Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3d8d4a18 authored by Jeremy Meyer's avatar Jeremy Meyer
Browse files

Error on duplicate resource with same disabled flag 

Also realized I hadn't handled flag negation so added that as well.

Test: Automated
Bug: 329436914
Flag: EXEMPT Aconfig not supported on host tools
Change-Id: If90ae71070306f8e0c367be7e652da9c7bd0bb22
parent b7ab38e1

core/tests/resourceflaggingtests/src/com/android/resourceflaggingtests/ResourceFlaggingTest.java

+10 −0
Original line number Diff line number Diff line
@@ -77,6 +77,16 @@ public class ResourceFlaggingTest { 
        assertThat(mResources.getBoolean(R.bool.bool2)).isTrue();

    }



    @Test

    public void testNegatedDisabledFlag() {

        assertThat(mResources.getBoolean(R.bool.bool5)).isTrue();

    }



    @Test

    public void testNegatedEnabledFlag() {

        assertThat(mResources.getBoolean(R.bool.bool6)).isTrue();

    }



    @Test

    public void testFlagEnabledDifferentCompilationUnit() {

        assertThat(mResources.getBoolean(R.bool.bool3)).isTrue();

tools/aapt2/Debug.cpp

+15 −0
Original line number Diff line number Diff line
@@ -346,6 +346,21 @@ void Debug::PrintTable(const ResourceTable& table, const DebugPrintTableOptions& 
            value->value->Accept(&body_printer);

            printer->Undent();

          }

          printer->Println("Flag disabled values:");

          for (const auto& value : entry.flag_disabled_values) {

            printer->Print("(");

            printer->Print(value->config.to_string());

            printer->Print(") ");

            value->value->Accept(&headline_printer);

            if (options.show_sources && !value->value->GetSource().path.empty()) {

              printer->Print(" src=");

              printer->Print(value->value->GetSource().to_string());

            }

            printer->Println();

            printer->Indent();

            value->value->Accept(&body_printer);

            printer->Undent();

          }

          printer->Undent();

        }

      }

tools/aapt2/Resource.h

+11 −0
Original line number Diff line number Diff line
@@ -71,6 +71,17 @@ enum class ResourceType { 


enum class FlagStatus { NoFlag = 0, Disabled = 1, Enabled = 2 };



struct FeatureFlagAttribute {

  std::string name;

  bool negated = false;



  std::string ToString() {

    return (negated ? "!" : "") + name;

  }



  bool operator==(const FeatureFlagAttribute& o) const = default;

};



android::StringPiece to_string(ResourceType type);



/**

tools/aapt2/ResourceParser.cpp

+33 −36
Original line number Diff line number Diff line
@@ -107,9 +107,10 @@ struct ParsedResource { 
  Visibility::Level visibility_level = Visibility::Level::kUndefined;

  bool staged_api = false;

  bool allow_new = false;

  FlagStatus flag_status = FlagStatus::NoFlag;

  std::optional<OverlayableItem> overlayable_item;

  std::optional<StagedId> staged_alias;

  std::optional<FeatureFlagAttribute> flag;

  FlagStatus flag_status;



  std::string comment;

  std::unique_ptr<Value> value;
@@ -151,6 +152,7 @@ static bool AddResourcesToTable(ResourceTable* table, android::IDiagnostics* dia 
  }



  if (res->value != nullptr) {

    res->value->SetFlag(res->flag);

    res->value->SetFlagStatus(res->flag_status);

    // Attach the comment, source and config to the value.

    res->value->SetComment(std::move(res->comment));
@@ -162,8 +164,6 @@ static bool AddResourcesToTable(ResourceTable* table, android::IDiagnostics* dia 
    res_builder.SetStagedId(res->staged_alias.value());

  }



  res_builder.SetFlagStatus(res->flag_status);



  bool error = false;

  if (!res->name.entry.empty()) {

    if (!table->AddResource(res_builder.Build(), diag)) {
@@ -547,11 +547,15 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, 
  });



  std::string resource_type = parser->element_name();

  auto flag_status = GetFlagStatus(parser);

  if (!flag_status) {

  out_resource->flag = GetFlag(parser);

  std::string error;

  auto flag_status = GetFlagStatus(out_resource->flag, options_.feature_flag_values, &error);

  if (flag_status) {

    out_resource->flag_status = flag_status.value();

  } else {

    diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number())) << error);

    return false;

  }

  out_resource->flag_status = flag_status.value();



  // The value format accepted for this resource.

  uint32_t resource_format = 0u;
@@ -733,31 +737,20 @@ bool ResourceParser::ParseResource(xml::XmlPullParser* parser, 
  return false;

}



std::optional<FlagStatus> ResourceParser::GetFlagStatus(xml::XmlPullParser* parser) {

  auto flag_status = FlagStatus::NoFlag;



  std::optional<StringPiece> flag = xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag");

  if (flag) {

    auto flag_it = options_.feature_flag_values.find(flag.value());

    if (flag_it == options_.feature_flag_values.end()) {

      diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))

                   << "Resource flag value undefined");

      return {};

    }

    const auto& flag_properties = flag_it->second;

    if (!flag_properties.read_only) {

      diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))

                   << "Only read only flags may be used with resources");

      return {};

std::optional<FeatureFlagAttribute> ResourceParser::GetFlag(xml::XmlPullParser* parser) {

  auto name = xml::FindAttribute(parser, xml::kSchemaAndroid, "featureFlag");

  if (name) {

    FeatureFlagAttribute flag;

    if (name->starts_with('!')) {

      flag.negated = true;

      flag.name = name->substr(1);

    } else {

      flag.name = name.value();

    }

    if (!flag_properties.enabled.has_value()) {

      diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))

                   << "Only flags with a value may be used with resources");

    return flag;

  } else {

    return {};

  }

    flag_status = flag_properties.enabled.value() ? FlagStatus::Enabled : FlagStatus::Disabled;

  }

  return flag_status;

}



bool ResourceParser::ParseItem(xml::XmlPullParser* parser,
@@ -1666,21 +1659,25 @@ bool ResourceParser::ParseArrayImpl(xml::XmlPullParser* parser, 
    const std::string& element_namespace = parser->element_namespace();

    const std::string& element_name = parser->element_name();

    if (element_namespace.empty() && element_name == "item") {

      auto flag_status = GetFlagStatus(parser);

      if (!flag_status) {

        error = true;

        continue;

      }

      auto flag = GetFlag(parser);

      std::unique_ptr<Item> item = ParseXml(parser, typeMask, kNoRawString);

      if (!item) {

        diag_->Error(android::DiagMessage(item_source) << "could not parse array item");

        error = true;

        continue;

      }

      item->SetFlagStatus(flag_status.value());

      item->SetFlag(flag);

      std::string err;

      auto status = GetFlagStatus(flag, options_.feature_flag_values, &err);

      if (status) {

        item->SetFlagStatus(status.value());

      } else {

        diag_->Error(android::DiagMessage(item_source) << err);

        error = true;

        continue;

      }

      item->SetSource(item_source);

      array->elements.emplace_back(std::move(item));



    } else if (!ShouldIgnoreElement(element_namespace, element_name)) {

      diag_->Error(android::DiagMessage(source_.WithLine(parser->line_number()))

                   << "unknown tag <" << element_namespace << ":" << element_name << ">");

tools/aapt2/ResourceParser.h

+1 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ class ResourceParser { 
 private:

  DISALLOW_COPY_AND_ASSIGN(ResourceParser);



  std::optional<FlagStatus> GetFlagStatus(xml::XmlPullParser* parser);

  std::optional<FeatureFlagAttribute> GetFlag(xml::XmlPullParser* parser);



  std::optional<FlattenedXmlSubTree> CreateFlattenSubTree(xml::XmlPullParser* parser);