OSDN Git Service

AAPT2: Allow compatible duplicate Attributes
authorAdam Lesinski <adamlesinski@google.com>
Sat, 9 Dec 2017 00:06:10 +0000 (16:06 -0800)
committerAdam Lesinski <adamlesinski@google.com>
Thu, 11 Jan 2018 21:54:11 +0000 (13:54 -0800)
If a resource XML file defines two compatible Attributes, they should
be merged without throwing an error. Ex:

<declare-styleable>
  <attr name="conflict" format="string" />
</declare-styleable>

<declare-styleable>
  <attr name="conflict" format="string|reference" />
</declare-styleable>

In this case, string|reference and string are the same, so these should
merge correctly.

Bug: 65699599
Test: make aapt2_tests
Test: make AaptBasicTest
Change-Id: I7b0f956d2332f7f0b458acd59ca0a606b2cfdf95

22 files changed:
tools/aapt2/ResourceParser.cpp
tools/aapt2/ResourceTable.cpp
tools/aapt2/ResourceTable.h
tools/aapt2/ResourceTable_test.cpp
tools/aapt2/ResourceUtils_test.cpp
tools/aapt2/ResourceValues.cpp
tools/aapt2/ResourceValues.h
tools/aapt2/ResourceValues_test.cpp
tools/aapt2/cmd/Link.cpp
tools/aapt2/format/binary/BinaryResourceParser.cpp
tools/aapt2/format/binary/TableFlattener_test.cpp
tools/aapt2/format/proto/ProtoDeserialize.cpp
tools/aapt2/format/proto/ProtoSerialize_test.cpp
tools/aapt2/integration-tests/BasicTest/Android.mk [new file with mode: 0644]
tools/aapt2/integration-tests/BasicTest/AndroidManifest.xml [new file with mode: 0644]
tools/aapt2/integration-tests/BasicTest/res/values/values.xml [new file with mode: 0644]
tools/aapt2/java/JavaClassGenerator_test.cpp
tools/aapt2/link/XmlCompatVersioner_test.cpp
tools/aapt2/link/XmlReferenceLinker.cpp
tools/aapt2/process/SymbolTable.cpp
tools/aapt2/test/Builders.cpp
tools/aapt2/test/Builders.h

index 24b28dd..7cffeea 100644 (file)
@@ -17,6 +17,7 @@
 #include "ResourceParser.h"
 
 #include <functional>
+#include <limits>
 #include <sstream>
 
 #include "android-base/logging.h"
@@ -987,8 +988,7 @@ bool ResourceParser::ParseAttrImpl(xml::XmlPullParser* parser,
     type_mask = ParseFormatAttribute(maybe_format.value());
     if (type_mask == 0) {
       diag_->Error(DiagMessage(source_.WithLine(parser->line_number()))
-                   << "invalid attribute format '" << maybe_format.value()
-                   << "'");
+                   << "invalid attribute format '" << maybe_format.value() << "'");
       return false;
     }
   }
@@ -1000,8 +1000,7 @@ bool ResourceParser::ParseAttrImpl(xml::XmlPullParser* parser,
     if (!min_str.empty()) {
       std::u16string min_str16 = util::Utf8ToUtf16(min_str);
       android::Res_value value;
-      if (android::ResTable::stringToInt(min_str16.data(), min_str16.size(),
-                                         &value)) {
+      if (android::ResTable::stringToInt(min_str16.data(), min_str16.size(), &value)) {
         maybe_min = static_cast<int32_t>(value.data);
       }
     }
@@ -1018,8 +1017,7 @@ bool ResourceParser::ParseAttrImpl(xml::XmlPullParser* parser,
     if (!max_str.empty()) {
       std::u16string max_str16 = util::Utf8ToUtf16(max_str);
       android::Res_value value;
-      if (android::ResTable::stringToInt(max_str16.data(), max_str16.size(),
-                                         &value)) {
+      if (android::ResTable::stringToInt(max_str16.data(), max_str16.size(), &value)) {
         maybe_max = static_cast<int32_t>(value.data);
       }
     }
@@ -1061,8 +1059,7 @@ bool ResourceParser::ParseAttrImpl(xml::XmlPullParser* parser,
     const Source item_source = source_.WithLine(parser->line_number());
     const std::string& element_namespace = parser->element_namespace();
     const std::string& element_name = parser->element_name();
-    if (element_namespace.empty() &&
-        (element_name == "flag" || element_name == "enum")) {
+    if (element_namespace.empty() && (element_name == "flag" || element_name == "enum")) {
       if (element_name == "enum") {
         if (type_mask & android::ResTable_map::TYPE_FLAGS) {
           diag_->Error(DiagMessage(item_source)
@@ -1120,17 +1117,12 @@ bool ResourceParser::ParseAttrImpl(xml::XmlPullParser* parser,
     return false;
   }
 
-  std::unique_ptr<Attribute> attr = util::make_unique<Attribute>(weak);
+  std::unique_ptr<Attribute> attr = util::make_unique<Attribute>(
+      type_mask ? type_mask : uint32_t{android::ResTable_map::TYPE_ANY});
+  attr->SetWeak(weak);
   attr->symbols = std::vector<Attribute::Symbol>(items.begin(), items.end());
-  attr->type_mask =
-      type_mask ? type_mask : uint32_t(android::ResTable_map::TYPE_ANY);
-  if (maybe_min) {
-    attr->min_int = maybe_min.value();
-  }
-
-  if (maybe_max) {
-    attr->max_int = maybe_max.value();
-  }
+  attr->min_int = maybe_min.value_or_default(std::numeric_limits<int32_t>::min());
+  attr->max_int = maybe_max.value_or_default(std::numeric_limits<int32_t>::max());
   out_resource->value = std::move(attr);
   return true;
 }
@@ -1445,11 +1437,9 @@ bool ResourceParser::ParseDeclareStyleable(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 == "attr") {
-      Maybe<StringPiece> maybe_name =
-          xml::FindNonEmptyAttribute(parser, "name");
+      Maybe<StringPiece> maybe_name = xml::FindNonEmptyAttribute(parser, "name");
       if (!maybe_name) {
-        diag_->Error(DiagMessage(item_source)
-                     << "<attr> tag must have a 'name' attribute");
+        diag_->Error(DiagMessage(item_source) << "<attr> tag must have a 'name' attribute");
         error = true;
         continue;
       }
@@ -1457,8 +1447,7 @@ bool ResourceParser::ParseDeclareStyleable(xml::XmlPullParser* parser,
       // If this is a declaration, the package name may be in the name. Separate
       // these out.
       // Eg. <attr name="android:text" />
-      Maybe<Reference> maybe_ref =
-          ResourceUtils::ParseXmlAttributeName(maybe_name.value());
+      Maybe<Reference> maybe_ref = ResourceUtils::ParseXmlAttributeName(maybe_name.value());
       if (!maybe_ref) {
         diag_->Error(DiagMessage(item_source) << "<attr> tag has invalid name '"
                                               << maybe_name.value() << "'");
index 3172892..9905f82 100644 (file)
@@ -262,9 +262,8 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist
   // attributes all-over, we do special handling to see
   // which definition sticks.
   //
-  if (existing_attr->type_mask == incoming_attr->type_mask) {
-    // The two attributes are both DECLs, but they are plain attributes
-    // with the same formats.
+  if (existing_attr->IsCompatibleWith(*incoming_attr)) {
+    // The two attributes are both DECLs, but they are plain attributes with compatible formats.
     // Keep the strongest one.
     return existing_attr->IsWeak() ? CollisionResult::kTakeNew : CollisionResult::kKeepOriginal;
   }
index eaa2d0b..95e30c4 100644 (file)
@@ -81,10 +81,7 @@ class ResourceConfigValue {
   DISALLOW_COPY_AND_ASSIGN(ResourceConfigValue);
 };
 
-/**
- * Represents a resource entry, which may have
- * varying values for each defined configuration.
- */
+// Represents a resource entry, which may have varying values for each defined configuration.
 class ResourceEntry {
  public:
   // The name of the resource. Immutable, as this determines the order of this resource
index eb75f94..7fa8ea2 100644 (file)
@@ -102,23 +102,37 @@ TEST(ResourceTableTest, AddMultipleResources) {
 TEST(ResourceTableTest, OverrideWeakResourceValue) {
   ResourceTable table;
 
-  ASSERT_TRUE(table.AddResource(
-      test::ParseNameOrDie("android:attr/foo"), ConfigDescription{}, "",
-      util::make_unique<Attribute>(true), test::GetDiagnostics()));
+  ASSERT_TRUE(table.AddResource(test::ParseNameOrDie("android:attr/foo"), ConfigDescription{}, "",
+                                test::AttributeBuilder().SetWeak(true).Build(),
+                                test::GetDiagnostics()));
 
   Attribute* attr = test::GetValue<Attribute>(&table, "android:attr/foo");
   ASSERT_THAT(attr, NotNull());
   EXPECT_TRUE(attr->IsWeak());
 
-  ASSERT_TRUE(table.AddResource(
-      test::ParseNameOrDie("android:attr/foo"), ConfigDescription{}, "",
-      util::make_unique<Attribute>(false), test::GetDiagnostics()));
+  ASSERT_TRUE(table.AddResource(test::ParseNameOrDie("android:attr/foo"), ConfigDescription{}, "",
+                                util::make_unique<Attribute>(), test::GetDiagnostics()));
 
   attr = test::GetValue<Attribute>(&table, "android:attr/foo");
   ASSERT_THAT(attr, NotNull());
   EXPECT_FALSE(attr->IsWeak());
 }
 
+TEST(ResourceTableTest, AllowCompatibleDuplicateAttributes) {
+  ResourceTable table;
+
+  const ResourceName name = test::ParseNameOrDie("android:attr/foo");
+  Attribute attr_one(android::ResTable_map::TYPE_STRING);
+  attr_one.SetWeak(true);
+  Attribute attr_two(android::ResTable_map::TYPE_STRING | android::ResTable_map::TYPE_REFERENCE);
+  attr_two.SetWeak(true);
+
+  ASSERT_TRUE(table.AddResource(name, ConfigDescription{}, "",
+                                util::make_unique<Attribute>(attr_one), test::GetDiagnostics()));
+  ASSERT_TRUE(table.AddResource(name, ConfigDescription{}, "",
+                                util::make_unique<Attribute>(attr_two), test::GetDiagnostics()));
+}
+
 TEST(ResourceTableTest, ProductVaryingValues) {
   ResourceTable table;
 
index e637c3e..cb786d3 100644 (file)
@@ -179,12 +179,11 @@ TEST(ResourceUtilsTest, ParseStyleParentReference) {
 }
 
 TEST(ResourceUtilsTest, ParseEmptyFlag) {
-  std::unique_ptr<Attribute> attr =
-      test::AttributeBuilder(false)
-          .SetTypeMask(ResTable_map::TYPE_FLAGS)
-          .AddItem("one", 0x01)
-          .AddItem("two", 0x02)
-          .Build();
+  std::unique_ptr<Attribute> attr = test::AttributeBuilder()
+                                        .SetTypeMask(ResTable_map::TYPE_FLAGS)
+                                        .AddItem("one", 0x01)
+                                        .AddItem("two", 0x02)
+                                        .Build();
 
   std::unique_ptr<BinaryPrimitive> result = ResourceUtils::TryParseFlagSymbol(attr.get(), "");
   ASSERT_THAT(result, NotNull());
index a782cd3..77cee06 100644 (file)
@@ -507,17 +507,10 @@ void BinaryPrimitive::PrettyPrint(Printer* printer) const {
   }
 }
 
-Attribute::Attribute()
-    : type_mask(0u),
-      min_int(std::numeric_limits<int32_t>::min()),
-      max_int(std::numeric_limits<int32_t>::max()) {
-}
-
-Attribute::Attribute(bool w, uint32_t t)
+Attribute::Attribute(uint32_t t)
     : type_mask(t),
       min_int(std::numeric_limits<int32_t>::min()),
       max_int(std::numeric_limits<int32_t>::max()) {
-  weak_ = w;
 }
 
 std::ostream& operator<<(std::ostream& out, const Attribute::Symbol& s) {
@@ -568,6 +561,20 @@ bool Attribute::Equals(const Value* value) const {
                     });
 }
 
+bool Attribute::IsCompatibleWith(const Attribute& attr) const {
+  // If the high bits are set on any of these attribute type masks, then they are incompatible.
+  // We don't check that flags and enums are identical.
+  if ((type_mask & ~android::ResTable_map::TYPE_ANY) != 0 ||
+      (attr.type_mask & ~android::ResTable_map::TYPE_ANY) != 0) {
+    return false;
+  }
+
+  // Every attribute accepts a reference.
+  uint32_t this_type_mask = type_mask | android::ResTable_map::TYPE_REFERENCE;
+  uint32_t that_type_mask = attr.type_mask | android::ResTable_map::TYPE_REFERENCE;
+  return this_type_mask == that_type_mask;
+}
+
 Attribute* Attribute::Clone(StringPool* /*new_pool*/) const {
   return new Attribute(*this);
 }
index b2ec8bd..6371c4c 100644 (file)
@@ -300,10 +300,15 @@ struct Attribute : public BaseValue<Attribute> {
   int32_t max_int;
   std::vector<Symbol> symbols;
 
-  Attribute();
-  explicit Attribute(bool w, uint32_t t = 0u);
+  explicit Attribute(uint32_t t = 0u);
 
   bool Equals(const Value* value) const override;
+
+  // Returns true if this Attribute's format is compatible with the given Attribute. The basic
+  // rule is that TYPE_REFERENCE can be ignored for both of the Attributes, and TYPE_FLAGS and
+  // TYPE_ENUMS are never compatible.
+  bool IsCompatibleWith(const Attribute& attr) const;
+
   Attribute* Clone(StringPool* new_pool) const override;
   std::string MaskString() const;
   void Print(std::ostream* out) const override;
index a80a9dc..c4a1108 100644 (file)
@@ -24,6 +24,18 @@ using ::testing::StrEq;
 
 namespace aapt {
 
+namespace {
+
+// Attribute types.
+constexpr const uint32_t TYPE_DIMENSION = android::ResTable_map::TYPE_DIMENSION;
+constexpr const uint32_t TYPE_ENUM = android::ResTable_map::TYPE_ENUM;
+constexpr const uint32_t TYPE_FLAGS = android::ResTable_map::TYPE_FLAGS;
+constexpr const uint32_t TYPE_INTEGER = android::ResTable_map::TYPE_INTEGER;
+constexpr const uint32_t TYPE_REFERENCE = android::Res_value::TYPE_REFERENCE;
+constexpr const uint32_t TYPE_STRING = android::ResTable_map::TYPE_STRING;
+
+}  // namespace
+
 TEST(ResourceValuesTest, PluralEquals) {
   StringPool pool;
 
@@ -206,23 +218,19 @@ TEST(ResourcesValuesTest, EmptyReferenceFlattens) {
   android::Res_value value = {};
   ASSERT_TRUE(Reference().Flatten(&value));
 
-  EXPECT_EQ(android::Res_value::TYPE_REFERENCE, value.dataType);
-  EXPECT_EQ(0x0u, value.data);
+  EXPECT_THAT(value.dataType, Eq(android::Res_value::TYPE_REFERENCE));
+  EXPECT_THAT(value.data, Eq(0u));
 }
 
 TEST(ResourcesValuesTest, AttributeMatches) {
-  constexpr const uint32_t TYPE_DIMENSION = android::ResTable_map::TYPE_DIMENSION;
-  constexpr const uint32_t TYPE_ENUM = android::ResTable_map::TYPE_ENUM;
-  constexpr const uint32_t TYPE_FLAGS = android::ResTable_map::TYPE_FLAGS;
-  constexpr const uint32_t TYPE_INTEGER = android::ResTable_map::TYPE_INTEGER;
   constexpr const uint8_t TYPE_INT_DEC = android::Res_value::TYPE_INT_DEC;
 
-  Attribute attr1(false /*weak*/, TYPE_DIMENSION);
+  Attribute attr1(TYPE_DIMENSION);
   EXPECT_FALSE(attr1.Matches(*ResourceUtils::TryParseColor("#7fff00")));
   EXPECT_TRUE(attr1.Matches(*ResourceUtils::TryParseFloat("23dp")));
   EXPECT_TRUE(attr1.Matches(*ResourceUtils::TryParseReference("@android:string/foo")));
 
-  Attribute attr2(false /*weak*/, TYPE_INTEGER | TYPE_ENUM);
+  Attribute attr2(TYPE_INTEGER | TYPE_ENUM);
   attr2.min_int = 0;
   attr2.symbols.push_back(Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/foo")),
                                             static_cast<uint32_t>(-1)});
@@ -231,7 +239,7 @@ TEST(ResourcesValuesTest, AttributeMatches) {
   EXPECT_TRUE(attr2.Matches(BinaryPrimitive(TYPE_INT_DEC, 1u)));
   EXPECT_FALSE(attr2.Matches(BinaryPrimitive(TYPE_INT_DEC, static_cast<uint32_t>(-2))));
 
-  Attribute attr3(false /*weak*/, TYPE_INTEGER | TYPE_FLAGS);
+  Attribute attr3(TYPE_INTEGER | TYPE_FLAGS);
   attr3.max_int = 100;
   attr3.symbols.push_back(
       Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/foo")), 0x01u});
@@ -251,11 +259,33 @@ TEST(ResourcesValuesTest, AttributeMatches) {
   // Not a flag and greater than max_int.
   EXPECT_FALSE(attr3.Matches(BinaryPrimitive(TYPE_INT_DEC, 127u)));
 
-  Attribute attr4(false /*weak*/, TYPE_ENUM);
+  Attribute attr4(TYPE_ENUM);
   attr4.symbols.push_back(
       Attribute::Symbol{Reference(test::ParseNameOrDie("android:id/foo")), 0x01u});
   EXPECT_TRUE(attr4.Matches(BinaryPrimitive(TYPE_INT_DEC, 0x01u)));
   EXPECT_FALSE(attr4.Matches(BinaryPrimitive(TYPE_INT_DEC, 0x02u)));
 }
 
+TEST(ResourcesValuesTest, AttributeIsCompatible) {
+  Attribute attr_one(TYPE_STRING | TYPE_REFERENCE);
+  Attribute attr_two(TYPE_STRING);
+  Attribute attr_three(TYPE_ENUM);
+  Attribute attr_four(TYPE_REFERENCE);
+
+  EXPECT_TRUE(attr_one.IsCompatibleWith(attr_one));
+  EXPECT_TRUE(attr_one.IsCompatibleWith(attr_two));
+  EXPECT_FALSE(attr_one.IsCompatibleWith(attr_three));
+  EXPECT_FALSE(attr_one.IsCompatibleWith(attr_four));
+
+  EXPECT_TRUE(attr_two.IsCompatibleWith(attr_one));
+  EXPECT_TRUE(attr_two.IsCompatibleWith(attr_two));
+  EXPECT_FALSE(attr_two.IsCompatibleWith(attr_three));
+  EXPECT_FALSE(attr_two.IsCompatibleWith(attr_four));
+
+  EXPECT_FALSE(attr_three.IsCompatibleWith(attr_one));
+  EXPECT_FALSE(attr_three.IsCompatibleWith(attr_two));
+  EXPECT_FALSE(attr_three.IsCompatibleWith(attr_three));
+  EXPECT_FALSE(attr_three.IsCompatibleWith(attr_four));
+}
+
 } // namespace aapt
index 72e07dc..c9e272c 100644 (file)
@@ -388,10 +388,8 @@ ResourceFileFlattener::ResourceFileFlattener(const ResourceFileFlattenerOptions&
   // generated from the attribute definitions themselves (b/62028956).
   if (const SymbolTable::Symbol* s = symm->FindById(R::attr::paddingHorizontal)) {
     std::vector<ReplacementAttr> replacements{
-        {"paddingLeft", R::attr::paddingLeft,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
-        {"paddingRight", R::attr::paddingRight,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
+        {"paddingLeft", R::attr::paddingLeft, Attribute(android::ResTable_map::TYPE_DIMENSION)},
+        {"paddingRight", R::attr::paddingRight, Attribute(android::ResTable_map::TYPE_DIMENSION)},
     };
     rules_[R::attr::paddingHorizontal] =
         util::make_unique<DegradeToManyRule>(std::move(replacements));
@@ -399,10 +397,8 @@ ResourceFileFlattener::ResourceFileFlattener(const ResourceFileFlattenerOptions&
 
   if (const SymbolTable::Symbol* s = symm->FindById(R::attr::paddingVertical)) {
     std::vector<ReplacementAttr> replacements{
-        {"paddingTop", R::attr::paddingTop,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
-        {"paddingBottom", R::attr::paddingBottom,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
+        {"paddingTop", R::attr::paddingTop, Attribute(android::ResTable_map::TYPE_DIMENSION)},
+        {"paddingBottom", R::attr::paddingBottom, Attribute(android::ResTable_map::TYPE_DIMENSION)},
     };
     rules_[R::attr::paddingVertical] =
         util::make_unique<DegradeToManyRule>(std::move(replacements));
@@ -411,9 +407,9 @@ ResourceFileFlattener::ResourceFileFlattener(const ResourceFileFlattenerOptions&
   if (const SymbolTable::Symbol* s = symm->FindById(R::attr::layout_marginHorizontal)) {
     std::vector<ReplacementAttr> replacements{
         {"layout_marginLeft", R::attr::layout_marginLeft,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
+         Attribute(android::ResTable_map::TYPE_DIMENSION)},
         {"layout_marginRight", R::attr::layout_marginRight,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
+         Attribute(android::ResTable_map::TYPE_DIMENSION)},
     };
     rules_[R::attr::layout_marginHorizontal] =
         util::make_unique<DegradeToManyRule>(std::move(replacements));
@@ -422,9 +418,9 @@ ResourceFileFlattener::ResourceFileFlattener(const ResourceFileFlattenerOptions&
   if (const SymbolTable::Symbol* s = symm->FindById(R::attr::layout_marginVertical)) {
     std::vector<ReplacementAttr> replacements{
         {"layout_marginTop", R::attr::layout_marginTop,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
+         Attribute(android::ResTable_map::TYPE_DIMENSION)},
         {"layout_marginBottom", R::attr::layout_marginBottom,
-         Attribute(false, android::ResTable_map::TYPE_DIMENSION)},
+         Attribute(android::ResTable_map::TYPE_DIMENSION)},
     };
     rules_[R::attr::layout_marginVertical] =
         util::make_unique<DegradeToManyRule>(std::move(replacements));
index 8d079ff..8215ddf 100644 (file)
@@ -504,8 +504,8 @@ std::unique_ptr<Style> BinaryResourceParser::ParseStyle(const ResourceNameRef& n
 std::unique_ptr<Attribute> BinaryResourceParser::ParseAttr(const ResourceNameRef& name,
                                                            const ConfigDescription& config,
                                                            const ResTable_map_entry* map) {
-  const bool is_weak = (util::DeviceToHost16(map->flags) & ResTable_entry::FLAG_WEAK) != 0;
-  std::unique_ptr<Attribute> attr = util::make_unique<Attribute>(is_weak);
+  std::unique_ptr<Attribute> attr = util::make_unique<Attribute>();
+  attr->SetWeak((util::DeviceToHost16(map->flags) & ResTable_entry::FLAG_WEAK) != 0);
 
   // First we must discover what type of attribute this is. Find the type mask.
   auto type_mask_iter = std::find_if(begin(map), end(map), [](const ResTable_map& entry) -> bool {
index 51ccdc7..bab7010 100644 (file)
@@ -215,7 +215,7 @@ TEST_F(TableFlattenerTest, FlattenEntriesWithGapsInIds) {
 }
 
 TEST_F(TableFlattenerTest, FlattenMinMaxAttributes) {
-  Attribute attr(false);
+  Attribute attr;
   attr.type_mask = android::ResTable_map::TYPE_INTEGER;
   attr.min_int = 10;
   attr.max_int = 23;
index 3d6975d..d8635a9 100644 (file)
@@ -635,8 +635,7 @@ std::unique_ptr<Value> DeserializeValueFromPb(const pb::Value& pb_value,
     switch (pb_compound_value.value_case()) {
       case pb::CompoundValue::kAttr: {
         const pb::Attribute& pb_attr = pb_compound_value.attr();
-        std::unique_ptr<Attribute> attr = util::make_unique<Attribute>();
-        attr->type_mask = pb_attr.format_flags();
+        std::unique_ptr<Attribute> attr = util::make_unique<Attribute>(pb_attr.format_flags());
         attr->min_int = pb_attr.min_int();
         attr->max_int = pb_attr.max_int();
         for (const pb::Attribute_Symbol& pb_symbol : pb_attr.symbol()) {
index d7f83fd..ccba5c6 100644 (file)
@@ -180,7 +180,7 @@ TEST(ProtoSerializeTest, SerializeAndDeserializeXml) {
   attr.name = "name";
   attr.namespace_uri = xml::kSchemaAndroid;
   attr.value = "23dp";
-  attr.compiled_attribute = xml::AaptAttribute({}, ResourceId(0x01010000));
+  attr.compiled_attribute = xml::AaptAttribute(Attribute{}, ResourceId(0x01010000));
   attr.compiled_value =
       ResourceUtils::TryParseItemForAttribute(attr.value, android::ResTable_map::TYPE_DIMENSION);
   attr.compiled_value->SetSource(Source().WithLine(25));
diff --git a/tools/aapt2/integration-tests/BasicTest/Android.mk b/tools/aapt2/integration-tests/BasicTest/Android.mk
new file mode 100644 (file)
index 0000000..6d2aac6
--- /dev/null
@@ -0,0 +1,23 @@
+#
+# Copyright (C) 2017 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+LOCAL_PATH := $(call my-dir)
+
+include $(CLEAR_VARS)
+LOCAL_USE_AAPT2 := true
+LOCAL_PACKAGE_NAME := AaptBasicTest
+LOCAL_MODULE_TAGS := tests
+include $(BUILD_PACKAGE)
diff --git a/tools/aapt2/integration-tests/BasicTest/AndroidManifest.xml b/tools/aapt2/integration-tests/BasicTest/AndroidManifest.xml
new file mode 100644 (file)
index 0000000..3743c40
--- /dev/null
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2017 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    package="com.android.aapt.basic">
+
+    <uses-sdk android:minSdkVersion="21" />
+</manifest>
diff --git a/tools/aapt2/integration-tests/BasicTest/res/values/values.xml b/tools/aapt2/integration-tests/BasicTest/res/values/values.xml
new file mode 100644 (file)
index 0000000..8d6bb43
--- /dev/null
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (C) 2017 The Android Open Source Project
+
+     Licensed under the Apache License, Version 2.0 (the "License");
+     you may not use this file except in compliance with the License.
+     You may obtain a copy of the License at
+
+          http://www.apache.org/licenses/LICENSE-2.0
+
+     Unless required by applicable law or agreed to in writing, software
+     distributed under the License is distributed on an "AS IS" BASIS,
+     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+     See the License for the specific language governing permissions and
+     limitations under the License.
+-->
+
+<resources>
+    <declare-styleable name="AttrConflictStyleableOne">
+        <attr name="format_conflict" format="string|reference" />
+    </declare-styleable>
+
+    <declare-styleable name="AttrConflictStyleableTwo">
+        <attr name="format_conflict" format="string" />
+    </declare-styleable>
+</resources>
index 5beb594..e449546 100644 (file)
@@ -57,7 +57,7 @@ TEST(JavaClassGeneratorTest, TransformInvalidJavaIdentifierCharacter) {
           .SetPackageId("android", 0x01)
           .AddSimple("android:id/hey-man", ResourceId(0x01020000))
           .AddValue("android:attr/cool.attr", ResourceId(0x01010000),
-                    test::AttributeBuilder(false).Build())
+                    test::AttributeBuilder().Build())
           .AddValue("android:styleable/hey.dude", ResourceId(0x01030000),
                     test::StyleableBuilder()
                         .AddItem("android:attr/cool.attr", ResourceId(0x01010000))
@@ -229,10 +229,8 @@ TEST(JavaClassGeneratorTest, EmitOtherPackagesAttributesInStyleable) {
       test::ResourceTableBuilder()
           .SetPackageId("android", 0x01)
           .SetPackageId("com.lib", 0x02)
-          .AddValue("android:attr/bar", ResourceId(0x01010000),
-                    test::AttributeBuilder(false).Build())
-          .AddValue("com.lib:attr/bar", ResourceId(0x02010000),
-                    test::AttributeBuilder(false).Build())
+          .AddValue("android:attr/bar", ResourceId(0x01010000), test::AttributeBuilder().Build())
+          .AddValue("com.lib:attr/bar", ResourceId(0x02010000), test::AttributeBuilder().Build())
           .AddValue("android:styleable/foo", ResourceId(0x01030000),
                     test::StyleableBuilder()
                         .AddItem("android:attr/bar", ResourceId(0x01010000))
@@ -290,7 +288,7 @@ TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) {
 TEST(JavaClassGeneratorTest, CommentsForEnumAndFlagAttributesArePresent) {}
 
 TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) {
-  Attribute attr(false);
+  Attribute attr;
   attr.SetComment(StringPiece("This is an attribute"));
 
   Styleable styleable;
@@ -375,7 +373,7 @@ TEST(JavaClassGeneratorTest, StyleableAndIndicesAreColocated) {
 }
 
 TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) {
-  Attribute attr(false);
+  Attribute attr;
   attr.SetComment(StringPiece("removed"));
 
   std::unique_ptr<ResourceTable> table =
@@ -413,7 +411,7 @@ TEST(JavaClassGeneratorTest, GenerateOnResourcesLoadedCallbackForSharedLibrary)
   std::unique_ptr<ResourceTable> table =
       test::ResourceTableBuilder()
           .SetPackageId("android", 0x00)
-          .AddValue("android:attr/foo", ResourceId(0x00010000), util::make_unique<Attribute>(false))
+          .AddValue("android:attr/foo", ResourceId(0x00010000), util::make_unique<Attribute>())
           .AddValue("android:id/foo", ResourceId(0x00020000), util::make_unique<Id>())
           .AddValue(
               "android:style/foo", ResourceId(0x00030000),
index 29ad25f..1ed4536 100644 (file)
@@ -54,17 +54,17 @@ class XmlCompatVersionerTest : public ::testing::Test {
             .AddSymbolSource(
                 test::StaticSymbolSourceBuilder()
                     .AddPublicSymbol("android:attr/paddingLeft", R::attr::paddingLeft,
-                                     util::make_unique<Attribute>(false, TYPE_DIMENSION))
+                                     util::make_unique<Attribute>(TYPE_DIMENSION))
                     .AddPublicSymbol("android:attr/paddingRight", R::attr::paddingRight,
-                                     util::make_unique<Attribute>(false, TYPE_DIMENSION))
+                                     util::make_unique<Attribute>(TYPE_DIMENSION))
                     .AddPublicSymbol("android:attr/progressBarPadding", R::attr::progressBarPadding,
-                                     util::make_unique<Attribute>(false, TYPE_DIMENSION))
+                                     util::make_unique<Attribute>(TYPE_DIMENSION))
                     .AddPublicSymbol("android:attr/paddingStart", R::attr::paddingStart,
-                                     util::make_unique<Attribute>(false, TYPE_DIMENSION))
+                                     util::make_unique<Attribute>(TYPE_DIMENSION))
                     .AddPublicSymbol("android:attr/paddingHorizontal", R::attr::paddingHorizontal,
-                                     util::make_unique<Attribute>(false, TYPE_DIMENSION))
+                                     util::make_unique<Attribute>(TYPE_DIMENSION))
                     .AddSymbol("com.app:attr/foo", ResourceId(0x7f010000),
-                               util::make_unique<Attribute>(false, TYPE_STRING))
+                               util::make_unique<Attribute>(TYPE_STRING))
                     .Build())
             .Build();
   }
@@ -126,9 +126,8 @@ TEST_F(XmlCompatVersionerTest, SingleRule) {
   XmlCompatVersioner::Rules rules;
   rules[R::attr::paddingHorizontal] =
       util::make_unique<DegradeToManyRule>(std::vector<ReplacementAttr>(
-          {ReplacementAttr{"paddingLeft", R::attr::paddingLeft, Attribute(false, TYPE_DIMENSION)},
-           ReplacementAttr{"paddingRight", R::attr::paddingRight,
-                           Attribute(false, TYPE_DIMENSION)}}));
+          {ReplacementAttr{"paddingLeft", R::attr::paddingLeft, Attribute(TYPE_DIMENSION)},
+           ReplacementAttr{"paddingRight", R::attr::paddingRight, Attribute(TYPE_DIMENSION)}}));
 
   const util::Range<ApiVersion> api_range{SDK_GINGERBREAD, SDK_O + 1};
 
@@ -187,12 +186,11 @@ TEST_F(XmlCompatVersionerTest, ChainedRule) {
   XmlCompatVersioner::Rules rules;
   rules[R::attr::progressBarPadding] =
       util::make_unique<DegradeToManyRule>(std::vector<ReplacementAttr>(
-          {ReplacementAttr{"paddingLeft", R::attr::paddingLeft, Attribute(false, TYPE_DIMENSION)},
-           ReplacementAttr{"paddingRight", R::attr::paddingRight,
-                           Attribute(false, TYPE_DIMENSION)}}));
+          {ReplacementAttr{"paddingLeft", R::attr::paddingLeft, Attribute(TYPE_DIMENSION)},
+           ReplacementAttr{"paddingRight", R::attr::paddingRight, Attribute(TYPE_DIMENSION)}}));
   rules[R::attr::paddingHorizontal] =
       util::make_unique<DegradeToManyRule>(std::vector<ReplacementAttr>({ReplacementAttr{
-          "progressBarPadding", R::attr::progressBarPadding, Attribute(false, TYPE_DIMENSION)}}));
+          "progressBarPadding", R::attr::progressBarPadding, Attribute(TYPE_DIMENSION)}}));
 
   const util::Range<ApiVersion> api_range{SDK_GINGERBREAD, SDK_O + 1};
 
@@ -267,9 +265,8 @@ TEST_F(XmlCompatVersionerTest, DegradeRuleOverridesExistingAttribute) {
   XmlCompatVersioner::Rules rules;
   rules[R::attr::paddingHorizontal] =
       util::make_unique<DegradeToManyRule>(std::vector<ReplacementAttr>(
-          {ReplacementAttr{"paddingLeft", R::attr::paddingLeft, Attribute(false, TYPE_DIMENSION)},
-           ReplacementAttr{"paddingRight", R::attr::paddingRight,
-                           Attribute(false, TYPE_DIMENSION)}}));
+          {ReplacementAttr{"paddingLeft", R::attr::paddingLeft, Attribute(TYPE_DIMENSION)},
+           ReplacementAttr{"paddingRight", R::attr::paddingRight, Attribute(TYPE_DIMENSION)}}));
 
   const util::Range<ApiVersion> api_range{SDK_GINGERBREAD, SDK_O + 1};
 
index 8852c8e..160ff92 100644 (file)
@@ -79,16 +79,15 @@ class XmlVisitor : public xml::PackageAwareVisitor {
 
   void Visit(xml::Element* el) override {
     // The default Attribute allows everything except enums or flags.
-    constexpr const static uint32_t kDefaultTypeMask =
-        0xffffffffu & ~(android::ResTable_map::TYPE_ENUM | android::ResTable_map::TYPE_FLAGS);
-    const static Attribute kDefaultAttribute(true /* weak */, kDefaultTypeMask);
+    Attribute default_attribute(android::ResTable_map::TYPE_ANY);
+    default_attribute.SetWeak(true);
 
     const Source source = source_.WithLine(el->line_number);
     for (xml::Attribute& attr : el->attributes) {
       // If the attribute has no namespace, interpret values as if
       // they were assigned to the default Attribute.
 
-      const Attribute* attribute = &kDefaultAttribute;
+      const Attribute* attribute = &default_attribute;
 
       if (Maybe<xml::ExtractedPackage> maybe_package =
               xml::ExtractPackageFromNamespace(attr.namespace_uri)) {
index 3cae0e8..2e97a2f 100644 (file)
@@ -243,7 +243,7 @@ static std::unique_ptr<SymbolTable::Symbol> LookupAttributeInTable(
   // Check to see if it is an attribute.
   for (size_t i = 0; i < (size_t)count; i++) {
     if (entry[i].map.name.ident == android::ResTable_map::ATTR_TYPE) {
-      s->attribute = std::make_shared<Attribute>(false, entry[i].map.value.data);
+      s->attribute = std::make_shared<Attribute>(entry[i].map.value.data);
       break;
     }
   }
index 495a48a..c4eab12 100644 (file)
@@ -156,8 +156,8 @@ std::unique_ptr<BinaryPrimitive> BuildPrimitive(uint8_t type, uint32_t data) {
   return util::make_unique<BinaryPrimitive>(value);
 }
 
-AttributeBuilder::AttributeBuilder(bool weak) : attr_(util::make_unique<Attribute>(weak)) {
-  attr_->type_mask = android::ResTable_map::TYPE_ANY;
+AttributeBuilder::AttributeBuilder()
+    : attr_(util::make_unique<Attribute>(android::ResTable_map::TYPE_ANY)) {
 }
 
 AttributeBuilder& AttributeBuilder::SetTypeMask(uint32_t typeMask) {
@@ -165,6 +165,11 @@ AttributeBuilder& AttributeBuilder::SetTypeMask(uint32_t typeMask) {
   return *this;
 }
 
+AttributeBuilder& AttributeBuilder::SetWeak(bool weak) {
+  attr_->SetWeak(weak);
+  return *this;
+}
+
 AttributeBuilder& AttributeBuilder::AddItem(const StringPiece& name, uint32_t value) {
   attr_->symbols.push_back(
       Attribute::Symbol{Reference(ResourceName({}, ResourceType::kId, name)), value});
index 0d7451b..fd5262a 100644 (file)
@@ -113,8 +113,9 @@ class ValueBuilder {
 
 class AttributeBuilder {
  public:
-  explicit AttributeBuilder(bool weak = false);
+  AttributeBuilder();
   AttributeBuilder& SetTypeMask(uint32_t typeMask);
+  AttributeBuilder& SetWeak(bool weak);
   AttributeBuilder& AddItem(const android::StringPiece& name, uint32_t value);
   std::unique_ptr<Attribute> Build();