Use a specific type for missing definitions in tracereferences
Bug: 169127026
Bug: 169546956
Change-Id: I1e5e374c9a4830ab59c2e5146f68b42449b09a07
diff --git a/src/main/java/com/android/tools/r8/tracereferences/MissingDefinitionsDiagnostic.java b/src/main/java/com/android/tools/r8/tracereferences/MissingDefinitionsDiagnostic.java
new file mode 100644
index 0000000..db31dac
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/tracereferences/MissingDefinitionsDiagnostic.java
@@ -0,0 +1,89 @@
+// Copyright (c) 2020, 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.tracereferences;
+
+import com.android.tools.r8.Diagnostic;
+import com.android.tools.r8.Keep;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.position.Position;
+import com.android.tools.r8.tracereferences.Tracer.TracedClassImpl;
+import com.android.tools.r8.tracereferences.Tracer.TracedFieldImpl;
+import com.android.tools.r8.tracereferences.Tracer.TracedMethodImpl;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Set;
+
+@Keep
+public class MissingDefinitionsDiagnostic implements Diagnostic {
+
+ private final Set<TracedClassImpl> missingClasses;
+ private final Set<TracedFieldImpl> missingFields;
+ private final Set<TracedMethodImpl> missingMethods;
+
+ MissingDefinitionsDiagnostic(
+ Set<TracedClassImpl> missingClasses,
+ Set<TracedFieldImpl> missingFields,
+ Set<TracedMethodImpl> missingMethods) {
+ this.missingClasses = missingClasses;
+ this.missingFields = missingFields;
+ this.missingMethods = missingMethods;
+ }
+
+ @Override
+ public Origin getOrigin() {
+ return Origin.unknown();
+ }
+
+ @Override
+ public Position getPosition() {
+ return Position.UNKNOWN;
+ }
+
+ @Override
+ public String getDiagnosticMessage() {
+ StringBuilder builder = new StringBuilder("Tracereferences found ");
+ List<String> components = new ArrayList<>();
+ if (missingClasses.size() > 0) {
+ components.add("" + missingClasses.size() + " classes");
+ }
+ if (missingFields.size() > 0) {
+ components.add("" + missingClasses.size() + " fields");
+ }
+ if (missingMethods.size() > 0) {
+ components.add("" + missingClasses.size() + " methods");
+ }
+ assert components.size() > 0;
+ for (int i = 0; i < components.size(); i++) {
+ if (i != 0) {
+ builder.append(i < components.size() - 1 ? ", " : " and ");
+ }
+ builder.append(components.get(i));
+ }
+ builder.append(" without definition");
+ builder.append(System.lineSeparator());
+ builder.append(System.lineSeparator());
+ builder.append("Classes without definition:");
+ missingClasses.forEach(
+ clazz ->
+ builder
+ .append(" ")
+ .append(clazz.getReference().toString())
+ .append(System.lineSeparator()));
+ builder.append("Fields without definition");
+ missingFields.forEach(
+ field ->
+ builder
+ .append(" ")
+ .append(field.getReference().toString())
+ .append(System.lineSeparator()));
+ builder.append("Methods without definition");
+ missingMethods.forEach(
+ method ->
+ builder
+ .append(" ")
+ .append(method.getReference().toString())
+ .append(System.lineSeparator()));
+ return builder.toString();
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/tracereferences/Tracer.java b/src/main/java/com/android/tools/r8/tracereferences/Tracer.java
index 1149b20..110fb8b 100644
--- a/src/main/java/com/android/tools/r8/tracereferences/Tracer.java
+++ b/src/main/java/com/android/tools/r8/tracereferences/Tracer.java
@@ -41,7 +41,6 @@
import com.android.tools.r8.tracereferences.TraceReferencesConsumer.TracedReference;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.InternalOptions;
-import com.android.tools.r8.utils.StringDiagnostic;
import com.android.tools.r8.utils.Timing;
import java.io.IOException;
import java.util.HashSet;
@@ -251,6 +250,7 @@
clazz.forEachField(useCollector::registerField);
}
consumer.finished();
+ useCollector.reportMissingDefinitions();
}
class UseCollector extends UseRegistry {
@@ -259,7 +259,9 @@
private final TraceReferencesConsumer consumer;
private DexProgramClass context;
private final DiagnosticsHandler diagnostics;
- private Set<TracedReference<?, ?>> missingDefinitionReported = new HashSet<>();
+ private final Set<TracedClassImpl> missingClasses = new HashSet<>();
+ private final Set<TracedFieldImpl> missingFields = new HashSet<>();
+ private final Set<TracedMethodImpl> missingMethods = new HashSet<>();
UseCollector(
DexItemFactory factory, TraceReferencesConsumer consumer, DiagnosticsHandler diagnostics) {
@@ -283,7 +285,7 @@
}
DexClass clazz = appInfo.definitionFor(type);
TracedClassImpl tracedClass = new TracedClassImpl(type, clazz);
- checkDiagnostics(tracedClass);
+ checkMissingDefinition(tracedClass);
if (isTargetType(type) || tracedClass.isMissingDefinition()) {
consumer.acceptType(tracedClass);
if (!tracedClass.isMissingDefinition()
@@ -301,7 +303,7 @@
}
addType(field.holder);
TracedFieldImpl tracedField = new TracedFieldImpl(field, baseField);
- checkDiagnostics(tracedField);
+ checkMissingDefinition(tracedField);
if (isTargetType(field.holder) || tracedField.isMissingDefinition()) {
consumer.acceptField(tracedField);
if (!tracedField.isMissingDefinition()
@@ -322,7 +324,7 @@
TracedMethodImpl tracedMethod = new TracedMethodImpl(method, definition);
if (isTargetType(method.holder) || tracedMethod.isMissingDefinition()) {
consumer.acceptMethod(tracedMethod);
- checkDiagnostics(tracedMethod);
+ checkMissingDefinition(tracedMethod);
if (!tracedMethod.isMissingDefinition()
&& definition.accessFlags.isVisibilityDependingOnPackage()) {
consumer.acceptPackage(Reference.packageFromString(definition.holder().getPackageName()));
@@ -330,11 +332,29 @@
}
}
- private void checkDiagnostics(TracedReferenceBase<?, ?> tracedReference) {
- if (tracedReference.isMissingDefinition() && missingDefinitionReported.add(tracedReference)) {
+ private void checkMissingDefinition(TracedClassImpl tracedClass) {
+ collectMissing(tracedClass, missingClasses);
+ }
+
+ private void checkMissingDefinition(TracedFieldImpl tracedField) {
+ collectMissing(tracedField, missingFields);
+ }
+
+ private void checkMissingDefinition(TracedMethodImpl tracedMethod) {
+ collectMissing(tracedMethod, missingMethods);
+ }
+
+ private <T extends TracedReferenceBase<?, ?>> void collectMissing(
+ T tracedReference, Set<T> missingCollection) {
+ if (tracedReference.isMissingDefinition()) {
+ missingCollection.add(tracedReference);
+ }
+ }
+
+ private void reportMissingDefinitions() {
+ if (missingClasses.size() > 0 || missingFields.size() > 0 || missingMethods.size() > 0) {
diagnostics.error(
- new StringDiagnostic(
- "Missing definition of " + tracedReference.getKindName() + " " + tracedReference));
+ new MissingDefinitionsDiagnostic(missingClasses, missingFields, missingMethods));
}
}
diff --git a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
index 730d952..09e32da 100644
--- a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
+++ b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
@@ -60,6 +60,10 @@
diagnostics.stream().anyMatch(d -> d.getDiagnosticMessage().contains(snippet)));
}
+ public void checkErrorsContains(String snippet) {
+ checkContains(snippet, errors);
+ }
+
public void checkWarningsContains(String snippet) {
checkContains(snippet, warnings);
}
diff --git a/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesCommandTest.java b/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesCommandTest.java
index a312c2f..430af64 100644
--- a/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesCommandTest.java
+++ b/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesCommandTest.java
@@ -14,6 +14,7 @@
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.references.Reference;
import com.android.tools.r8.tracereferences.TraceReferencesFormattingConsumer.OutputFormat;
import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.DescriptorUtils;
@@ -23,6 +24,8 @@
import com.google.common.collect.ImmutableList;
import java.io.BufferedOutputStream;
import java.io.IOException;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
@@ -135,14 +138,13 @@
}
private String formatName(OutputFormat format) {
- if (format == TraceReferencesFormattingConsumer.OutputFormat.PRINTUSAGE) {
+ if (format == OutputFormat.PRINTUSAGE) {
return "printuses";
}
- if (format == TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES) {
+ if (format == OutputFormat.KEEP_RULES) {
return "keep";
}
- assertSame(
- format, TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION);
+ assertSame(format, OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION);
return "keepallowobfuscation";
}
@@ -233,12 +235,12 @@
runAndCheckOutput(
ImmutableList.of(Target.class),
ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.PRINTUSAGE,
+ OutputFormat.PRINTUSAGE,
StringUtils.lines(
ImmutableList.of(
"com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target",
"com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target: void"
- + " target(int)",
+ + " method(int)",
"com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target: int"
+ " field")));
}
@@ -248,12 +250,12 @@
runAndCheckOutput(
ImmutableList.of(Target.class),
ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES,
+ OutputFormat.KEEP_RULES,
StringUtils.lines(
ImmutableList.of(
"-keep class com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target"
+ " {",
- " public static void target(int);",
+ " public static void method(int);",
" int field;",
"}",
"-keeppackagenames com.android.tools.r8.tracereferences")));
@@ -264,31 +266,43 @@
runAndCheckOutput(
ImmutableList.of(Target.class),
ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
+ OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
StringUtils.lines(
ImmutableList.of(
"-keep,allowobfuscation class"
+ " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target {",
- " public static void target(int);",
+ " public static void method(int);",
" int field;",
"}",
"-keeppackagenames com.android.tools.r8.tracereferences")));
}
+ private void checkTargetMissing(DiagnosticsChecker diagnosticsChecker) {
+ Field field;
+ Method method;
+ try {
+ field = Target.class.getField("field");
+ method = Target.class.getMethod("method", int.class);
+ } catch (ReflectiveOperationException e) {
+ throw new RuntimeException(e);
+ }
+ assertEquals(1, diagnosticsChecker.errors.size());
+ assertEquals(0, diagnosticsChecker.warnings.size());
+ assertEquals(0, diagnosticsChecker.infos.size());
+ diagnosticsChecker.checkErrorsContains(Reference.classFromClass(Target.class).toString());
+ diagnosticsChecker.checkErrorsContains(Reference.fieldFromField(field).toString());
+ diagnosticsChecker.checkErrorsContains(Reference.methodFromMethod(method).toString());
+ }
+
@Test
public void testNoReferences_printUses() throws Throwable {
- String targetTypeName = Target.class.getTypeName();
try {
runAndCheckOutput(
ImmutableList.of(OtherTarget.class),
ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.PRINTUSAGE,
+ OutputFormat.PRINTUSAGE,
StringUtils.lines(),
- diagnosticsChecker -> {
- assertEquals(3, diagnosticsChecker.errors.size());
- assertEquals(0, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ this::checkTargetMissing);
fail("Expected compilation to fail");
} catch (CompilationFailedException e) {
// Expected.
@@ -297,18 +311,15 @@
@Test
public void testMissingReference_keepRules() throws Throwable {
- String targetTypeName = Target.class.getTypeName();
+ Field field = Target.class.getField("field");
+ Method method = Target.class.getMethod("method", int.class);
try {
runAndCheckOutput(
ImmutableList.of(OtherTarget.class),
ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES,
+ OutputFormat.KEEP_RULES,
StringUtils.lines(),
- diagnosticsChecker -> {
- assertEquals(3, diagnosticsChecker.errors.size());
- assertEquals(0, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ this::checkTargetMissing);
fail("Expected compilation to fail");
} catch (CompilationFailedException e) {
// Expected.
@@ -316,18 +327,14 @@
}
@Test
- public void testNoReferences_keepRulesAllowObfuscation() throws Throwable {
+ public void testMissingReference_keepRulesAllowObfuscation() throws Throwable {
try {
runAndCheckOutput(
ImmutableList.of(OtherTarget.class),
ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
+ OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
StringUtils.lines(),
- diagnosticsChecker -> {
- assertEquals(3, diagnosticsChecker.errors.size());
- assertEquals(0, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ this::checkTargetMissing);
fail("Expected compilation to fail");
} catch (CompilationFailedException e) {
// Expected.
@@ -344,6 +351,22 @@
}
}
+ private void checkTargetPartlyMissing(DiagnosticsChecker diagnosticsChecker) {
+ Field field;
+ Method method;
+ try {
+ field = Target.class.getField("field");
+ method = Target.class.getMethod("method", int.class);
+ } catch (ReflectiveOperationException e) {
+ throw new RuntimeException(e);
+ }
+ assertEquals(1, diagnosticsChecker.errors.size());
+ assertEquals(0, diagnosticsChecker.warnings.size());
+ assertEquals(0, diagnosticsChecker.infos.size());
+ diagnosticsChecker.checkErrorsContains(Reference.fieldFromField(field).toString());
+ diagnosticsChecker.checkErrorsContains(Reference.methodFromMethod(method).toString());
+ }
+
@Test
public void testMissingDefinition_printUses() throws Throwable {
Path dir = temp.newFolder().toPath();
@@ -358,15 +381,11 @@
runAndCheckOutput(
targetJar,
sourceJar,
- TraceReferencesFormattingConsumer.OutputFormat.PRINTUSAGE,
+ OutputFormat.PRINTUSAGE,
StringUtils.lines(
ImmutableList.of(
"com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target")),
- diagnosticsChecker -> {
- assertEquals(2, diagnosticsChecker.errors.size());
- assertEquals(0, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ this::checkTargetPartlyMissing);
fail("Expected compilation to fail");
} catch (CompilationFailedException e) {
// Expected.
@@ -387,7 +406,7 @@
runAndCheckOutput(
targetJar,
sourceJar,
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES,
+ OutputFormat.KEEP_RULES,
StringUtils.lines(
ImmutableList.of(
"-keep class"
@@ -395,11 +414,7 @@
+ " {",
"}",
"-keeppackagenames com.android.tools.r8.tracereferences")),
- diagnosticsChecker -> {
- assertEquals(2, diagnosticsChecker.errors.size());
- assertEquals(0, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ this::checkTargetPartlyMissing);
fail("Expected compilation to fail");
} catch (CompilationFailedException e) {
// Expected.
@@ -420,18 +435,14 @@
runAndCheckOutput(
targetJar,
sourceJar,
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
+ OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
StringUtils.lines(
ImmutableList.of(
"-keep,allowobfuscation class"
+ " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target {",
"}",
"-keeppackagenames com.android.tools.r8.tracereferences")),
- diagnosticsChecker -> {
- assertEquals(2, diagnosticsChecker.errors.size());
- assertEquals(0, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ this::checkTargetPartlyMissing);
fail("Expected compilation to fail");
} catch (CompilationFailedException e) {
// Expected.
@@ -440,7 +451,7 @@
private byte[] getClassWithTargetRemoved() throws IOException {
return transformer(Target.class)
- .removeMethods((access, name, descriptor, signature, exceptions) -> name.equals("target"))
+ .removeMethods((access, name, descriptor, signature, exceptions) -> name.equals("method"))
.removeFields((access, name, descriptor, signature, value) -> name.equals("field"))
.transform();
}
@@ -448,16 +459,16 @@
static class Target {
public static int field;
- public static void target(int i) {}
+ public static void method(int i) {}
}
static class OtherTarget {
- public static void target() {}
+ public static void method() {}
}
static class Source {
public static void source() {
- Target.target(Target.field);
+ Target.method(Target.field);
}
}
}
