diff options
-rw-r--r-- | tools/aapt2/Android.bp | 2 | ||||
-rw-r--r-- | tools/aapt2/ResourceUtils.cpp | 16 | ||||
-rw-r--r-- | tools/aapt2/ResourceUtils.h | 3 | ||||
-rw-r--r-- | tools/aapt2/ResourceValues.cpp | 183 | ||||
-rw-r--r-- | tools/aapt2/ResourceValues.h | 32 | ||||
-rw-r--r-- | tools/aapt2/ResourceValues_test.cpp | 32 | ||||
-rw-r--r-- | tools/aapt2/link/TableMerger.cpp | 93 | ||||
-rw-r--r-- | tools/aapt2/link/TableMerger_test.cpp | 93 | ||||
-rw-r--r-- | tools/aapt2/test/Common.h | 63 | ||||
-rw-r--r-- | tools/aapt2/test/Test.h | 1 |
10 files changed, 346 insertions, 172 deletions
diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp index bf189492b082..b2f1f62647e6 100644 --- a/tools/aapt2/Android.bp +++ b/tools/aapt2/Android.bp @@ -160,7 +160,7 @@ cc_library_host_shared { cc_test_host { name: "aapt2_tests", srcs: ["**/*_test.cpp"], - static_libs: ["libaapt2"], + static_libs: ["libaapt2", "libgmock"], defaults: ["aapt_defaults"], } diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp index 1bb7d9beee45..818c8cec30f9 100644 --- a/tools/aapt2/ResourceUtils.cpp +++ b/tools/aapt2/ResourceUtils.cpp @@ -496,19 +496,17 @@ Maybe<int> ParseSdkVersion(const StringPiece& str) { std::unique_ptr<BinaryPrimitive> TryParseBool(const StringPiece& str) { if (Maybe<bool> maybe_result = ParseBool(str)) { - android::Res_value value = {}; - value.dataType = android::Res_value::TYPE_INT_BOOLEAN; - - if (maybe_result.value()) { - value.data = 0xffffffffu; - } else { - value.data = 0; - } - return util::make_unique<BinaryPrimitive>(value); + const uint32_t data = maybe_result.value() ? 0xffffffffu : 0u; + return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_INT_BOOLEAN, data); } return {}; } +std::unique_ptr<BinaryPrimitive> MakeBool(bool val) { + return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_INT_BOOLEAN, + val ? 0xffffffffu : 0u); +} + std::unique_ptr<BinaryPrimitive> TryParseInt(const StringPiece& str) { std::u16string str16 = util::Utf8ToUtf16(str); android::Res_value value; diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h index 48922b72fefa..da0fc8ee314a 100644 --- a/tools/aapt2/ResourceUtils.h +++ b/tools/aapt2/ResourceUtils.h @@ -147,6 +147,9 @@ std::unique_ptr<BinaryPrimitive> TryParseColor(const android::StringPiece& str); */ std::unique_ptr<BinaryPrimitive> TryParseBool(const android::StringPiece& str); +// Returns a boolean BinaryPrimitive. +std::unique_ptr<BinaryPrimitive> MakeBool(bool val); + /* * Returns a BinaryPrimitve object representing an integer if the string was * parsed as one. diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp index 34bd2b4361ca..abfdec48df67 100644 --- a/tools/aapt2/ResourceValues.cpp +++ b/tools/aapt2/ResourceValues.cpp @@ -29,6 +29,11 @@ namespace aapt { +std::ostream& operator<<(std::ostream& out, const Value& value) { + value.Print(&out); + return out; +} + template <typename Derived> void BaseValue<Derived>::Accept(RawValueVisitor* visitor) { visitor->Visit(static_cast<Derived*>(this)); @@ -346,6 +351,15 @@ Attribute::Attribute(bool w, uint32_t t) weak_ = w; } +std::ostream& operator<<(std::ostream& out, const Attribute::Symbol& s) { + if (s.symbol.name) { + out << s.symbol.name.value().entry; + } else { + out << "???"; + } + return out << "=" << s.value; +} + template <typename T> constexpr T* add_pointer(T& val) { return &val; @@ -361,31 +375,27 @@ bool Attribute::Equals(const Value* value) const { return false; } - if (type_mask != other->type_mask || min_int != other->min_int || - max_int != other->max_int) { + if (type_mask != other->type_mask || min_int != other->min_int || max_int != other->max_int) { return false; } std::vector<const Symbol*> sorted_a; std::transform(symbols.begin(), symbols.end(), std::back_inserter(sorted_a), add_pointer<const Symbol>); - std::sort(sorted_a.begin(), sorted_a.end(), - [](const Symbol* a, const Symbol* b) -> bool { - return a->symbol.name < b->symbol.name; - }); + std::sort(sorted_a.begin(), sorted_a.end(), [](const Symbol* a, const Symbol* b) -> bool { + return a->symbol.name < b->symbol.name; + }); std::vector<const Symbol*> sorted_b; - std::transform(other->symbols.begin(), other->symbols.end(), - std::back_inserter(sorted_b), add_pointer<const Symbol>); - std::sort(sorted_b.begin(), sorted_b.end(), - [](const Symbol* a, const Symbol* b) -> bool { - return a->symbol.name < b->symbol.name; - }); + std::transform(other->symbols.begin(), other->symbols.end(), std::back_inserter(sorted_b), + add_pointer<const Symbol>); + std::sort(sorted_b.begin(), sorted_b.end(), [](const Symbol* a, const Symbol* b) -> bool { + return a->symbol.name < b->symbol.name; + }); return std::equal(sorted_a.begin(), sorted_a.end(), sorted_b.begin(), [](const Symbol* a, const Symbol* b) -> bool { - return a->symbol.Equals(&b->symbol) && - a->value == b->value; + return a->symbol.Equals(&b->symbol) && a->value == b->value; }); } @@ -588,14 +598,50 @@ bool Attribute::Matches(const Item* item, DiagMessage* out_msg) const { return true; } +std::ostream& operator<<(std::ostream& out, const Style::Entry& entry) { + if (entry.key.name) { + out << entry.key.name.value(); + } else if (entry.key.id) { + out << entry.key.id.value(); + } else { + out << "???"; + } + out << " = " << entry.value; + return out; +} + +template <typename T> +std::vector<T*> ToPointerVec(std::vector<T>& src) { + std::vector<T*> dst; + dst.reserve(src.size()); + for (T& in : src) { + dst.push_back(&in); + } + return dst; +} + +template <typename T> +std::vector<const T*> ToPointerVec(const std::vector<T>& src) { + std::vector<const T*> dst; + dst.reserve(src.size()); + for (const T& in : src) { + dst.push_back(&in); + } + return dst; +} + +static bool KeyNameComparator(const Style::Entry* a, const Style::Entry* b) { + return a->key.name < b->key.name; +} + bool Style::Equals(const Value* value) const { const Style* other = ValueCast<Style>(value); if (!other) { return false; } + if (bool(parent) != bool(other->parent) || - (parent && other->parent && - !parent.value().Equals(&other->parent.value()))) { + (parent && other->parent && !parent.value().Equals(&other->parent.value()))) { return false; } @@ -603,26 +649,15 @@ bool Style::Equals(const Value* value) const { return false; } - std::vector<const Entry*> sorted_a; - std::transform(entries.begin(), entries.end(), std::back_inserter(sorted_a), - add_pointer<const Entry>); - std::sort(sorted_a.begin(), sorted_a.end(), - [](const Entry* a, const Entry* b) -> bool { - return a->key.name < b->key.name; - }); + std::vector<const Entry*> sorted_a = ToPointerVec(entries); + std::sort(sorted_a.begin(), sorted_a.end(), KeyNameComparator); - std::vector<const Entry*> sorted_b; - std::transform(other->entries.begin(), other->entries.end(), - std::back_inserter(sorted_b), add_pointer<const Entry>); - std::sort(sorted_b.begin(), sorted_b.end(), - [](const Entry* a, const Entry* b) -> bool { - return a->key.name < b->key.name; - }); + std::vector<const Entry*> sorted_b = ToPointerVec(other->entries); + std::sort(sorted_b.begin(), sorted_b.end(), KeyNameComparator); return std::equal(sorted_a.begin(), sorted_a.end(), sorted_b.begin(), [](const Entry* a, const Entry* b) -> bool { - return a->key.Equals(&b->key) && - a->value->Equals(b->value.get()); + return a->key.Equals(&b->key) && a->value->Equals(b->value.get()); }); } @@ -633,8 +668,7 @@ Style* Style::Clone(StringPool* new_pool) const { style->comment_ = comment_; style->source_ = source_; for (auto& entry : entries) { - style->entries.push_back( - Entry{entry.key, std::unique_ptr<Item>(entry.value->Clone(new_pool))}); + style->entries.push_back(Entry{entry.key, std::unique_ptr<Item>(entry.value->Clone(new_pool))}); } return style; } @@ -642,26 +676,73 @@ Style* Style::Clone(StringPool* new_pool) const { void Style::Print(std::ostream* out) const { *out << "(style) "; if (parent && parent.value().name) { - if (parent.value().private_reference) { + const Reference& parent_ref = parent.value(); + if (parent_ref.private_reference) { *out << "*"; } - *out << parent.value().name.value(); + *out << parent_ref.name.value(); } *out << " [" << util::Joiner(entries, ", ") << "]"; } -static ::std::ostream& operator<<(::std::ostream& out, - const Style::Entry& value) { - if (value.key.name) { - out << value.key.name.value(); - } else if (value.key.id) { - out << value.key.id.value(); - } else { - out << "???"; +Style::Entry CloneEntry(const Style::Entry& entry, StringPool* pool) { + Style::Entry cloned_entry{entry.key}; + if (entry.value != nullptr) { + cloned_entry.value.reset(entry.value->Clone(pool)); } - out << " = "; - value.value->Print(&out); - return out; + return cloned_entry; +} + +void Style::MergeWith(Style* other, StringPool* pool) { + if (other->parent) { + parent = other->parent; + } + + // We can't assume that the entries are sorted alphabetically since they're supposed to be + // sorted by Resource Id. Not all Resource Ids may be set though, so we can't sort and merge + // them keying off that. + // + // Instead, sort the entries of each Style by their name in a separate structure. Then merge + // those. + + std::vector<Entry*> this_sorted = ToPointerVec(entries); + std::sort(this_sorted.begin(), this_sorted.end(), KeyNameComparator); + + std::vector<Entry*> other_sorted = ToPointerVec(other->entries); + std::sort(other_sorted.begin(), other_sorted.end(), KeyNameComparator); + + auto this_iter = this_sorted.begin(); + const auto this_end = this_sorted.end(); + + auto other_iter = other_sorted.begin(); + const auto other_end = other_sorted.end(); + + std::vector<Entry> merged_entries; + while (this_iter != this_end) { + if (other_iter != other_end) { + if ((*this_iter)->key.name < (*other_iter)->key.name) { + merged_entries.push_back(std::move(**this_iter)); + ++this_iter; + } else { + // The other overrides. + merged_entries.push_back(CloneEntry(**other_iter, pool)); + if ((*this_iter)->key.name == (*other_iter)->key.name) { + ++this_iter; + } + ++other_iter; + } + } else { + merged_entries.push_back(std::move(**this_iter)); + ++this_iter; + } + } + + while (other_iter != other_end) { + merged_entries.push_back(CloneEntry(**other_iter, pool)); + ++other_iter; + } + + entries = std::move(merged_entries); } bool Array::Equals(const Value* value) const { @@ -758,11 +839,6 @@ void Plural::Print(std::ostream* out) const { } } -static ::std::ostream& operator<<(::std::ostream& out, - const std::unique_ptr<Item>& item) { - return out << *item; -} - bool Styleable::Equals(const Value* value) const { const Styleable* other = ValueCast<Styleable>(value); if (!other) { @@ -810,8 +886,7 @@ struct NameOnlyComparator { void Styleable::MergeWith(Styleable* other) { // Compare only names, because some References may already have their IDs - // assigned - // (framework IDs that don't change). + // assigned (framework IDs that don't change). std::set<Reference, NameOnlyComparator> references; references.insert(entries.begin(), entries.end()); references.insert(other->entries.begin(), other->entries.end()); diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h index 06f949f9555c..ac5795fb9774 100644 --- a/tools/aapt2/ResourceValues.h +++ b/tools/aapt2/ResourceValues.h @@ -40,7 +40,8 @@ struct RawValueVisitor; // type specific operations is to check the Value's type() and // cast it to the appropriate subclass. This isn't super clean, // but it is the simplest strategy. -struct Value { +class Value { + public: virtual ~Value() = default; // Whether this value is weak and can be overridden without warning or error. Default is false. @@ -82,6 +83,8 @@ struct Value { // Human readable printout of this value. virtual void Print(std::ostream* out) const = 0; + friend std::ostream& operator<<(std::ostream& out, const Value& value); + protected: Source source_; std::string comment_; @@ -245,6 +248,8 @@ struct Attribute : public BaseValue<Attribute> { struct Symbol { Reference symbol; uint32_t value; + + friend std::ostream& operator<<(std::ostream& out, const Symbol& symbol); }; uint32_t type_mask; @@ -266,6 +271,8 @@ struct Style : public BaseValue<Style> { struct Entry { Reference key; std::unique_ptr<Item> value; + + friend std::ostream& operator<<(std::ostream& out, const Entry& entry); }; Maybe<Reference> parent; @@ -278,6 +285,10 @@ struct Style : public BaseValue<Style> { bool Equals(const Value* value) const override; Style* Clone(StringPool* new_pool) const override; void Print(std::ostream* out) const override; + + // Merges `style` into this Style. All identical attributes of `style` take precedence, including + // the parent, if there is one. + void MergeWith(Style* style, StringPool* pool); }; struct Array : public BaseValue<Array> { @@ -307,20 +318,15 @@ struct Styleable : public BaseValue<Styleable> { void MergeWith(Styleable* styleable); }; -// Stream operator for printing Value objects. -inline ::std::ostream& operator<<(::std::ostream& out, const Value& value) { - value.Print(&out); - return out; -} - -inline ::std::ostream& operator<<(::std::ostream& out, - const Attribute::Symbol& s) { - if (s.symbol.name) { - out << s.symbol.name.value().entry; +template <typename T> +typename std::enable_if<std::is_base_of<Value, T>::value, std::ostream&>::type operator<<( + std::ostream& out, const std::unique_ptr<T>& value) { + if (value == nullptr) { + out << "NULL"; } else { - out << "???"; + value->Print(&out); } - return out << "=" << s.value; + return out; } } // namespace aapt diff --git a/tools/aapt2/ResourceValues_test.cpp b/tools/aapt2/ResourceValues_test.cpp index 692258003471..e154d93b6f85 100644 --- a/tools/aapt2/ResourceValues_test.cpp +++ b/tools/aapt2/ResourceValues_test.cpp @@ -148,4 +148,36 @@ TEST(ResourceValuesTest, StyleClone) { EXPECT_TRUE(a->Equals(b.get())); } +TEST(ResourceValuesTest, StyleMerges) { + StringPool pool_a; + StringPool pool_b; + + std::unique_ptr<Style> a = + test::StyleBuilder() + .SetParent("android:style/Parent") + .AddItem("android:attr/a", util::make_unique<String>(pool_a.MakeRef("FooA"))) + .AddItem("android:attr/b", util::make_unique<String>(pool_a.MakeRef("FooB"))) + .Build(); + + std::unique_ptr<Style> b = + test::StyleBuilder() + .SetParent("android:style/OverlayParent") + .AddItem("android:attr/c", util::make_unique<String>(pool_b.MakeRef("OverlayFooC"))) + .AddItem("android:attr/a", util::make_unique<String>(pool_b.MakeRef("OverlayFooA"))) + .Build(); + + a->MergeWith(b.get(), &pool_a); + + StringPool pool; + std::unique_ptr<Style> expected = + test::StyleBuilder() + .SetParent("android:style/OverlayParent") + .AddItem("android:attr/a", util::make_unique<String>(pool.MakeRef("OverlayFooA"))) + .AddItem("android:attr/b", util::make_unique<String>(pool.MakeRef("FooB"))) + .AddItem("android:attr/c", util::make_unique<String>(pool.MakeRef("OverlayFooC"))) + .Build(); + + EXPECT_TRUE(a->Equals(expected.get())); +} + } // namespace aapt diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index 931109125263..14c23efac291 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -179,32 +179,39 @@ static bool MergeEntry(IAaptContext* context, const Source& src, return true; } -/** - * Modified CollisionResolver which will merge Styleables. Used with overlays. - * - * Styleables are not actual resources, but they are treated as such during the - * compilation phase. Styleables don't simply overlay each other, their - * definitions merge - * and accumulate. If both values are Styleables, we just merge them into the - * existing value. - */ -static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, - Value* incoming) { +// Modified CollisionResolver which will merge Styleables and Styles. Used with overlays. +// +// Styleables are not actual resources, but they are treated as such during the +// compilation phase. +// +// Styleables and Styles don't simply overlay each other, their definitions merge +// and accumulate. If both values are Styleables/Styles, we just merge them into the +// existing value. +static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Value* incoming, + StringPool* pool) { if (Styleable* existing_styleable = ValueCast<Styleable>(existing)) { if (Styleable* incoming_styleable = ValueCast<Styleable>(incoming)) { // Styleables get merged. existing_styleable->MergeWith(incoming_styleable); return ResourceTable::CollisionResult::kKeepOriginal; } + } else if (Style* existing_style = ValueCast<Style>(existing)) { + if (Style* incoming_style = ValueCast<Style>(incoming)) { + // Styles get merged. + existing_style->MergeWith(incoming_style, pool); + return ResourceTable::CollisionResult::kKeepOriginal; + } } // Delegate to the default handler. return ResourceTable::ResolveValueCollision(existing, incoming); } -static ResourceTable::CollisionResult MergeConfigValue( - IAaptContext* context, const ResourceNameRef& res_name, const bool overlay, - ResourceConfigValue* dst_config_value, - ResourceConfigValue* src_config_value) { +static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context, + const ResourceNameRef& res_name, + const bool overlay, + ResourceConfigValue* dst_config_value, + ResourceConfigValue* src_config_value, + StringPool* pool) { using CollisionResult = ResourceTable::CollisionResult; Value* dst_value = dst_config_value->value.get(); @@ -212,10 +219,9 @@ static ResourceTable::CollisionResult MergeConfigValue( CollisionResult collision_result; if (overlay) { - collision_result = ResolveMergeCollision(dst_value, src_value); + collision_result = ResolveMergeCollision(dst_value, src_value, pool); } else { - collision_result = - ResourceTable::ResolveValueCollision(dst_value, src_value); + collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value); } if (collision_result == CollisionResult::kConflict) { @@ -224,10 +230,9 @@ static ResourceTable::CollisionResult MergeConfigValue( } // Error! - context->GetDiagnostics()->Error( - DiagMessage(src_value->GetSource()) - << "resource '" << res_name << "' has a conflicting value for " - << "configuration (" << src_config_value->config << ")"); + context->GetDiagnostics()->Error(DiagMessage(src_value->GetSource()) + << "resource '" << res_name << "' has a conflicting value for " + << "configuration (" << src_config_value->config << ")"); context->GetDiagnostics()->Note(DiagMessage(dst_value->GetSource()) << "originally defined here"); return CollisionResult::kConflict; @@ -243,8 +248,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, bool error = false; for (auto& src_type : src_package->types) { - ResourceTableType* dst_type = - master_package_->FindOrCreateType(src_type->type); + ResourceTableType* dst_type = master_package_->FindOrCreateType(src_type->type); if (!MergeType(context_, src, dst_type, src_type.get())) { error = true; continue; @@ -253,8 +257,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, for (auto& src_entry : src_type->entries) { std::string entry_name = src_entry->name; if (mangle_package) { - entry_name = - NameMangler::MangleEntry(src_package->name, src_entry->name); + entry_name = NameMangler::MangleEntry(src_package->name, src_entry->name); } ResourceEntry* dst_entry; @@ -264,16 +267,14 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, dst_entry = dst_type->FindEntry(entry_name); } - const ResourceNameRef res_name(src_package->name, src_type->type, - src_entry->name); + const ResourceNameRef res_name(src_package->name, src_type->type, src_entry->name); if (!dst_entry) { - context_->GetDiagnostics()->Error( - DiagMessage(src) << "resource " << res_name - << " does not override an existing resource"); - context_->GetDiagnostics()->Note( - DiagMessage(src) << "define an <add-resource> tag or use " - << "--auto-add-overlay"); + context_->GetDiagnostics()->Error(DiagMessage(src) + << "resource " << res_name + << " does not override an existing resource"); + context_->GetDiagnostics()->Note(DiagMessage(src) << "define an <add-resource> tag or use " + << "--auto-add-overlay"); error = true; continue; } @@ -291,7 +292,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, if (dst_config_value) { CollisionResult collision_result = MergeConfigValue(context_, res_name, overlay, dst_config_value, - src_config_value.get()); + src_config_value.get(), &master_table_->string_pool); if (collision_result == CollisionResult::kConflict) { error = true; continue; @@ -299,25 +300,22 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table, continue; } } else { - dst_config_value = dst_entry->FindOrCreateValue( - src_config_value->config, src_config_value->product); + dst_config_value = + dst_entry->FindOrCreateValue(src_config_value->config, src_config_value->product); } // Continue if we're taking the new resource. - if (FileReference* f = - ValueCast<FileReference>(src_config_value->value.get())) { + if (FileReference* f = ValueCast<FileReference>(src_config_value->value.get())) { std::unique_ptr<FileReference> new_file_ref; if (mangle_package) { new_file_ref = CloneAndMangleFile(src_package->name, *f); } else { - new_file_ref = std::unique_ptr<FileReference>( - f->Clone(&master_table_->string_pool)); + new_file_ref = std::unique_ptr<FileReference>(f->Clone(&master_table_->string_pool)); } if (callback) { - if (!callback(res_name, src_config_value->config, - new_file_ref.get(), f)) { + if (!callback(res_name, src_config_value->config, new_file_ref.get(), f)) { error = true; continue; } @@ -341,18 +339,15 @@ std::unique_ptr<FileReference> TableMerger::CloneAndMangleFile( std::string mangled_entry = NameMangler::MangleEntry(package, entry.to_string()); std::string newPath = prefix.to_string() + mangled_entry + suffix.to_string(); std::unique_ptr<FileReference> new_file_ref = - util::make_unique<FileReference>( - master_table_->string_pool.MakeRef(newPath)); + util::make_unique<FileReference>(master_table_->string_pool.MakeRef(newPath)); new_file_ref->SetComment(file_ref.GetComment()); new_file_ref->SetSource(file_ref.GetSource()); return new_file_ref; } - return std::unique_ptr<FileReference>( - file_ref.Clone(&master_table_->string_pool)); + return std::unique_ptr<FileReference>(file_ref.Clone(&master_table_->string_pool)); } -bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file, - bool overlay) { +bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file, bool overlay) { ResourceTable table; std::string path = ResourceUtils::BuildResourceFileName(file_desc); std::unique_ptr<FileReference> file_ref = diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp index 742f5a7a570a..3b791b5cf92b 100644 --- a/tools/aapt2/link/TableMerger_test.cpp +++ b/tools/aapt2/link/TableMerger_test.cpp @@ -20,6 +20,14 @@ #include "io/FileSystem.h" #include "test/Test.h" +using ::aapt::test::ValueEq; +using ::testing::Contains; +using ::testing::NotNull; +using ::testing::UnorderedElementsAreArray; +using ::testing::Pointee; +using ::testing::Field; +using ::testing::Eq; + namespace aapt { struct TableMergerTest : public ::testing::Test { @@ -62,26 +70,20 @@ TEST_F(TableMergerTest, SimpleMerge) { io::FileCollection collection; ASSERT_TRUE(merger.Merge({}, table_a.get())); - ASSERT_TRUE( - merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection)); + ASSERT_TRUE(merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection)); EXPECT_TRUE(merger.merged_packages().count("com.app.b") != 0); // Entries from com.app.a should not be mangled. - AAPT_EXPECT_TRUE( - final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo"))); - AAPT_EXPECT_TRUE( - final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar"))); - AAPT_EXPECT_TRUE(final_table.FindResource( - test::ParseNameOrDie("com.app.a:styleable/view"))); + EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo"))); + EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar"))); + EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:styleable/view"))); // The unmangled name should not be present. - AAPT_EXPECT_FALSE( - final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo"))); + EXPECT_FALSE(final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo"))); // Look for the mangled name. - AAPT_EXPECT_TRUE(final_table.FindResource( - test::ParseNameOrDie("com.app.a:id/com.app.b$foo"))); + EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/com.app.b$foo"))); } TEST_F(TableMergerTest, MergeFile) { @@ -100,7 +102,7 @@ TEST_F(TableMergerTest, MergeFile) { FileReference* file = test::GetValueForConfig<FileReference>( &final_table, "com.app.a:layout/main", test::ParseConfigOrDie("hdpi-v4")); - ASSERT_NE(nullptr, file); + ASSERT_THAT(file, NotNull()); EXPECT_EQ(std::string("res/layout-hdpi-v4/main.xml"), *file->path); } @@ -137,17 +139,14 @@ TEST_F(TableMergerTest, MergeFileReferences) { collection.InsertFile("res/xml/file.xml"); ASSERT_TRUE(merger.Merge({}, table_a.get())); - ASSERT_TRUE( - merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection)); + ASSERT_TRUE(merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection)); - FileReference* f = - test::GetValue<FileReference>(&final_table, "com.app.a:xml/file"); - ASSERT_NE(f, nullptr); + FileReference* f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/file"); + ASSERT_THAT(f, NotNull()); EXPECT_EQ(std::string("res/xml/file.xml"), *f->path); - f = test::GetValue<FileReference>(&final_table, - "com.app.a:xml/com.app.b$file"); - ASSERT_NE(f, nullptr); + f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/com.app.b$file"); + ASSERT_THAT(f, NotNull()); EXPECT_EQ(std::string("res/xml/com.app.b$file.xml"), *f->path); } @@ -171,10 +170,9 @@ TEST_F(TableMergerTest, OverrideResourceWithOverlay) { ASSERT_TRUE(merger.Merge({}, base.get())); ASSERT_TRUE(merger.MergeOverlay({}, overlay.get())); - BinaryPrimitive* foo = - test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo"); - ASSERT_NE(nullptr, foo); - EXPECT_EQ(0x0u, foo->value.data); + BinaryPrimitive* foo = test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo"); + ASSERT_THAT(foo, + Pointee(Field(&BinaryPrimitive::value, Field(&android::Res_value::data, Eq(0u))))); } TEST_F(TableMergerTest, OverrideSameResourceIdsWithOverlay) { @@ -301,7 +299,7 @@ TEST_F(TableMergerTest, FailToMergeNewResourceWithoutAutoAddOverlay) { ASSERT_FALSE(merger.MergeOverlay({}, table_b.get())); } -TEST_F(TableMergerTest, OverlaidStyleablesShouldBeMerged) { +TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) { std::unique_ptr<ResourceTable> table_a = test::ResourceTableBuilder() .SetPackageId("com.app.a", 0x7f) @@ -310,15 +308,27 @@ TEST_F(TableMergerTest, OverlaidStyleablesShouldBeMerged) { .AddItem("com.app.a:attr/bar") .AddItem("com.app.a:attr/foo", ResourceId(0x01010000)) .Build()) + .AddValue("com.app.a:style/Theme", + test::StyleBuilder() + .SetParent("com.app.a:style/Parent") + .AddItem("com.app.a:attr/bar", util::make_unique<Id>()) + .AddItem("com.app.a:attr/foo", ResourceUtils::MakeBool(false)) + .Build()) .Build(); std::unique_ptr<ResourceTable> table_b = test::ResourceTableBuilder() .SetPackageId("com.app.a", 0x7f) - .AddValue("com.app.a:styleable/Foo", - test::StyleableBuilder() - .AddItem("com.app.a:attr/bat") - .AddItem("com.app.a:attr/foo") + .AddValue("com.app.a:styleable/Foo", test::StyleableBuilder() + .AddItem("com.app.a:attr/bat") + .AddItem("com.app.a:attr/foo") + .Build()) + .AddValue("com.app.a:style/Theme", + test::StyleBuilder() + .SetParent("com.app.a:style/OverlayParent") + .AddItem("com.app.a:attr/bat", util::make_unique<Id>()) + .AddItem("com.app.a:attr/foo", ResourceId(0x01010000), + ResourceUtils::MakeBool(true)) .Build()) .Build(); @@ -330,18 +340,29 @@ TEST_F(TableMergerTest, OverlaidStyleablesShouldBeMerged) { ASSERT_TRUE(merger.Merge({}, table_a.get())); ASSERT_TRUE(merger.MergeOverlay({}, table_b.get())); - Styleable* styleable = - test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo"); - ASSERT_NE(nullptr, styleable); + Styleable* styleable = test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo"); + ASSERT_THAT(styleable, NotNull()); std::vector<Reference> expected_refs = { Reference(test::ParseNameOrDie("com.app.a:attr/bar")), Reference(test::ParseNameOrDie("com.app.a:attr/bat")), - Reference(test::ParseNameOrDie("com.app.a:attr/foo"), - ResourceId(0x01010000)), + Reference(test::ParseNameOrDie("com.app.a:attr/foo"), ResourceId(0x01010000)), }; + EXPECT_THAT(styleable->entries, UnorderedElementsAreArray(expected_refs)); + + Style* style = test::GetValue<Style>(&final_table, "com.app.a:style/Theme"); + ASSERT_THAT(style, NotNull()); + + std::vector<Reference> extracted_refs; + for (const auto& entry : style->entries) { + extracted_refs.push_back(entry.key); + } + EXPECT_THAT(extracted_refs, UnorderedElementsAreArray(expected_refs)); - EXPECT_EQ(expected_refs, styleable->entries); + const auto expected = ResourceUtils::MakeBool(true); + EXPECT_THAT(style->entries, Contains(Field(&Style::Entry::value, Pointee(ValueEq(*expected))))); + EXPECT_THAT(style->parent, + Eq(make_value(Reference(test::ParseNameOrDie("com.app.a:style/OverlayParent"))))); } } // namespace aapt diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index 248921f3afea..a937de8869a6 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -22,12 +22,14 @@ #include "android-base/logging.h" #include "android-base/macros.h" #include "androidfw/StringPiece.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "ConfigDescription.h" #include "Debug.h" #include "ResourceTable.h" #include "ResourceUtils.h" +#include "ResourceValues.h" #include "ValueVisitor.h" #include "io/File.h" #include "process/IResourceTableConsumer.h" @@ -51,13 +53,11 @@ struct DummyDiagnosticsImpl : public IDiagnostics { return; case Level::Warn: - std::cerr << actual_msg.source << ": warn: " << actual_msg.message - << "." << std::endl; + 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; + std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl; break; } } @@ -84,11 +84,9 @@ template <typename T> T* GetValueForConfigAndProduct(ResourceTable* table, const android::StringPiece& res_name, const ConfigDescription& config, const android::StringPiece& product) { - Maybe<ResourceTable::SearchResult> result = - table->FindResource(ParseNameOrDie(res_name)); + Maybe<ResourceTable::SearchResult> result = table->FindResource(ParseNameOrDie(res_name)); if (result) { - ResourceConfigValue* config_value = - result.value().entry->FindValue(config, product); + ResourceConfigValue* config_value = result.value().entry->FindValue(config, product); if (config_value) { return ValueCast<T>(config_value->value.get()); } @@ -111,9 +109,13 @@ class TestFile : public io::IFile { public: explicit TestFile(const android::StringPiece& path) : source_(path) {} - std::unique_ptr<io::IData> OpenAsData() override { return {}; } + std::unique_ptr<io::IData> OpenAsData() override { + return {}; + } - const Source& GetSource() const override { return source_; } + const Source& GetSource() const override { + return source_; + } private: DISALLOW_COPY_AND_ASSIGN(TestFile); @@ -122,6 +124,47 @@ class TestFile : public io::IFile { }; } // namespace test + +// Workaround gtest bug (https://github.com/google/googletest/issues/443) +// that does not select base class operator<< for derived class T. +template <typename T> +typename std::enable_if<std::is_base_of<Value, T>::value, std::ostream&>::type operator<<( + std::ostream& out, const T& value) { + value.Print(&out); + return out; +} + +template std::ostream& operator<<<Item>(std::ostream&, const Item&); +template std::ostream& operator<<<Reference>(std::ostream&, const Reference&); +template std::ostream& operator<<<Id>(std::ostream&, const Id&); +template std::ostream& operator<<<RawString>(std::ostream&, const RawString&); +template std::ostream& operator<<<String>(std::ostream&, const String&); +template std::ostream& operator<<<StyledString>(std::ostream&, const StyledString&); +template std::ostream& operator<<<FileReference>(std::ostream&, const FileReference&); +template std::ostream& operator<<<BinaryPrimitive>(std::ostream&, const BinaryPrimitive&); +template std::ostream& operator<<<Attribute>(std::ostream&, const Attribute&); +template std::ostream& operator<<<Style>(std::ostream&, const Style&); +template std::ostream& operator<<<Array>(std::ostream&, const Array&); +template std::ostream& operator<<<Plural>(std::ostream&, const Plural&); + +// Add a print method to Maybe. +template <typename T> +void PrintTo(const Maybe<T>& value, std::ostream* out) { + if (value) { + *out << ::testing::PrintToString(value.value()); + } else { + *out << "Nothing"; + } +} + +namespace test { + +MATCHER_P(ValueEq, a, + std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) { + return arg.Equals(&a); +} + +} // namespace test } // namespace aapt #endif /* AAPT_TEST_COMMON_H */ diff --git a/tools/aapt2/test/Test.h b/tools/aapt2/test/Test.h index ec07432fa51e..a24c01cd4137 100644 --- a/tools/aapt2/test/Test.h +++ b/tools/aapt2/test/Test.h @@ -17,6 +17,7 @@ #ifndef AAPT_TEST_TEST_H #define AAPT_TEST_TEST_H +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "test/Builders.h" |