commit 3001f0311ddf4a6cc8b328ae697314feb1fd00f7
author Jake Wharton <jakew@google.com> Mon Jun 11 12:24:11 2018 -0400
committer Jake Wharton <jakew@google.com> Mon Jun 11 17:21:28 2018 -0400
tree 0dd7ae595263f5e063f46842012053df56fdea47
parent 420785e6d0bdecd6849aa9e609e39fb1906a5bc0

Only keep methods with correct signature for view/menu click

For View's onClick attribute the method must have a single argument of type android.view.View. For a MenuItem's onClick attribute the method must have a single argument of type android.view.MenuItem. Since these rules match all types and any return type, matching by signature is the only available specificity that can be added.

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

tools/aapt2/java/ProguardRules.cpp

diff --git a/tools/aapt2/java/ProguardRules.cpp b/tools/aapt2/java/ProguardRules.cpp
index d7ebd8c..d03cdb3 100644
--- a/tools/aapt2/java/ProguardRules.cpp
+++ b/tools/aapt2/java/ProguardRules.cpp
@@ -79,8 +79,10 @@
     keep_set_->AddConditionalClass({file_.name, file_.source.WithLine(line_number)}, class_name);
   }
 
-  void AddMethod(size_t line_number, const std::string& method_name) {
-    keep_set_->AddMethod({file_.name, file_.source.WithLine(line_number)}, method_name);
+  void AddMethod(size_t line_number, const std::string& method_name,
+                 const std::string& method_signature) {
+    keep_set_->AddMethod({file_.name, file_.source.WithLine(line_number)},
+        {method_name, method_signature});
   }
 
   void AddReference(size_t line_number, Reference* ref) {
@@ -125,7 +127,7 @@
         AddClass(node->line_number, attr.value);
       } else if (attr.namespace_uri == xml::kSchemaAndroid &&
                  attr.name == "onClick") {
-        AddMethod(node->line_number, attr.value);
+        AddMethod(node->line_number, attr.value, "android.view.View");
       }
     }
 
@@ -149,7 +151,7 @@
               util::IsJavaClassName(attr.value)) {
             AddClass(node->line_number, attr.value);
           } else if (attr.name == "onClick") {
-            AddMethod(node->line_number, attr.value);
+            AddMethod(node->line_number, attr.value, "android.view.MenuItem");
           }
         }
       }
@@ -396,7 +398,8 @@
     for (const UsageLocation& location : entry.second) {
       printer.Print("# Referenced at ").Println(location.source.to_string());
     }
-    printer.Print("-keepclassmembers class * { *** ").Print(entry.first).Println("(...); }");
+    printer.Print("-keepclassmembers class * { *** ").Print(entry.first.name)
+        .Print("(").Print(entry.first.signature).Println("); }");
     printer.Println();
   }
 }

tools/aapt2/java/ProguardRules.h

diff --git a/tools/aapt2/java/ProguardRules.h b/tools/aapt2/java/ProguardRules.h
index 343272e..acaceac 100644
--- a/tools/aapt2/java/ProguardRules.h
+++ b/tools/aapt2/java/ProguardRules.h
@@ -40,6 +40,11 @@
   Source source;
 };
 
+struct NameAndSignature {
+  std::string name;
+  std::string signature;
+};
+
 class KeepSet {
  public:
   KeepSet() = default;
@@ -55,8 +60,8 @@
     conditional_class_set_[class_name].insert(file);
   }
 
-  inline void AddMethod(const UsageLocation& file, const std::string& method_name) {
-    method_set_[method_name].insert(file);
+  inline void AddMethod(const UsageLocation& file, const NameAndSignature& name_and_signature) {
+    method_set_[name_and_signature].insert(file);
   }
 
   inline void AddReference(const UsageLocation& file, const ResourceName& resource_name) {
@@ -71,7 +76,7 @@
 
   bool conditional_keep_rules_ = false;
   std::map<std::string, std::set<UsageLocation>> manifest_class_set_;
-  std::map<std::string, std::set<UsageLocation>> method_set_;
+  std::map<NameAndSignature, std::set<UsageLocation>> method_set_;
   std::map<std::string, std::set<UsageLocation>> conditional_class_set_;
   std::map<ResourceName, std::set<UsageLocation>> reference_set_;
 };
@@ -100,6 +105,20 @@
   return lhs.name.compare(rhs.name);
 }
 
+//
+// NameAndSignature implementation.
+//
+
+inline bool operator<(const NameAndSignature& lhs, const NameAndSignature& rhs) {
+  if (lhs.name < rhs.name) {
+    return true;
+  }
+  if (lhs.name == rhs.name) {
+    return lhs.signature < rhs.signature;
+  }
+  return false;
+}
+
 }  // namespace proguard
 }  // namespace aapt
 

tools/aapt2/java/ProguardRules_test.cpp

diff --git a/tools/aapt2/java/ProguardRules_test.cpp b/tools/aapt2/java/ProguardRules_test.cpp
index a548f4a..b5e27e0 100644
--- a/tools/aapt2/java/ProguardRules_test.cpp
+++ b/tools/aapt2/java/ProguardRules_test.cpp
@@ -239,7 +239,8 @@
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keepclassmembers class * { *** bar_method(...); }"));
+  EXPECT_THAT(actual, HasSubstr(
+      "-keepclassmembers class * { *** bar_method(android.view.View); }"));
 }
 
 TEST(ProguardRulesTest, MenuRulesAreEmitted) {
@@ -258,7 +259,8 @@
 
   std::string actual = GetKeepSetString(set);
 
-  EXPECT_THAT(actual, HasSubstr("-keepclassmembers class * { *** on_click(...); }"));
+  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, Not(HasSubstr("com.foo.Bat")));