From 1c1544ff96e7f87154c2eff62c1da3aee46b569f Mon Sep 17 00:00:00 2001
From: Narayan Kamath <narayan@google.com>
Date: Thu, 13 Feb 2020 14:33:47 +0000
Subject: [PATCH] aapt2: Preserve annotation parameters for SystemApi.

SystemAPI can now be parameterized. Note that this change preserves
the content of the annotation params without attempting to interpret
them in any way. This means that references to types outside of
java.lang must be fully qualified.

Test: atest aapt2_tests
Bug: 147581540
Change-Id: I0630fb1c7c7a7f5918ff2dca32fb8e078b367751
---
 tools/aapt2/java/AnnotationProcessor.cpp      | 31 +++++++++++++++----
 tools/aapt2/java/AnnotationProcessor.h        |  3 +-
 tools/aapt2/java/AnnotationProcessor_test.cpp | 15 +++++++++
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/tools/aapt2/java/AnnotationProcessor.cpp b/tools/aapt2/java/AnnotationProcessor.cpp
index a4610b2575b9c..cec59e75831da 100644
--- a/tools/aapt2/java/AnnotationProcessor.cpp
+++ b/tools/aapt2/java/AnnotationProcessor.cpp
@@ -18,6 +18,7 @@
 
 #include <algorithm>
 #include <array>
+#include <regex>
 
 #include "text/Unicode.h"
 #include "text/Utf8Iterator.h"
@@ -65,14 +66,26 @@ void AnnotationProcessor::AppendCommentLine(std::string comment) {
 
   // Treat deprecated specially, since we don't remove it from the source comment.
   if (comment.find(sDeprecated) != std::string::npos) {
-    annotation_bit_mask_ |= AnnotationRule::kDeprecated;
+    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) {
-      annotation_bit_mask_ |= rule.bit_mask;
-      comment.erase(comment.begin() + idx, comment.begin() + idx + rule.doc_str.size());
+      // Captures all parameters associated with the specified annotation rule
+      // by matching the first pair of parantheses after the rule.
+      std::regex re(rule.doc_str.to_string() + "\\s*\\((.+)\\)");
+      std::smatch match_result;
+      const bool is_match = std::regex_search(comment, match_result, re);
+      // We currently only capture and preserve parameters for SystemApi.
+      if (is_match && rule.bit_mask == AnnotationRule::kSystemApi) {
+        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());
+      }
     }
   }
 
@@ -119,13 +132,19 @@ void AnnotationProcessor::Print(Printer* printer) const {
     printer->Println(" */");
   }
 
-  if (annotation_bit_mask_ & AnnotationRule::kDeprecated) {
+  if (annotation_parameter_map_.find(AnnotationRule::kDeprecated) !=
+        annotation_parameter_map_.end()) {
     printer->Println("@Deprecated");
   }
 
   for (const AnnotationRule& rule : sAnnotationRules) {
-    if (annotation_bit_mask_ & rule.bit_mask) {
-      printer->Println(rule.annotation);
+    const auto& it = annotation_parameter_map_.find(rule.bit_mask);
+    if (it != annotation_parameter_map_.end()) {
+      printer->Print(rule.annotation);
+      if (!it->second.empty()) {
+        printer->Print("(").Print(it->second).Print(")");
+      }
+      printer->Print("\n");
     }
   }
 }
diff --git a/tools/aapt2/java/AnnotationProcessor.h b/tools/aapt2/java/AnnotationProcessor.h
index ae7bdb0c3ae2a..fdb58468d995a 100644
--- a/tools/aapt2/java/AnnotationProcessor.h
+++ b/tools/aapt2/java/AnnotationProcessor.h
@@ -19,6 +19,7 @@
 
 #include <sstream>
 #include <string>
+#include <unordered_map>
 
 #include "androidfw/StringPiece.h"
 
@@ -70,7 +71,7 @@ class AnnotationProcessor {
   std::stringstream comment_;
   std::stringstream mAnnotations;
   bool has_comments_ = false;
-  uint32_t annotation_bit_mask_ = 0;
+  std::unordered_map<uint32_t, std::string> annotation_parameter_map_;
 
   void AppendCommentLine(std::string line);
 };
diff --git a/tools/aapt2/java/AnnotationProcessor_test.cpp b/tools/aapt2/java/AnnotationProcessor_test.cpp
index 69f49c8b97c3e..7d0a4e9af6321 100644
--- a/tools/aapt2/java/AnnotationProcessor_test.cpp
+++ b/tools/aapt2/java/AnnotationProcessor_test.cpp
@@ -61,6 +61,21 @@ TEST(AnnotationProcessorTest, EmitsSystemApiAnnotationAndRemovesFromComment) {
   EXPECT_THAT(annotations, HasSubstr("This is a system API"));
 }
 
+TEST(AnnotationProcessorTest, EmitsSystemApiAnnotationParamsAndRemovesFromComment) {
+  AnnotationProcessor processor;
+  processor.AppendComment("@SystemApi (p1=k1,p2=k2) This is a system API");
+
+  std::string annotations;
+  StringOutputStream out(&annotations);
+  Printer printer(&out);
+  processor.Print(&printer);
+  out.Flush();
+
+  EXPECT_THAT(annotations, HasSubstr("@android.annotation.SystemApi(p1=k1,p2=k2)"));
+  EXPECT_THAT(annotations, Not(HasSubstr("@SystemApi")));
+  EXPECT_THAT(annotations, HasSubstr("This is a system API"));
+}
+
 TEST(AnnotationProcessorTest, EmitsTestApiAnnotationAndRemovesFromComment) {
   AnnotationProcessor processor;
   processor.AppendComment("@TestApi This is a test API");