Permalink
Browse files

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.
  • Loading branch information...
1 parent fbd5efe commit 1e0707786b118e3e33379e7acdc75306b45e6547 @xeno-by xeno-by committed Jan 31, 2012
@@ -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)
@@ -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
}
@@ -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
@@ -0,0 +1,2 @@
+A
+true
@@ -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]))
+}
@@ -0,0 +1,2 @@
+Test.A
+true
@@ -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]))
+}
@@ -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>
@@ -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]))
+ """
+}
@@ -0,0 +1,2 @@
+C.this.A
+true
@@ -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]))
+}
@@ -0,0 +1,4 @@
+Test.A1
+true
+Test.this.A2
+true
@@ -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]))
+}
No changes.
@@ -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]))
+ }
+}
No changes.
@@ -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))
+}
Oops, something went wrong.

0 comments on commit 1e07077

Please sign in to comment.