Enable verbose printing of methods when using StackTraceParser

This CL will allow for outputting verbose method information when
retracing. We only implement verbose for the StackTraceParser since it
is not easy to figure out where to insert method formals and return
types in an arbitrary string defined by a regular expression.

Bug: 132850880
Change-Id: Ia567107d39b37c5ac3d9fc924ccc0eb6799d08ea
diff --git a/src/main/java/com/android/tools/r8/retrace/Retrace.java b/src/main/java/com/android/tools/r8/retrace/Retrace.java
index 79d9721..c6dad1b 100644
--- a/src/main/java/com/android/tools/r8/retrace/Retrace.java
+++ b/src/main/java/com/android/tools/r8/retrace/Retrace.java
@@ -128,7 +128,8 @@
                 .retrace();
       } else {
         result =
-            new RetraceStackTrace(retraceBase, command.stackTrace, command.diagnosticsHandler)
+            new RetraceStackTrace(
+                    retraceBase, command.stackTrace, command.diagnosticsHandler, command.isVerbose)
                 .retrace();
       }
       command.retracedStackTraceConsumer.accept(result.getNodes());
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java b/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java
index b2692a2..a7f194f 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceCommand.java
@@ -67,8 +67,8 @@
     }
 
     /** Set if the produced stack trace should have additional information. */
-    public Builder isVerbose() {
-      this.isVerbose = true;
+    public Builder setVerbose(boolean verbose) {
+      this.isVerbose = verbose;
       return this;
     }
 
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceStackTrace.java b/src/main/java/com/android/tools/r8/retrace/RetraceStackTrace.java
index 3a564d8..3b68d3d 100644
--- a/src/main/java/com/android/tools/r8/retrace/RetraceStackTrace.java
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceStackTrace.java
@@ -4,6 +4,7 @@
 
 package com.android.tools.r8.retrace;
 
+import static com.android.tools.r8.retrace.RetraceUtils.methodDescriptionFromMethodReference;
 import static com.google.common.base.Predicates.not;
 
 import com.android.tools.r8.DiagnosticsHandler;
@@ -11,7 +12,7 @@
 import com.android.tools.r8.references.MethodReference;
 import com.android.tools.r8.references.Reference;
 import com.android.tools.r8.utils.DescriptorUtils;
-import com.google.common.base.Strings;
+import com.android.tools.r8.utils.StringUtils;
 import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
 import java.util.List;
@@ -42,17 +43,21 @@
       if (lines.get(0).asAtLine().isAmbiguous) {
         lines.sort(new AtStackTraceLineComparator());
       }
-      String previousClazz = "";
+      boolean shouldPrintOr = false;
       for (StackTraceLine line : lines) {
         assert line.isAtLine();
         AtLine atLine = line.asAtLine();
-        if (atLine.isAmbiguous) {
+        if (atLine.isAmbiguous && shouldPrintOr) {
+          String atLineString = atLine.toString();
+          int firstNonWhitespaceCharacter = StringUtils.firstNonWhitespaceCharacter(atLineString);
           strings.add(
-              atLine.toString(previousClazz.isEmpty() ? atLine.at : "<OR> " + atLine.at, ""));
+              atLineString.substring(0, firstNonWhitespaceCharacter)
+                  + "<OR> "
+                  + atLineString.substring(firstNonWhitespaceCharacter));
         } else {
           strings.add(atLine.toString());
         }
-        previousClazz = atLine.clazz;
+        shouldPrintOr = true;
       }
     }
   }
@@ -84,12 +89,17 @@
   private final RetraceBase retraceBase;
   private final List<String> stackTrace;
   private final DiagnosticsHandler diagnosticsHandler;
+  private final boolean verbose;
 
   RetraceStackTrace(
-      RetraceBase retraceBase, List<String> stackTrace, DiagnosticsHandler diagnosticsHandler) {
+      RetraceBase retraceBase,
+      List<String> stackTrace,
+      DiagnosticsHandler diagnosticsHandler,
+      boolean verbose) {
     this.retraceBase = retraceBase;
     this.stackTrace = stackTrace;
     this.diagnosticsHandler = diagnosticsHandler;
+    this.verbose = verbose;
   }
 
   public RetraceCommandLineResult retrace() {
@@ -107,7 +117,7 @@
       return;
     }
     StackTraceLine stackTraceLine = parseLine(index + 1, stackTrace.get(index));
-    List<StackTraceLine> retraced = stackTraceLine.retrace(retraceBase);
+    List<StackTraceLine> retraced = stackTraceLine.retrace(retraceBase, verbose);
     StackTraceNode node = new StackTraceNode(retraced);
     result.add(node);
     retraceLine(stackTrace, index + 1, result);
@@ -115,7 +125,7 @@
 
   abstract static class StackTraceLine {
 
-    abstract List<StackTraceLine> retrace(RetraceBase retraceBase);
+    abstract List<StackTraceLine> retrace(RetraceBase retraceBase, boolean verbose);
 
     static int firstNonWhiteSpaceCharacterFromIndex(String line, int index) {
       return firstFromIndex(line, index, not(Character::isWhitespace));
@@ -215,7 +225,7 @@
     }
 
     @Override
-    List<StackTraceLine> retrace(RetraceBase retraceBase) {
+    List<StackTraceLine> retrace(RetraceBase retraceBase, boolean verbose) {
       List<StackTraceLine> exceptionLines = new ArrayList<>();
       retraceBase
           .retrace(Reference.classFromTypeName(exceptionClass))
@@ -267,6 +277,7 @@
     private final String at;
     private final String clazz;
     private final String method;
+    private final String methodAsString;
     private final String fileName;
     private final int linePosition;
     private final boolean isAmbiguous;
@@ -276,6 +287,7 @@
         String at,
         String clazz,
         String method,
+        String methodAsString,
         String fileName,
         int linePosition,
         boolean isAmbiguous) {
@@ -283,6 +295,7 @@
       this.at = at;
       this.clazz = clazz;
       this.method = method;
+      this.methodAsString = methodAsString;
       this.fileName = fileName;
       this.linePosition = linePosition;
       this.isAmbiguous = isAmbiguous;
@@ -335,11 +348,14 @@
       } else {
         fileName = line.substring(parensStart + 1, parensEnd);
       }
+      String className = line.substring(classStartIndex, methodSeparator);
+      String methodName = line.substring(methodSeparator + 1, parensStart);
       return new AtLine(
           line.substring(0, firstNonWhiteSpace),
           line.substring(firstNonWhiteSpace, classStartIndex),
-          line.substring(classStartIndex, methodSeparator),
-          line.substring(methodSeparator + 1, parensStart),
+          className,
+          methodName,
+          className + "." + methodName,
           fileName,
           position,
           false);
@@ -350,49 +366,38 @@
     }
 
     @Override
-    List<StackTraceLine> retrace(RetraceBase retraceBase) {
+    List<StackTraceLine> retrace(RetraceBase retraceBase, boolean verbose) {
       List<StackTraceLine> lines = new ArrayList<>();
       ClassReference classReference = Reference.classFromTypeName(clazz);
-      RetraceMethodResult retraceMethodResult =
-          retraceBase
-              .retrace(classReference)
-              .lookupMethod(method)
-              .narrowByLine(linePosition)
-              .forEach(
-                  methodElement -> {
-                    MethodReference methodReference = methodElement.getMethodReference();
-                    lines.add(
-                        new AtLine(
-                            startingWhitespace,
-                            at,
-                            methodReference.getHolderClass().getTypeName(),
-                            methodReference.getMethodName(),
-                            retraceBase.retraceSourceFile(
-                                classReference, fileName, methodReference.getHolderClass(), true),
-                            hasLinePosition()
-                                ? methodElement.getOriginalLineNumber(linePosition)
-                                : linePosition,
-                            methodElement.getRetraceMethodResult().isAmbiguous()));
-                  });
+      retraceBase
+          .retrace(classReference)
+          .lookupMethod(method)
+          .narrowByLine(linePosition)
+          .forEach(
+              methodElement -> {
+                MethodReference methodReference = methodElement.getMethodReference();
+                lines.add(
+                    new AtLine(
+                        startingWhitespace,
+                        at,
+                        methodReference.getHolderClass().getTypeName(),
+                        methodReference.getMethodName(),
+                        methodDescriptionFromMethodReference(methodReference, verbose),
+                        retraceBase.retraceSourceFile(
+                            classReference, fileName, methodReference.getHolderClass(), true),
+                        hasLinePosition()
+                            ? methodElement.getOriginalLineNumber(linePosition)
+                            : linePosition,
+                        methodElement.getRetraceMethodResult().isAmbiguous()));
+              });
       return lines;
     }
 
     @Override
     public String toString() {
-      return toString(at, "");
-    }
-
-    protected String toString(String at, String previousClass) {
       StringBuilder sb = new StringBuilder(startingWhitespace);
       sb.append(at);
-      String commonPrefix = Strings.commonPrefix(clazz, previousClass);
-      if (commonPrefix.length() == clazz.length()) {
-        sb.append(Strings.repeat(" ", clazz.length() + 1));
-      } else {
-        sb.append(Strings.padStart(clazz.substring(commonPrefix.length()), clazz.length(), ' '));
-        sb.append(".");
-      }
-      sb.append(method);
+      sb.append(methodAsString);
       sb.append("(");
       sb.append(fileName);
       if (linePosition != NO_POSITION) {
@@ -443,7 +448,7 @@
     }
 
     @Override
-    List<StackTraceLine> retrace(RetraceBase retraceBase) {
+    List<StackTraceLine> retrace(RetraceBase retraceBase, boolean verbose) {
       return ImmutableList.of(new MoreLine(line));
     }
 
@@ -461,7 +466,7 @@
     }
 
     @Override
-    List<StackTraceLine> retrace(RetraceBase retraceBase) {
+    List<StackTraceLine> retrace(RetraceBase retraceBase, boolean verbose) {
       return ImmutableList.of(new UnknownLine(line));
     }
 
diff --git a/src/main/java/com/android/tools/r8/retrace/RetraceUtils.java b/src/main/java/com/android/tools/r8/retrace/RetraceUtils.java
new file mode 100644
index 0000000..81c2272
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/retrace/RetraceUtils.java
@@ -0,0 +1,38 @@
+// Copyright (c) 2019, 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.retrace;
+
+import com.android.tools.r8.references.MethodReference;
+import com.android.tools.r8.references.TypeReference;
+
+public class RetraceUtils {
+
+  public static String methodDescriptionFromMethodReference(
+      MethodReference methodReference, boolean verbose) {
+    if (!verbose || methodReference.isUnknown()) {
+      return methodReference.getHolderClass().getTypeName() + "." + methodReference.getMethodName();
+    }
+    StringBuilder sb = new StringBuilder();
+    sb.append(
+        methodReference.getReturnType() == null
+            ? "void"
+            : methodReference.getReturnType().getTypeName());
+    sb.append(" ");
+    sb.append(methodReference.getHolderClass().getTypeName());
+    sb.append(".");
+    sb.append(methodReference.getMethodName());
+    sb.append("(");
+    boolean seenFirstIndex = false;
+    for (TypeReference formalType : methodReference.getFormalTypes()) {
+      if (seenFirstIndex) {
+        sb.append(",");
+      }
+      seenFirstIndex = true;
+      sb.append(formalType.getTypeName());
+    }
+    sb.append(")");
+    return sb.toString();
+  }
+}
diff --git a/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineWithoutNullCheck.java b/src/test/java/com/android/tools/r8/retrace/InlineWithoutNullCheckTest.java
similarity index 94%
rename from src/test/java/com/android/tools/r8/retrace/stacktraces/InlineWithoutNullCheck.java
rename to src/test/java/com/android/tools/r8/retrace/InlineWithoutNullCheckTest.java
index 6126c4b..c46d3ce 100644
--- a/src/test/java/com/android/tools/r8/retrace/stacktraces/InlineWithoutNullCheck.java
+++ b/src/test/java/com/android/tools/r8/retrace/InlineWithoutNullCheckTest.java
@@ -2,7 +2,7 @@
 // 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.retrace.stacktraces;
+package com.android.tools.r8.retrace;
 
 import static com.android.tools.r8.naming.retrace.StackTrace.isSame;
 import static com.android.tools.r8.naming.retrace.StackTrace.isSameExceptForFileNameAndLineNumber;
@@ -25,7 +25,7 @@
 import org.junit.runners.Parameterized;
 
 @RunWith(Parameterized.class)
-public class InlineWithoutNullCheck extends TestBase {
+public class InlineWithoutNullCheckTest extends TestBase {
 
   private final TestParameters parameters;
 
@@ -34,7 +34,7 @@
     return getTestParameters().withAllRuntimes().withAllApiLevels().build();
   }
 
-  public InlineWithoutNullCheck(TestParameters parameters) {
+  public InlineWithoutNullCheckTest(TestParameters parameters) {
     this.parameters = parameters;
   }
 
@@ -47,21 +47,21 @@
     // Get the expected stack traces by running on the runtime to test.
     expectedStackTraceForInlineMethod =
         testForRuntime(parameters.getRuntime(), parameters.getApiLevel())
-            .addInnerClasses(InlineWithoutNullCheck.class)
+            .addInnerClasses(InlineWithoutNullCheckTest.class)
             .run(parameters.getRuntime(), TestClassForInlineMethod.class)
             .writeProcessResult(System.out)
             .assertFailure()
             .getStackTrace();
     expectedStackTraceForInlineField =
         testForRuntime(parameters.getRuntime(), parameters.getApiLevel())
-            .addInnerClasses(InlineWithoutNullCheck.class)
+            .addInnerClasses(InlineWithoutNullCheckTest.class)
             .run(parameters.getRuntime(), TestClassForInlineField.class)
             .writeProcessResult(System.out)
             .assertFailure()
             .getStackTrace();
     expectedStackTraceForInlineStaticField =
         testForRuntime(parameters.getRuntime(), parameters.getApiLevel())
-            .addInnerClasses(InlineWithoutNullCheck.class)
+            .addInnerClasses(InlineWithoutNullCheckTest.class)
             .run(parameters.getRuntime(), TestClassForInlineStaticField.class)
             .writeProcessResult(System.out)
             .assertFailure()
@@ -104,7 +104,7 @@
   @Test
   public void testInlineMethodWhichChecksNullReceiverBeforeAnySideEffectMethod() throws Exception {
     testForR8(parameters.getBackend())
-        .addInnerClasses(InlineWithoutNullCheck.class)
+        .addInnerClasses(InlineWithoutNullCheckTest.class)
         .addKeepMainRule(TestClassForInlineMethod.class)
         .enableInliningAnnotations()
         .setMinApi(parameters.getApiLevel())
@@ -132,7 +132,7 @@
   @Test
   public void testInlineMethodWhichChecksNullReceiverBeforeAnySideEffectField() throws Exception {
     testForR8(parameters.getBackend())
-        .addInnerClasses(InlineWithoutNullCheck.class)
+        .addInnerClasses(InlineWithoutNullCheckTest.class)
         .addKeepMainRule(TestClassForInlineField.class)
         .enableInliningAnnotations()
         .setMinApi(parameters.getApiLevel())
@@ -161,7 +161,7 @@
   public void testInlineMethodWhichChecksNullReceiverBeforeAnySideEffectStaticField()
       throws Exception {
     testForR8(parameters.getBackend())
-        .addInnerClasses(InlineWithoutNullCheck.class)
+        .addInnerClasses(InlineWithoutNullCheckTest.class)
         .addKeepMainRule(TestClassForInlineStaticField.class)
         .enableInliningAnnotations()
         .setMinApi(parameters.getApiLevel())
diff --git a/src/test/java/com/android/tools/r8/retrace/RetraceVerboseTests.java b/src/test/java/com/android/tools/r8/retrace/RetraceVerboseTests.java
new file mode 100644
index 0000000..277d190
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/retrace/RetraceVerboseTests.java
@@ -0,0 +1,54 @@
+// Copyright (c) 2019, 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.retrace;
+
+import static junit.framework.TestCase.assertEquals;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestDiagnosticMessagesImpl;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.retrace.stacktraces.FoundMethodVerboseStackTrace;
+import com.android.tools.r8.retrace.stacktraces.StackTraceForTest;
+import com.android.tools.r8.retrace.stacktraces.UnknownMethodVerboseStackTrace;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class RetraceVerboseTests extends TestBase {
+
+  @Parameters(name = "{0}")
+  public static TestParametersCollection data() {
+    return getTestParameters().withNoneRuntime().build();
+  }
+
+  public RetraceVerboseTests(TestParameters parameters) {}
+
+  @Test
+  public void testFoundMethod() {
+    runRetraceTest(new FoundMethodVerboseStackTrace());
+  }
+
+  @Test
+  public void testUnknownMethod() {
+    runRetraceTest(new UnknownMethodVerboseStackTrace());
+  }
+
+  private TestDiagnosticMessagesImpl runRetraceTest(StackTraceForTest stackTraceForTest) {
+    TestDiagnosticMessagesImpl diagnosticsHandler = new TestDiagnosticMessagesImpl();
+    RetraceCommand retraceCommand =
+        RetraceCommand.builder(diagnosticsHandler)
+            .setProguardMapProducer(stackTraceForTest::mapping)
+            .setStackTrace(stackTraceForTest.obfuscatedStackTrace())
+            .setVerbose(true)
+            .setRetracedStackTraceConsumer(
+                retraced -> assertEquals(stackTraceForTest.retracedStackTrace(), retraced))
+            .build();
+    Retrace.run(retraceCommand);
+    return diagnosticsHandler;
+  }
+}
diff --git a/src/test/java/com/android/tools/r8/retrace/stacktraces/FoundMethodVerboseStackTrace.java b/src/test/java/com/android/tools/r8/retrace/stacktraces/FoundMethodVerboseStackTrace.java
new file mode 100644
index 0000000..4d9418c
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/retrace/stacktraces/FoundMethodVerboseStackTrace.java
@@ -0,0 +1,38 @@
+// Copyright (c) 2019, 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.retrace.stacktraces;
+
+import com.android.tools.r8.utils.StringUtils;
+import java.util.Arrays;
+import java.util.List;
+
+public class FoundMethodVerboseStackTrace implements StackTraceForTest {
+
+  @Override
+  public List<String> obfuscatedStackTrace() {
+    return Arrays.asList(
+        "Exception in thread \"main\" java.lang.NullPointerException", "\tat a.a.a(Foo.java:7)");
+  }
+
+  @Override
+  public String mapping() {
+    return StringUtils.lines(
+        "com.android.tools.r8.naming.retrace.Main -> a.a:",
+        "    7:7:com.android.Foo main(java.lang.String[],com.android.Bar):102 -> a");
+  }
+
+  @Override
+  public List<String> retracedStackTrace() {
+    return Arrays.asList(
+        "Exception in thread \"main\" java.lang.NullPointerException",
+        "\tat com.android.Foo com.android.tools.r8.naming.retrace.Main.main(java.lang.String[],"
+            + "com.android.Bar)(Main.java:102)");
+  }
+
+  @Override
+  public int expectedWarnings() {
+    return 0;
+  }
+}
diff --git a/src/test/java/com/android/tools/r8/retrace/stacktraces/UnknownMethodVerboseStackTrace.java b/src/test/java/com/android/tools/r8/retrace/stacktraces/UnknownMethodVerboseStackTrace.java
new file mode 100644
index 0000000..3eb9710
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/retrace/stacktraces/UnknownMethodVerboseStackTrace.java
@@ -0,0 +1,48 @@
+// Copyright (c) 2019, 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.retrace.stacktraces;
+
+import com.android.tools.r8.utils.StringUtils;
+import java.util.Arrays;
+import java.util.List;
+
+public class UnknownMethodVerboseStackTrace implements StackTraceForTest {
+
+  @Override
+  public List<String> obfuscatedStackTrace() {
+    return Arrays.asList(
+        "Exception in thread \"main\" java.lang.NullPointerException",
+        "\tat a.a.c(Foo.java)",
+        "\tat a.a.b(Bar.java)",
+        "\tat a.a.a(Baz.java)");
+  }
+
+  @Override
+  public String mapping() {
+    return StringUtils.lines(
+        "com.android.tools.r8.naming.retrace.Main -> a.a:",
+        "    com.android.Foo main(java.lang.String[],com.android.Bar) -> a",
+        "    com.android.Foo main(java.lang.String[]) -> b",
+        "    com.android.Bar main(com.android.Bar) -> b");
+  }
+
+  @Override
+  public List<String> retracedStackTrace() {
+    return Arrays.asList(
+        "Exception in thread \"main\" java.lang.NullPointerException",
+        "\tat com.android.tools.r8.naming.retrace.Main.c(Main.java)",
+        "\tat com.android.Foo com.android.tools.r8.naming.retrace.Main.main("
+            + "java.lang.String[])(Main.java)",
+        "\t<OR> at com.android.Bar com.android.tools.r8.naming.retrace.Main.main("
+            + "com.android.Bar)(Main.java)",
+        "\tat com.android.Foo com.android.tools.r8.naming.retrace.Main.main("
+            + "java.lang.String[],com.android.Bar)(Main.java)");
+  }
+
+  @Override
+  public int expectedWarnings() {
+    return 0;
+  }
+}