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

Commit 46078bac authored by Jeremy Meyer's avatar Jeremy Meyer Committed by Android (Google) Code Review
Browse files

Merge "Error on duplicate resource with same disabled flag" into main

parents c92ba5f9 3d8d4a18
Loading
Loading
Loading
Loading
+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();
+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();
        }
      }
+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);

/**
+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 << ">");
+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);

Loading