diff --git a/tools/aapt2/java/ClassDefinition.cpp b/tools/aapt2/java/ClassDefinition.cpp index d7508d257db46..c139b73db2963 100644 --- a/tools/aapt2/java/ClassDefinition.cpp +++ b/tools/aapt2/java/ClassDefinition.cpp @@ -41,18 +41,21 @@ void MethodDefinition::WriteToStream(const StringPiece& prefix, bool final, ClassDefinition::Result ClassDefinition::AddMember(std::unique_ptr member) { Result result = Result::kAdded; - auto iter = members_.find(member); - if (iter != members_.end()) { - members_.erase(iter); + auto iter = indexed_members_.find(member->GetName()); + if (iter != indexed_members_.end()) { + // Overwrite the entry. + ordered_members_[iter->second].reset(); result = Result::kOverridden; } - members_.insert(std::move(member)); + + indexed_members_[member->GetName()] = ordered_members_.size(); + ordered_members_.push_back(std::move(member)); return result; } bool ClassDefinition::empty() const { - for (const std::unique_ptr& member : members_) { - if (!member->empty()) { + for (const std::unique_ptr& member : ordered_members_) { + if (member != nullptr && !member->empty()) { return false; } } @@ -61,7 +64,7 @@ bool ClassDefinition::empty() const { void ClassDefinition::WriteToStream(const StringPiece& prefix, bool final, std::ostream* out) const { - if (members_.empty() && !create_if_empty_) { + if (empty() && !create_if_empty_) { return; } @@ -76,9 +79,14 @@ void ClassDefinition::WriteToStream(const StringPiece& prefix, bool final, std::string new_prefix = prefix.to_string(); new_prefix.append(kIndent); - for (const std::unique_ptr& member : members_) { - member->WriteToStream(new_prefix, final, out); - *out << "\n"; + for (const std::unique_ptr& member : ordered_members_) { + // There can be nullptr members when a member is added to the ClassDefinition + // and takes precedence over a previous member with the same name. The overridden member is + // set to nullptr. + if (member != nullptr) { + member->WriteToStream(new_prefix, final, out); + *out << "\n"; + } } *out << prefix << "}"; diff --git a/tools/aapt2/java/ClassDefinition.h b/tools/aapt2/java/ClassDefinition.h index 6c4bcad83d2ab..28a3489e71a43 100644 --- a/tools/aapt2/java/ClassDefinition.h +++ b/tools/aapt2/java/ClassDefinition.h @@ -18,8 +18,9 @@ #define AAPT_JAVA_CLASSDEFINITION_H #include -#include #include +#include +#include #include "android-base/macros.h" #include "androidfw/StringPiece.h" @@ -73,21 +74,18 @@ class PrimitiveMember : public ClassMember { void WriteToStream(const android::StringPiece& prefix, bool final, std::ostream* out) const override { ClassMember::WriteToStream(prefix, final, out); - - *out << prefix << "public static " << (final ? "final " : "") << "int " - << name_ << "=" << val_ << ";"; + *out << prefix << "public static " << (final ? "final " : "") << "int " << name_ << "=" << val_ + << ";"; } private: + DISALLOW_COPY_AND_ASSIGN(PrimitiveMember); + std::string name_; T val_; - - DISALLOW_COPY_AND_ASSIGN(PrimitiveMember); }; -/** - * Specialization for strings so they get the right type and are quoted with "". - */ +// Specialization for strings so they get the right type and are quoted with "". template <> class PrimitiveMember : public ClassMember { public: @@ -111,10 +109,10 @@ class PrimitiveMember : public ClassMember { } private: + DISALLOW_COPY_AND_ASSIGN(PrimitiveMember); + std::string name_; std::string val_; - - DISALLOW_COPY_AND_ASSIGN(PrimitiveMember); }; using IntMember = PrimitiveMember; @@ -160,10 +158,10 @@ class PrimitiveArrayMember : public ClassMember { } private: + DISALLOW_COPY_AND_ASSIGN(PrimitiveArrayMember); + std::string name_; std::vector elements_; - - DISALLOW_COPY_AND_ASSIGN(PrimitiveArrayMember); }; using ResourceArrayMember = PrimitiveArrayMember; @@ -185,12 +183,16 @@ class MethodDefinition : public ClassMember { } // Even if the method is empty, we always want to write the method signature. - bool empty() const override { return false; } + bool empty() const override { + return false; + } void WriteToStream(const android::StringPiece& prefix, bool final, std::ostream* out) const override; private: + DISALLOW_COPY_AND_ASSIGN(MethodDefinition); + std::string signature_; std::vector statements_; }; @@ -222,19 +224,13 @@ class ClassDefinition : public ClassMember { std::ostream* out) const override; private: - struct ClassMemberCompare { - using T = std::unique_ptr; - bool operator()(const T& a, const T& b) const { - return a->GetName() < b->GetName(); - } - }; + DISALLOW_COPY_AND_ASSIGN(ClassDefinition); std::string name_; ClassQualifier qualifier_; bool create_if_empty_; - std::set, ClassMemberCompare> members_; - - DISALLOW_COPY_AND_ASSIGN(ClassDefinition); + std::vector> ordered_members_; + std::unordered_map indexed_members_; }; } // namespace aapt diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 271279ff5e92a..84bf04134ad9c 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -23,6 +23,8 @@ #include "util/Util.h" using android::StringPiece; +using ::testing::Lt; +using ::testing::Ne; namespace aapt { @@ -329,6 +331,53 @@ TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) EXPECT_NE(std::string::npos, actual.find(styleable.GetComment().data())); } +TEST(JavaClassGeneratorTest, StyleableAndIndicesAreColocated) { + std::unique_ptr table = + test::ResourceTableBuilder() + .SetPackageId("android", 0x01) + .AddValue("android:attr/layout_gravity", util::make_unique()) + .AddValue("android:attr/background", util::make_unique()) + .AddValue("android:styleable/ActionBar", + test::StyleableBuilder() + .AddItem("android:attr/background", ResourceId(0x01010000)) + .Build()) + .AddValue("android:styleable/ActionBar.LayoutParams", + test::StyleableBuilder() + .AddItem("android:attr/layout_gravity", ResourceId(0x01010001)) + .Build()) + .Build(); + + std::unique_ptr context = + test::ContextBuilder() + .AddSymbolSource(util::make_unique(table.get())) + .SetNameManglerPolicy(NameManglerPolicy{"android"}) + .Build(); + + JavaClassGeneratorOptions options; + JavaClassGenerator generator(context.get(), table.get(), {}); + std::stringstream out; + ASSERT_TRUE(generator.Generate("android", &out)); + std::string output = out.str(); + + std::string::size_type actionbar_pos = output.find("int[] ActionBar"); + ASSERT_THAT(actionbar_pos, Ne(std::string::npos)); + + std::string::size_type actionbar_background_pos = output.find("int ActionBar_background"); + ASSERT_THAT(actionbar_background_pos, Ne(std::string::npos)); + + std::string::size_type actionbar_layout_params_pos = output.find("int[] ActionBar_LayoutParams"); + ASSERT_THAT(actionbar_layout_params_pos, Ne(std::string::npos)); + + std::string::size_type actionbar_layout_params_layout_gravity_pos = + output.find("int ActionBar_LayoutParams_layout_gravity"); + ASSERT_THAT(actionbar_layout_params_layout_gravity_pos, Ne(std::string::npos)); + + EXPECT_THAT(actionbar_pos, Lt(actionbar_background_pos)); + EXPECT_THAT(actionbar_pos, Lt(actionbar_layout_params_pos)); + EXPECT_THAT(actionbar_background_pos, Lt(actionbar_layout_params_pos)); + EXPECT_THAT(actionbar_layout_params_pos, Lt(actionbar_layout_params_layout_gravity_pos)); +} + TEST(JavaClassGeneratorTest, CommentsForRemovedAttributesAreNotPresentInClass) { Attribute attr(false); attr.SetComment(StringPiece("removed"));