Improve resolution error diagnostics

Introduce an AST node for identifiers instead of just using String, and
use the diagnostic position to improve resolution errors.

MOE_MIGRATED_REVID=216452604

4 files changed, 78 insertions, 22 deletions

diff --git a/javatests/com/google/turbine/binder/BinderErrorTest.java b/javatests/com/google/turbine/binder/BinderErrorTest.java
index e49c5cc..bd2bfb0 100644
--- a/javatests/com/google/turbine/binder/BinderErrorTest.java
+++ b/javatests/com/google/turbine/binder/BinderErrorTest.java
@@ -288,7 +288,7 @@ public class BinderErrorTest {
         {
           "<>:2: error: symbol not found java.lang.Deprecated$NoSuch", //
           "  @Deprecated.NoSuch int x;",
-          "   ^",
+          "              ^",
         },
       },
       {
@@ -412,7 +412,7 @@ public class BinderErrorTest {
         {
           "<>:2: error: could not resolve NoSuch", //
           "  NoSuch f;",
-          "         ^",
+          "  ^",
         },
       },
       {
@@ -441,6 +441,45 @@ public class BinderErrorTest {
           "                ^",
         },
       },
+      {
+        {
+          "import java.util.Map;", //
+          "class Foo {",
+          "  Map.Entry.NoSuch<List> ys;",
+          "}",
+        },
+        {
+          "<>:3: error: symbol not found java.util.Map$Entry$NoSuch", //
+          "  Map.Entry.NoSuch<List> ys;",
+          "            ^",
+        },
+      },
+      {
+        {
+          "import java.util.List;", //
+          "class Foo {",
+          "  NoSuch<List> xs;",
+          "}",
+        },
+        {
+          "<>:3: error: could not resolve NoSuch", //
+          "  NoSuch<List> xs;",
+          "  ^",
+        },
+      },
+      {
+        {
+          "import java.util.List;", //
+          "class Foo {",
+          "  java.util.NoSuch<List> xs;",
+          "}",
+        },
+        {
+          "<>:3: error: could not resolve java.util.NoSuch", //
+          "  java.util.NoSuch<List> xs;",
+          "  ^",
+        },
+      },
     };
     return Arrays.asList((Object[][]) testCases);
   }
diff --git a/javatests/com/google/turbine/binder/ClassPathBinderTest.java b/javatests/com/google/turbine/binder/ClassPathBinderTest.java
index bd9bde3..60af2a3 100644
--- a/javatests/com/google/turbine/binder/ClassPathBinderTest.java
+++ b/javatests/com/google/turbine/binder/ClassPathBinderTest.java
@@ -33,11 +33,11 @@ import com.google.turbine.binder.lookup.Scope;
 import com.google.turbine.binder.sym.ClassSymbol;
 import com.google.turbine.model.TurbineFlag;
 import com.google.turbine.model.TurbineTyKind;
+import com.google.turbine.tree.Tree.Ident;
 import java.io.IOError;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Arrays;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -54,11 +54,11 @@ public class ClassPathBinderTest {
 
     Scope javaLang = TURBINE_BOOTCLASSPATH.index().lookupPackage(ImmutableList.of("java", "lang"));
 
-    LookupResult result = javaLang.lookup(new LookupKey(Arrays.asList("String")));
+    LookupResult result = javaLang.lookup(new LookupKey(ImmutableList.of(new Ident(-1, "String"))));
     assertThat(result.remaining()).isEmpty();
     assertThat(result.sym()).isEqualTo(new ClassSymbol("java/lang/String"));
 
-    result = javaLang.lookup(new LookupKey(Arrays.asList("Object")));
+    result = javaLang.lookup(new LookupKey(ImmutableList.of(new Ident(-1, "Object"))));
     assertThat(result.remaining()).isEmpty();
     assertThat(result.sym()).isEqualTo(new ClassSymbol("java/lang/Object"));
   }
diff --git a/javatests/com/google/turbine/binder/JimageClassBinderTest.java b/javatests/com/google/turbine/binder/JimageClassBinderTest.java
index ffbbf87..bbcb245 100644
--- a/javatests/com/google/turbine/binder/JimageClassBinderTest.java
+++ b/javatests/com/google/turbine/binder/JimageClassBinderTest.java
@@ -16,6 +16,7 @@
 
 package com.google.turbine.binder;
 
+import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableList;
@@ -23,6 +24,7 @@ import com.google.turbine.binder.bytecode.BytecodeBoundClass;
 import com.google.turbine.binder.lookup.LookupKey;
 import com.google.turbine.binder.lookup.LookupResult;
 import com.google.turbine.binder.sym.ClassSymbol;
+import com.google.turbine.tree.Tree.Ident;
 import java.io.IOException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -49,7 +51,7 @@ public class JimageClassBinderTest {
         binder
             .index()
             .lookupPackage(ImmutableList.of("java", "lang"))
-            .lookup(new LookupKey(ImmutableList.of("Object")));
+            .lookup(new LookupKey(ImmutableList.of(new Ident(-1, "Object"))));
     assertThat(((ClassSymbol) objectSym.sym()).binaryName()).isEqualTo("java/lang/Object");
     assertThat(objectSym.remaining()).isEmpty();
 
@@ -57,16 +59,22 @@ public class JimageClassBinderTest {
         binder
             .index()
             .lookupPackage(ImmutableList.of("java", "util"))
-            .lookup(new LookupKey(ImmutableList.of("Map", "Entry")));
+            .lookup(new LookupKey(ImmutableList.of(new Ident(-1, "Map"), new Ident(-1, "Entry"))));
     assertThat(((ClassSymbol) entrySym.sym()).binaryName()).isEqualTo("java/util/Map");
-    assertThat(entrySym.remaining()).containsExactly("Entry");
+    assertThat(getOnlyElement(entrySym.remaining()).value()).isEqualTo("Entry");
 
     entrySym =
         binder
             .index()
             .scope()
-            .lookup(new LookupKey(ImmutableList.of("java", "util", "Map", "Entry")));
+            .lookup(
+                new LookupKey(
+                    ImmutableList.of(
+                        new Ident(-1, "java"),
+                        new Ident(-1, "util"),
+                        new Ident(-1, "Map"),
+                        new Ident(-1, "Entry"))));
     assertThat(((ClassSymbol) entrySym.sym()).binaryName()).isEqualTo("java/util/Map");
-    assertThat(entrySym.remaining()).containsExactly("Entry");
+    assertThat(getOnlyElement(entrySym.remaining()).value()).isEqualTo("Entry");
   }
 }
diff --git a/javatests/com/google/turbine/binder/lookup/TopLevelIndexTest.java b/javatests/com/google/turbine/binder/lookup/TopLevelIndexTest.java
index fe8b49e..022e47c 100644
--- a/javatests/com/google/turbine/binder/lookup/TopLevelIndexTest.java
+++ b/javatests/com/google/turbine/binder/lookup/TopLevelIndexTest.java
@@ -16,11 +16,13 @@
 
 package com.google.turbine.binder.lookup;
 
+import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
 import com.google.turbine.binder.sym.ClassSymbol;
+import com.google.turbine.tree.Tree.Ident;
 import java.util.NoSuchElementException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -41,8 +43,7 @@ public class TopLevelIndexTest {
 
   @Test
   public void simple() {
-    LookupResult result =
-        index.scope().lookup(new LookupKey(ImmutableList.of("java", "util", "Map")));
+    LookupResult result = index.scope().lookup(lookupKey(ImmutableList.of("java", "util", "Map")));
     assertThat(result.sym()).isEqualTo(new ClassSymbol("java/util/Map"));
     assertThat(result.remaining()).isEmpty();
   }
@@ -50,14 +51,14 @@ public class TopLevelIndexTest {
   @Test
   public void nested() {
     LookupResult result =
-        index.scope().lookup(new LookupKey(ImmutableList.of("java", "util", "Map", "Entry")));
+        index.scope().lookup(lookupKey(ImmutableList.of("java", "util", "Map", "Entry")));
     assertThat(result.sym()).isEqualTo(new ClassSymbol("java/util/Map"));
-    assertThat(result.remaining()).containsExactly("Entry");
+    assertThat(getOnlyElement(result.remaining()).value()).isEqualTo("Entry");
   }
 
   @Test
   public void empty() {
-    assertThat(index.scope().lookup(new LookupKey(ImmutableList.of("java", "NoSuch", "Entry"))))
+    assertThat(index.scope().lookup(lookupKey(ImmutableList.of("java", "NoSuch", "Entry"))))
         .isNull();
     assertThat(index.lookupPackage(ImmutableList.of("java", "math"))).isNull();
     assertThat(index.lookupPackage(ImmutableList.of("java", "util", "Map"))).isNull();
@@ -67,11 +68,11 @@ public class TopLevelIndexTest {
   public void packageScope() {
     Scope scope = index.lookupPackage(ImmutableList.of("java", "util"));
 
-    assertThat(scope.lookup(new LookupKey(ImmutableList.of("Map"))).sym())
+    assertThat(scope.lookup(lookupKey(ImmutableList.of("Map"))).sym())
         .isEqualTo(new ClassSymbol("java/util/Map"));
-    assertThat(scope.lookup(new LookupKey(ImmutableList.of("List"))).sym())
+    assertThat(scope.lookup(lookupKey(ImmutableList.of("List"))).sym())
         .isEqualTo(new ClassSymbol("java/util/List"));
-    assertThat(scope.lookup(new LookupKey(ImmutableList.of("NoSuch")))).isNull();
+    assertThat(scope.lookup(lookupKey(ImmutableList.of("NoSuch")))).isNull();
   }
 
   @Test
@@ -82,7 +83,7 @@ public class TopLevelIndexTest {
           SimpleTopLevelIndex.of(
               ImmutableList.of(new ClassSymbol("java/Foo"), new ClassSymbol("java/Foo/Bar")));
 
-      LookupResult result = index.scope().lookup(new LookupKey(ImmutableList.of("java", "Foo")));
+      LookupResult result = index.scope().lookup(lookupKey(ImmutableList.of("java", "Foo")));
       assertThat(result.sym()).isEqualTo(new ClassSymbol("java/Foo"));
       assertThat(result.remaining()).isEmpty();
     }
@@ -92,11 +93,11 @@ public class TopLevelIndexTest {
           SimpleTopLevelIndex.of(
               ImmutableList.of(new ClassSymbol("java/Foo/Bar"), new ClassSymbol("java/Foo")));
 
-      assertThat(index.scope().lookup(new LookupKey(ImmutableList.of("java", "Foo")))).isNull();
+      assertThat(index.scope().lookup(lookupKey(ImmutableList.of("java", "Foo")))).isNull();
       LookupResult packageResult =
           index
               .lookupPackage(ImmutableList.of("java", "Foo"))
-              .lookup(new LookupKey(ImmutableList.of("Bar")));
+              .lookup(lookupKey(ImmutableList.of("Bar")));
       assertThat(packageResult.sym()).isEqualTo(new ClassSymbol("java/Foo/Bar"));
       assertThat(packageResult.remaining()).isEmpty();
     }
@@ -104,7 +105,7 @@ public class TopLevelIndexTest {
 
   @Test
   public void emptyLookup() {
-    LookupKey key = new LookupKey(ImmutableList.of("java", "util", "List"));
+    LookupKey key = lookupKey(ImmutableList.of("java", "util", "List"));
     key = key.rest();
     key = key.rest();
     try {
@@ -114,4 +115,12 @@ public class TopLevelIndexTest {
       // expected
     }
   }
+
+  private LookupKey lookupKey(ImmutableList<String> names) {
+    ImmutableList.Builder<Ident> result = ImmutableList.builder();
+    for (String name : names) {
+      result.add(new Ident(-1, name));
+    }
+    return new LookupKey(result.build());
+  }
 }