diff --git a/tools/aapt2/java/ClassDefinition.cpp b/tools/aapt2/java/ClassDefinition.cpp index 45130a4d76812..6ad0dd68cb6ae 100644 --- a/tools/aapt2/java/ClassDefinition.cpp +++ b/tools/aapt2/java/ClassDefinition.cpp @@ -39,6 +39,17 @@ void MethodDefinition::WriteToStream(const StringPiece& prefix, bool final, *out << prefix << "}"; } +ClassDefinition::Result ClassDefinition::AddMember(std::unique_ptr member) { + Result result = Result::kAdded; + auto iter = members_.find(member); + if (iter != members_.end()) { + members_.erase(iter); + result = Result::kOverridden; + } + members_.insert(std::move(member)); + return result; +} + bool ClassDefinition::empty() const { for (const std::unique_ptr& member : members_) { if (!member->empty()) { diff --git a/tools/aapt2/java/ClassDefinition.h b/tools/aapt2/java/ClassDefinition.h index ca76421390d6a..6c4bcad83d2ab 100644 --- a/tools/aapt2/java/ClassDefinition.h +++ b/tools/aapt2/java/ClassDefinition.h @@ -18,6 +18,7 @@ #define AAPT_JAVA_CLASSDEFINITION_H #include +#include #include #include "android-base/macros.h" @@ -37,10 +38,14 @@ class ClassMember { public: virtual ~ClassMember() = default; - AnnotationProcessor* GetCommentBuilder() { return &processor_; } + AnnotationProcessor* GetCommentBuilder() { + return &processor_; + } virtual bool empty() const = 0; + virtual const std::string& GetName() const = 0; + // Writes the class member to the out stream. Subclasses should derive this method // to write their own data. Call this base method from the subclass to write out // this member's comments/annotations. @@ -57,7 +62,13 @@ class PrimitiveMember : public ClassMember { PrimitiveMember(const android::StringPiece& name, const T& val) : name_(name.to_string()), val_(val) {} - bool empty() const override { return false; } + bool empty() const override { + return false; + } + + const std::string& GetName() const override { + return name_; + } void WriteToStream(const android::StringPiece& prefix, bool final, std::ostream* out) const override { @@ -83,7 +94,13 @@ class PrimitiveMember : public ClassMember { PrimitiveMember(const android::StringPiece& name, const std::string& val) : name_(name.to_string()), val_(val) {} - bool empty() const override { return false; } + bool empty() const override { + return false; + } + + const std::string& GetName() const override { + return name_; + } void WriteToStream(const android::StringPiece& prefix, bool final, std::ostream* out) const override { @@ -109,9 +126,17 @@ class PrimitiveArrayMember : public ClassMember { public: explicit PrimitiveArrayMember(const android::StringPiece& name) : name_(name.to_string()) {} - void AddElement(const T& val) { elements_.push_back(val); } + void AddElement(const T& val) { + elements_.push_back(val); + } - bool empty() const override { return false; } + bool empty() const override { + return false; + } + + const std::string& GetName() const override { + return name_; + } void WriteToStream(const android::StringPiece& prefix, bool final, std::ostream* out) const override { @@ -154,6 +179,11 @@ class MethodDefinition : public ClassMember { // formatting may be broken. void AppendStatement(const android::StringPiece& statement); + // Not quite the same as a name, but good enough. + const std::string& GetName() const override { + return signature_; + } + // Even if the method is empty, we always want to write the method signature. bool empty() const override { return false; } @@ -175,19 +205,34 @@ class ClassDefinition : public ClassMember { ClassDefinition(const android::StringPiece& name, ClassQualifier qualifier, bool createIfEmpty) : name_(name.to_string()), qualifier_(qualifier), create_if_empty_(createIfEmpty) {} - void AddMember(std::unique_ptr member) { - members_.push_back(std::move(member)); - } + enum class Result { + kAdded, + kOverridden, + }; + + Result AddMember(std::unique_ptr member); bool empty() const override; + + const std::string& GetName() const override { + return name_; + } + void WriteToStream(const android::StringPiece& prefix, bool final, 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(); + } + }; + std::string name_; ClassQualifier qualifier_; bool create_if_empty_; - std::vector> members_; + std::set, ClassMemberCompare> members_; DISALLOW_COPY_AND_ASSIGN(ClassDefinition); }; diff --git a/tools/aapt2/java/ManifestClassGenerator.cpp b/tools/aapt2/java/ManifestClassGenerator.cpp index 4ef32c92dd848..c4b36176aa71a 100644 --- a/tools/aapt2/java/ManifestClassGenerator.cpp +++ b/tools/aapt2/java/ManifestClassGenerator.cpp @@ -68,7 +68,10 @@ static bool WriteSymbol(const Source& source, IDiagnostics* diag, xml::Element* util::make_unique(result.value(), attr->value); string_member->GetCommentBuilder()->AppendComment(el->comment); - class_def->AddMember(std::move(string_member)); + if (class_def->AddMember(std::move(string_member)) == ClassDefinition::Result::kOverridden) { + diag->Warn(DiagMessage(source.WithLine(el->line_number)) + << "duplicate definitions of '" << result.value() << "', overriding previous"); + } return true; } diff --git a/tools/aapt2/java/ManifestClassGenerator_test.cpp b/tools/aapt2/java/ManifestClassGenerator_test.cpp index c744e9bd4fc9b..ada563409d19b 100644 --- a/tools/aapt2/java/ManifestClassGenerator_test.cpp +++ b/tools/aapt2/java/ManifestClassGenerator_test.cpp @@ -121,6 +121,21 @@ TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) { EXPECT_THAT(actual, HasSubstr(expected_test)); } +// This is bad but part of public API behaviour so we need to preserve it. +TEST(ManifestClassGeneratorTest, LastSeenPermissionWithSameLeafNameTakesPrecedence) { + std::unique_ptr context = test::ContextBuilder().Build(); + std::unique_ptr manifest = test::BuildXmlDom(R"( + + + + )"); + + std::string actual; + ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual)); + EXPECT_THAT(actual, HasSubstr("ACCESS_INTERNET=\"com.android.aapt.test.ACCESS_INTERNET\";")); + EXPECT_THAT(actual, Not(HasSubstr("ACCESS_INTERNET=\"android.permission.ACCESS_INTERNET\";"))); +} + static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res, std::string* out_str) { std::unique_ptr manifest_class =