Skip to content

Commit

Permalink
Hardens classToType logic
Browse files Browse the repository at this point in the history
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
xeno-by committed Feb 1, 2012
1 parent fbd5efe commit 1e07077
Show file tree
Hide file tree
Showing 19 changed files with 247 additions and 62 deletions.
118 changes: 87 additions & 31 deletions src/compiler/scala/reflect/runtime/JavaToScala.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

/**
Expand Down Expand Up @@ -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)
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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]
}

/**
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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
Expand Down Expand Up @@ -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
}
30 changes: 2 additions & 28 deletions src/compiler/scala/tools/nsc/interpreter/IMain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/files/run/t5256a.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
A
true
9 changes: 9 additions & 0 deletions test/files/run/t5256a.scala
Original file line number Diff line number Diff line change
@@ -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]))
}
2 changes: 2 additions & 0 deletions test/files/run/t5256b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Test.A
true
8 changes: 8 additions & 0 deletions test/files/run/t5256b.scala
Original file line number Diff line number Diff line change
@@ -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]))
}
20 changes: 20 additions & 0 deletions test/files/run/t5256d.check
Original file line number Diff line number Diff line change
@@ -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>
10 changes: 10 additions & 0 deletions test/files/run/t5256d.scala
Original file line number Diff line number Diff line change
@@ -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]))
"""
}
2 changes: 2 additions & 0 deletions test/files/run/t5256e.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
C.this.A
true
9 changes: 9 additions & 0 deletions test/files/run/t5256e.scala
Original file line number Diff line number Diff line change
@@ -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]))
}
4 changes: 4 additions & 0 deletions test/files/run/t5256f.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Test.A1
true
Test.this.A2
true
19 changes: 19 additions & 0 deletions test/files/run/t5256f.scala
Original file line number Diff line number Diff line change
@@ -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]))
}
Empty file added test/pending/run/t5256c.check
Empty file.
10 changes: 10 additions & 0 deletions test/pending/run/t5256c.scala
Original file line number Diff line number Diff line change
@@ -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]))
}
}
Empty file added test/pending/run/t5256g.check
Empty file.
11 changes: 11 additions & 0 deletions test/pending/run/t5256g.scala
Original file line number Diff line number Diff line change
@@ -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))
}
Loading

0 comments on commit 1e07077

Please sign in to comment.