Make missing references an error in tracereferences
Bug: 169127026
Bug: 169546956
Change-Id: Ifc8ad05452a6e9a8b84b7dcbefaa46e3096866e2
diff --git a/src/main/java/com/android/tools/r8/tracereferences/Formatter.java b/src/main/java/com/android/tools/r8/tracereferences/Formatter.java
index 01b7f99..b572f26 100644
--- a/src/main/java/com/android/tools/r8/tracereferences/Formatter.java
+++ b/src/main/java/com/android/tools/r8/tracereferences/Formatter.java
@@ -4,7 +4,6 @@
package com.android.tools.r8.tracereferences;
import com.android.tools.r8.references.ClassReference;
-import com.android.tools.r8.references.FieldReference;
import com.android.tools.r8.references.MethodReference;
import com.android.tools.r8.references.PackageReference;
import com.android.tools.r8.references.TypeReference;
@@ -42,10 +41,6 @@
output.append(StringUtils.lines(string));
}
- protected void appendLine() {
- appendLine("");
- }
-
protected void printArguments(MethodReference method) {
StringUtils.append(
output,
@@ -56,33 +51,10 @@
protected abstract void printConstructorName(MethodReference method);
- private void printError(String message) {
- append("# Error: " + message);
- }
-
protected abstract void printField(TracedField field);
protected abstract void printMethod(TracedMethod method);
- private void printFieldError(FieldReference field) {
- appendLine(
- field.getFieldType().getTypeName()
- + " "
- + field.getHolderClass().getTypeName()
- + "."
- + field.getFieldName());
- }
-
- private void printMethodError(MethodReference method) {
- printReturn(method);
- append(" ");
- append(method.getHolderClass().getTypeName());
- append(".");
- append(method.getMethodName());
- printArguments(method);
- appendLine();
- }
-
protected abstract void printPackageNames(List<String> packageNames);
protected void printReturn(MethodReference method) {
@@ -104,23 +76,20 @@
protected abstract void printTypeFooter();
void format(TraceReferencesResult result) {
- int errors =
- print(
- result.types,
- result.keepPackageNames,
- result.fields,
- result.methods,
- result.missingDefinition);
- assert errors == result.missingDefinition.size();
+ print(
+ result.types,
+ result.keepPackageNames,
+ result.fields,
+ result.methods,
+ result.missingDefinition);
}
- private int print(
+ private void print(
Set<TracedClass> types,
Set<PackageReference> keepPackageNames,
Map<ClassReference, Set<TracedField>> fields,
Map<ClassReference, Set<TracedMethod>> methods,
Set<Object> missingDefinition) {
- int errors = 0;
List<TracedClass> sortedTypes = new ArrayList<>(types);
sortedTypes.sort(Comparator.comparing(tracedClass -> tracedClass.getReference().getTypeName()));
for (TracedClass type : sortedTypes) {
@@ -128,21 +97,13 @@
methods.getOrDefault(type.getReference(), Collections.emptySet());
Set<TracedField> fieldsForClass =
fields.getOrDefault(type.getReference(), Collections.emptySet());
- boolean typeMissing = missingDefinition.contains(type.getReference());
- if (typeMissing) {
- printError("Could not find definition for type " + type.getReference().getTypeName());
- appendLine();
- errors++;
- } else {
- printTypeHeader(type);
+ if (missingDefinition.contains(type.getReference())) {
+ continue;
}
+ printTypeHeader(type);
List<TracedMethod> sortedMethods = new ArrayList<>(methodsForClass.size());
for (TracedMethod method : methodsForClass) {
- assert method.isMissingDefinition() || !typeMissing;
if (method.isMissingDefinition()) {
- printError("Could not find definition for method ");
- printMethodError(method.getReference());
- errors++;
continue;
}
assert method.getAccessFlags() != null;
@@ -156,18 +117,12 @@
List<TracedField> sortedFields = new ArrayList<>(fieldsForClass);
sortedFields.sort(Comparator.comparing(tracedField -> tracedField.getReference().toString()));
for (TracedField field : sortedFields) {
- assert field.isMissingDefinition() || !typeMissing;
if (field.isMissingDefinition()) {
- printError("Could not find definition for field ");
- printFieldError(field.getReference());
- errors++;
continue;
}
printField(field);
}
- if (!typeMissing) {
- printTypeFooter();
- }
+ printTypeFooter();
}
List<String> packageNamesToKeep =
keepPackageNames.stream()
@@ -175,6 +130,5 @@
.sorted()
.collect(Collectors.toList());
printPackageNames(packageNamesToKeep);
- return errors;
}
}
diff --git a/src/main/java/com/android/tools/r8/tracereferences/TraceReferencesAbortException.java b/src/main/java/com/android/tools/r8/tracereferences/TraceReferencesAbortException.java
deleted file mode 100644
index 8afab19..0000000
--- a/src/main/java/com/android/tools/r8/tracereferences/TraceReferencesAbortException.java
+++ /dev/null
@@ -1,6 +0,0 @@
-// 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;
-
-public class TraceReferencesAbortException extends RuntimeException {}
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 4d44912..1149b20 100644
--- a/src/main/java/com/android/tools/r8/tracereferences/Tracer.java
+++ b/src/main/java/com/android/tools/r8/tracereferences/Tracer.java
@@ -332,7 +332,7 @@
private void checkDiagnostics(TracedReferenceBase<?, ?> tracedReference) {
if (tracedReference.isMissingDefinition() && missingDefinitionReported.add(tracedReference)) {
- diagnostics.warning(
+ diagnostics.error(
new StringDiagnostic(
"Missing definition of " + tracedReference.getKindName() + " " + tracedReference));
}
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 42e867d..a312c2f 100644
--- a/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesCommandTest.java
+++ b/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesCommandTest.java
@@ -5,6 +5,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.DiagnosticsChecker;
@@ -156,20 +157,27 @@
Path output = dir.resolve("output.txt");
DiagnosticsChecker diagnosticsChecker = new DiagnosticsChecker();
TraceReferencesFormattingConsumer consumer = new TraceReferencesFormattingConsumer(format);
- TraceReferences.run(
- TraceReferencesCommand.builder(diagnosticsChecker)
- .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
- .addTargetFiles(targetJar)
- .addSourceFiles(sourceJar)
- .setConsumer(consumer)
- .build());
- assertEquals(expected, consumer.get());
- if (diagnosticsCheckerConsumer != null) {
- diagnosticsCheckerConsumer.accept(diagnosticsChecker);
- } else {
- assertEquals(0, diagnosticsChecker.errors.size());
- assertEquals(0, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
+ try {
+ TraceReferences.run(
+ TraceReferencesCommand.builder(diagnosticsChecker)
+ .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
+ .addTargetFiles(targetJar)
+ .addSourceFiles(sourceJar)
+ .setConsumer(consumer)
+ .build());
+ assertEquals(expected, consumer.get());
+ if (diagnosticsCheckerConsumer != null) {
+ diagnosticsCheckerConsumer.accept(diagnosticsChecker);
+ } else {
+ assertEquals(0, diagnosticsChecker.errors.size());
+ assertEquals(0, diagnosticsChecker.warnings.size());
+ assertEquals(0, diagnosticsChecker.infos.size());
+ }
+ } catch (CompilationFailedException e) {
+ if (diagnosticsCheckerConsumer != null) {
+ diagnosticsCheckerConsumer.accept(diagnosticsChecker);
+ }
+ throw e;
}
TraceReferences.run(
@@ -270,64 +278,60 @@
@Test
public void testNoReferences_printUses() throws Throwable {
String targetTypeName = Target.class.getTypeName();
- runAndCheckOutput(
- ImmutableList.of(OtherTarget.class),
- ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.PRINTUSAGE,
- StringUtils.lines(
- ImmutableList.of(
- "# Error: Could not find definition for type " + targetTypeName,
- "# Error: Could not find definition for method void "
- + targetTypeName
- + ".target(int)",
- "# Error: Could not find definition for field int " + targetTypeName + ".field")),
- diagnosticsChecker -> {
- assertEquals(0, diagnosticsChecker.errors.size());
- assertEquals(3, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ try {
+ runAndCheckOutput(
+ ImmutableList.of(OtherTarget.class),
+ ImmutableList.of(Source.class),
+ TraceReferencesFormattingConsumer.OutputFormat.PRINTUSAGE,
+ StringUtils.lines(),
+ diagnosticsChecker -> {
+ assertEquals(3, diagnosticsChecker.errors.size());
+ assertEquals(0, diagnosticsChecker.warnings.size());
+ assertEquals(0, diagnosticsChecker.infos.size());
+ });
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
}
@Test
public void testMissingReference_keepRules() throws Throwable {
String targetTypeName = Target.class.getTypeName();
- runAndCheckOutput(
- ImmutableList.of(OtherTarget.class),
- ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES,
- StringUtils.lines(
- ImmutableList.of(
- "# Error: Could not find definition for type " + targetTypeName,
- "# Error: Could not find definition for method void "
- + targetTypeName
- + ".target(int)",
- "# Error: Could not find definition for field int " + targetTypeName + ".field")),
- diagnosticsChecker -> {
- assertEquals(0, diagnosticsChecker.errors.size());
- assertEquals(3, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ try {
+ runAndCheckOutput(
+ ImmutableList.of(OtherTarget.class),
+ ImmutableList.of(Source.class),
+ TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES,
+ StringUtils.lines(),
+ diagnosticsChecker -> {
+ assertEquals(3, diagnosticsChecker.errors.size());
+ assertEquals(0, diagnosticsChecker.warnings.size());
+ assertEquals(0, diagnosticsChecker.infos.size());
+ });
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
}
@Test
public void testNoReferences_keepRulesAllowObfuscation() throws Throwable {
- String targetTypeName = Target.class.getTypeName();
- runAndCheckOutput(
- ImmutableList.of(OtherTarget.class),
- ImmutableList.of(Source.class),
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
- StringUtils.lines(
- ImmutableList.of(
- "# Error: Could not find definition for type " + targetTypeName,
- "# Error: Could not find definition for method void "
- + targetTypeName
- + ".target(int)",
- "# Error: Could not find definition for field int " + targetTypeName + ".field")),
- diagnosticsChecker -> {
- assertEquals(0, diagnosticsChecker.errors.size());
- assertEquals(3, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ try {
+ runAndCheckOutput(
+ ImmutableList.of(OtherTarget.class),
+ ImmutableList.of(Source.class),
+ TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
+ StringUtils.lines(),
+ diagnosticsChecker -> {
+ assertEquals(3, diagnosticsChecker.errors.size());
+ assertEquals(0, diagnosticsChecker.warnings.size());
+ assertEquals(0, diagnosticsChecker.infos.size());
+ });
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
}
public static void zip(Path zipFile, String path, byte[] data) throws IOException {
@@ -350,24 +354,23 @@
sourceJar,
ToolHelper.getClassPathForTests(),
ToolHelper.getClassFileForTestClass(Source.class));
- runAndCheckOutput(
- targetJar,
- sourceJar,
- TraceReferencesFormattingConsumer.OutputFormat.PRINTUSAGE,
- StringUtils.lines(
- ImmutableList.of(
- "com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target",
- "# Error: Could not find definition for method void"
- + " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target"
- + ".target(int)",
- "# Error: Could not find definition for field int"
- + " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target"
- + ".field")),
- diagnosticsChecker -> {
- assertEquals(0, diagnosticsChecker.errors.size());
- assertEquals(2, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ try {
+ runAndCheckOutput(
+ targetJar,
+ sourceJar,
+ TraceReferencesFormattingConsumer.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());
+ });
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
}
@Test
@@ -380,27 +383,27 @@
sourceJar,
ToolHelper.getClassPathForTests(),
ToolHelper.getClassFileForTestClass(Source.class));
- runAndCheckOutput(
- targetJar,
- sourceJar,
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES,
- StringUtils.lines(
- ImmutableList.of(
- "-keep class com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target"
- + " {",
- "# Error: Could not find definition for method void"
- + " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target"
- + ".target(int)",
- "# Error: Could not find definition for field int"
- + " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target"
- + ".field",
- "}",
- "-keeppackagenames com.android.tools.r8.tracereferences")),
- diagnosticsChecker -> {
- assertEquals(0, diagnosticsChecker.errors.size());
- assertEquals(2, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ try {
+ runAndCheckOutput(
+ targetJar,
+ sourceJar,
+ TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES,
+ StringUtils.lines(
+ ImmutableList.of(
+ "-keep 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());
+ });
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
}
@Test
@@ -413,25 +416,26 @@
sourceJar,
ToolHelper.getClassPathForTests(),
ToolHelper.getClassFileForTestClass(Source.class));
- runAndCheckOutput(
- targetJar,
- sourceJar,
- TraceReferencesFormattingConsumer.OutputFormat.KEEP_RULES_WITH_ALLOWOBFUSCATION,
- StringUtils.lines(
- ImmutableList.of(
- "-keep,allowobfuscation class"
- + " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target {",
- "# Error: Could not find definition for method void"
- + " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target.target(int)",
- "# Error: Could not find definition for field int"
- + " com.android.tools.r8.tracereferences.TraceReferencesCommandTest$Target.field",
- "}",
- "-keeppackagenames com.android.tools.r8.tracereferences")),
- diagnosticsChecker -> {
- assertEquals(0, diagnosticsChecker.errors.size());
- assertEquals(2, diagnosticsChecker.warnings.size());
- assertEquals(0, diagnosticsChecker.infos.size());
- });
+ try {
+ runAndCheckOutput(
+ targetJar,
+ sourceJar,
+ TraceReferencesFormattingConsumer.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());
+ });
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
}
private byte[] getClassWithTargetRemoved() throws IOException {
diff --git a/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesMissingReferencesInDexTest.java b/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesMissingReferencesInDexTest.java
index 8a080e3..8b59d47 100644
--- a/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesMissingReferencesInDexTest.java
+++ b/src/test/java/com/android/tools/r8/tracereferences/TraceReferencesMissingReferencesInDexTest.java
@@ -6,7 +6,9 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.DiagnosticsChecker;
import com.android.tools.r8.TestBase;
import com.android.tools.r8.TestParameters;
@@ -69,12 +71,17 @@
DiagnosticsChecker diagnosticsChecker = new DiagnosticsChecker();
MissingReferencesConsumer consumer = new MissingReferencesConsumer();
- TraceReferences.run(
- TraceReferencesCommand.builder(diagnosticsChecker)
- .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
- .addSourceFiles(sourceDex)
- .setConsumer(consumer)
- .build());
+ try {
+ TraceReferences.run(
+ TraceReferencesCommand.builder(diagnosticsChecker)
+ .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
+ .addSourceFiles(sourceDex)
+ .setConsumer(consumer)
+ .build());
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
assertTrue(consumer.acceptTypeCalled);
assertTrue(consumer.acceptFieldCalled);
@@ -93,12 +100,17 @@
DiagnosticsChecker diagnosticsChecker = new DiagnosticsChecker();
MissingReferencesConsumer consumer = new MissingReferencesConsumer();
- TraceReferences.run(
- TraceReferencesCommand.builder(diagnosticsChecker)
- .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
- .addSourceFiles(sourceDex)
- .setConsumer(consumer)
- .build());
+ try {
+ TraceReferences.run(
+ TraceReferencesCommand.builder(diagnosticsChecker)
+ .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
+ .addSourceFiles(sourceDex)
+ .setConsumer(consumer)
+ .build());
+ fail("Expected compilation to fail");
+ } catch (CompilationFailedException e) {
+ // Expected.
+ }
assertFalse(consumer.acceptTypeCalled);
assertTrue(consumer.acceptFieldCalled);
