Make missing symbol diagnostics more consistent

and record arguments in TurbineError. MOE_MIGRATED_REVID=192525271
google · Apr 11, 2018 · 822abec · 822abec
1 parent d0fcced
commit 822abec
Show file tree

Hide file tree

Showing 15 changed files with 117 additions and 60 deletions.
diff --git a/java/com/google/turbine/binder/ConstBinder.java b/java/com/google/turbine/binder/ConstBinder.java
@@ -182,7 +182,7 @@ private static RetentionPolicy bindRetention(AnnoInfo annotation) {
       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
@@ -199,13 +199,16 @@ private ClassSymbol resolveClass(ClassTy classTy) {
     }
     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
@@ -31,6 +31,7 @@
 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 @@ private ClassSymbol resolveClass(Tree.ClassTy ty) {
     // 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
@@ -206,13 +206,15 @@ private ClassSymbol resolve(int pos, ImmutableList<String> simpleNames) {
     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
@@ -107,14 +107,23 @@ public CanonicalResolver(
     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
@@ -560,14 +560,11 @@ private ImmutableList<AnnoInfo> bindAnnotations(
     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 @@ private ImmutableList<AnnoInfo> bindAnnotations(
     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 @@ private Type bindClassTy(CompoundScope scope, Tree.ClassTy t) {
     // 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 @@ private Type bindClassTyRest(
             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
@@ -83,7 +83,7 @@ public BytecodeBoundClass(
             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
@@ -103,7 +103,10 @@ private static ImportScope namedImport(
     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
@@ -40,10 +40,15 @@ interface ResolveFunction {
     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
@@ -85,7 +85,7 @@ public TopLevelIndex build() {
 
     /** 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
@@ -47,7 +47,7 @@ public int hashCode() {
 
   @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
@@ -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 enum ErrorKind {
     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 static TurbineError format(SourceFile source, ErrorKind kind, Object... a
     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 static TurbineError format(
         .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
@@ -106,9 +106,9 @@ public String toString() {
       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('<');