Make missing symbol diagnostics more consistent

and record arguments in TurbineError.

MOE_MIGRATED_REVID=192525271

15 files changed, 117 insertions, 60 deletions

diff --git a/java/com/google/turbine/binder/ConstBinder.java b/java/com/google/turbine/binder/ConstBinder.java
index 4cb40a2..4b48dd3 100644
--- a/java/com/google/turbine/binder/ConstBinder.java
+++ b/java/com/google/turbine/binder/ConstBinder.java
@@ -182,7 +182,7 @@ public class ConstBinder {
       return null;
     }
     EnumConstantValue enumValue = (EnumConstantValue) value;
-    if (!enumValue.sym().owner().toString().equals("java/lang/annotation/RetentionPolicy")) {
+    if (!enumValue.sym().owner().binaryName().equals("java/lang/annotation/RetentionPolicy")) {
       return null;
     }
     return RetentionPolicy.valueOf(enumValue.sym().name());
diff --git a/java/com/google/turbine/binder/ConstEvaluator.java b/java/com/google/turbine/binder/ConstEvaluator.java
index d661fb9..ad9d117 100644
--- a/java/com/google/turbine/binder/ConstEvaluator.java
+++ b/java/com/google/turbine/binder/ConstEvaluator.java
@@ -199,13 +199,16 @@ public strictfp class ConstEvaluator {
     }
     LookupResult result = scope.lookup(new LookupKey(flat));
     if (result == null) {
-      throw error(classTy.position(), ErrorKind.SYMBOL_NOT_FOUND, flat.peekFirst());
+      throw error(classTy.position(), ErrorKind.CANNOT_RESOLVE, flat.peekFirst());
     }
     ClassSymbol classSym = (ClassSymbol) result.sym();
     for (String bit : result.remaining()) {
       classSym = Resolve.resolve(env, origin, classSym, bit);
       if (classSym == null) {
-        throw error(classTy.position(), ErrorKind.SYMBOL_NOT_FOUND, bit);
+        throw error(
+            classTy.position(),
+            ErrorKind.SYMBOL_NOT_FOUND,
+            new ClassSymbol(classSym.binaryName() + '$' + bit));
       }
     }
     return classSym;
diff --git a/java/com/google/turbine/binder/HierarchyBinder.java b/java/com/google/turbine/binder/HierarchyBinder.java
index c3e82ba..2545c17 100644
--- a/java/com/google/turbine/binder/HierarchyBinder.java
+++ b/java/com/google/turbine/binder/HierarchyBinder.java
@@ -31,6 +31,7 @@ import com.google.turbine.diag.TurbineError;
 import com.google.turbine.diag.TurbineError.ErrorKind;
 import com.google.turbine.model.TurbineTyKind;
 import com.google.turbine.tree.Tree;
+import com.google.turbine.tree.Tree.ClassTy;
 import java.util.ArrayDeque;
 
 /** Type hierarchy binding. */
@@ -116,24 +117,31 @@ public class HierarchyBinder {
     // Resolve the base symbol in the qualified name.
     LookupResult result = lookup(ty, new LookupKey(flat));
     if (result == null) {
-      throw TurbineError.format(base.source(), ty.position(), ErrorKind.SYMBOL_NOT_FOUND, ty);
+      throw TurbineError.format(base.source(), ty.position(), ErrorKind.CANNOT_RESOLVE, ty);
     }
     // Resolve pieces in the qualified name referring to member types.
     // This needs to consider member type declarations inherited from supertypes and interfaces.
     ClassSymbol sym = (ClassSymbol) result.sym();
     for (String bit : result.remaining()) {
-      try {
-        sym = Resolve.resolve(env, origin, sym, bit);
-      } catch (LazyBindingError e) {
-        throw error(ty.position(), ErrorKind.CYCLIC_HIERARCHY, e.getMessage());
-      }
-      if (sym == null) {
-        throw error(ty.position(), ErrorKind.SYMBOL_NOT_FOUND, bit);
-      }
+      sym = resolveNext(ty, sym, bit);
     }
     return sym;
   }
 
+  private ClassSymbol resolveNext(ClassTy ty, ClassSymbol sym, String bit) {
+    ClassSymbol next;
+    try {
+      next = Resolve.resolve(env, origin, sym, bit);
+    } catch (LazyBindingError e) {
+      throw error(ty.position(), ErrorKind.CYCLIC_HIERARCHY, e.getMessage());
+    }
+    if (next == null) {
+      throw error(
+          ty.position(), ErrorKind.SYMBOL_NOT_FOUND, new ClassSymbol(sym.binaryName() + '$' + bit));
+    }
+    return next;
+  }
+
   /** Resolve a qualified type name to a symbol. */
   private LookupResult lookup(Tree tree, LookupKey lookup) {
     // Handle any lexically enclosing class declarations (if we're binding a member class).
diff --git a/java/com/google/turbine/binder/ModuleBinder.java b/java/com/google/turbine/binder/ModuleBinder.java
index 2cf3d53..312ec45 100644
--- a/java/com/google/turbine/binder/ModuleBinder.java
+++ b/java/com/google/turbine/binder/ModuleBinder.java
@@ -206,13 +206,15 @@ public class ModuleBinder {
     LookupKey key = new LookupKey(simpleNames);
     LookupResult result = scope.lookup(key);
     if (result == null) {
-      throw error(ErrorKind.SYMBOL_NOT_FOUND, pos, Joiner.on('.').join(simpleNames));
+      throw error(
+          ErrorKind.SYMBOL_NOT_FOUND, pos, new ClassSymbol(Joiner.on('/').join(simpleNames)));
     }
     ClassSymbol sym = (ClassSymbol) result.sym();
     for (String name : result.remaining()) {
       sym = Resolve.resolve(env, /* origin= */ null, sym, name);
       if (sym == null) {
-        throw error(ErrorKind.SYMBOL_NOT_FOUND, pos, name);
+        throw error(
+            ErrorKind.SYMBOL_NOT_FOUND, pos, new ClassSymbol(sym.binaryName() + '$' + name));
       }
     }
     return sym;
diff --git a/java/com/google/turbine/binder/Resolve.java b/java/com/google/turbine/binder/Resolve.java
index d06dbc1..d19f97d 100644
--- a/java/com/google/turbine/binder/Resolve.java
+++ b/java/com/google/turbine/binder/Resolve.java
@@ -107,14 +107,23 @@ public class Resolve {
     public ClassSymbol resolve(SourceFile source, int position, LookupResult result) {
       ClassSymbol sym = (ClassSymbol) result.sym();
       for (String bit : result.remaining()) {
-        sym = resolveOne(sym, bit);
-        if (sym == null) {
-          throw TurbineError.format(source, position, ErrorKind.SYMBOL_NOT_FOUND, bit);
-        }
+        sym = resolveNext(source, position, sym, bit);
       }
       return sym;
     }
 
+    private ClassSymbol resolveNext(SourceFile source, int position, ClassSymbol sym, String bit) {
+      ClassSymbol next = resolveOne(sym, bit);
+      if (next == null) {
+        throw TurbineError.format(
+            source,
+            position,
+            ErrorKind.SYMBOL_NOT_FOUND,
+            new ClassSymbol(sym.binaryName() + '$' + bit));
+      }
+      return next;
+    }
+
     @Override
     public ClassSymbol resolveOne(ClassSymbol sym, String bit) {
       BoundClass ci = env.get(sym);
diff --git a/java/com/google/turbine/binder/TypeBinder.java b/java/com/google/turbine/binder/TypeBinder.java
index 964958b..e0546ac 100644
--- a/java/com/google/turbine/binder/TypeBinder.java
+++ b/java/com/google/turbine/binder/TypeBinder.java
@@ -560,14 +560,11 @@ public class TypeBinder {
     for (Tree.Anno tree : trees) {
       LookupResult lookupResult = scope.lookup(new LookupKey(tree.name()));
       if (lookupResult == null) {
-        throw error(tree.position(), ErrorKind.SYMBOL_NOT_FOUND, Joiner.on('.').join(tree.name()));
+        throw error(tree.position(), ErrorKind.CANNOT_RESOLVE, Joiner.on('.').join(tree.name()));
       }
       ClassSymbol sym = (ClassSymbol) lookupResult.sym();
       for (String name : lookupResult.remaining()) {
-        sym = Resolve.resolve(env, owner, sym, name);
-        if (sym == null) {
-          throw error(tree.position(), ErrorKind.SYMBOL_NOT_FOUND, name);
-        }
+        sym = resolveNext(tree.position(), sym, name);
       }
       if (env.get(sym).kind() != TurbineTyKind.ANNOTATION) {
         throw error(tree.position(), ErrorKind.NOT_AN_ANNOTATION, sym);
@@ -577,6 +574,15 @@ public class TypeBinder {
     return result.build();
   }
 
+  private ClassSymbol resolveNext(int position, ClassSymbol sym, String bit) {
+    ClassSymbol next = Resolve.resolve(env, owner, sym, bit);
+    if (next == null) {
+      throw error(
+          position, ErrorKind.SYMBOL_NOT_FOUND, new ClassSymbol(sym.binaryName() + '$' + bit));
+    }
+    return next;
+  }
+
   private ImmutableList<Type> bindTyArgs(CompoundScope scope, ImmutableList<Tree.Type> targs) {
     ImmutableList.Builder<Type> result = ImmutableList.builder();
     for (Tree.Type ty : targs) {
@@ -627,7 +633,7 @@ public class TypeBinder {
     // resolve the prefix to a symbol
     LookupResult result = scope.lookup(new LookupKey(names));
     if (result == null || result.sym() == null) {
-      throw error(t.position(), ErrorKind.SYMBOL_NOT_FOUND, t);
+      throw error(t.position(), ErrorKind.CANNOT_RESOLVE, t);
     }
     Symbol sym = result.sym();
     int annoIdx = flat.size() - result.remaining().size() - 1;
@@ -660,10 +666,8 @@ public class TypeBinder {
             sym, bindTyArgs(scope, flat.get(idx++).tyargs()), annotations));
     for (; idx < flat.size(); idx++) {
       Tree.ClassTy curr = flat.get(idx);
-      sym = Resolve.resolve(env, owner, sym, curr.name());
-      if (sym == null) {
-        throw error(curr.position(), ErrorKind.CANNOT_RESOLVE, curr.name());
-      }
+      sym = resolveNext(curr.position(), sym, curr.name());
+
       annotations = bindAnnotations(scope, curr.annos());
       classes.add(
           new Type.ClassTy.SimpleClassTy(sym, bindTyArgs(scope, curr.tyargs()), annotations));
diff --git a/java/com/google/turbine/binder/bytecode/BytecodeBoundClass.java b/java/com/google/turbine/binder/bytecode/BytecodeBoundClass.java
index 0d9a6f9..cfab044 100644
--- a/java/com/google/turbine/binder/bytecode/BytecodeBoundClass.java
+++ b/java/com/google/turbine/binder/bytecode/BytecodeBoundClass.java
@@ -83,7 +83,7 @@ public class BytecodeBoundClass implements BoundClass, HeaderBoundClass, TypeBou
             new Supplier<ClassFile>() {
               @Override
               public ClassFile get() {
-                ClassFile cf = ClassReader.read(jarFile + "!" + sym, bytes.get());
+                ClassFile cf = ClassReader.read(jarFile + "!" + sym.binaryName(), bytes.get());
                 verify(
                     cf.name().equals(sym.binaryName()),
                     "expected class data for %s, saw %s instead",
diff --git a/java/com/google/turbine/binder/lookup/ImportIndex.java b/java/com/google/turbine/binder/lookup/ImportIndex.java
index 305bbfd..d644db2 100644
--- a/java/com/google/turbine/binder/lookup/ImportIndex.java
+++ b/java/com/google/turbine/binder/lookup/ImportIndex.java
@@ -103,7 +103,10 @@ public class ImportIndex implements ImportScope {
     LookupResult result = cpi.scope().lookup(new LookupKey(i.type()));
     if (result == null) {
       throw TurbineError.format(
-          source, i.position(), ErrorKind.SYMBOL_NOT_FOUND, Joiner.on('.').join(i.type()));
+          source,
+          i.position(),
+          ErrorKind.SYMBOL_NOT_FOUND,
+          new ClassSymbol(Joiner.on('/').join(i.type())));
     }
     ClassSymbol sym = resolve.resolve(source, i.position(), result);
     return new ImportScope() {
diff --git a/java/com/google/turbine/binder/lookup/ImportScope.java b/java/com/google/turbine/binder/lookup/ImportScope.java
index 69ab78d..949b3d1 100644
--- a/java/com/google/turbine/binder/lookup/ImportScope.java
+++ b/java/com/google/turbine/binder/lookup/ImportScope.java
@@ -40,10 +40,15 @@ public interface ImportScope {
     default ClassSymbol resolve(SourceFile source, int position, LookupResult result) {
       ClassSymbol sym = (ClassSymbol) result.sym();
       for (String bit : result.remaining()) {
-        sym = resolveOne(sym, bit);
-        if (sym == null) {
-          throw TurbineError.format(source, position, ErrorKind.SYMBOL_NOT_FOUND, bit);
+        ClassSymbol next = resolveOne(sym, bit);
+        if (next == null) {
+          throw TurbineError.format(
+              source,
+              position,
+              ErrorKind.SYMBOL_NOT_FOUND,
+              new ClassSymbol(sym.binaryName() + '$' + bit));
         }
+        sym = next;
       }
       return sym;
     }
diff --git a/java/com/google/turbine/binder/lookup/SimpleTopLevelIndex.java b/java/com/google/turbine/binder/lookup/SimpleTopLevelIndex.java
index 24e5775..7fc7913 100644
--- a/java/com/google/turbine/binder/lookup/SimpleTopLevelIndex.java
+++ b/java/com/google/turbine/binder/lookup/SimpleTopLevelIndex.java
@@ -85,7 +85,7 @@ public class SimpleTopLevelIndex implements TopLevelIndex {
 
     /** Inserts a {@link ClassSymbol} into the index, creating any needed packages. */
     public boolean insert(ClassSymbol sym) {
-      Iterator<String> it = Splitter.on('/').split(sym.toString()).iterator();
+      Iterator<String> it = Splitter.on('/').split(sym.binaryName()).iterator();
       Node curr = root;
       while (it.hasNext()) {
         String simpleName = it.next();
diff --git a/java/com/google/turbine/binder/sym/ClassSymbol.java b/java/com/google/turbine/binder/sym/ClassSymbol.java
index 63dcfb1..2adf15f 100644
--- a/java/com/google/turbine/binder/sym/ClassSymbol.java
+++ b/java/com/google/turbine/binder/sym/ClassSymbol.java
@@ -47,7 +47,7 @@ public class ClassSymbol implements Symbol {
 
   @Override
   public String toString() {
-    return className;
+    return className.replace('/', '.');
   }
 
   @Override
diff --git a/java/com/google/turbine/diag/TurbineError.java b/java/com/google/turbine/diag/TurbineError.java
index 8c75345..cd7d9e1 100644
--- a/java/com/google/turbine/diag/TurbineError.java
+++ b/java/com/google/turbine/diag/TurbineError.java
@@ -17,9 +17,13 @@
 package com.google.turbine.diag;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.Iterables.getOnlyElement;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
+import com.google.turbine.binder.sym.ClassSymbol;
 
 /** A compilation error. */
 public class TurbineError extends Error {
@@ -37,7 +41,7 @@ public class TurbineError extends Error {
     TYPE_PARAMETER_QUALIFIER("type parameter used as type qualifier"),
     UNEXPECTED_TOKEN("unexpected token: %s"),
     INVALID_ANNOTATION_ARGUMENT("invalid annotation argument"),
-    CANNOT_RESOLVE("cannot resolve %s"),
+    CANNOT_RESOLVE("could not resolve %s"),
     EXPRESSION_ERROR("could not evaluate constant expression"),
     CYCLIC_HIERARCHY("cycle in class hierarchy: %s"),
     NOT_AN_ANNOTATION("%s is not an annotation"),
@@ -67,7 +71,7 @@ public class TurbineError extends Error {
     String path = firstNonNull(source.path(), "<>");
     String message = kind.format(args);
     String diagnostic = path + ": error: " + message.trim() + System.lineSeparator();
-    return new TurbineError(kind, diagnostic);
+    return new TurbineError(kind, diagnostic, ImmutableList.copyOf(args));
   }
 
   /**
@@ -92,18 +96,37 @@ public class TurbineError extends Error {
         .append(System.lineSeparator());
     sb.append(Strings.repeat(" ", column)).append('^');
     String diagnostic = sb.toString();
-    return new TurbineError(kind, diagnostic);
+    return new TurbineError(kind, diagnostic, ImmutableList.copyOf(args));
   }
 
-  final ErrorKind kind;
+  private final ErrorKind kind;
+  private final ImmutableList<Object> args;
 
-  private TurbineError(ErrorKind kind, String diagnostic) {
+  private TurbineError(ErrorKind kind, String diagnostic, ImmutableList<Object> args) {
     super(diagnostic);
+    switch (kind) {
+      case SYMBOL_NOT_FOUND:
+        {
+          checkArgument(
+              args.size() == 1 && getOnlyElement(args) instanceof ClassSymbol,
+              "diagnostic (%s) has invalid argument args %s",
+              diagnostic,
+              args);
+          break;
+        }
+      default: // fall out
+    }
     this.kind = kind;
+    this.args = args;
   }
 
   /** The diagnostic kind. */
   public ErrorKind kind() {
     return kind;
   }
+
+  /** The diagnostic arguments. */
+  public ImmutableList<Object> args() {
+    return args;
+  }
 }
diff --git a/java/com/google/turbine/type/Type.java b/java/com/google/turbine/type/Type.java
index cdd8605..61a5bbe 100644
--- a/java/com/google/turbine/type/Type.java
+++ b/java/com/google/turbine/type/Type.java
@@ -106,9 +106,9 @@ public interface Type {
       for (SimpleClassTy c : classes) {
         if (!first) {
           sb.append('.');
-          sb.append(c.sym.toString().substring(c.sym.toString().lastIndexOf('$') + 1));
+          sb.append(c.sym.binaryName().substring(c.sym.binaryName().lastIndexOf('$') + 1));
         } else {
-          sb.append(c.sym);
+          sb.append(c.sym.binaryName());
         }
         if (!c.targs.isEmpty()) {
           sb.append('<');
diff --git a/javatests/com/google/turbine/binder/BinderErrorTest.java b/javatests/com/google/turbine/binder/BinderErrorTest.java
index a4c2e25..06088ab 100644
--- a/javatests/com/google/turbine/binder/BinderErrorTest.java
+++ b/javatests/com/google/turbine/binder/BinderErrorTest.java
@@ -46,7 +46,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:2: error: symbol not found NoSuch",
+          "<>:2: error: could not resolve NoSuch",
           "public class A extends NoSuch {",
           "                       ^",
         }
@@ -61,7 +61,7 @@ public class BinderErrorTest {
         },
         // TODO(cushon): we'd prefer the caret at NoSuch instead of A
         {
-          "<>:4: error: symbol not found NoSuch", //
+          "<>:4: error: symbol not found a.A$NoSuch", //
           "class B extends A.NoSuch {",
           "                ^",
         }
@@ -73,7 +73,7 @@ public class BinderErrorTest {
           "class B extends A<NoSuch> {}",
         },
         {
-          "<>:3: error: symbol not found NoSuch",
+          "<>:3: error: could not resolve NoSuch",
           "class B extends A<NoSuch> {}",
           "                  ^",
         }
@@ -84,7 +84,7 @@ public class BinderErrorTest {
           "@Anno(foo=100, bar=200) class Test {}",
         },
         {
-          "<>:2: error: cannot resolve foo", //
+          "<>:2: error: could not resolve foo", //
           "@Anno(foo=100, bar=200) class Test {}",
           "      ^",
         },
@@ -95,7 +95,7 @@ public class BinderErrorTest {
           "@Anno(foo=100, bar=200) class Test {}",
         },
         {
-          "<>:2: error: cannot resolve bar", //
+          "<>:2: error: could not resolve bar", //
           "@Anno(foo=100, bar=200) class Test {}",
           "               ^",
         },
@@ -146,8 +146,8 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:4: error: cycle in class hierarchy: p/OuterExtendsInner$Inner"
-              + " -> p/OuterExtendsInner$Inner",
+          "<>:4: error: cycle in class hierarchy: p.OuterExtendsInner$Inner"
+              + " -> p.OuterExtendsInner$Inner",
           "  public static class Inner extends Foo {}",
           "                                    ^"
         },
@@ -173,7 +173,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:2: error: symbol not found NoSuch", //
+          "<>:2: error: symbol not found java.util.List$NoSuch", //
           "import java.util.List.NoSuch;",
           "       ^"
         },
@@ -186,7 +186,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:2: error: symbol not found NoSuch", //
+          "<>:2: error: symbol not found java.util.List$NoSuch", //
           "import static java.util.List.NoSuch;",
           "              ^"
         },
@@ -199,7 +199,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:3: error: symbol not found NoSuchOther",
+          "<>:3: error: could not resolve NoSuchOther",
           "public class Test extends NoSuchOther {",
           "                          ^",
         },
@@ -212,7 +212,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:3: error: symbol not found NoSuchOther",
+          "<>:3: error: could not resolve NoSuchOther",
           "public class Test extends NoSuchOther {",
           "                          ^",
         },
@@ -225,7 +225,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:3: error: symbol not found NoSuchOther",
+          "<>:3: error: could not resolve NoSuchOther",
           "public class Test extends NoSuchOther {",
           "                          ^",
         },
@@ -238,7 +238,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:3: error: symbol not found NoSuchOther",
+          "<>:3: error: could not resolve NoSuchOther",
           "public class Test extends NoSuchOther {",
           "                          ^",
         },
@@ -250,7 +250,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:2: error: java/lang/Object is not an annotation", //
+          "<>:2: error: java.lang.Object is not an annotation", //
           "  @Object int x;",
           "   ^",
         },
@@ -262,7 +262,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:2: error: java/lang/Deprecated is not @Repeatable", //
+          "<>:2: error: java.lang.Deprecated is not @Repeatable", //
           "  @Deprecated @Deprecated int x;",
           "   ^",
         },
@@ -274,7 +274,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:2: error: symbol not found NoSuch.NoSuch", //
+          "<>:2: error: could not resolve NoSuch.NoSuch", //
           "  @NoSuch.NoSuch int x;",
           "   ^",
         },
@@ -286,7 +286,7 @@ public class BinderErrorTest {
           "}",
         },
         {
-          "<>:2: error: symbol not found NoSuch", //
+          "<>:2: error: symbol not found java.lang.Deprecated$NoSuch", //
           "  @Deprecated.NoSuch int x;",
           "   ^",
         },
diff --git a/javatests/com/google/turbine/binder/BinderTest.java b/javatests/com/google/turbine/binder/BinderTest.java
index 9ba5705..700baf0 100644
--- a/javatests/com/google/turbine/binder/BinderTest.java
+++ b/javatests/com/google/turbine/binder/BinderTest.java
@@ -195,7 +195,7 @@ public class BinderTest {
           /* moduleVersion=*/ Optional.absent());
       fail();
     } catch (TurbineError e) {
-      assertThat(e.getMessage()).contains("cycle in class hierarchy: a/A -> b/B -> a/A");
+      assertThat(e.getMessage()).contains("cycle in class hierarchy: a.A -> b.B -> a.A");
     }
   }