Merge pull request #2035 from scalamacros/ticket/6989

SI-6989 privateWithin is now populated in reflect

7 files changed, 332 insertions, 17 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
index 976f7e038b..a517a33279 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala
@@ -548,8 +548,8 @@ abstract class ClassfileParser {
     skipMembers() // methods
     if (!isScala) {
       clazz setFlag sflags
-      setPrivateWithin(clazz, jflags)
-      setPrivateWithin(staticModule, jflags)
+      importPrivateWithinFromJavaFlags(clazz, jflags)
+      importPrivateWithinFromJavaFlags(staticModule, jflags)
       clazz.setInfo(classInfo)
       moduleClass setInfo staticInfo
       staticModule.setInfo(moduleClass.tpe)
@@ -612,7 +612,7 @@ abstract class ClassfileParser {
         if (isEnum) ConstantType(Constant(sym))
         else info
       }
-      setPrivateWithin(sym, jflags)
+      importPrivateWithinFromJavaFlags(sym, jflags)
       parseAttributes(sym, info)
       getScope(jflags).enter(sym)
 
@@ -663,7 +663,7 @@ abstract class ClassfileParser {
               info = MethodType(newParams, clazz.tpe)
           }
         sym.setInfo(info)
-        setPrivateWithin(sym, jflags)
+        importPrivateWithinFromJavaFlags(sym, jflags)
         parseAttributes(sym, info)
         if ((jflags & JAVA_ACC_VARARGS) != 0) {
           sym.setInfo(arrayToRepeated(sym.info))
@@ -1264,19 +1264,6 @@ abstract class ClassfileParser {
   protected def getScope(flags: Int): Scope =
     if (isStatic(flags)) staticScope else instanceScope
 
-  private def setPrivateWithin(sym: Symbol, jflags: Int) {
-    if ((jflags & (JAVA_ACC_PRIVATE | JAVA_ACC_PROTECTED | JAVA_ACC_PUBLIC)) == 0)
-      // See ticket #1687 for an example of when topLevelClass is NoSymbol: it
-      // apparently occurs when processing v45.3 bytecode.
-      if (sym.enclosingTopLevelClass != NoSymbol)
-        sym.privateWithin = sym.enclosingTopLevelClass.owner
-
-    // protected in java means package protected. #3946
-    if ((jflags & JAVA_ACC_PROTECTED) != 0)
-      if (sym.enclosingTopLevelClass != NoSymbol)
-        sym.privateWithin = sym.enclosingTopLevelClass.owner
-  }
-
   private def isPrivate(flags: Int)     = (flags & JAVA_ACC_PRIVATE) != 0
   private def isStatic(flags: Int)      = (flags & JAVA_ACC_STATIC) != 0
   private def hasAnnotation(flags: Int) = (flags & JAVA_ACC_ANNOTATION) != 0
diff --git a/src/reflect/scala/reflect/internal/PrivateWithin.scala b/src/reflect/scala/reflect/internal/PrivateWithin.scala
new file mode 100644
index 0000000000..9b99b94b41
--- /dev/null
+++ b/src/reflect/scala/reflect/internal/PrivateWithin.scala
@@ -0,0 +1,23 @@
+package scala.reflect
+package internal
+
+import ClassfileConstants._
+
+trait PrivateWithin {
+  self: SymbolTable =>
+
+  def importPrivateWithinFromJavaFlags(sym: Symbol, jflags: Int): Symbol = {
+    if ((jflags & (JAVA_ACC_PRIVATE | JAVA_ACC_PROTECTED | JAVA_ACC_PUBLIC)) == 0)
+      // See ticket #1687 for an example of when topLevelClass is NoSymbol: it
+      // apparently occurs when processing v45.3 bytecode.
+      if (sym.enclosingTopLevelClass != NoSymbol)
+        sym.privateWithin = sym.enclosingTopLevelClass.owner
+
+    // protected in java means package protected. #3946
+    if ((jflags & JAVA_ACC_PROTECTED) != 0)
+      if (sym.enclosingTopLevelClass != NoSymbol)
+        sym.privateWithin = sym.enclosingTopLevelClass.owner
+
+    sym
+  }
+}
\ No newline at end of file
diff --git a/src/reflect/scala/reflect/internal/SymbolTable.scala b/src/reflect/scala/reflect/internal/SymbolTable.scala
index 02ac59a461..f75855f1ec 100644
--- a/src/reflect/scala/reflect/internal/SymbolTable.scala
+++ b/src/reflect/scala/reflect/internal/SymbolTable.scala
@@ -38,6 +38,7 @@ abstract class SymbolTable extends macros.Universe
                               with StdAttachments
                               with StdCreators
                               with BuildUtils
+                              with PrivateWithin
 {
 
   val gen = new TreeGen { val global: SymbolTable.this.type = SymbolTable.this }
diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index ea2fc4afe9..778c826dc0 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -639,6 +639,7 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
       /** used to avoid cycles while initializing classes */
       private var parentsLevel = 0
       private var pendingLoadActions: List[() => Unit] = Nil
+      private val relatedSymbols = clazz +: (if (module != NoSymbol) List(module, module.moduleClass) else Nil)
 
       override def load(sym: Symbol): Unit = {
         debugInfo("completing from Java " + sym + "/" + clazz.fullName)//debug
@@ -650,6 +651,7 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
           module.moduleClass setFlag (flags & PRIVATE | JAVA)
         }
 
+        relatedSymbols foreach (importPrivateWithinFromJavaFlags(_, jclazz.getModifiers))
         copyAnnotations(clazz, jclazz)
         // to do: annotations to set also for module?
 
@@ -1068,6 +1070,7 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
           .newValue(newTermName(jfield.getName), NoPosition, toScalaFieldFlags(jfield.getModifiers))
           .setInfo(typeToScala(jfield.getGenericType))
       fieldCache enter (jfield, field)
+      importPrivateWithinFromJavaFlags(field, jfield.getModifiers)
       copyAnnotations(field, jfield)
       field
     }
@@ -1093,6 +1096,7 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
       val paramtpes = jmeth.getGenericParameterTypes.toList map typeToScala
       val resulttpe = typeToScala(jmeth.getGenericReturnType)
       setMethType(meth, tparams, paramtpes, resulttpe)
+      importPrivateWithinFromJavaFlags(meth, jmeth.getModifiers)
       copyAnnotations(meth, jmeth)
       if ((jmeth.getModifiers & JAVA_ACC_VARARGS) != 0) meth.setInfo(arrayToRepeated(meth.info))
       meth
@@ -1116,6 +1120,7 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
       val paramtpes = jconstr.getGenericParameterTypes.toList map typeToScala
       setMethType(constr, tparams, paramtpes, clazz.tpe)
       constr setInfo GenPolyType(tparams, MethodType(clazz.newSyntheticValueParams(paramtpes), clazz.tpe))
+      importPrivateWithinFromJavaFlags(constr, jconstr.getModifiers)
       copyAnnotations(constr, jconstr)
       constr
     }
diff --git a/test/files/run/t6989.check b/test/files/run/t6989.check
new file mode 100644
index 0000000000..8943792115
--- /dev/null
+++ b/test/files/run/t6989.check
@@ -0,0 +1,216 @@
+============
+sym = class PackagePrivateJavaClass, signature = ClassInfoType(...), owner = package foo
+isPrivate = false
+isProtected = false
+isPublic = false
+privateWithin = package foo
+============
+sym = constructor PackagePrivateJavaClass, signature = (x$1: Int, x$2: Int)foo.PackagePrivateJavaClass, owner = class PackagePrivateJavaClass
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = variable privateField, signature = Int, owner = class PackagePrivateJavaClass
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = method privateMethod, signature = ()Unit, owner = class PackagePrivateJavaClass
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = variable protectedField, signature = Int, owner = class PackagePrivateJavaClass
+isPrivate = false
+isProtected = true
+isPublic = false
+privateWithin = package foo
+============
+sym = method protectedMethod, signature = ()Unit, owner = class PackagePrivateJavaClass
+isPrivate = false
+isProtected = true
+isPublic = false
+privateWithin = package foo
+============
+sym = variable publicField, signature = Int, owner = class PackagePrivateJavaClass
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = method publicMethod, signature = ()Unit, owner = class PackagePrivateJavaClass
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = object PackagePrivateJavaClass, signature = foo.PackagePrivateJavaClass.type, owner = package foo
+isPrivate = false
+isProtected = false
+isPublic = false
+privateWithin = package foo
+============
+sym = variable privateStaticField, signature = Int, owner = object PackagePrivateJavaClass
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = method privateStaticMethod, signature = ()Unit, owner = object PackagePrivateJavaClass
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = variable protectedStaticField, signature = Int, owner = object PackagePrivateJavaClass
+isPrivate = false
+isProtected = true
+isPublic = false
+privateWithin = package foo
+============
+sym = method protectedStaticMethod, signature = ()Unit, owner = object PackagePrivateJavaClass
+isPrivate = false
+isProtected = true
+isPublic = false
+privateWithin = package foo
+============
+sym = variable publicStaticField, signature = Int, owner = object PackagePrivateJavaClass
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = method publicStaticMethod, signature = ()Unit, owner = object PackagePrivateJavaClass
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = class JavaClass_1, signature = ClassInfoType(...), owner = package foo
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = class $PrivateJavaClass, signature = ClassInfoType(...), owner = class JavaClass_1
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = value this$0, signature = foo.JavaClass_1, owner = class $PrivateJavaClass
+isPrivate = false
+isProtected = false
+isPublic = false
+privateWithin = package foo
+============
+sym = object $PrivateJavaClass, signature = JavaClass_1.this.$PrivateJavaClass.type, owner = class JavaClass_1
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = class $ProtectedJavaClass, signature = ClassInfoType(...), owner = class JavaClass_1
+isPrivate = false
+isProtected = true
+isPublic = false
+privateWithin = package foo
+============
+sym = value this$0, signature = foo.JavaClass_1, owner = class $ProtectedJavaClass
+isPrivate = false
+isProtected = false
+isPublic = false
+privateWithin = package foo
+============
+sym = object $ProtectedJavaClass, signature = JavaClass_1.this.$ProtectedJavaClass.type, owner = class JavaClass_1
+isPrivate = false
+isProtected = false
+isPublic = false
+privateWithin = package foo
+============
+sym = class $PublicJavaClass, signature = ClassInfoType(...), owner = class JavaClass_1
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = constructor $PublicJavaClass, signature = (x$1: foo.JavaClass_1)JavaClass_1.this.$PublicJavaClass, owner = class $PublicJavaClass
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = value this$0, signature = foo.JavaClass_1, owner = class $PublicJavaClass
+isPrivate = false
+isProtected = false
+isPublic = false
+privateWithin = package foo
+============
+sym = object $PublicJavaClass, signature = JavaClass_1.this.$PublicJavaClass.type, owner = class JavaClass_1
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = constructor JavaClass_1, signature = ()foo.JavaClass_1, owner = class JavaClass_1
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = object JavaClass_1, signature = foo.JavaClass_1.type, owner = package foo
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = class PrivateStaticJavaClass, signature = ClassInfoType(...), owner = object JavaClass_1
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = object PrivateStaticJavaClass, signature = foo.JavaClass_1.PrivateStaticJavaClass.type, owner = object JavaClass_1
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = class ProtectedStaticJavaClass, signature = ClassInfoType(...), owner = object JavaClass_1
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = object ProtectedStaticJavaClass, signature = foo.JavaClass_1.ProtectedStaticJavaClass.type, owner = object JavaClass_1
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
+============
+sym = class PublicStaticJavaClass, signature = ClassInfoType(...), owner = object JavaClass_1
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = constructor PublicStaticJavaClass, signature = ()foo.JavaClass_1.PublicStaticJavaClass, owner = class PublicStaticJavaClass
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = object PublicStaticJavaClass, signature = foo.JavaClass_1.PublicStaticJavaClass.type, owner = object JavaClass_1
+isPrivate = false
+isProtected = false
+isPublic = true
+privateWithin = <none>
+============
+sym = variable staticField, signature = Int, owner = object JavaClass_1
+isPrivate = true
+isProtected = false
+isPublic = false
+privateWithin = <none>
diff --git a/test/files/run/t6989/JavaClass_1.java b/test/files/run/t6989/JavaClass_1.java
new file mode 100644
index 0000000000..eb26a08700
--- /dev/null
+++ b/test/files/run/t6989/JavaClass_1.java
@@ -0,0 +1,41 @@
+package foo;
+
+// Originally composed to accommodate pull request feedback, this test has
+// uncovered a handful of bugs in FromJavaClassCompleter, namely:
+// * SI-7071 non-public ctors get lost
+// * SI-7072 inner classes are read incorrectly
+
+// I'm leaving the incorrect results of FromJavaClassCompleters in the check
+// file, so that we get notified when something changes there.
+
+class PackagePrivateJavaClass {
+  private int privateField = 0;
+  protected int protectedField = 1;
+  public int publicField = 2;
+
+  private static int privateStaticField = 3;
+  protected static int protectedStaticField = 4;
+  public static int publicStaticField = 5;
+
+  private void privateMethod() {}
+  protected void protectedMethod() {}
+  public void publicMethod() {}
+
+  private static void privateStaticMethod() {}
+  protected static void protectedStaticMethod() {}
+  public static void publicStaticMethod() {}
+
+  private PackagePrivateJavaClass() {}
+  protected PackagePrivateJavaClass(int x) {}
+  public PackagePrivateJavaClass(int x, int y) {}
+}
+
+public class JavaClass_1 {
+  private class PrivateJavaClass {}
+  private static class PrivateStaticJavaClass {}
+  protected class ProtectedJavaClass {}
+  private static class ProtectedStaticJavaClass {}
+  public class PublicJavaClass {}
+  public static class PublicStaticJavaClass {}
+  private static int staticField = 0;
+}
\ No newline at end of file
diff --git a/test/files/run/t6989/Test_2.scala b/test/files/run/t6989/Test_2.scala
new file mode 100644
index 0000000000..e48e82422d
--- /dev/null
+++ b/test/files/run/t6989/Test_2.scala
@@ -0,0 +1,42 @@
+import scala.reflect.runtime.universe._
+
+// Originally composed to accommodate pull request feedback, this test has
+// uncovered a handful of bugs in FromJavaClassCompleter, namely:
+// * SI-7071 non-public ctors get lost
+// * SI-7072 inner classes are read incorrectly
+
+// I'm leaving the incorrect results of FromJavaClassCompleters in the check
+// file, so that we get notified when something changes there.
+
+package object foo {
+  def testAll(): Unit = {
+    test(typeOf[foo.PackagePrivateJavaClass].typeSymbol)
+    test(typeOf[foo.PackagePrivateJavaClass].typeSymbol.companionSymbol)
+    test(typeOf[foo.JavaClass_1].typeSymbol)
+    test(typeOf[foo.JavaClass_1].typeSymbol.companionSymbol)
+  }
+
+  def test(sym: Symbol): Unit = {
+    printSymbolDetails(sym)
+    if (sym.isClass || sym.isModule) {
+      sym.typeSignature.declarations.toList.sortBy(_.name.toString) foreach test
+    }
+  }
+
+  def printSymbolDetails(sym: Symbol): Unit = {
+    def stableSignature(sym: Symbol) = sym.typeSignature match {
+      case ClassInfoType(_, _, _) => "ClassInfoType(...)"
+      case tpe => tpe.toString
+    }
+    println("============")
+    println(s"sym = $sym, signature = ${stableSignature(sym)}, owner = ${sym.owner}")
+    println(s"isPrivate = ${sym.isPrivate}")
+    println(s"isProtected = ${sym.isProtected}")
+    println(s"isPublic = ${sym.isPublic}")
+    println(s"privateWithin = ${sym.privateWithin}")
+  }
+}
+
+object Test extends App {
+  foo.testAll()
+}
\ No newline at end of file