SI-5999 a real fix to the packageless problem

After a discussion on a reflection meeting on Jul 17
we concluded that we should split staticModule into
staticModule and staticPackage to remove the ambiguity
between packageless objects and packageless packages
(more in the comments in the body of the commit).

The motivation is verbosely outlined in the comments,
but the bottom line is that Scala allows packages and
packageless objects to have the same name within the same program.
Therefore at times we need to disambiguate, hence the introduction
of the staticPackage method.

As of such staticModule no longer works for packages.
In the same fashion staticPackage doesn't work for modules.
This is done to ensure robustness of reification.

I would like to do the same for getModule in Definitions,
but we have to maintain backward compatibility. That's why I retained
the old behavior, but replaced getModule invocations with getPackage
where appropriate to be in line with staticModule and staticPackage.

Another important thing that follows from the discussion is that
both staticClass and staticModule prefer parent packages over parent objects
in cases of ambiguity. Say, if we have the following snippet of code:
object B { class C } next to package B { class C }
then staticClass("B.C") will never even consider a C inside the object B.
This is how scalac operates, so we decided to be consistent here.

Finally reification logic got changed to distinguish between
staticModule and staticPackage, and to allow for the fact that
staticClass and staticModule prefer parent packages to parent objects.

3 files changed, 36 insertions, 7 deletions

diff --git a/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala b/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala
index 9b0777580b..38c8fedac5 100644
--- a/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala
+++ b/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala
@@ -37,10 +37,40 @@ trait GenSymbols {
       mirrorMirrorSelect(nme.EmptyPackageClass)
     else if (sym.isModuleClass)
       Select(Select(reify(sym.sourceModule), nme.asModuleSymbol), nme.moduleClass)
+    else if (sym.isPackage)
+      mirrorMirrorCall(nme.staticPackage, reify(sym.fullName))
     else if (sym.isLocatable) {
-      // [Eugene] am I doing this right?
-//      if (sym.isStaticOwner) { // no good for us, because it returns false for packages
-      if (sym.isStatic && (sym.isClass || sym.isModule)) {
+      /** This is a fancy conundrum that stems from the fact that Scala allows
+       *  packageless packages and packageless objects with the same names in the same program.
+       *
+       *  For more details read the docs to staticModule and staticPackage.
+       *  Here I'll just provide the examples of how reify works for different kinds of symbols.
+       *
+       *    // 1) packageless
+       *    // packageless classes are non-ambiguous, but modules vs packages might be
+       *    // that's why we have separate methods to reify those
+       *    // note that staticModule will never resolve to a package if an object is missing and an homonymous package is present and vice versa
+       *    // otherwise reification would be unsound
+       *    class C => staticClass("C")
+       *    object B => staticModule("B")
+       *    package B => staticPackage("B")
+       *
+       *    // 2) classes and modules enclosed in a package
+       *    // staticXXX methods always look into parent packages and ignores parent modules, so for fully qualified names they are non-ambiguous
+       *    // namely even if there's an object B { class C } next to package B { class C }, then staticClass("B.C") will resolve to a packageful class
+       *    // this closely mirrors Scala's behavior, read up the docs to staticModule/staticPackage for more information
+       *    package B { class C } => staticClass("B.C")
+       *    package B { object B } => staticModule("B.B")
+       *    package B { package B } => staticPackage("B.B")
+       *
+       *    // 3) classes and modules enclosed in a packageless module
+       *    // staticClass/staticModule won't look into EmptyPackageClass, so we reify such symbols in a roundabout way
+       *    object B { class C } => selectType(staticModule("B"), "C")
+       *    object B { object B } => selectType(staticModule("B"), "B")
+       *    object B { package B } => impossible
+       */
+      val hasPackagelessParent = sym.ownerChain.tail.tail exists (_.isEmptyPackageClass)
+      if (sym.isStatic && (sym.isClass || sym.isModule) && !hasPackagelessParent) {
         val resolver = if (sym.isType) nme.staticClass else nme.staticModule
         mirrorMirrorCall(resolver, reify(sym.fullName))
       } else {
diff --git a/src/compiler/scala/reflect/reify/codegen/GenTypes.scala b/src/compiler/scala/reflect/reify/codegen/GenTypes.scala
index 82951a2434..c49e5b3342 100644
--- a/src/compiler/scala/reflect/reify/codegen/GenTypes.scala
+++ b/src/compiler/scala/reflect/reify/codegen/GenTypes.scala
@@ -41,8 +41,7 @@ trait GenTypes {
       case tpe @ ThisType(empty) if empty.isEmptyPackageClass =>
         mirrorBuildCall(nme.thisPrefix, mirrorMirrorSelect(nme.EmptyPackageClass))
       case tpe @ ThisType(clazz) if clazz.isModuleClass && clazz.isStatic =>
-        // [Eugene++ to Martin] makes sense?
-        val module = mirrorMirrorCall(nme.staticModule, reify(clazz.fullName))
+        val module = reify(clazz.sourceModule)
         val moduleClass = Select(Select(module, nme.asModuleSymbol), nme.moduleClass)
         mirrorFactoryCall(nme.ThisType, moduleClass)
       case tpe @ ThisType(_) =>
diff --git a/src/compiler/scala/reflect/reify/utils/Extractors.scala b/src/compiler/scala/reflect/reify/utils/Extractors.scala
index 52e4ff08c1..86265ec77a 100644
--- a/src/compiler/scala/reflect/reify/utils/Extractors.scala
+++ b/src/compiler/scala/reflect/reify/utils/Extractors.scala
@@ -27,7 +27,7 @@ trait Extractors {
   //       def applyImpl[U >: Nothing <: scala.reflect.api.Universe with Singleton]($m$untyped: scala.reflect.base.MirrorOf[U]): U#Tree = {
   //         val $u: U = $m$untyped.universe;
   //         val $m: $u.Mirror = $m$untyped.asInstanceOf[$u.Mirror];
-  //         $u.Apply($u.Select($u.Select($u.build.This($m.staticModule("scala.collection.immutable").moduleClass), $u.newTermName("List")), $u.newTermName("apply")), List($u.Literal($u.Constant(1)), $u.Literal($u.Constant(2))))
+  //         $u.Apply($u.Select($u.Select($u.build.This($m.staticPackage("scala.collection.immutable").moduleClass), $u.newTermName("List")), $u.newTermName("apply")), List($u.Literal($u.Constant(1)), $u.Literal($u.Constant(2))))
   //       }
   //     };
   //     new $treecreator1()
@@ -40,7 +40,7 @@ trait Extractors {
   //       def apply[U >: Nothing <: scala.reflect.base.Universe with Singleton]($m$untyped: scala.reflect.base.MirrorOf[U]): U#Type = {
   //         val $u: U = $m$untyped.universe;
   //         val $m: $u.Mirror = $m$untyped.asInstanceOf[$u.Mirror];
-  //         $u.TypeRef($u.ThisType($m.staticModule("scala.collection.immutable").moduleClass), $m.staticClass("scala.collection.immutable.List"), List($m.staticClass("scala.Int").asTypeConstructor))
+  //         $u.TypeRef($u.ThisType($m.staticPackage("scala.collection.immutable").moduleClass), $m.staticClass("scala.collection.immutable.List"), List($m.staticClass("scala.Int").asTypeConstructor))
   //       }
   //     };
   //     new $typecreator1()