From bab4ef56d7803f3a50ccfaca2729509338fcbb23 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Thu, 1 Jun 2017 15:22:57 -0700 Subject: [PATCH] AAPT2: Allow undefined resources (placeholders) A resource defined like so: should be assigned the value @null. The only exception is for resources, which are given the empty string value (since is ambiguous). The decision to use "" is based off the fact that old AAPT used to assign "" to all undefined resources, even non-string ones. Bug: 38425050 Test: make aapt2_tests Change-Id: Ib3e0f6f83d16ddd8b279c9fd44a07a37867b85e9 --- tools/aapt2/Android.bp | 5 +- tools/aapt2/ResourceParser.cpp | 5 ++ tools/aapt2/ResourceParser.h | 30 +++---- tools/aapt2/ResourceParser_test.cpp | 64 ++++++++------- tools/aapt2/ResourceUtils.cpp | 78 ++++++++++--------- tools/aapt2/ResourceUtils.h | 11 ++- tools/aapt2/ResourceUtils_test.cpp | 21 +++++ tools/aapt2/ResourceValues.cpp | 33 +++++--- tools/aapt2/ResourceValues_test.cpp | 10 +++ .../AppOne/res/values/test.xml | 3 + tools/aapt2/link/ReferenceLinker.cpp | 5 +- tools/aapt2/proto/TableProtoDeserializer.cpp | 13 +--- tools/aapt2/proto/TableProtoSerializer.cpp | 3 +- tools/aapt2/test/Builders.h | 10 +-- tools/aapt2/test/Common.cpp | 60 ++++++++++++++ tools/aapt2/test/Common.h | 34 +++----- 16 files changed, 245 insertions(+), 140 deletions(-) create mode 100644 tools/aapt2/test/Common.cpp diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp index 064d864114633..1dc47f88ab927 100644 --- a/tools/aapt2/Android.bp +++ b/tools/aapt2/Android.bp @@ -160,7 +160,10 @@ cc_library_host_shared { // ========================================================== cc_test_host { name: "aapt2_tests", - srcs: ["**/*_test.cpp"], + srcs: [ + "test/Common.cpp", + "**/*_test.cpp", + ], static_libs: [ "libaapt2", "libgmock", diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index 084325a44d93b..9a37913f0edcb 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -623,6 +623,11 @@ std::unique_ptr ResourceParser::ParseXml(xml::XmlPullParser* parser, return std::move(string); } + // If the text is empty, and the value is not allowed to be a string, encode it as a @null. + if (util::TrimWhitespace(raw_value).empty()) { + return ResourceUtils::MakeNull(); + } + if (allow_raw_value) { // We can't parse this so return a RawString if we are allowed. return util::make_unique( diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index 8258019958624..5631dc2ad29c4 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -78,42 +78,32 @@ class ResourceParser { * Item, then a * RawString is returned. Otherwise this returns false; */ - std::unique_ptr ParseXml(xml::XmlPullParser* parser, - const uint32_t type_mask, + std::unique_ptr ParseXml(xml::XmlPullParser* parser, const uint32_t type_mask, const bool allow_raw_value); bool ParseResources(xml::XmlPullParser* parser); bool ParseResource(xml::XmlPullParser* parser, ParsedResource* out_resource); - bool ParseItem(xml::XmlPullParser* parser, ParsedResource* out_resource, - uint32_t format); + bool ParseItem(xml::XmlPullParser* parser, ParsedResource* out_resource, uint32_t format); bool ParseString(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParsePublic(xml::XmlPullParser* parser, ParsedResource* out_resource); - bool ParsePublicGroup(xml::XmlPullParser* parser, - ParsedResource* out_resource); - bool ParseSymbolImpl(xml::XmlPullParser* parser, - ParsedResource* out_resource); + bool ParsePublicGroup(xml::XmlPullParser* parser, ParsedResource* out_resource); + bool ParseSymbolImpl(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseSymbol(xml::XmlPullParser* parser, ParsedResource* out_resource); - bool ParseAddResource(xml::XmlPullParser* parser, - ParsedResource* out_resource); + bool ParseAddResource(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseAttr(xml::XmlPullParser* parser, ParsedResource* out_resource); - bool ParseAttrImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, - bool weak); + bool ParseAttrImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, bool weak); Maybe ParseEnumOrFlagItem(xml::XmlPullParser* parser, const android::StringPiece& tag); bool ParseStyle(const ResourceType type, xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseStyleItem(xml::XmlPullParser* parser, Style* style); - bool ParseDeclareStyleable(xml::XmlPullParser* parser, - ParsedResource* out_resource); + bool ParseDeclareStyleable(xml::XmlPullParser* parser, ParsedResource* out_resource); bool ParseArray(xml::XmlPullParser* parser, ParsedResource* out_resource); - bool ParseIntegerArray(xml::XmlPullParser* parser, - ParsedResource* out_resource); - bool ParseStringArray(xml::XmlPullParser* parser, - ParsedResource* out_resource); - bool ParseArrayImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, - uint32_t typeMask); + bool ParseIntegerArray(xml::XmlPullParser* parser, ParsedResource* out_resource); + bool ParseStringArray(xml::XmlPullParser* parser, ParsedResource* out_resource); + bool ParseArrayImpl(xml::XmlPullParser* parser, ParsedResource* out_resource, uint32_t typeMask); bool ParsePlural(xml::XmlPullParser* parser, ParsedResource* out_resource); IDiagnostics* diag_; diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index e60ef66a21dbd..5352ca8679b3e 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -25,19 +25,20 @@ #include "test/Test.h" #include "xml/XmlPullParser.h" +using ::aapt::test::ValueEq; using ::android::StringPiece; using ::testing::Eq; using ::testing::NotNull; +using ::testing::Pointee; namespace aapt { -constexpr const char* kXmlPreamble = - "\n"; +constexpr const char* kXmlPreamble = "\n"; TEST(ResourceParserSingleTest, FailToParseWithNoRootResourcesElement) { std::unique_ptr context = test::ContextBuilder().Build(); std::stringstream input(kXmlPreamble); - input << "" << std::endl; + input << R"()" << std::endl; ResourceTable table; ResourceParser parser(context->GetDiagnostics(), &table, Source{"test"}, {}); xml::XmlPullParser xml_parser(input); @@ -46,19 +47,20 @@ TEST(ResourceParserSingleTest, FailToParseWithNoRootResourcesElement) { class ResourceParserTest : public ::testing::Test { public: - void SetUp() override { context_ = test::ContextBuilder().Build(); } + void SetUp() override { + context_ = test::ContextBuilder().Build(); + } ::testing::AssertionResult TestParse(const StringPiece& str) { return TestParse(str, ConfigDescription{}); } - ::testing::AssertionResult TestParse(const StringPiece& str, - const ConfigDescription& config) { + ::testing::AssertionResult TestParse(const StringPiece& str, const ConfigDescription& config) { std::stringstream input(kXmlPreamble); input << "\n" << str << "\n" << std::endl; ResourceParserOptions parserOptions; - ResourceParser parser(context_->GetDiagnostics(), &table_, Source{"test"}, - config, parserOptions); + ResourceParser parser(context_->GetDiagnostics(), &table_, Source{"test"}, config, + parserOptions); xml::XmlPullParser xmlParser(input); if (parser.Parse(&xmlParser)) { return ::testing::AssertionSuccess(); @@ -205,18 +207,18 @@ TEST_F(ResourceParserTest, ParseNull) { // a non-existing value, and this causes problems in styles when trying to // resolve an attribute. Null values must be encoded as android::Res_value::TYPE_REFERENCE // with a data value of 0. - BinaryPrimitive* integer = test::GetValue(&table_, "integer/foo"); - ASSERT_NE(nullptr, integer); - EXPECT_EQ(uint16_t(android::Res_value::TYPE_REFERENCE), integer->value.dataType); - EXPECT_EQ(0u, integer->value.data); + Reference* null_ref = test::GetValue(&table_, "integer/foo"); + ASSERT_THAT(null_ref, NotNull()); + EXPECT_FALSE(null_ref->name); + EXPECT_FALSE(null_ref->id); + EXPECT_EQ(Reference::Type::kResource, null_ref->reference_type); } TEST_F(ResourceParserTest, ParseEmpty) { std::string input = "@empty"; ASSERT_TRUE(TestParse(input)); - BinaryPrimitive* integer = - test::GetValue(&table_, "integer/foo"); + BinaryPrimitive* integer = test::GetValue(&table_, "integer/foo"); ASSERT_NE(nullptr, integer); EXPECT_EQ(uint16_t(android::Res_value::TYPE_NULL), integer->value.dataType); EXPECT_EQ(uint32_t(android::Res_value::DATA_NULL_EMPTY), integer->value.data); @@ -241,22 +243,18 @@ TEST_F(ResourceParserTest, ParseAttr) { // ultimately // stored them with the default configuration. Check that we have the same // behavior. -TEST_F(ResourceParserTest, - ParseAttrAndDeclareStyleableUnderConfigButRecordAsNoConfig) { +TEST_F(ResourceParserTest, ParseAttrAndDeclareStyleableUnderConfigButRecordAsNoConfig) { const ConfigDescription watch_config = test::ParseConfigOrDie("watch"); - std::string input = R"EOF( - - - - )EOF"; + std::string input = R"( + + + + )"; ASSERT_TRUE(TestParse(input, watch_config)); - EXPECT_EQ(nullptr, test::GetValueForConfig(&table_, "attr/foo", - watch_config)); - EXPECT_EQ(nullptr, test::GetValueForConfig(&table_, "attr/baz", - watch_config)); - EXPECT_EQ(nullptr, test::GetValueForConfig( - &table_, "styleable/bar", watch_config)); + EXPECT_EQ(nullptr, test::GetValueForConfig(&table_, "attr/foo", watch_config)); + EXPECT_EQ(nullptr, test::GetValueForConfig(&table_, "attr/baz", watch_config)); + EXPECT_EQ(nullptr, test::GetValueForConfig(&table_, "styleable/bar", watch_config)); EXPECT_NE(nullptr, test::GetValue(&table_, "attr/foo")); EXPECT_NE(nullptr, test::GetValue(&table_, "attr/baz")); @@ -833,4 +831,16 @@ TEST_F(ResourceParserTest, ParseBagElement) { EXPECT_NE(nullptr, ValueCast(val->entries[0].value.get())); } +TEST_F(ResourceParserTest, ParseElementWithNoValue) { + std::string input = R"( + + )"; + ASSERT_TRUE(TestParse(input)); + ASSERT_THAT(test::GetValue(&table_, "drawable/foo"), Pointee(ValueEq(Reference()))); + + String* str = test::GetValue(&table_, "string/foo"); + ASSERT_THAT(str, NotNull()); + EXPECT_THAT(*str->value, Eq("")); +} + } // namespace aapt diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index 818c8cec30f9e..deeef6ebbcb75 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -305,21 +305,25 @@ std::unique_ptr TryParseReference(const StringPiece& str, return {}; } -std::unique_ptr TryParseNullOrEmpty(const StringPiece& str) { - StringPiece trimmed_str(util::TrimWhitespace(str)); - android::Res_value value = {}; +std::unique_ptr TryParseNullOrEmpty(const StringPiece& str) { + const StringPiece trimmed_str(util::TrimWhitespace(str)); if (trimmed_str == "@null") { - // TYPE_NULL with data set to 0 is interpreted by the runtime as an error. - // Instead we set the data type to TYPE_REFERENCE with a value of 0. - value.dataType = android::Res_value::TYPE_REFERENCE; + return MakeNull(); } else if (trimmed_str == "@empty") { - // TYPE_NULL with value of DATA_NULL_EMPTY is handled fine by the runtime. - value.dataType = android::Res_value::TYPE_NULL; - value.data = android::Res_value::DATA_NULL_EMPTY; - } else { - return {}; + return MakeEmpty(); } - return util::make_unique(value); + return {}; +} + +std::unique_ptr MakeNull() { + // TYPE_NULL with data set to 0 is interpreted by the runtime as an error. + // Instead we set the data type to TYPE_REFERENCE with a value of 0. + return util::make_unique(); +} + +std::unique_ptr MakeEmpty() { + return util::make_unique(android::Res_value::TYPE_NULL, + android::Res_value::DATA_NULL_EMPTY); } std::unique_ptr TryParseEnumSymbol(const Attribute* enum_attr, @@ -569,13 +573,15 @@ uint32_t AndroidTypeToAttributeTypeMask(uint16_t type) { std::unique_ptr TryParseItemForAttribute( const StringPiece& value, uint32_t type_mask, const std::function& on_create_reference) { - std::unique_ptr null_or_empty = TryParseNullOrEmpty(value); + using android::ResTable_map; + + auto null_or_empty = TryParseNullOrEmpty(value); if (null_or_empty) { - return std::move(null_or_empty); + return null_or_empty; } bool create = false; - std::unique_ptr reference = TryParseReference(value, &create); + auto reference = TryParseReference(value, &create); if (reference) { if (create && on_create_reference) { on_create_reference(reference->name.value()); @@ -583,39 +589,37 @@ std::unique_ptr TryParseItemForAttribute( return std::move(reference); } - if (type_mask & android::ResTable_map::TYPE_COLOR) { + if (type_mask & ResTable_map::TYPE_COLOR) { // Try parsing this as a color. - std::unique_ptr color = TryParseColor(value); + auto color = TryParseColor(value); if (color) { return std::move(color); } } - if (type_mask & android::ResTable_map::TYPE_BOOLEAN) { + if (type_mask & ResTable_map::TYPE_BOOLEAN) { // Try parsing this as a boolean. - std::unique_ptr boolean = TryParseBool(value); + auto boolean = TryParseBool(value); if (boolean) { return std::move(boolean); } } - if (type_mask & android::ResTable_map::TYPE_INTEGER) { + if (type_mask & ResTable_map::TYPE_INTEGER) { // Try parsing this as an integer. - std::unique_ptr integer = TryParseInt(value); + auto integer = TryParseInt(value); if (integer) { return std::move(integer); } } - const uint32_t float_mask = android::ResTable_map::TYPE_FLOAT | - android::ResTable_map::TYPE_DIMENSION | - android::ResTable_map::TYPE_FRACTION; + const uint32_t float_mask = + ResTable_map::TYPE_FLOAT | ResTable_map::TYPE_DIMENSION | ResTable_map::TYPE_FRACTION; if (type_mask & float_mask) { // Try parsing this as a float. - std::unique_ptr floating_point = TryParseFloat(value); + auto floating_point = TryParseFloat(value); if (floating_point) { - if (type_mask & - AndroidTypeToAttributeTypeMask(floating_point->value.dataType)) { + if (type_mask & AndroidTypeToAttributeTypeMask(floating_point->value.dataType)) { return std::move(floating_point); } } @@ -630,24 +634,25 @@ std::unique_ptr TryParseItemForAttribute( std::unique_ptr TryParseItemForAttribute( const StringPiece& str, const Attribute* attr, const std::function& on_create_reference) { + using android::ResTable_map; + const uint32_t type_mask = attr->type_mask; - std::unique_ptr value = - TryParseItemForAttribute(str, type_mask, on_create_reference); + auto value = TryParseItemForAttribute(str, type_mask, on_create_reference); if (value) { return value; } - if (type_mask & android::ResTable_map::TYPE_ENUM) { + if (type_mask & ResTable_map::TYPE_ENUM) { // Try parsing this as an enum. - std::unique_ptr enum_value = TryParseEnumSymbol(attr, str); + auto enum_value = TryParseEnumSymbol(attr, str); if (enum_value) { return std::move(enum_value); } } - if (type_mask & android::ResTable_map::TYPE_FLAGS) { + if (type_mask & ResTable_map::TYPE_FLAGS) { // Try parsing this as a flag. - std::unique_ptr flag_value = TryParseFlagSymbol(attr, str); + auto flag_value = TryParseFlagSymbol(attr, str); if (flag_value) { return std::move(flag_value); } @@ -655,8 +660,7 @@ std::unique_ptr TryParseItemForAttribute( return {}; } -std::string BuildResourceFileName(const ResourceFile& res_file, - const NameMangler* mangler) { +std::string BuildResourceFileName(const ResourceFile& res_file, const NameMangler* mangler) { std::stringstream out; out << "res/" << res_file.name.type; if (res_file.config != ConfigDescription{}) { @@ -719,9 +723,9 @@ std::unique_ptr ParseBinaryResValue(const ResourceType& type, const Config ref_type = Reference::Type::kAttribute; } - if (data == 0) { + if (data == 0u) { // A reference of 0, must be the magic @null reference. - return util::make_unique(android::Res_value::TYPE_REFERENCE, 0u); + return util::make_unique(); } // This is a normal reference. diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h index da0fc8ee314ae..36f6c2bda8f81 100644 --- a/tools/aapt2/ResourceUtils.h +++ b/tools/aapt2/ResourceUtils.h @@ -133,7 +133,16 @@ std::unique_ptr TryParseReference(const android::StringPiece& str, * Returns a BinaryPrimitve object representing @null or @empty if the string * was parsed as one. */ -std::unique_ptr TryParseNullOrEmpty(const android::StringPiece& str); +std::unique_ptr TryParseNullOrEmpty(const android::StringPiece& str); + +// Returns a Reference representing @null. +// Due to runtime compatibility issues, this is encoded as a reference with ID 0. +// The runtime will convert this to TYPE_NULL. +std::unique_ptr MakeNull(); + +// Returns a BinaryPrimitive representing @empty. This is encoded as a Res_value with +// type Res_value::TYPE_NULL and data Res_value::DATA_NULL_EMPTY. +std::unique_ptr MakeEmpty(); /* * Returns a BinaryPrimitve object representing a color if the string was parsed diff --git a/tools/aapt2/ResourceUtils_test.cpp b/tools/aapt2/ResourceUtils_test.cpp index 048c69252e472..cdc34f1ec752f 100644 --- a/tools/aapt2/ResourceUtils_test.cpp +++ b/tools/aapt2/ResourceUtils_test.cpp @@ -19,6 +19,9 @@ #include "Resource.h" #include "test/Test.h" +using ::aapt::test::ValueEq; +using ::testing::Pointee; + namespace aapt { TEST(ResourceUtilsTest, ParseBool) { @@ -200,4 +203,22 @@ TEST(ResourceUtilsTest, ParseEmptyFlag) { EXPECT_EQ(0u, result->value.data); } +TEST(ResourceUtilsTest, NullIsEmptyReference) { + auto null_value = ResourceUtils::MakeNull(); + ASSERT_THAT(null_value, Pointee(ValueEq(Reference()))); + + auto value = ResourceUtils::TryParseNullOrEmpty("@null"); + ASSERT_THAT(value, Pointee(ValueEq(Reference()))); +} + +TEST(ResourceUtilsTest, EmptyIsBinaryPrimitive) { + auto empty_value = ResourceUtils::MakeEmpty(); + ASSERT_THAT(empty_value, Pointee(ValueEq(BinaryPrimitive(android::Res_value::TYPE_NULL, + android::Res_value::DATA_NULL_EMPTY)))); + + auto value = ResourceUtils::TryParseNullOrEmpty("@empty"); + ASSERT_THAT(value, Pointee(ValueEq(BinaryPrimitive(android::Res_value::TYPE_NULL, + android::Res_value::DATA_NULL_EMPTY)))); +} + } // namespace aapt diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index abfdec48df67b..e808984c75f53 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -94,8 +94,8 @@ bool Reference::Equals(const Value* value) const { bool Reference::Flatten(android::Res_value* out_value) const { const ResourceId resid = id.value_or_default(ResourceId(0)); - const bool dynamic = - (resid.package_id() != kFrameworkPackageId && resid.package_id() != kAppPackageId); + const bool dynamic = resid.is_valid_dynamic() && resid.package_id() != kFrameworkPackageId && + resid.package_id() != kAppPackageId; if (reference_type == Reference::Type::kResource) { if (dynamic) { @@ -119,22 +119,29 @@ Reference* Reference::Clone(StringPool* /*new_pool*/) const { } void Reference::Print(std::ostream* out) const { - *out << "(reference) "; - if (reference_type == Reference::Type::kResource) { - *out << "@"; - if (private_reference) { - *out << "*"; + if (reference_type == Type::kResource) { + *out << "(reference) @"; + if (!name && !id) { + *out << "null"; + return; } } else { - *out << "?"; + *out << "(attr-reference) ?"; + } + + if (private_reference) { + *out << "*"; } if (name) { *out << name.value(); } - if (id && !Res_INTERNALID(id.value().id)) { - *out << " " << id.value(); + if (id && id.value().is_valid_dynamic()) { + if (name) { + *out << " "; + } + *out << id.value(); } } @@ -314,7 +321,11 @@ BinaryPrimitive* BinaryPrimitive::Clone(StringPool* /*new_pool*/) const { void BinaryPrimitive::Print(std::ostream* out) const { switch (value.dataType) { case android::Res_value::TYPE_NULL: - *out << "(null)"; + if (value.data == android::Res_value::DATA_NULL_EMPTY) { + *out << "(empty)"; + } else { + *out << "(null)"; + } break; case android::Res_value::TYPE_INT_DEC: *out << "(integer) " << static_cast(value.data); diff --git a/tools/aapt2/ResourceValues_test.cpp b/tools/aapt2/ResourceValues_test.cpp index e154d93b6f857..69f8e67530f69 100644 --- a/tools/aapt2/ResourceValues_test.cpp +++ b/tools/aapt2/ResourceValues_test.cpp @@ -180,4 +180,14 @@ TEST(ResourceValuesTest, StyleMerges) { EXPECT_TRUE(a->Equals(expected.get())); } +// TYPE_NULL is encoded as TYPE_REFERENCE with a value of 0. This is represented in AAPT2 +// by a default constructed Reference value. +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); +} + } // namespace aapt diff --git a/tools/aapt2/integration-tests/AppOne/res/values/test.xml b/tools/aapt2/integration-tests/AppOne/res/values/test.xml index 5104f8212d8be..2c9e8b8775657 100644 --- a/tools/aapt2/integration-tests/AppOne/res/values/test.xml +++ b/tools/aapt2/integration-tests/AppOne/res/values/test.xml @@ -38,4 +38,7 @@ + + + diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index 18498e3e3c67d..a8e510cd6140b 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -296,7 +296,10 @@ bool ReferenceLinker::LinkReference(const CallSite& callsite, Reference* referen IAaptContext* context, SymbolTable* symbols, xml::IPackageDeclStack* decls) { CHECK(reference != nullptr); - CHECK(reference->name || reference->id); + if (!reference->name && !reference->id) { + // This is @null. + return true; + } Reference transformed_reference = *reference; TransformReferenceFromNamespace(decls, context->GetCompilationPackage(), &transformed_reference); diff --git a/tools/aapt2/proto/TableProtoDeserializer.cpp b/tools/aapt2/proto/TableProtoDeserializer.cpp index 1d0041b2ac5a5..4b5619235c06c 100644 --- a/tools/aapt2/proto/TableProtoDeserializer.cpp +++ b/tools/aapt2/proto/TableProtoDeserializer.cpp @@ -343,26 +343,19 @@ class PackagePbDeserializer { return value; } - bool DeserializeReferenceFromPb(const pb::Reference& pb_ref, - Reference* out_ref) { + bool DeserializeReferenceFromPb(const pb::Reference& pb_ref, Reference* out_ref) { out_ref->reference_type = DeserializeReferenceTypeFromPb(pb_ref.type()); out_ref->private_reference = pb_ref.private_(); - if (!pb_ref.has_id() && !pb_ref.has_symbol_idx()) { - return false; - } - if (pb_ref.has_id()) { out_ref->id = ResourceId(pb_ref.id()); } if (pb_ref.has_symbol_idx()) { - const std::string str_symbol = - util::GetString(*symbol_pool_, pb_ref.symbol_idx()); + const std::string str_symbol = util::GetString(*symbol_pool_, pb_ref.symbol_idx()); ResourceNameRef name_ref; if (!ResourceUtils::ParseResourceName(str_symbol, &name_ref, nullptr)) { - diag_->Error(DiagMessage(source_) << "invalid reference name '" - << str_symbol << "'"); + diag_->Error(DiagMessage(source_) << "invalid reference name '" << str_symbol << "'"); return false; } diff --git a/tools/aapt2/proto/TableProtoSerializer.cpp b/tools/aapt2/proto/TableProtoSerializer.cpp index de10bb80bf7e6..d87d64e1cb46b 100644 --- a/tools/aapt2/proto/TableProtoSerializer.cpp +++ b/tools/aapt2/proto/TableProtoSerializer.cpp @@ -190,8 +190,7 @@ class PbSerializerVisitor : public RawValueVisitor { } if (ref.name) { - StringPool::Ref symbol_ref = - symbol_pool_->MakeRef(ref.name.value().ToString()); + StringPool::Ref symbol_ref = symbol_pool_->MakeRef(ref.name.value().ToString()); pb_ref->set_symbol_idx(static_cast(symbol_ref.index())); } diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index 02acedbd8ac5a..f3da780cacbdf 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -59,8 +59,7 @@ class ResourceTableBuilder { ResourceTableBuilder& AddReference(const android::StringPiece& name, const ResourceId& id, const android::StringPiece& ref) { - return AddValue(name, id, - util::make_unique(ParseNameOrDie(ref))); + return AddValue(name, id, util::make_unique(ParseNameOrDie(ref))); } ResourceTableBuilder& AddString(const android::StringPiece& name, @@ -111,8 +110,8 @@ class ResourceTableBuilder { ResourceTableBuilder& AddValue(const android::StringPiece& name, const ConfigDescription& config, const ResourceId& id, std::unique_ptr value) { ResourceName res_name = ParseNameOrDie(name); - CHECK(table_->AddResourceAllowMangled(res_name, id, config, {}, - std::move(value), &diagnostics_)); + CHECK(table_->AddResourceAllowMangled(res_name, id, config, {}, std::move(value), + GetDiagnostics())); return *this; } @@ -122,7 +121,7 @@ class ResourceTableBuilder { Symbol symbol; symbol.state = state; symbol.allow_new = allow_new; - CHECK(table_->SetSymbolStateAllowMangled(res_name, id, symbol, &diagnostics_)); + CHECK(table_->SetSymbolStateAllowMangled(res_name, id, symbol, GetDiagnostics())); return *this; } @@ -131,7 +130,6 @@ class ResourceTableBuilder { private: DISALLOW_COPY_AND_ASSIGN(ResourceTableBuilder); - DummyDiagnosticsImpl diagnostics_; std::unique_ptr table_ = util::make_unique(); }; diff --git a/tools/aapt2/test/Common.cpp b/tools/aapt2/test/Common.cpp new file mode 100644 index 0000000000000..0fabbc4070e7f --- /dev/null +++ b/tools/aapt2/test/Common.cpp @@ -0,0 +1,60 @@ +/* + * 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. + */ + +#include "test/Common.h" + +namespace aapt { +namespace test { + +struct DummyDiagnosticsImpl : public IDiagnostics { + void Log(Level level, DiagMessageActual& actual_msg) override { + switch (level) { + case Level::Note: + return; + + case Level::Warn: + std::cerr << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl; + break; + + case Level::Error: + std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl; + break; + } + } +}; + +IDiagnostics* GetDiagnostics() { + static DummyDiagnosticsImpl diag; + return &diag; +} + +template <> +Value* GetValueForConfigAndProduct(ResourceTable* table, + const android::StringPiece& res_name, + const ConfigDescription& config, + const android::StringPiece& product) { + Maybe result = table->FindResource(ParseNameOrDie(res_name)); + if (result) { + ResourceConfigValue* config_value = result.value().entry->FindValue(config, product); + if (config_value) { + return config_value->value.get(); + } + } + return nullptr; +} + +} // namespace test +} // namespace aapt diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index a937de8869a62..05851485e2164 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -46,27 +46,7 @@ namespace aapt { namespace test { -struct DummyDiagnosticsImpl : public IDiagnostics { - void Log(Level level, DiagMessageActual& actual_msg) override { - switch (level) { - case Level::Note: - return; - - case Level::Warn: - std::cerr << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl; - break; - - case Level::Error: - std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl; - break; - } - } -}; - -inline IDiagnostics* GetDiagnostics() { - static DummyDiagnosticsImpl diag; - return &diag; -} +IDiagnostics* GetDiagnostics(); inline ResourceName ParseNameOrDie(const android::StringPiece& str) { ResourceNameRef ref; @@ -80,7 +60,7 @@ inline ConfigDescription ParseConfigOrDie(const android::StringPiece& str) { return config; } -template +template T* GetValueForConfigAndProduct(ResourceTable* table, const android::StringPiece& res_name, const ConfigDescription& config, const android::StringPiece& product) { @@ -94,13 +74,19 @@ T* GetValueForConfigAndProduct(ResourceTable* table, const android::StringPiece& return nullptr; } -template +template <> +Value* GetValueForConfigAndProduct(ResourceTable* table, + const android::StringPiece& res_name, + const ConfigDescription& config, + const android::StringPiece& product); + +template T* GetValueForConfig(ResourceTable* table, const android::StringPiece& res_name, const ConfigDescription& config) { return GetValueForConfigAndProduct(table, res_name, config, {}); } -template +template T* GetValue(ResourceTable* table, const android::StringPiece& res_name) { return GetValueForConfig(table, res_name, {}); }