fixes handling of fancy nested classes in runtime reflection

Replaces the `jclazz.isMemberClass` check for whether we have an
inner/nested class with `jclazz.getEnclosingClass != null`, because there
exist classes produced by javac (see the attached jar file and the test log)
which have the following properties:
  * They are nested within a parent class
  * getEnclosingClass returns a non-null value
  * isMemberClass returns false

Previously such classes were incorrectly treated as non-nested, were
incorrectly put into an enclosing package rather than an enclosing class,
and had their names trimmed in the process, leading to situations when
a package has multiple declarations with the same name. This is now fixed.

When changing the check, we need to be careful with interpretation of
what Class.getEnclosingXXX methods return. If getEnclosingClass produces
a non-null result, this doesn't mean that the class is inner or nested,
because getEnclosingClass is also not null for local classes (the ones
with getEnclosingMethod != null || getEnclosingConstructor != null).
This is expressed in the order of pattern match clauses in `sOwner`.

Now when the bug is fixed, I also revert b18a2f8798b2, restoring a very
important integrity check in runtime reflection, which I had to disable
a couple hours ago to fix a master breakage. More details at scala-internals:
https://groups.google.com/forum/#!topic/scala-internals/hcnUFk75MgQ

3 files changed, 29 insertions, 31 deletions

diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index 1e2dd6b7d3..8e822ca4f0 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -18,7 +18,7 @@ import internal.pickling.ByteCodecs
 import internal.pickling.UnPickler
 import scala.collection.mutable.{ HashMap, ListBuffer }
 import internal.Flags._
-import ReflectionUtils.{staticSingletonInstance, innerSingletonInstance, scalacShouldntLoadClass}
+import ReflectionUtils._
 import scala.language.existentials
 import scala.runtime.{ScalaRunTime, BoxesRunTime}
 
@@ -777,33 +777,19 @@ private[reflect] trait JavaMirrors extends internal.SymbolTable with api.JavaUni
     /**
      * The Scala owner of the Scala class corresponding to the Java class `jclazz`
      */
-    private def sOwner(jclazz: jClass[_]): Symbol =
-      if (jclazz.isMemberClass) {
-        val jEnclosingClass = jclazz.getEnclosingClass
-        val sEnclosingClass = classToScala(jEnclosingClass)
-        followStatic(sEnclosingClass, jclazz.javaFlags)
-      } else if (jclazz.isLocalClass0) {
-        val jEnclosingMethod = jclazz.getEnclosingMethod
-        if (jEnclosingMethod != null) {
-          methodToScala(jEnclosingMethod)
-        } else {
-          val jEnclosingConstructor = jclazz.getEnclosingConstructor
-          constructorToScala(jEnclosingConstructor)
-        }
-      } else if (jclazz.isPrimitive || jclazz.isArray) {
-        ScalaPackageClass
-      } else if (jclazz.getPackage != null) {
-        val jPackage = jclazz.getPackage
-        packageToScala(jPackage).moduleClass
-      } 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
-        // [martin] I think it's better to be forgiving here. Restoring packageNameToScala.
-        val jPackageName = jclazz.getName take jclazz.getName.lastIndexOf('.')
-        packageNameToScala(jPackageName).moduleClass
-      }
+    // @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
+    // [martin] I think it's better to be forgiving here. Restoring packageNameToScala.
+    private def sOwner(jclazz: jClass[_]): Symbol = jclazz match {
+      case PrimitiveOrArray()            => ScalaPackageClass
+      case EnclosedInMethod(jowner)      => methodToScala(jowner)
+      case EnclosedInConstructor(jowner) => constructorToScala(jowner)
+      case EnclosedInClass(jowner)       => followStatic(classToScala(jowner), jclazz.javaFlags)
+      case EnclosedInPackage(jowner)     => packageToScala(jowner).moduleClass
+      case _                             => packageNameToScala(jclazz.getName take jclazz.getName.lastIndexOf('.')).moduleClass
+    }
 
     /**
      * The Scala owner of the Scala symbol corresponding to the Java member `jmember`
diff --git a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala
index 710ec02acd..813c0e1386 100644
--- a/src/reflect/scala/reflect/runtime/ReflectionUtils.scala
+++ b/src/reflect/scala/reflect/runtime/ReflectionUtils.scala
@@ -84,4 +84,18 @@ private[scala] object ReflectionUtils {
   def scalacShouldntLoadClassfile(fileName: String) = isTraitImplementation(fileName)
 
   def scalacShouldntLoadClass(name: scala.reflect.internal.SymbolTable#Name) = scalacShouldntLoadClassfile(name + ".class")
+
+  object PrimitiveOrArray {
+    def unapply(jclazz: jClass[_]) = jclazz.isPrimitive || jclazz.isArray
+  }
+
+  class EnclosedIn[T](enclosure: jClass[_] => T) {
+    def unapply(jclazz: jClass[_]): Option[T] = if (enclosure(jclazz) != null) Some(enclosure(jclazz)) else None
+  }
+
+  object EnclosedInMethod extends EnclosedIn(_.getEnclosingMethod)
+  object EnclosedInConstructor extends EnclosedIn(_.getEnclosingConstructor)
+  object EnclosedInClass extends EnclosedIn(_.getEnclosingClass)
+  object EnclosedInPackage extends EnclosedIn(_.getPackage)
 }
+
diff --git a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala
index c6059ac402..30a3855d70 100644
--- a/src/reflect/scala/reflect/runtime/SymbolLoaders.scala
+++ b/src/reflect/scala/reflect/runtime/SymbolLoaders.scala
@@ -97,9 +97,7 @@ private[reflect] trait SymbolLoaders { self: SymbolTable =>
       if (isCompilerUniverse) super.enter(sym)
       else {
         val existing = super.lookupEntry(sym.name)
-        // commented out to provide a hotfix for strange class files that javac sometimes emits
-        // see more details at: https://groups.google.com/forum/#!topic/scala-internals/hcnUFk75MgQ
-        // assert(existing == null || existing.sym.isMethod, s"pkgClass = $pkgClass, sym = $sym, existing = $existing")
+        assert(existing == null || existing.sym.isMethod, s"pkgClass = $pkgClass, sym = $sym, existing = $existing")
         super.enter(sym)
       }
     }