Merge "Do not warn about implements/extends rules that use wildcards"
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java b/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java
index 747dea9..dc8925a 100644
--- a/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java
+++ b/src/main/java/com/android/tools/r8/shaking/ProguardTypeMatcher.java
@@ -112,6 +112,10 @@
return null;
}
+ public final boolean matchesSpecificType() {
+ return getSpecificType() != null;
+ }
+
private static class MatchAllTypes extends ProguardTypeMatcher {
private static final ProguardTypeMatcher MATCH_ALL_TYPES = new MatchAllTypes();
diff --git a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
index de0537f..7485f31 100644
--- a/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
+++ b/src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
@@ -593,19 +593,23 @@
implementsExpected = satisfyImplementsRule(clazz, rule);
}
if (extendsExpected || implementsExpected) {
- // Warn if users got it wrong, but only warn once.
- if (rule.getInheritanceIsExtends()) {
- if (implementsExpected && rulesThatUseExtendsOrImplementsWrong.add(rule)) {
+ // Warn if users got it wrong, but only warn once. Also, only warn if rule is actually
+ // specific (there is no correct way to write "keep class X that extends or implements from
+ // a class or interface in package Y").
+ if (rule.getInheritanceClassName().matchesSpecificType()) {
+ if (rule.getInheritanceIsExtends()) {
+ if (implementsExpected && rulesThatUseExtendsOrImplementsWrong.add(rule)) {
+ assert options.testing.allowProguardRulesThatUseExtendsOrImplementsWrong;
+ options.reporter.warning(
+ new StringDiagnostic(
+ "The rule `" + rule + "` uses extends but actually matches implements."));
+ }
+ } else if (extendsExpected && rulesThatUseExtendsOrImplementsWrong.add(rule)) {
assert options.testing.allowProguardRulesThatUseExtendsOrImplementsWrong;
options.reporter.warning(
new StringDiagnostic(
- "The rule `" + rule + "` uses extends but actually matches implements."));
+ "The rule `" + rule + "` uses implements but actually matches extends."));
}
- } else if (extendsExpected && rulesThatUseExtendsOrImplementsWrong.add(rule)) {
- assert options.testing.allowProguardRulesThatUseExtendsOrImplementsWrong;
- options.reporter.warning(
- new StringDiagnostic(
- "The rule `" + rule + "` uses implements but actually matches extends."));
}
return true;
}
diff --git a/src/test/java/com/android/tools/r8/configuration/WrongUseOfImplementsOrExtendsWarningTest.java b/src/test/java/com/android/tools/r8/configuration/WrongUseOfImplementsOrExtendsWarningTest.java
new file mode 100644
index 0000000..cc07ce3
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/configuration/WrongUseOfImplementsOrExtendsWarningTest.java
@@ -0,0 +1,105 @@
+// Copyright (c) 2018, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.configuration;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import org.junit.Test;
+
+public class WrongUseOfImplementsOrExtendsWarningTest extends TestBase {
+
+ @Test
+ public void testCorrectUseOfExtends() throws Exception {
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(WrongUseOfImplementsOrExtendsWarningTest.class)
+ .addKeepRules(
+ "-keep class " + B.class.getTypeName() + " extends " + A.class.getTypeName())
+ .compile()
+ .assertNoMessages()
+ .inspector();
+ assertThat(inspector.clazz(B.class), isPresent());
+ }
+
+ @Test
+ public void testWrongUseOfExtends() throws Exception {
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(WrongUseOfImplementsOrExtendsWarningTest.class)
+ .addKeepRules(
+ "-keep class " + B.class.getTypeName() + " extends " + I.class.getTypeName())
+ .compile()
+ .assertWarningMessageThatMatches(
+ containsString("uses extends but actually matches implements"))
+ .inspector();
+ assertThat(inspector.clazz(B.class), isPresent());
+ }
+
+ @Test
+ public void testUseOfExtendsWithWildcards() throws Exception {
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(WrongUseOfImplementsOrExtendsWarningTest.class)
+ .addKeepRules(
+ "-keep class " + B.class.getTypeName() + " extends **$" + I.class.getSimpleName())
+ .compile()
+ .assertNoMessages()
+ .inspector();
+ assertThat(inspector.clazz(B.class), isPresent());
+ }
+
+ @Test
+ public void testCorrectUseOfImplements() throws Exception {
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(WrongUseOfImplementsOrExtendsWarningTest.class)
+ .addKeepRules(
+ "-keep class " + B.class.getTypeName() + " implements " + I.class.getTypeName())
+ .compile()
+ .assertNoMessages()
+ .inspector();
+ assertThat(inspector.clazz(B.class), isPresent());
+ }
+
+ @Test
+ public void testWrongUseOfImplements() throws Exception {
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(WrongUseOfImplementsOrExtendsWarningTest.class)
+ .addKeepRules(
+ "-keep class " + B.class.getTypeName() + " implements " + A.class.getTypeName())
+ .compile()
+ .assertWarningMessageThatMatches(
+ containsString("uses implements but actually matches extends"))
+ .inspector();
+ assertThat(inspector.clazz(B.class), isPresent());
+ }
+
+ @Test
+ public void testUseOfImplementsWithWildcards() throws Exception {
+ CodeInspector inspector =
+ testForR8(Backend.DEX)
+ .addInnerClasses(WrongUseOfImplementsOrExtendsWarningTest.class)
+ .addKeepRules(
+ "-keep class "
+ + B.class.getTypeName()
+ + " implements **$"
+ + A.class.getSimpleName())
+ .compile()
+ .assertNoMessages()
+ .inspector();
+ assertThat(inspector.clazz(B.class), isPresent());
+ }
+
+ static class A {}
+
+ static class B extends A implements I {}
+
+ interface I {}
+}