diff options
22 files changed, 217 insertions, 113 deletions
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 24b28dd7d970..7cffeea6fe2c 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -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() << "'"); diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index 3172892d7172..9905f827d663 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -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; } diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index eaa2d0b8af7d..95e30c442042 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -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 diff --git a/tools/aapt2/ResourceTable_test.cpp b/tools/aapt2/ResourceTable_test.cpp index eb75f947e0be..7fa8ea2f7f94 100644 --- a/tools/aapt2/ResourceTable_test.cpp +++ b/tools/aapt2/ResourceTable_test.cpp @@ -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; diff --git a/tools/aapt2/ResourceUtils_test.cpp b/tools/aapt2/ResourceUtils_test.cpp index e637c3ee2f3c..cb786d3794c2 100644 --- a/tools/aapt2/ResourceUtils_test.cpp +++ b/tools/aapt2/ResourceUtils_test.cpp @@ -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()); diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index a782cd3672d1..77cee0683f3e 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -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); } diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h index b2ec8bdd7c77..6371c4cbb7b5 100644 --- a/tools/aapt2/ResourceValues.h +++ b/tools/aapt2/ResourceValues.h @@ -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; diff --git a/tools/aapt2/ResourceValues_test.cpp b/tools/aapt2/ResourceValues_test.cpp index a80a9dc177f1..c4a1108ac62a 100644 --- a/tools/aapt2/ResourceValues_test.cpp +++ b/tools/aapt2/ResourceValues_test.cpp @@ -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 diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index 72e07dc23725..c9e272c32eae 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -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)); diff --git a/tools/aapt2/format/binary/BinaryResourceParser.cpp b/tools/aapt2/format/binary/BinaryResourceParser.cpp index 8d079ffecb63..8215ddf0461c 100644 --- a/tools/aapt2/format/binary/BinaryResourceParser.cpp +++ b/tools/aapt2/format/binary/BinaryResourceParser.cpp @@ -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 { diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp index 51ccdc7359b2..bab70109fc65 100644 --- a/tools/aapt2/format/binary/TableFlattener_test.cpp +++ b/tools/aapt2/format/binary/TableFlattener_test.cpp @@ -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; diff --git a/tools/aapt2/format/proto/ProtoDeserialize.cpp b/tools/aapt2/format/proto/ProtoDeserialize.cpp index 3d6975d8d559..d8635a989bed 100644 --- a/tools/aapt2/format/proto/ProtoDeserialize.cpp +++ b/tools/aapt2/format/proto/ProtoDeserialize.cpp @@ -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()) { diff --git a/tools/aapt2/format/proto/ProtoSerialize_test.cpp b/tools/aapt2/format/proto/ProtoSerialize_test.cpp index d7f83fd9ecdc..ccba5c652822 100644 --- a/tools/aapt2/format/proto/ProtoSerialize_test.cpp +++ b/tools/aapt2/format/proto/ProtoSerialize_test.cpp @@ -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 index 000000000000..6d2aac6ac936 --- /dev/null +++ b/tools/aapt2/integration-tests/BasicTest/Android.mk @@ -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 index 000000000000..3743c4012b60 --- /dev/null +++ b/tools/aapt2/integration-tests/BasicTest/AndroidManifest.xml @@ -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 index 000000000000..8d6bb432bf6b --- /dev/null +++ b/tools/aapt2/integration-tests/BasicTest/res/values/values.xml @@ -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> diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 5beb594bd92f..e449546f9399 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -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), diff --git a/tools/aapt2/link/XmlCompatVersioner_test.cpp b/tools/aapt2/link/XmlCompatVersioner_test.cpp index 29ad25f79ab9..1ed4536c4566 100644 --- a/tools/aapt2/link/XmlCompatVersioner_test.cpp +++ b/tools/aapt2/link/XmlCompatVersioner_test.cpp @@ -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}; diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp index 8852c8e8c66e..160ff925f6cc 100644 --- a/tools/aapt2/link/XmlReferenceLinker.cpp +++ b/tools/aapt2/link/XmlReferenceLinker.cpp @@ -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)) { diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index 3cae0e8ae462..2e97a2f3ae7f 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -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; } } diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp index 495a48a830f7..c4eab1269e4e 100644 --- a/tools/aapt2/test/Builders.cpp +++ b/tools/aapt2/test/Builders.cpp @@ -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}); diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index 0d7451b5a6da..fd5262af6e48 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -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(); |