Add API test for keep rule diagnostics.
Bug: b/143432055
Bug: b/248371950
Change-Id: I4be26b4cd3abab915dbcdcf05d23e8abb5d31c9a
diff --git a/src/main/java/com/android/tools/r8/errors/ProguardKeepRuleDiagnostic.java b/src/main/java/com/android/tools/r8/errors/ProguardKeepRuleDiagnostic.java
index dcb6bb9..10d8c1f 100644
--- a/src/main/java/com/android/tools/r8/errors/ProguardKeepRuleDiagnostic.java
+++ b/src/main/java/com/android/tools/r8/errors/ProguardKeepRuleDiagnostic.java
@@ -4,6 +4,8 @@
package com.android.tools.r8.errors;
import com.android.tools.r8.Diagnostic;
+import com.android.tools.r8.Keep;
-/** Base class for diagnostics related to proguard keep rules. */
-public abstract class ProguardKeepRuleDiagnostic implements Diagnostic {}
+/** Base interface for diagnostics related to proguard keep rules. */
+@Keep
+public interface ProguardKeepRuleDiagnostic extends Diagnostic {}
diff --git a/src/main/java/com/android/tools/r8/errors/UnusedProguardKeepRuleDiagnostic.java b/src/main/java/com/android/tools/r8/errors/UnusedProguardKeepRuleDiagnostic.java
index 396f88b..2d2b326 100644
--- a/src/main/java/com/android/tools/r8/errors/UnusedProguardKeepRuleDiagnostic.java
+++ b/src/main/java/com/android/tools/r8/errors/UnusedProguardKeepRuleDiagnostic.java
@@ -9,7 +9,7 @@
import com.android.tools.r8.shaking.ProguardConfigurationRule;
@Keep
-public class UnusedProguardKeepRuleDiagnostic extends ProguardKeepRuleDiagnostic {
+public class UnusedProguardKeepRuleDiagnostic implements ProguardKeepRuleDiagnostic {
private final ProguardConfigurationRule rule;
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index f07a328..6bf0a8f 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -1988,8 +1988,7 @@
public boolean allowInvokeErrors = false;
public boolean allowUnnecessaryDontWarnWildcards = true;
public boolean allowUnusedDontWarnRules = true;
- public boolean reportUnusedProguardConfigurationRules =
- System.getProperty("com.android.tools.r8.reportUnusedProguardConfigurationRules") != null;
+ public boolean reportUnusedProguardConfigurationRules = true;
public boolean alwaysUseExistingAccessInfoCollectionsInMemberRebinding = true;
public boolean alwaysUsePessimisticRegisterAllocation = false;
public boolean enableCheckCastAndInstanceOfRemoval = true;
diff --git a/src/test/java/com/android/tools/r8/L8TestBuilder.java b/src/test/java/com/android/tools/r8/L8TestBuilder.java
index 4a22769..f5a808a 100644
--- a/src/test/java/com/android/tools/r8/L8TestBuilder.java
+++ b/src/test/java/com/android/tools/r8/L8TestBuilder.java
@@ -6,8 +6,10 @@
import static junit.framework.Assert.assertNull;
import static junit.framework.TestCase.assertTrue;
+import static org.junit.Assert.fail;
import com.android.tools.r8.TestBase.Backend;
+import com.android.tools.r8.errors.UnusedProguardKeepRuleDiagnostic;
import com.android.tools.r8.ir.desugar.desugaredlibrary.DesugaredLibrarySpecification;
import com.android.tools.r8.ir.desugar.desugaredlibrary.DesugaredLibrarySpecificationParser;
import com.android.tools.r8.origin.Origin;
@@ -249,14 +251,20 @@
|| warnings.stream()
.allMatch(warn -> warn.getDiagnosticMessage().contains("org.testng.Assert")));
List<Diagnostic> infos = diagnosticsMessages.getInfos();
- // The rewriting confuses the generic signatures in some methods. Such signatures are never
- // used by tools (they use the non library desugared version) and are stripped when compiling
- // with R8 anyway.
- // TODO(b/243483320): Investigate the Invalid signature.
- assertTrue(
- infos.isEmpty()
- || infos.stream()
- .allMatch(info -> info.getDiagnosticMessage().contains("Invalid signature ")));
+ for (Diagnostic info : infos) {
+ // The rewriting confuses the generic signatures in some methods. Such signatures are never
+ // used by tools (they use the non library desugared version) and are stripped when compiling
+ // with R8 anyway.
+ // TODO(b/243483320): Investigate the Invalid signature.
+ if (info.getDiagnosticMessage().contains("Invalid signature ")) {
+ continue;
+ }
+ // TODO(b/248371950): Should we have L8 shinking builds with these?
+ if (info instanceof UnusedProguardKeepRuleDiagnostic) {
+ continue;
+ }
+ fail("Unexpected info diagnostic: " + info);
+ }
}
private L8Command.Builder addProgramClassFileData(L8Command.Builder builder) {
diff --git a/src/test/java/com/android/tools/r8/compilerapi/CompilerApiTestCollection.java b/src/test/java/com/android/tools/r8/compilerapi/CompilerApiTestCollection.java
index 73dbba2..fd3e16d 100644
--- a/src/test/java/com/android/tools/r8/compilerapi/CompilerApiTestCollection.java
+++ b/src/test/java/com/android/tools/r8/compilerapi/CompilerApiTestCollection.java
@@ -13,6 +13,7 @@
import com.android.tools.r8.compilerapi.assertionconfiguration.AssertionConfigurationTest;
import com.android.tools.r8.compilerapi.classconflictresolver.ClassConflictResolverTest;
import com.android.tools.r8.compilerapi.desugardependencies.DesugarDependenciesTest;
+import com.android.tools.r8.compilerapi.diagnostics.ProguardKeepRuleDiagnosticsApiTest;
import com.android.tools.r8.compilerapi.diagnostics.UnsupportedFeaturesDiagnosticApiTest;
import com.android.tools.r8.compilerapi.globalsynthetics.GlobalSyntheticsTest;
import com.android.tools.r8.compilerapi.inputdependencies.InputDependenciesTest;
@@ -56,7 +57,8 @@
ImmutableList.of(
ArtProfilesForRewritingApiTest.ApiTest.class,
StartupProfileApiTest.ApiTest.class,
- ClassConflictResolverTest.ApiTest.class);
+ ClassConflictResolverTest.ApiTest.class,
+ ProguardKeepRuleDiagnosticsApiTest.ApiTest.class);
private final TemporaryFolder temp;
diff --git a/src/test/java/com/android/tools/r8/compilerapi/diagnostics/ProguardKeepRuleDiagnosticsApiTest.java b/src/test/java/com/android/tools/r8/compilerapi/diagnostics/ProguardKeepRuleDiagnosticsApiTest.java
new file mode 100644
index 0000000..fedb449
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/compilerapi/diagnostics/ProguardKeepRuleDiagnosticsApiTest.java
@@ -0,0 +1,117 @@
+// Copyright (c) 2022, 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.compilerapi.diagnostics;
+
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType;
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.containsString;
+
+import com.android.tools.r8.DexIndexedConsumer;
+import com.android.tools.r8.Diagnostic;
+import com.android.tools.r8.DiagnosticsHandler;
+import com.android.tools.r8.DiagnosticsLevel;
+import com.android.tools.r8.R8;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.TestDiagnosticMessagesImpl;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.compilerapi.CompilerApiTest;
+import com.android.tools.r8.compilerapi.CompilerApiTestRunner;
+import com.android.tools.r8.errors.ProguardKeepRuleDiagnostic;
+import com.android.tools.r8.errors.UnusedProguardKeepRuleDiagnostic;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.ThrowingConsumer;
+import java.util.Collections;
+import org.junit.Test;
+
+public class ProguardKeepRuleDiagnosticsApiTest extends CompilerApiTestRunner {
+
+ public ProguardKeepRuleDiagnosticsApiTest(TestParameters parameters) {
+ super(parameters);
+ }
+
+ @Override
+ public Class<? extends CompilerApiTest> binaryTestClass() {
+ return ApiTest.class;
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ ApiTest test = new ApiTest(ApiTest.PARAMETERS);
+ runTest(test::runR8);
+ }
+
+ private void runTest(ThrowingConsumer<DiagnosticsHandler, Exception> test) throws Exception {
+ TestDiagnosticMessagesImpl diagnostics = new TestDiagnosticMessagesImpl();
+ test.accept(diagnostics);
+ diagnostics
+ .assertOnlyWarnings()
+ // Check the diagnostic is an instance of both types.
+ .assertWarningsMatch(
+ allOf(
+ diagnosticType(UnusedProguardKeepRuleDiagnostic.class),
+ diagnosticMessage(containsString("does not match anything"))))
+ .assertWarningsMatch(
+ allOf(
+ diagnosticType(ProguardKeepRuleDiagnostic.class),
+ diagnosticMessage(containsString("does not match anything"))));
+ }
+
+ public static class ApiTest extends CompilerApiTest {
+
+ public ApiTest(Object parameters) {
+ super(parameters);
+ }
+
+ public void runR8(DiagnosticsHandler handler) throws Exception {
+ R8.run(
+ R8Command.builder(
+ new DiagnosticsHandler() {
+ @Override
+ public DiagnosticsLevel modifyDiagnosticsLevel(
+ DiagnosticsLevel level, Diagnostic diagnostic) {
+ if (diagnostic instanceof ProguardKeepRuleDiagnostic
+ || diagnostic instanceof UnusedProguardKeepRuleDiagnostic) {
+ return handler.modifyDiagnosticsLevel(DiagnosticsLevel.WARNING, diagnostic);
+ }
+ return handler.modifyDiagnosticsLevel(level, diagnostic);
+ }
+
+ @Override
+ public void error(Diagnostic error) {
+ handler.error(error);
+ }
+
+ @Override
+ public void warning(Diagnostic warning) {
+ handler.warning(warning);
+ }
+
+ @Override
+ public void info(Diagnostic info) {
+ handler.info(info);
+ }
+ })
+ .addClassProgramData(getBytesForClass(getMockClass()), Origin.unknown())
+ .addProguardConfiguration(getKeepMainRules(getMockClass()), Origin.unknown())
+ .addProguardConfiguration(
+ Collections.singletonList("-keep class NotPresent {}"), Origin.unknown())
+ .addLibraryFiles(getJava8RuntimeJar())
+ .setProgramConsumer(DexIndexedConsumer.emptyConsumer())
+ .setMinApiLevel(1)
+ .build());
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ runR8(
+ new DiagnosticsHandler() {
+ @Override
+ public void warning(Diagnostic warning) {
+ // ignore the warnings.
+ }
+ });
+ }
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryWarningTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryWarningTest.java
index 2bd0f31..9d0d78e 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryWarningTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/DesugaredLibraryWarningTest.java
@@ -5,6 +5,7 @@
package com.android.tools.r8.desugar.desugaredlibrary;
import static com.android.tools.r8.DiagnosticsMatcher.diagnosticMessage;
+import static com.android.tools.r8.DiagnosticsMatcher.diagnosticType;
import static com.android.tools.r8.desugar.desugaredlibrary.test.CompilationSpecification.D8_L8DEBUG;
import static com.android.tools.r8.desugar.desugaredlibrary.test.CompilationSpecification.D8_L8SHRINK;
import static com.android.tools.r8.desugar.desugaredlibrary.test.LibraryDesugaringSpecification.JDK8;
@@ -14,6 +15,7 @@
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.desugar.desugaredlibrary.test.CompilationSpecification;
import com.android.tools.r8.desugar.desugaredlibrary.test.LibraryDesugaringSpecification;
+import com.android.tools.r8.errors.UnusedProguardKeepRuleDiagnostic;
import com.google.common.collect.ImmutableList;
import java.util.List;
import org.junit.Test;
@@ -64,14 +66,16 @@
.compile()
.inspectDiagnosticMessages(
diagnosticsHandler -> {
+ diagnosticsHandler.assertNoErrors();
if (libraryDesugaringSpecification != JDK8) {
- diagnosticsHandler.assertNoErrors();
diagnosticsHandler.assertAllWarningsMatch(
diagnosticMessage(containsString("Specification conversion")));
} else {
-
- diagnosticsHandler.assertNoMessages();
+ diagnosticsHandler.assertNoWarnings();
}
+ // TODO(b/248371950): Should we have L8 shinking builds with these?
+ diagnosticsHandler.assertAllInfosMatch(
+ diagnosticType(UnusedProguardKeepRuleDiagnostic.class));
});
}
}
