Only keep methods with correct signature for more types

- For transition and pathMotion attributes the method must have Context and AttributeSet parameters.
- For actionViewClass and actionProviderClass attributes the constructor must have a single Context parameter.
- For Fragment's class or name attributes the constructor must have zero parameters.

Bug: 37123156
Test: make aapt2_tests
Change-Id: I34017abd182867ba95172835051d114cb2f3b3ac

3 files changed, 53 insertions, 38 deletions

diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp
index 92487f4e6854..63f9dd327c0b 100644
--- a/tools/aapt2/java/ProguardRules.cpp
+++ b/tools/aapt2/java/ProguardRules.cpp
@@ -50,11 +50,11 @@ class BaseVisitor : public xml::Visitor {
         // This is a custom view, let's figure out the class name from this.
         std::string package = maybe_package.value().package + "." + node->name;
         if (util::IsJavaClassName(package)) {
-          AddClass(node->line_number, package);
+          AddClass(node->line_number, package, "...");
         }
       }
     } else if (util::IsJavaClassName(node->name)) {
-      AddClass(node->line_number, node->name);
+      AddClass(node->line_number, node->name, "...");
     }
 
     for (const auto& child : node->children) {
@@ -75,8 +75,10 @@ class BaseVisitor : public xml::Visitor {
   ResourceFile file_;
   KeepSet* keep_set_;
 
-  virtual void AddClass(size_t line_number, const std::string& class_name) {
-    keep_set_->AddConditionalClass({file_.name, file_.source.WithLine(line_number)}, class_name);
+  virtual void AddClass(size_t line_number, const std::string& class_name,
+                        const std::string& ctor_signature) {
+    keep_set_->AddConditionalClass({file_.name, file_.source.WithLine(line_number)},
+        {class_name, ctor_signature});
   }
 
   void AddMethod(size_t line_number, const std::string& method_name,
@@ -106,27 +108,33 @@ class LayoutVisitor : public BaseVisitor {
   }
 
   void Visit(xml::Element* node) override {
-    bool check_class = false;
-    bool check_name = false;
+    bool is_view = false;
+    bool is_fragment = false;
     if (node->namespace_uri.empty()) {
       if (node->name == "view") {
-        check_class = true;
+        is_view = true;
       } else if (node->name == "fragment") {
-        check_class = check_name = true;
+        is_fragment = true;
       }
     } else if (node->namespace_uri == xml::kSchemaAndroid) {
-      check_name = node->name == "fragment";
+      is_fragment = node->name == "fragment";
     }
 
     for (const auto& attr : node->attributes) {
-      if (check_class && attr.namespace_uri.empty() && attr.name == "class" &&
-          util::IsJavaClassName(attr.value)) {
-        AddClass(node->line_number, attr.value);
-      } else if (check_name && attr.namespace_uri == xml::kSchemaAndroid &&
-                 attr.name == "name" && util::IsJavaClassName(attr.value)) {
-        AddClass(node->line_number, attr.value);
-      } else if (attr.namespace_uri == xml::kSchemaAndroid &&
-                 attr.name == "onClick") {
+      if (attr.namespace_uri.empty() && attr.name == "class") {
+        if (util::IsJavaClassName(attr.value)) {
+          if (is_view) {
+            AddClass(node->line_number, attr.value,
+                "android.content.Context, android.util.AttributeSet");
+          } else if (is_fragment) {
+            AddClass(node->line_number, attr.value, "");
+          }
+        }
+      } else if (attr.namespace_uri == xml::kSchemaAndroid && attr.name == "name") {
+        if (is_fragment && util::IsJavaClassName(attr.value)) {
+          AddClass(node->line_number, attr.value, "");
+        }
+      } else if (attr.namespace_uri == xml::kSchemaAndroid && attr.name == "onClick") {
         AddMethod(node->line_number, attr.value, "android.view.View");
       }
     }
@@ -149,7 +157,7 @@ class MenuVisitor : public BaseVisitor {
         if (attr.namespace_uri == xml::kSchemaAndroid) {
           if ((attr.name == "actionViewClass" || attr.name == "actionProviderClass") &&
               util::IsJavaClassName(attr.value)) {
-            AddClass(node->line_number, attr.value);
+            AddClass(node->line_number, attr.value, "android.content.Context");
           } else if (attr.name == "onClick") {
             AddMethod(node->line_number, attr.value, "android.view.MenuItem");
           }
@@ -180,7 +188,7 @@ class XmlResourceVisitor : public BaseVisitor {
       xml::Attribute* attr =
           node->FindAttribute(xml::kSchemaAndroid, "fragment");
       if (attr && util::IsJavaClassName(attr->value)) {
-        AddClass(node->line_number, attr->value);
+        AddClass(node->line_number, attr->value, "");
       }
     }
 
@@ -202,7 +210,7 @@ class NavigationVisitor : public BaseVisitor {
     if (attr != nullptr && !attr->value.empty()) {
       std::string name = (attr->value[0] == '.') ? package_ + attr->value : attr->value;
       if (util::IsJavaClassName(name)) {
-        AddClass(node->line_number, name);
+        AddClass(node->line_number, name, "...");
       }
     }
 
@@ -225,7 +233,8 @@ class TransitionVisitor : public BaseVisitor {
     if (check_class) {
       xml::Attribute* attr = node->FindAttribute({}, "class");
       if (attr && util::IsJavaClassName(attr->value)) {
-        AddClass(node->line_number, attr->value);
+        AddClass(node->line_number, attr->value,
+            "android.content.Context, android.util.AttributeSet");
       }
     }
 
@@ -256,14 +265,14 @@ class ManifestVisitor : public BaseVisitor {
         if (attr) {
           Maybe<std::string> result = util::GetFullyQualifiedClassName(package_, attr->value);
           if (result) {
-            AddClass(node->line_number, result.value());
+            AddClass(node->line_number, result.value(), "");
           }
         }
         attr = node->FindAttribute(xml::kSchemaAndroid, "appComponentFactory");
         if (attr) {
           Maybe<std::string> result = util::GetFullyQualifiedClassName(package_, attr->value);
           if (result) {
-            AddClass(node->line_number, result.value());
+            AddClass(node->line_number, result.value(), "");
           }
         }
         if (main_dex_only_) {
@@ -294,7 +303,7 @@ class ManifestVisitor : public BaseVisitor {
         if (get_name) {
           Maybe<std::string> result = util::GetFullyQualifiedClassName(package_, attr->value);
           if (result) {
-            AddClass(node->line_number, result.value());
+            AddClass(node->line_number, result.value(), "");
           }
         }
       }
@@ -302,7 +311,8 @@ class ManifestVisitor : public BaseVisitor {
     BaseVisitor::Visit(node);
   }
 
-  virtual void AddClass(size_t line_number, const std::string& class_name) override {
+  virtual void AddClass(size_t line_number, const std::string& class_name,
+                        const std::string& ctor_signature) override {
     keep_set_->AddManifestClass({file_.name, file_.source.WithLine(line_number)}, class_name);
   }
 
@@ -390,13 +400,15 @@ void WriteKeepSet(const KeepSet& keep_set, OutputStream* out) {
         printer.Print("-if class **.R$layout { int ")
             .Print(JavaClassGenerator::TransformToFieldName(location.name.entry))
             .Println("; }");
-        printer.Print("-keep class ").Print(entry.first).Println(" { <init>(...); }");
+        printer.Print("-keep class ").Print(entry.first.name).Print(" { <init>(")
+            .Print(entry.first.signature).Println("); }");
       }
     } else {
       for (const UsageLocation& location : entry.second) {
         printer.Print("# Referenced at ").Println(location.source.to_string());
       }
-      printer.Print("-keep class ").Print(entry.first).Println(" { <init>(...); }");
+      printer.Print("-keep class ").Print(entry.first.name).Print(" { <init>(")
+          .Print(entry.first.signature).Println("); }");
     }
     printer.Println();
   }
diff --git a/tools/aapt2/java/ProguardRules.h b/tools/aapt2/java/ProguardRules.h
index acaceac41237..01dad0b08aea 100644
--- a/tools/aapt2/java/ProguardRules.h
+++ b/tools/aapt2/java/ProguardRules.h
@@ -56,8 +56,9 @@ class KeepSet {
     manifest_class_set_[class_name].insert(file);
   }
 
-  inline void AddConditionalClass(const UsageLocation& file, const std::string& class_name) {
-    conditional_class_set_[class_name].insert(file);
+  inline void AddConditionalClass(const UsageLocation& file,
+                                  const NameAndSignature& class_and_signature) {
+    conditional_class_set_[class_and_signature].insert(file);
   }
 
   inline void AddMethod(const UsageLocation& file, const NameAndSignature& name_and_signature) {
@@ -77,7 +78,7 @@ class KeepSet {
   bool conditional_keep_rules_ = false;
   std::map<std::string, std::set<UsageLocation>> manifest_class_set_;
   std::map<NameAndSignature, std::set<UsageLocation>> method_set_;
-  std::map<std::string, std::set<UsageLocation>> conditional_class_set_;
+  std::map<NameAndSignature, std::set<UsageLocation>> conditional_class_set_;
   std::map<ResourceName, std::set<UsageLocation>> reference_set_;
 };
 
diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp
index aeb9eb6e91c4..a41a8ca3394a 100644
--- a/tools/aapt2/java/ProguardRules_test.cpp
+++ b/tools/aapt2/java/ProguardRules_test.cpp
@@ -77,7 +77,7 @@ TEST(ProguardRulesTest, FragmentNameRuleIsEmitted) {
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));
+  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(); }"));
 }
 
 TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) {
@@ -91,7 +91,7 @@ TEST(ProguardRulesTest, FragmentClassRuleIsEmitted) {
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));
+  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(); }"));
 }
 
 TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) {
@@ -107,8 +107,8 @@ TEST(ProguardRulesTest, FragmentNameAndClassRulesAreEmitted) {
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(...); }"));
+  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(); }"));
+  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(); }"));
 }
 
 TEST(ProguardRulesTest, NavigationFragmentNameAndClassRulesAreEmitted) {
@@ -267,8 +267,8 @@ TEST(ProguardRulesTest, MenuRulesAreEmitted) {
 
   EXPECT_THAT(actual, HasSubstr(
       "-keepclassmembers class * { *** on_click(android.view.MenuItem); }"));
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(...); }"));
+  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(android.content.Context); }"));
+  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Baz { <init>(android.content.Context); }"));
   EXPECT_THAT(actual, Not(HasSubstr("com.foo.Bat")));
 }
 
@@ -285,7 +285,8 @@ TEST(ProguardRulesTest, TransitionPathMotionRulesAreEmitted) {
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));
+  EXPECT_THAT(actual, HasSubstr(
+      "-keep class com.foo.Bar { <init>(android.content.Context, android.util.AttributeSet); }"));
 }
 
 TEST(ProguardRulesTest, TransitionRulesAreEmitted) {
@@ -301,7 +302,8 @@ TEST(ProguardRulesTest, TransitionRulesAreEmitted) {
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keep class com.foo.Bar { <init>(...); }"));
+  EXPECT_THAT(actual, HasSubstr(
+      "-keep class com.foo.Bar { <init>(android.content.Context, android.util.AttributeSet); }"));
 }
 
 }  // namespace aapt