Check visibility of static on-demand type imports

Move initial modifier handling earlier in binding, and use it to filter
private class and fields from on-demand imports.

MOE_MIGRATED_REVID=142092576

12 files changed, 124 insertions, 41 deletions

diff --git a/java/com/google/turbine/binder/Binder.java b/java/com/google/turbine/binder/Binder.java
index 9262a1f..bdc6c23 100644
--- a/java/com/google/turbine/binder/Binder.java
+++ b/java/com/google/turbine/binder/Binder.java
@@ -52,6 +52,7 @@ import com.google.turbine.binder.sym.FieldSymbol;
 import com.google.turbine.model.Const;
 import com.google.turbine.model.TurbineFlag;
 import com.google.turbine.model.TurbineTyKind;
+import com.google.turbine.model.TurbineVisibility;
 import com.google.turbine.tree.Tree;
 import com.google.turbine.tree.Tree.CompUnit;
 import com.google.turbine.tree.Tree.PkgDecl;
@@ -141,9 +142,11 @@ public class Binder {
       for (Tree.TyDecl decl : decls) {
         ClassSymbol sym = new ClassSymbol(packagename + decl.name());
         ImmutableMap<String, ClassSymbol> children =
-            bindSourceBoundClassMembers(envbuilder, sym, decl.members(), toplevels, unit);
+            bindSourceBoundClassMembers(
+                envbuilder, sym, decl.members(), toplevels, unit, decl.tykind());
         if (envbuilder.putIfAbsent(
-            sym, new SourceBoundClass(decl, null, decl.tykind(), children))) {
+            sym,
+            new SourceBoundClass(decl, null, decl.tykind(), children, access(null, decl.mods())))) {
           toplevels.put(unit, sym);
         }
         tliBuilder.insert(sym);
@@ -152,6 +155,25 @@ public class Binder {
     return envbuilder.build();
   }
 
+  private static int access(TurbineTyKind enclosing, ImmutableSet<TurbineModifier> mods) {
+    int access = 0;
+    for (TurbineModifier m : mods) {
+      access |= m.flag();
+    }
+    // types declared in interfaces and annotations are implicitly public (JLS 9.5)
+    if (enclosing != null) {
+      switch (enclosing) {
+        case INTERFACE:
+        case ANNOTATION:
+          access = TurbineVisibility.PUBLIC.setAccess(access);
+          break;
+        default:
+          break;
+      }
+    }
+    return access;
+  }
+
   /** Fakes up the synthetic type declaration for a package-info.java file. */
   private static TyDecl packageInfoTree(PkgDecl pkgDecl) {
     return new TyDecl(
@@ -172,7 +194,8 @@ public class Binder {
       ClassSymbol owner,
       ImmutableList<Tree> members,
       Multimap<CompUnit, ClassSymbol> toplevels,
-      CompUnit unit) {
+      CompUnit unit,
+      TurbineTyKind enclosing) {
     ImmutableMap.Builder<String, ClassSymbol> result = ImmutableMap.builder();
     for (Tree member : members) {
       if (member.kind() == Tree.Kind.TY_DECL) {
@@ -181,8 +204,11 @@ public class Binder {
         toplevels.put(unit, sym);
         result.put(decl.name(), sym);
         ImmutableMap<String, ClassSymbol> children =
-            bindSourceBoundClassMembers(env, sym, decl.members(), toplevels, unit);
-        env.putIfAbsent(sym, new SourceBoundClass(decl, owner, decl.tykind(), children));
+            bindSourceBoundClassMembers(env, sym, decl.members(), toplevels, unit, decl.tykind());
+        env.putIfAbsent(
+            sym,
+            new SourceBoundClass(
+                decl, owner, decl.tykind(), children, access(enclosing, decl.mods())));
       }
     }
     return result.build();
@@ -204,7 +230,8 @@ public class Binder {
           unit.pkg().isPresent() ? unit.pkg().get().name() : ImmutableList.of();
       Scope packageScope = tli.lookupPackage(packagename);
       CanonicalSymbolResolver importResolver =
-          new CanonicalResolver(CompoundEnv.<ClassSymbol, BoundClass>of(ienv).append(classPathEnv));
+          new CanonicalResolver(
+              packagename, CompoundEnv.<ClassSymbol, BoundClass>of(ienv).append(classPathEnv));
       Scope importScope = ImportIndex.create(importResolver, tli, unit.imports());
       Scope wildImportScope = WildImportIndex.create(importResolver, tli, unit.imports());
       MemberImportIndex memberImports = new MemberImportIndex(importResolver, tli, unit.imports());
diff --git a/java/com/google/turbine/binder/ConstEvaluator.java b/java/com/google/turbine/binder/ConstEvaluator.java
index b744339..7156a87 100644
--- a/java/com/google/turbine/binder/ConstEvaluator.java
+++ b/java/com/google/turbine/binder/ConstEvaluator.java
@@ -235,9 +235,15 @@ public strictfp class ConstEvaluator {
     Iterator<ClassSymbol> it = base.memberImports().onDemandImports();
     while (it.hasNext()) {
       field = Resolve.resolveField(env, owner, it.next(), simpleName);
-      if (field != null) {
-        return field;
+      if (field == null) {
+        continue;
       }
+      // resolve handles visibility of inherited members; on-demand imports of private members are
+      // a special case
+      if ((field.access() & TurbineFlag.ACC_PRIVATE) == TurbineFlag.ACC_PRIVATE) {
+        continue;
+      }
+      return field;
     }
     return null;
   }
diff --git a/java/com/google/turbine/binder/HierarchyBinder.java b/java/com/google/turbine/binder/HierarchyBinder.java
index 8416202..cc96eaf 100644
--- a/java/com/google/turbine/binder/HierarchyBinder.java
+++ b/java/com/google/turbine/binder/HierarchyBinder.java
@@ -31,9 +31,7 @@ import com.google.turbine.diag.TurbineError;
 import com.google.turbine.diag.TurbineError.ErrorKind;
 import com.google.turbine.model.TurbineFlag;
 import com.google.turbine.model.TurbineTyKind;
-import com.google.turbine.model.TurbineVisibility;
 import com.google.turbine.tree.Tree;
-import com.google.turbine.tree.TurbineModifier;
 import java.util.ArrayDeque;
 
 /** Type hierarchy binding. */
@@ -62,11 +60,7 @@ public class HierarchyBinder {
 
   private SourceHeaderBoundClass bind() {
     Tree.TyDecl decl = base.decl();
-
-    int access = 0;
-    for (TurbineModifier m : decl.mods()) {
-      access |= m.flag();
-    }
+    int access = base.access();
     switch (decl.tykind()) {
       case CLASS:
         access |= TurbineFlag.ACC_SUPER;
@@ -84,11 +78,6 @@ public class HierarchyBinder {
         throw new AssertionError(decl.tykind());
     }
 
-    // types declared in interfaces  annotations are implicitly public (JLS 9.5)
-    if (enclosedByInterface(base)) {
-      access = TurbineVisibility.PUBLIC.setAccess(access);
-    }
-
     if ((access & TurbineFlag.ACC_STATIC) == 0 && implicitStatic(base)) {
       access |= TurbineFlag.ACC_STATIC;
     }
@@ -170,21 +159,6 @@ public class HierarchyBinder {
     }
   }
 
-  /** Returns true if the given type is declared in an interface. */
-  private boolean enclosedByInterface(BoundClass c) {
-    if (c.owner() != null) {
-      HeaderBoundClass info = env.get(c.owner());
-      switch (info.kind()) {
-        case INTERFACE:
-        case ANNOTATION:
-          return true;
-        default:
-          break;
-      }
-    }
-    return false;
-  }
-
   /**
    * Resolves the {@link ClassSymbol} for the given {@link Tree.ClassTy}, with handling for
    * non-canonical qualified type names.
diff --git a/java/com/google/turbine/binder/Resolve.java b/java/com/google/turbine/binder/Resolve.java
index 7b9e490..ad033cd 100644
--- a/java/com/google/turbine/binder/Resolve.java
+++ b/java/com/google/turbine/binder/Resolve.java
@@ -16,6 +16,8 @@
 
 package com.google.turbine.binder;
 
+import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
 import com.google.turbine.binder.bound.BoundClass;
 import com.google.turbine.binder.bound.HeaderBoundClass;
 import com.google.turbine.binder.bound.TypeBoundClass;
@@ -66,9 +68,12 @@ public class Resolve {
   }
 
   static class CanonicalResolver implements CanonicalSymbolResolver {
+    private final String packagename;
     private final CompoundEnv<ClassSymbol, BoundClass> env;
 
-    public CanonicalResolver(CompoundEnv<ClassSymbol, BoundClass> env) {
+    public CanonicalResolver(
+        ImmutableList<String> packagename, CompoundEnv<ClassSymbol, BoundClass> env) {
+      this.packagename = Joiner.on('/').join(packagename);
       this.env = env;
     }
 
@@ -94,8 +99,25 @@ public class Resolve {
       if (sym == null) {
         return null;
       }
+      if (!visible(sym, TurbineVisibility.fromAccess(env.get(sym).access()))) {
+        return null;
+      }
       return sym;
     }
+
+    boolean visible(ClassSymbol sym, TurbineVisibility visibility) {
+      switch (visibility) {
+        case PUBLIC:
+          return true;
+        case PROTECTED:
+        case PACKAGE:
+          return Objects.equals(packageName(sym), packagename);
+        case PRIVATE:
+          return false;
+        default:
+          throw new AssertionError(visibility);
+      }
+    }
   }
 
   /**
diff --git a/java/com/google/turbine/binder/TypeBinder.java b/java/com/google/turbine/binder/TypeBinder.java
index b770109..c100fd1 100644
--- a/java/com/google/turbine/binder/TypeBinder.java
+++ b/java/com/google/turbine/binder/TypeBinder.java
@@ -651,6 +651,9 @@ public class TypeBinder {
     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());
+      }
       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/bound/BoundClass.java b/java/com/google/turbine/binder/bound/BoundClass.java
index 29ef196..ad26af7 100644
--- a/java/com/google/turbine/binder/bound/BoundClass.java
+++ b/java/com/google/turbine/binder/bound/BoundClass.java
@@ -35,6 +35,9 @@ public interface BoundClass {
   @Nullable
   ClassSymbol owner();
 
+  /** Class access bits (see JVMS table 4.1). */
+  int access();
+
   /** Member type declarations, by simple name. */
   ImmutableMap<String, ClassSymbol> children();
 }
diff --git a/java/com/google/turbine/binder/bound/HeaderBoundClass.java b/java/com/google/turbine/binder/bound/HeaderBoundClass.java
index bc8edb3..14807bb 100644
--- a/java/com/google/turbine/binder/bound/HeaderBoundClass.java
+++ b/java/com/google/turbine/binder/bound/HeaderBoundClass.java
@@ -29,9 +29,6 @@ public interface HeaderBoundClass extends BoundClass {
   /** The interfaces of the declaration. */
   ImmutableList<ClassSymbol> interfaces();
 
-  /** Class access bits (see JVMS table 4.1). */
-  int access();
-
   /** Declared type parameters. */
   public ImmutableMap<String, TyVarSymbol> typeParameters();
 }
diff --git a/java/com/google/turbine/binder/bound/PackageSourceBoundClass.java b/java/com/google/turbine/binder/bound/PackageSourceBoundClass.java
index 6b708f9..946bd90 100644
--- a/java/com/google/turbine/binder/bound/PackageSourceBoundClass.java
+++ b/java/com/google/turbine/binder/bound/PackageSourceBoundClass.java
@@ -57,6 +57,11 @@ public class PackageSourceBoundClass implements BoundClass {
   }
 
   @Override
+  public int access() {
+    return base.access();
+  }
+
+  @Override
   public ImmutableMap<String, ClassSymbol> children() {
     return base.children();
   }
diff --git a/java/com/google/turbine/binder/bound/SourceBoundClass.java b/java/com/google/turbine/binder/bound/SourceBoundClass.java
index c9afc3b..3373cce 100644
--- a/java/com/google/turbine/binder/bound/SourceBoundClass.java
+++ b/java/com/google/turbine/binder/bound/SourceBoundClass.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap;
 import com.google.turbine.binder.sym.ClassSymbol;
 import com.google.turbine.model.TurbineTyKind;
 import com.google.turbine.tree.Tree;
+import com.google.turbine.tree.Tree.TyDecl;
 
 /** A {@link BoundClass} that corresponds to a source file being compiled. */
 public class SourceBoundClass implements BoundClass {
@@ -27,16 +28,19 @@ public class SourceBoundClass implements BoundClass {
   private final ClassSymbol owner;
   private final TurbineTyKind kind;
   private final ImmutableMap<String, ClassSymbol> children;
+  private final int access;
 
   public SourceBoundClass(
-      Tree.TyDecl decl,
+      TyDecl decl,
       ClassSymbol owner,
       TurbineTyKind kind,
-      ImmutableMap<String, ClassSymbol> children) {
+      ImmutableMap<String, ClassSymbol> children,
+      int access) {
     this.decl = decl;
     this.owner = owner;
     this.kind = kind;
     this.children = children;
+    this.access = access;
   }
 
   public Tree.TyDecl decl() {
@@ -54,6 +58,11 @@ public class SourceBoundClass implements BoundClass {
   }
 
   @Override
+  public int access() {
+    return access;
+  }
+
+  @Override
   public ImmutableMap<String, ClassSymbol> children() {
     return children;
   }
diff --git a/javatests/com/google/turbine/lower/LowerIntegrationTest.java b/javatests/com/google/turbine/lower/LowerIntegrationTest.java
index f3268d4..7b80d90 100644
--- a/javatests/com/google/turbine/lower/LowerIntegrationTest.java
+++ b/javatests/com/google/turbine/lower/LowerIntegrationTest.java
@@ -287,6 +287,8 @@ public class LowerIntegrationTest {
       "typaram_lookup.test",
       "typaram_lookup_enclosing.test",
       "B33513475.test",
+      "B33513475b.test",
+      "B33513475c.test",
     };
     List<Object[]> tests =
         ImmutableList.copyOf(testCases).stream().map(x -> new Object[] {x}).collect(toList());
diff --git a/javatests/com/google/turbine/lower/testdata/B33513475b.test b/javatests/com/google/turbine/lower/testdata/B33513475b.test
new file mode 100644
index 0000000..f92a40b
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/B33513475b.test
@@ -0,0 +1,19 @@
+=== p/A.java ===
+package p;
+import static p.A.I.*;
+import static p.B.*;
+class A {
+  static class I {
+    private static class Inner {
+    }
+  }
+  Inner.InnerMost i;
+}
+
+=== p/B.java ===
+package p;
+public class B {
+  public static class Inner {
+    protected static class InnerMost {}
+  }
+}
diff --git a/javatests/com/google/turbine/lower/testdata/B33513475c.test b/javatests/com/google/turbine/lower/testdata/B33513475c.test
new file mode 100644
index 0000000..abbf346
--- /dev/null
+++ b/javatests/com/google/turbine/lower/testdata/B33513475c.test
@@ -0,0 +1,16 @@
+=== p/A.java ===
+package p;
+import static p.A.I.*;
+import static p.B.*;
+class A {
+  static class I {
+    private static final int N = 1;
+  }
+  static final int M = N;
+}
+
+=== p/B.java ===
+package p;
+public class B {
+  public static final int N = 2;
+}