[aapt2] Ignore annotations in flag/enum attribute values

Bug: 258855262
Test: atest aapt2_tests
Change-Id: If67a73b02da719b049c19cee561a2f9b8945956b

7 files changed, 123 insertions, 27 deletions

diff --git a/tools/aapt2/java/AnnotationProcessor.cpp b/tools/aapt2/java/AnnotationProcessor.cpp
index 8c644cf83339..a7f6f5524e21 100644
--- a/tools/aapt2/java/AnnotationProcessor.cpp
+++ b/tools/aapt2/java/AnnotationProcessor.cpp
@@ -64,29 +64,31 @@ static std::array<AnnotationRule, 3> sAnnotationRules = {{
     {"@FlaggedApi", AnnotationRule::kFlaggedApi, "@android.annotation.FlaggedApi", true},
 }};
 
-void AnnotationProcessor::AppendCommentLine(std::string comment) {
+void AnnotationProcessor::AppendCommentLine(std::string comment, bool add_api_annotations) {
   static constexpr std::string_view sDeprecated = "@deprecated";
 
-  // Treat deprecated specially, since we don't remove it from the source comment.
-  if (comment.find(sDeprecated) != std::string::npos) {
-    annotation_parameter_map_[AnnotationRule::kDeprecated] = "";
-  }
+  if (add_api_annotations) {
+    // Treat deprecated specially, since we don't remove it from the source comment.
+    if (comment.find(sDeprecated) != std::string::npos) {
+      annotation_parameter_map_[AnnotationRule::kDeprecated] = "";
+    }
 
-  for (const AnnotationRule& rule : sAnnotationRules) {
-    std::string::size_type idx = comment.find(rule.doc_str.data());
-    if (idx != std::string::npos) {
-      // Captures all parameters associated with the specified annotation rule
-      // by matching the first pair of parentheses after the rule.
-      std::regex re(std::string(rule.doc_str).append(R"(\s*\((.+)\))"));
-      std::smatch match_result;
-      const bool is_match = std::regex_search(comment, match_result, re);
-      if (is_match && rule.preserve_params) {
-        annotation_parameter_map_[rule.bit_mask] = match_result[1].str();
-        comment.erase(comment.begin() + match_result.position(),
-                      comment.begin() + match_result.position() + match_result.length());
-      } else {
-        annotation_parameter_map_[rule.bit_mask] = "";
-        comment.erase(comment.begin() + idx, comment.begin() + idx + rule.doc_str.size());
+    for (const AnnotationRule& rule : sAnnotationRules) {
+      std::string::size_type idx = comment.find(rule.doc_str.data());
+      if (idx != std::string::npos) {
+        // Captures all parameters associated with the specified annotation rule
+        // by matching the first pair of parentheses after the rule.
+        std::regex re(std::string(rule.doc_str).append(R"(\s*\((.+)\))"));
+        std::smatch match_result;
+        const bool is_match = std::regex_search(comment, match_result, re);
+        if (is_match && rule.preserve_params) {
+          annotation_parameter_map_[rule.bit_mask] = match_result[1].str();
+          comment.erase(comment.begin() + match_result.position(),
+                        comment.begin() + match_result.position() + match_result.length());
+        } else {
+          annotation_parameter_map_[rule.bit_mask] = "";
+          comment.erase(comment.begin() + idx, comment.begin() + idx + rule.doc_str.size());
+        }
       }
     }
   }
@@ -109,12 +111,12 @@ void AnnotationProcessor::AppendCommentLine(std::string comment) {
   comment_ << "\n * " << std::move(comment);
 }
 
-void AnnotationProcessor::AppendComment(StringPiece comment) {
+void AnnotationProcessor::AppendComment(StringPiece comment, bool add_api_annotations) {
   // We need to process line by line to clean-up whitespace and append prefixes.
   for (StringPiece line : util::Tokenize(comment, '\n')) {
     line = util::TrimWhitespace(line);
     if (!line.empty()) {
-      AppendCommentLine(std::string(line));
+      AppendCommentLine(std::string(line), add_api_annotations);
     }
   }
 }
diff --git a/tools/aapt2/java/AnnotationProcessor.h b/tools/aapt2/java/AnnotationProcessor.h
index db3437e3b5b1..2217ab35705a 100644
--- a/tools/aapt2/java/AnnotationProcessor.h
+++ b/tools/aapt2/java/AnnotationProcessor.h
@@ -60,7 +60,10 @@ class AnnotationProcessor {
 
   // Adds more comments. Resources can have value definitions for various configurations, and
   // each of the definitions may have comments that need to be processed.
-  void AppendComment(android::StringPiece comment);
+  //
+  // If add_api_annotations is false, annotations found in the comment (e.g., "@SystemApi")
+  // will NOT be converted to Java annotations.
+  void AppendComment(android::StringPiece comment, bool add_api_annotations = true);
 
   void AppendNewLine();
 
@@ -73,7 +76,7 @@ class AnnotationProcessor {
   bool has_comments_ = false;
   std::unordered_map<uint32_t, std::string> annotation_parameter_map_;
 
-  void AppendCommentLine(std::string line);
+  void AppendCommentLine(std::string line, bool add_api_annotations);
 };
 
 }  // namespace aapt
diff --git a/tools/aapt2/java/AnnotationProcessor_test.cpp b/tools/aapt2/java/AnnotationProcessor_test.cpp
index e98e96ba3bc3..e5eee34f451c 100644
--- a/tools/aapt2/java/AnnotationProcessor_test.cpp
+++ b/tools/aapt2/java/AnnotationProcessor_test.cpp
@@ -136,7 +136,28 @@ TEST(AnnotationProcessorTest, NotEmitSystemApiAnnotation) {
   EXPECT_THAT(annotations, HasSubstr("This is a system API"));
 }
 
-TEST(AnnotationProcessor, ExtractsFirstSentence) {
+TEST(AnnotationProcessorTest, DoNotAddApiAnnotations) {
+  AnnotationProcessor processor;
+  processor.AppendComment(
+      "@SystemApi This is a system API\n"
+      "@FlaggedApi This is a flagged API\n"
+      "@TestApi This is a test API\n"
+      "@deprecated Deprecate me\n", /*add_api_annotations=*/
+      false);
+
+  std::string annotations;
+  StringOutputStream out(&annotations);
+  Printer printer(&out);
+  processor.Print(&printer);
+  out.Flush();
+
+  EXPECT_THAT(annotations, Not(HasSubstr("@android.annotation.SystemApi")));
+  EXPECT_THAT(annotations, Not(HasSubstr("@android.annotation.FlaggedApi")));
+  EXPECT_THAT(annotations, Not(HasSubstr("@android.annotation.TestApi")));
+  EXPECT_THAT(annotations, Not(HasSubstr("@Deprecated")));
+}
+
+TEST(AnnotationProcessorTest, ExtractsFirstSentence) {
   EXPECT_THAT(AnnotationProcessor::ExtractFirstSentence("This is the only sentence"),
               Eq("This is the only sentence"));
   EXPECT_THAT(AnnotationProcessor::ExtractFirstSentence(
diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp
index 58f656458177..6e73b017cce2 100644
--- a/tools/aapt2/java/JavaClassGenerator.cpp
+++ b/tools/aapt2/java/JavaClassGenerator.cpp
@@ -180,7 +180,10 @@ static void AddAttributeFormatDoc(AnnotationProcessor* processor, Attribute* att
            << "<td>" << std::hex << symbol.value << std::dec << "</td>"
            << "<td>" << util::TrimWhitespace(symbol.symbol.GetComment())
            << "</td></tr>";
-      processor->AppendComment(line.str());
+      // add_api_annotations is false since we don't want any annotations
+      // (e.g., "@deprecated")/ found in the enum/flag values to be propagated
+      // up to the attribute.
+      processor->AppendComment(line.str(), /*add_api_annotations=*/false);
     }
     processor->AppendComment("</table>");
   }
diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp
index 40395ed64fe3..bca9f4bc58c4 100644
--- a/tools/aapt2/java/JavaClassGenerator_test.cpp
+++ b/tools/aapt2/java/JavaClassGenerator_test.cpp
@@ -324,7 +324,58 @@ TEST(JavaClassGeneratorTest, CommentsForSimpleResourcesArePresent) {
   EXPECT_THAT(output, HasSubstr(expected_text));
 }
 
-TEST(JavaClassGeneratorTest, CommentsForEnumAndFlagAttributesArePresent) {}
+TEST(JavaClassGeneratorTest, CommentsForEnumAndFlagAttributesArePresent) {
+  std::unique_ptr<Attribute> flagAttr =
+      test::AttributeBuilder()
+          .SetTypeMask(android::ResTable_map::TYPE_FLAGS)
+          .SetComment("Flag attribute")
+          .AddItemWithComment("flagOne", 0x01, "Flag comment 1")
+          .AddItemWithComment("flagTwo", 0x02, "@deprecated Flag comment 2")
+          .Build();
+  std::unique_ptr<Attribute> enumAttr =
+      test::AttributeBuilder()
+          .SetTypeMask(android::ResTable_map::TYPE_ENUM)
+          .SetComment("Enum attribute")
+          .AddItemWithComment("enumOne", 0x01, "@TestApi Enum comment 1")
+          .AddItemWithComment("enumTwo", 0x02, "Enum comment 2")
+          .Build();
+
+  std::unique_ptr<ResourceTable> table = test::ResourceTableBuilder()
+                                             .AddValue("android:attr/one", std::move(flagAttr))
+                                             .AddValue("android:attr/two", std::move(enumAttr))
+                                             .Build();
+
+  std::unique_ptr<IAaptContext> context =
+      test::ContextBuilder()
+          .AddSymbolSource(util::make_unique<ResourceTableSymbolSource>(table.get()))
+          .SetNameManglerPolicy(NameManglerPolicy{"android"})
+          .Build();
+  JavaClassGeneratorOptions options;
+  options.use_final = false;
+  JavaClassGenerator generator(context.get(), table.get(), options);
+
+  std::string output;
+  StringOutputStream out(&output);
+  ASSERT_TRUE(generator.Generate("android", &out));
+  out.Flush();
+
+  // Special annotations from the enum/flag values should NOT generate
+  // annotations for the attribute value.
+  EXPECT_THAT(output, Not(HasSubstr("@Deprecated")));
+  EXPECT_THAT(output, Not(HasSubstr("@android.annotation.TestApi")));
+
+  EXPECT_THAT(output, HasSubstr("Flag attribute"));
+  EXPECT_THAT(output, HasSubstr("flagOne"));
+  EXPECT_THAT(output, HasSubstr("Flag comment 1"));
+  EXPECT_THAT(output, HasSubstr("flagTwo"));
+  EXPECT_THAT(output, HasSubstr("@deprecated Flag comment 2"));
+
+  EXPECT_THAT(output, HasSubstr("Enum attribute"));
+  EXPECT_THAT(output, HasSubstr("enumOne"));
+  EXPECT_THAT(output, HasSubstr("@TestApi Enum comment 1"));
+  EXPECT_THAT(output, HasSubstr("enumTwo"));
+  EXPECT_THAT(output, HasSubstr("Enum comment 2"));
+}
 
 TEST(JavaClassGeneratorTest, CommentsForStyleablesAndNestedAttributesArePresent) {
   Attribute attr;
diff --git a/tools/aapt2/test/Builders.cpp b/tools/aapt2/test/Builders.cpp
index 65f63dc68e54..b5934e40a2a3 100644
--- a/tools/aapt2/test/Builders.cpp
+++ b/tools/aapt2/test/Builders.cpp
@@ -177,12 +177,25 @@ AttributeBuilder& AttributeBuilder::SetWeak(bool weak) {
   return *this;
 }
 
+AttributeBuilder& AttributeBuilder::SetComment(StringPiece comment) {
+  attr_->SetComment(comment);
+  return *this;
+}
+
 AttributeBuilder& AttributeBuilder::AddItem(StringPiece name, uint32_t value) {
   attr_->symbols.push_back(
       Attribute::Symbol{Reference(ResourceName({}, ResourceType::kId, name)), value});
   return *this;
 }
 
+AttributeBuilder& AttributeBuilder::AddItemWithComment(StringPiece name, uint32_t value,
+                                                       StringPiece comment) {
+  Reference ref(ResourceName({}, ResourceType::kId, name));
+  ref.SetComment(comment);
+  attr_->symbols.push_back(Attribute::Symbol{ref, value});
+  return *this;
+}
+
 std::unique_ptr<Attribute> AttributeBuilder::Build() {
   return std::move(attr_);
 }
diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h
index 098535d8526f..9ee44ba6d04c 100644
--- a/tools/aapt2/test/Builders.h
+++ b/tools/aapt2/test/Builders.h
@@ -116,7 +116,10 @@ class AttributeBuilder {
   AttributeBuilder();
   AttributeBuilder& SetTypeMask(uint32_t typeMask);
   AttributeBuilder& SetWeak(bool weak);
+  AttributeBuilder& SetComment(android::StringPiece comment);
   AttributeBuilder& AddItem(android::StringPiece name, uint32_t value);
+  AttributeBuilder& AddItemWithComment(android::StringPiece name, uint32_t value,
+                                       android::StringPiece comment);
   std::unique_ptr<Attribute> Build();
 
  private: