Hardens classToType logic

Reflection now correctly processes classes, objects and inner classes
that are declared in classes and objects.

However classToType still crashes on compound types and local classes.
For more information on those, follow the links:
* Compound types: https://issues.scala-lang.org/browse/SI-5430
* Local classes: https://issues.scala-lang.org/browse/SI-5431

Fixes https://issues.scala-lang.org/browse/SI-5256.
Review by @paulp, @odersky.

19 files changed, 247 insertions, 62 deletions

diff --git a/src/compiler/scala/reflect/runtime/JavaToScala.scala b/src/compiler/scala/reflect/runtime/JavaToScala.scala
index b4bcc52a23..4c49c0221f 100644
--- a/src/compiler/scala/reflect/runtime/JavaToScala.scala
+++ b/src/compiler/scala/reflect/runtime/JavaToScala.scala
@@ -241,16 +241,32 @@ trait JavaToScala extends ConversionUtil { self: SymbolTable =>
    * The Scala owner of the Scala class corresponding to the Java class `jclazz`
    */
   private def sOwner(jclazz: jClass[_]): Symbol = {
-    if (jclazz.isMemberClass)
-      followStatic(classToScala(jclazz.getEnclosingClass), jclazz.getModifiers)
-    else if (jclazz.isLocalClass)
-      methodToScala(jclazz.getEnclosingMethod) orElse constrToScala(jclazz.getEnclosingConstructor)
-    else if (jclazz.isPrimitive || jclazz.isArray)
+    if (jclazz.isMemberClass) {
+      val jEnclosingClass = jclazz.getEnclosingClass
+      val sEnclosingClass = classToScala(jEnclosingClass)
+      followStatic(sEnclosingClass, jclazz.getModifiers)
+    } else if (jclazz.isLocalClass) {
+      val jEnclosingMethod = jclazz.getEnclosingMethod
+      if (jEnclosingMethod != null) {
+        methodToScala(jEnclosingMethod)
+      } else {
+        val jEnclosingConstructor = jclazz.getEnclosingConstructor
+        constrToScala(jEnclosingConstructor)
+      }
+    } else if (jclazz.isPrimitive || jclazz.isArray) {
       ScalaPackageClass
-    else if (jclazz.getPackage != null)
-      packageToScala(jclazz.getPackage)
-    else
+    } else if (jclazz.getPackage != null) {
+      val jPackage = jclazz.getPackage
+      packageToScala(jPackage)
+    } else {
+      // @eb: a weird classloader might return a null package for something with a non-empty package name
+      // for example, http://groups.google.com/group/scala-internals/browse_thread/thread/7be09ff8f67a1e5c
+      // in that case we could invoke packageNameToScala(jPackageName) and, probably, be okay
+      // however, I think, it's better to blow up, since weirdness of the class loader might bite us elsewhere
+      val jPackageName = jclazz.getName.substring(0, Math.max(jclazz.getName.lastIndexOf("."), 0))
+      assert(jPackageName == "")
       EmptyPackageClass
+    }
   }
 
   /**
@@ -295,8 +311,10 @@ trait JavaToScala extends ConversionUtil { self: SymbolTable =>
    *  @return A Scala method object that corresponds to `jmeth`.
    */
   def methodToScala(jmeth: jMethod): Symbol = methodCache.toScala(jmeth) {
-    val owner = followStatic(classToScala(jmeth.getDeclaringClass), jmeth.getModifiers)
-    lookup(owner, jmeth.getName) suchThat (erasesTo(_, jmeth)) orElse jmethodAsScala(jmeth)
+    val jOwner = jmeth.getDeclaringClass
+    var sOwner = classToScala(jOwner)
+    sOwner = followStatic(sOwner, jmeth.getModifiers)
+    lookup(sOwner, jmeth.getName) suchThat (erasesTo(_, jmeth)) orElse jmethodAsScala(jmeth)
   }
 
   /**
@@ -344,6 +362,18 @@ trait JavaToScala extends ConversionUtil { self: SymbolTable =>
     pkg.moduleClass
   }
 
+  private def scalaSimpleName(jclazz: jClass[_]): TypeName = {
+    val owner = sOwner(jclazz)
+    val enclosingClass = jclazz.getEnclosingClass
+    var prefix = if (enclosingClass != null) enclosingClass.getName else ""
+    val isObject = owner.isModuleClass && !owner.isPackageClass
+    if (isObject && !prefix.endsWith(nme.MODULE_SUFFIX_STRING)) prefix += nme.MODULE_SUFFIX_STRING
+    assert(jclazz.getName.startsWith(prefix))
+    var name = jclazz.getName.substring(prefix.length)
+    name = name.substring(name.lastIndexOf(".") + 1)
+    newTypeName(name)
+  }
+
   /**
    * The Scala class that corresponds to a given Java class.
    *  @param jclazz  The Java class
@@ -353,28 +383,54 @@ trait JavaToScala extends ConversionUtil { self: SymbolTable =>
    */
   def classToScala(jclazz: jClass[_]): Symbol = classCache.toScala(jclazz) {
     val jname = javaTypeName(jclazz)
-    def lookup = sOwner(jclazz).info.decl(newTypeName(jclazz.getSimpleName))
-    
-    if (jclazz.isMemberClass && !nme.isImplClassName(jname)) {
-      val sym = lookup
-      assert(sym.isType, sym+"/"+jclazz+"/"+sOwner(jclazz)+"/"+jclazz.getSimpleName)
-      sym.asInstanceOf[ClassSymbol]
-    }
-    else if (jclazz.isLocalClass || invalidClassName(jname)) {
-      // local classes and implementation classes not preserved by unpickling - treat as Java
-      jclassAsScala(jclazz)
-    }
-    else if (jclazz.isArray) {
-      ArrayClass
+    val owner = sOwner(jclazz)
+    val simpleName = scalaSimpleName(jclazz)
+
+    val sym = {
+      def lookup = {
+        def coreLookup(name: Name): Symbol = {
+          val sym = owner.info.decl(name)
+          sym orElse {
+            if (name.startsWith(nme.NAME_JOIN_STRING))
+              coreLookup(name.subName(1, name.length))
+            else
+              NoSymbol
+          }
+        }
+
+        if (nme.isModuleName(simpleName)) {
+          val moduleName = nme.stripModuleSuffix(simpleName).toTermName
+          val sym = coreLookup(moduleName)
+          if (sym == NoSymbol) sym else sym.moduleClass
+        } else {
+          coreLookup(simpleName)
+        }
+      }
+
+      if (jclazz.isMemberClass && !nme.isImplClassName(jname)) {
+        lookup
+      } else if (jclazz.isLocalClass || invalidClassName(jname)) {
+        // local classes and implementation classes not preserved by unpickling - treat as Java
+        jclassAsScala(jclazz)
+      } else if (jclazz.isArray) {
+        ArrayClass
+      } else javaTypeToValueClass(jclazz) orElse {
+        // jclazz is top-level - get signature
+        lookup
+        //        val (clazz, module) = createClassModule(
+        //          sOwner(jclazz), newTypeName(jclazz.getSimpleName), new TopClassCompleter(_, _))
+        //        classCache enter (jclazz, clazz)
+        //        clazz
+      }
     }
-    else javaTypeToValueClass(jclazz) orElse {
-      // jclazz is top-level - get signature
-      lookup
-      //        val (clazz, module) = createClassModule(
-      //          sOwner(jclazz), newTypeName(jclazz.getSimpleName), new TopClassCompleter(_, _))
-      //        classCache enter (jclazz, clazz)
-      //        clazz
+
+    if (!sym.isType) {
+      def msgNoSym = "no symbol could be loaded from %s (scala equivalent is %s) by name %s".format(owner, jclazz, simpleName)
+      def msgIsNotType = "not a type: symbol %s loaded from %s (scala equivalent is %s) by name %s".format(sym, owner, jclazz, simpleName)
+      assert(false, if (sym == NoSymbol) msgNoSym else msgIsNotType)
     }
+
+    sym.asInstanceOf[ClassSymbol]
   }
 
   /**
@@ -453,7 +509,7 @@ trait JavaToScala extends ConversionUtil { self: SymbolTable =>
   private def jclassAsScala(jclazz: jClass[_]): Symbol = jclassAsScala(jclazz, sOwner(jclazz))
 
   private def jclassAsScala(jclazz: jClass[_], owner: Symbol): Symbol = {
-    val name = newTypeName(jclazz.getSimpleName)
+    val name = scalaSimpleName(jclazz)
     val completer = (clazz: Symbol, module: Symbol) => new FromJavaClassCompleter(clazz, module, jclazz)
     val (clazz, module) = createClassModule(owner, name, completer)
     classCache enter (jclazz, clazz)
diff --git a/src/compiler/scala/tools/nsc/interpreter/AbstractFileClassLoader.scala b/src/compiler/scala/tools/nsc/interpreter/AbstractFileClassLoader.scala
index 70fa740eeb..3a605975f4 100644
--- a/src/compiler/scala/tools/nsc/interpreter/AbstractFileClassLoader.scala
+++ b/src/compiler/scala/tools/nsc/interpreter/AbstractFileClassLoader.scala
@@ -21,9 +21,6 @@ class AbstractFileClassLoader(root: AbstractFile, parent: ClassLoader)
 {
   // private val defined = mutable.Map[String, Class[_]]()
 
-  // Widening to public
-  override def getPackage(name: String) = super.getPackage(name)
-
   override protected def trace =
     sys.props contains "scala.debug.classloader"
 
@@ -47,6 +44,22 @@ class AbstractFileClassLoader(root: AbstractFile, parent: ClassLoader)
     }
   }
 
+  protected def dirNameToPath(name: String): String =
+    name.replace('.', '/')
+
+  protected def findAbstractDir(name: String): AbstractFile = {
+    var file: AbstractFile = root
+    val pathParts          = dirNameToPath(name) split '/'
+
+    for (dirPart <- pathParts) {
+      file = file.lookupName(dirPart, true)
+      if (file == null)
+        return null
+    }
+
+    return file
+  }
+
   override def getResourceAsStream(name: String) = findAbstractFile(name) match {
     case null => super.getResourceAsStream(name)
     case file => file.input
@@ -78,4 +91,24 @@ class AbstractFileClassLoader(root: AbstractFile, parent: ClassLoader)
   //   case null   => super.getResource(name)
   //   case file   => new URL(...)
   // }
+
+  private val packages = mutable.Map[String, Package]()
+
+  override def definePackage(name: String, specTitle: String, specVersion: String, specVendor: String, implTitle: String, implVersion: String, implVendor: String, sealBase: URL): Package = {
+    throw new UnsupportedOperationException()
+  }
+
+  override def getPackage(name: String): Package = {
+    findAbstractDir(name) match {
+      case null => super.getPackage(name)
+      case file => packages.getOrElseUpdate(name, {
+        val ctor = classOf[Package].getDeclaredConstructor(classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[String], classOf[URL], classOf[ClassLoader])
+        ctor.setAccessible(true)
+        ctor.newInstance(name, null, null, null, null, null, null, null, this)
+      })
+    }
+  }
+
+  override def getPackages(): Array[Package] =
+    root.iterator.filter(_.isDirectory).map(dir => getPackage(dir.name)).toArray
 }
diff --git a/src/compiler/scala/tools/nsc/interpreter/IMain.scala b/src/compiler/scala/tools/nsc/interpreter/IMain.scala
index 4ccea8afd6..6ae8d0e7d0 100644
--- a/src/compiler/scala/tools/nsc/interpreter/IMain.scala
+++ b/src/compiler/scala/tools/nsc/interpreter/IMain.scala
@@ -196,7 +196,7 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends
       def foreach[U](f: Tree => U): Unit = t foreach { x => f(x) ; () }
     }).toList
   }
-  
+
   implicit def installReplTypeOps(tp: Type): ReplTypeOps = new ReplTypeOps(tp)
   class ReplTypeOps(tp: Type) {
     def orElse(other: => Type): Type    = if (tp ne NoType) tp else other
@@ -314,26 +314,6 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends
   private class TranslatingClassLoader(parent: ClassLoader) extends AbstractFileClassLoader(virtualDirectory, parent) {
     private[IMain] var traceClassLoading = isReplTrace
     override protected def trace = super.trace || traceClassLoading
-    
-    private val packages = mutable.HashMap[String, Package]()
-    private def enclosingPackageNames(name: String): List[String] =
-      (name split '.').inits.toList drop 1 dropRight 1 map (_ mkString ".") reverse
-
-    // Here's what all those params to definePackage are after the package name:
-    //
-    // specTitle - The specification title
-    // specVersion - The specification version
-    // specVendor - The specification vendor
-    // implTitle - The implementation title
-    // implVersion - The implementation version
-    // implVendor - The implementation vendor
-    // sealBase - If not null, then this package is sealed with respect to the given code source URL object. Otherwise, the package is not sealed.
-    private def addPackageNames(name: String) {
-      enclosingPackageNames(name) filterNot (packages contains _) foreach { p =>
-        packages(p) = definePackage(p, "", "", "", "", "", "", null)
-        repltrace("Added " + packages(p) + " to repl classloader.")
-      }
-    }
 
     /** Overridden here to try translating a simple name to the generated
      *  class name if the original attempt fails.  This method is used by
@@ -348,12 +328,6 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends
           file
       }
     }
-    override def findClass(name: String): JClass = {
-      val clazz = super.findClass(name)
-      if (clazz ne null)
-        addPackageNames(clazz.getName)
-      clazz
-    }
   }
   private def makeClassLoader(): AbstractFileClassLoader =
     new TranslatingClassLoader(parentClassLoader match {
@@ -1104,7 +1078,7 @@ class IMain(initialSettings: Settings, protected val out: JPrintWriter) extends
       val clazz      = classOfTerm(id) getOrElse { return NoType }
       val staticSym  = tpe.typeSymbol
       val runtimeSym = getClassIfDefined(clazz.getName)
-      
+
       if ((runtimeSym != NoSymbol) && (runtimeSym != staticSym) && (runtimeSym isSubClass staticSym))
         runtimeSym.info
       else NoType
diff --git a/test/files/run/t5256a.check b/test/files/run/t5256a.check
new file mode 100644
index 0000000000..304f4ddd79
--- /dev/null
+++ b/test/files/run/t5256a.check
@@ -0,0 +1,2 @@
+A
+true
diff --git a/test/files/run/t5256a.scala b/test/files/run/t5256a.scala
new file mode 100644
index 0000000000..05a935c770
--- /dev/null
+++ b/test/files/run/t5256a.scala
@@ -0,0 +1,9 @@
+import scala.reflect.mirror._
+
+class A
+
+object Test extends App {
+  val c = classToType(classOf[A])
+  println(c)
+  println(c.typeSymbol == classToSymbol(classOf[A]))
+}
diff --git a/test/files/run/t5256b.check b/test/files/run/t5256b.check
new file mode 100644
index 0000000000..64f4c01166
--- /dev/null
+++ b/test/files/run/t5256b.check
@@ -0,0 +1,2 @@
+Test.A

+true
\ No newline at end of file
diff --git a/test/files/run/t5256b.scala b/test/files/run/t5256b.scala
new file mode 100644
index 0000000000..5575211641
--- /dev/null
+++ b/test/files/run/t5256b.scala
@@ -0,0 +1,8 @@
+import scala.reflect.mirror._
+
+object Test extends App {
+  class A
+  val c = classToType(classOf[A])
+  println(c)
+  println(c.typeSymbol == classToSymbol(classOf[A]))
+}
diff --git a/test/files/run/t5256d.check b/test/files/run/t5256d.check
new file mode 100644
index 0000000000..7924c15c5c
--- /dev/null
+++ b/test/files/run/t5256d.check
@@ -0,0 +1,20 @@
+Type in expressions to have them evaluated.

+Type :help for more information.

+

+scala> 

+

+scala> import scala.reflect.mirror._

+import scala.reflect.mirror._

+

+scala> class A

+defined class A

+

+scala> val c = classToType(classOf[A])

+c: reflect.mirror.Type = A

+

+scala> println(c.typeSymbol == classToSymbol(classOf[A]))

+true

+

+scala> 

+

+scala> 

diff --git a/test/files/run/t5256d.scala b/test/files/run/t5256d.scala
new file mode 100644
index 0000000000..86404a9b63
--- /dev/null
+++ b/test/files/run/t5256d.scala
@@ -0,0 +1,10 @@
+import scala.tools.partest.ReplTest
+
+object Test extends ReplTest {
+  def code = """
+import scala.reflect.mirror._
+class A
+val c = classToType(classOf[A])
+println(c.typeSymbol == classToSymbol(classOf[A]))
+  """
+}
diff --git a/test/files/run/t5256e.check b/test/files/run/t5256e.check
new file mode 100644
index 0000000000..e50f917e14
--- /dev/null
+++ b/test/files/run/t5256e.check
@@ -0,0 +1,2 @@
+C.this.A
+true
\ No newline at end of file
diff --git a/test/files/run/t5256e.scala b/test/files/run/t5256e.scala
new file mode 100644
index 0000000000..9ed422ca44
--- /dev/null
+++ b/test/files/run/t5256e.scala
@@ -0,0 +1,9 @@
+import scala.reflect.mirror._
+
+class C { class A }
+
+object Test extends App {
+  val c = classToType(classOf[C#A])
+  println(c)
+  println(c.typeSymbol == classToSymbol(classOf[C#A]))
+}
diff --git a/test/files/run/t5256f.check b/test/files/run/t5256f.check
new file mode 100644
index 0000000000..ad2f375d9a
--- /dev/null
+++ b/test/files/run/t5256f.check
@@ -0,0 +1,4 @@
+Test.A1
+true
+Test.this.A2
+true
diff --git a/test/files/run/t5256f.scala b/test/files/run/t5256f.scala
new file mode 100644
index 0000000000..45c80cbd63
--- /dev/null
+++ b/test/files/run/t5256f.scala
@@ -0,0 +1,19 @@
+import scala.reflect.mirror._
+
+object Test extends App {
+  class A1
+
+  val c1 = classToType(classOf[A1])
+  println(c1)
+  println(c1.typeSymbol == classToSymbol(classOf[A1]))
+
+  new Test
+}
+
+class Test {
+  class A2
+
+  val c2 = classToType(classOf[A2])
+  println(c2)
+  println(c2.typeSymbol == classToSymbol(classOf[A2]))
+}
diff --git a/test/pending/run/t5256c.check b/test/pending/run/t5256c.check
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/pending/run/t5256c.check
diff --git a/test/pending/run/t5256c.scala b/test/pending/run/t5256c.scala
new file mode 100644
index 0000000000..8ebb51a009
--- /dev/null
+++ b/test/pending/run/t5256c.scala
@@ -0,0 +1,10 @@
+import scala.reflect.mirror._
+
+object Test extends App {
+  {
+    class A
+    val c = classToType(classOf[A])
+    println(c)
+    println(c.typeSymbol == classToSymbol(classOf[A]))
+  }
+}
diff --git a/test/pending/run/t5256g.check b/test/pending/run/t5256g.check
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/test/pending/run/t5256g.check
diff --git a/test/pending/run/t5256g.scala b/test/pending/run/t5256g.scala
new file mode 100644
index 0000000000..6158a9281d
--- /dev/null
+++ b/test/pending/run/t5256g.scala
@@ -0,0 +1,11 @@
+import scala.reflect.mirror._
+
+class A
+trait B
+
+object Test extends App {
+  val mutant = new A with B
+  val c = classToType(mutant.getClass)
+  println(c)
+  println(c.typeSymbol == classToSymbol(mutant.getClass))
+}
diff --git a/test/pending/run/t5256h.check b/test/pending/run/t5256h.check
new file mode 100644
index 0000000000..4f9b8faf71
--- /dev/null
+++ b/test/pending/run/t5256h.check
@@ -0,0 +1,8 @@
+import scala.reflect.mirror._
+
+object Test extends App {
+  val mutant = new { val x = 2 }
+  val c = classToType(mutant.getClass)
+  println(c)
+  println(c.typeSymbol == classToSymbol(mutant.getClass))
+}
diff --git a/test/pending/run/t5256h.scala b/test/pending/run/t5256h.scala
new file mode 100644
index 0000000000..4f9b8faf71
--- /dev/null
+++ b/test/pending/run/t5256h.scala
@@ -0,0 +1,8 @@
+import scala.reflect.mirror._
+
+object Test extends App {
+  val mutant = new { val x = 2 }
+  val c = classToType(mutant.getClass)
+  println(c)
+  println(c.typeSymbol == classToSymbol(mutant.getClass))
+}