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.

10 files changed, 160 insertions, 51 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()
diff --git a/src/continuations/plugin/scala/tools/selectivecps/CPSUtils.scala b/src/continuations/plugin/scala/tools/selectivecps/CPSUtils.scala
index 3a1dc87a6a..46c644bcd6 100644
--- a/src/continuations/plugin/scala/tools/selectivecps/CPSUtils.scala
+++ b/src/continuations/plugin/scala/tools/selectivecps/CPSUtils.scala
@@ -36,7 +36,7 @@ trait CPSUtils {
   lazy val MarkerCPSAdaptMinus = rootMirror.getRequiredClass("scala.util.continuations.cpsMinus")
 
   lazy val Context = rootMirror.getRequiredClass("scala.util.continuations.ControlContext")
-  lazy val ModCPS = rootMirror.getRequiredModule("scala.util.continuations")
+  lazy val ModCPS = rootMirror.getRequiredPackage("scala.util.continuations")
 
   lazy val MethShiftUnit  = definitions.getMember(ModCPS, cpsNames.shiftUnit)
   lazy val MethShiftUnit0 = definitions.getMember(ModCPS, cpsNames.shiftUnit0)
diff --git a/src/library/scala/reflect/base/Base.scala b/src/library/scala/reflect/base/Base.scala
index a4e6256f4d..4457a6cf14 100644
--- a/src/library/scala/reflect/base/Base.scala
+++ b/src/library/scala/reflect/base/Base.scala
@@ -357,6 +357,9 @@ class Base extends Universe { self =>
     def staticModule(fullName: String): ModuleSymbol =
       mkStatic[ModuleSymbol](fullName)
 
+    def staticPackage(fullName: String): ModuleSymbol =
+      staticModule(fullName) // this toy universe doesn't care about the distinction between packages and modules
+
     private def mkStatic[S <: Symbol : ClassTag](fullName: String): S =
       cached(fullName) {
         val point = fullName lastIndexOf '.'
diff --git a/src/library/scala/reflect/base/MirrorOf.scala b/src/library/scala/reflect/base/MirrorOf.scala
index d030ace96c..6dc8090eee 100644
--- a/src/library/scala/reflect/base/MirrorOf.scala
+++ b/src/library/scala/reflect/base/MirrorOf.scala
@@ -25,7 +25,7 @@ abstract class MirrorOf[U <: base.Universe with Singleton] {
    *    scala> res0.fullName
    *    res1: String = scala.collection.immutable.List
    *
-   *    scala> cm.staticModule("scala")
+   *    scala> cm.staticPackage("scala")
    *    res2: reflect.runtime.universe.ModuleSymbol = package scala
    *
    *    scala> res2.moduleClass.typeSignature member newTypeName("List")
@@ -33,11 +33,58 @@ abstract class MirrorOf[U <: base.Universe with Singleton] {
    *
    *    scala> res3.fullName
    *    res4: String = scala.List
+   *
+   *  To be consistent with Scala name resolution rules, in case of ambiguity between
+   *  a package and an object, the object is never been considered.
+   *
+   *  For example for the following code:
+   *
+   *    package foo {
+   *      class B
+   *    }
+   *
+   *    object foo {
+   *      class A
+   *      class B
+   *    }
+   *
+   *  staticClass("foo.B") will resolve to the symbol corresponding to the class B declared in the package foo, and
+   *  staticClass("foo.A") will throw a MissingRequirementException (which is exactly what scalac would do if this
+   *  fully qualified class name is written inside any package in a Scala program).
+   *
+   *  In the example above, to load a symbol that corresponds to the class B declared in the object foo,
+   *  use staticModule("foo") to load the module symbol and then navigate typeSignature.members of its moduleClass.
    */
   def staticClass(fullName: String): U#ClassSymbol
 
   /** The symbol corresponding to the globally accessible object with the
    *  given fully qualified name `fullName`.
+   *
+   *  To be consistent with Scala name resolution rules, in case of ambiguity between
+   *  a package and an object, the object is never been considered.
+   *
+   *  For example for the following code:
+   *
+   *    package foo {
+   *      object B
+   *    }
+   *
+   *    object foo {
+   *      object A
+   *      object B
+   *    }
+   *
+   *  staticModule("foo.B") will resolve to the symbol corresponding to the object B declared in the package foo, and
+   *  staticModule("foo.A") will throw a MissingRequirementException (which is exactly what scalac would do if this
+   *  fully qualified class name is written inside any package in a Scala program).
+   *
+   *  In the example above, to load a symbol that corresponds to the object B declared in the object foo,
+   *  use staticModule("foo") to load the module symbol and then navigate typeSignature.members of its moduleClass.
    */
   def staticModule(fullName: String): U#ModuleSymbol
+
+  /** The symbol corresponding to a package with the
+   *  given fully qualified name `fullName`.
+   */
+  def staticPackage(fullName: String): U#ModuleSymbol
 }
diff --git a/src/partest/scala/tools/partest/CompilerTest.scala b/src/partest/scala/tools/partest/CompilerTest.scala
index 89c1a9c2ca..fec9893099 100644
--- a/src/partest/scala/tools/partest/CompilerTest.scala
+++ b/src/partest/scala/tools/partest/CompilerTest.scala
@@ -50,7 +50,7 @@ abstract class CompilerTest extends DirectTest {
   }
 
   class SymsInPackage(pkgName: String) {
-    def pkg     = rootMirror.getRequiredModule(pkgName)
+    def pkg     = rootMirror.getRequiredPackage(pkgName)
     def classes = allMembers(pkg) filter (_.isClass)
     def modules = allMembers(pkg) filter (_.isModule)
     def symbols = classes ++ terms filterNot (_ eq NoSymbol)
diff --git a/src/reflect/scala/reflect/internal/Mirrors.scala b/src/reflect/scala/reflect/internal/Mirrors.scala
index 100168a69d..f7561ae274 100644
--- a/src/reflect/scala/reflect/internal/Mirrors.scala
+++ b/src/reflect/scala/reflect/internal/Mirrors.scala
@@ -29,24 +29,14 @@ trait Mirrors extends api.Mirrors {
       else definitions.findNamedMember(segs.tail, RootClass.info member segs.head)
     }
 
-    /** If you're looking for a class, pass a type name.
-     *  If a module, a term name.
-     *
-     *  `tryPackageless` tells this function to search for requested a requested symbol in EmptyPackageClass as well.
-     *  Compiler might ignore them, but they should be loadable with macros if the programmer wishes.
-     *  More info here: http://groups.google.com/group/scala-internals/browse_thread/thread/5146021fd7c0cec
-     */
-    private def getModuleOrClass(path: Name, tryPackageless: Boolean): Symbol = getModuleOrClass(path, path.length, tryPackageless)
-
     /** Todo: organize similar to mkStatic in reflect.Base */
-    private def getModuleOrClass(path: Name, len: Int, tryPackageless: Boolean): Symbol = {
+    private def getModuleOrClass(path: Name, len: Int): Symbol = {
       val point = path lastPos('.', len - 1)
       val owner =
-        if (point > 0) getModuleOrClass(path.toTermName, point, tryPackageless)
+        if (point > 0) getModuleOrClass(path.toTermName, point)
         else RootClass
       val name = path subName (point + 1, len)
       var sym = owner.info member name
-      if (sym == NoSymbol && owner == RootClass && tryPackageless) sym = EmptyPackageClass.info decl name
       val result = if (path.isTermName) sym.suchThat(_ hasFlag MODULE) else sym
       if (result != NoSymbol) result
       else {
@@ -57,6 +47,27 @@ trait Mirrors extends api.Mirrors {
       }
     }
 
+    /** If you're looking for a class, pass a type name.
+     *  If a module, a term name.
+     *
+     *  Unlike `getModuleOrClass`, this function
+     *  loads unqualified names from the root package.
+     */
+    private def getModuleOrClass(path: Name): Symbol =
+      getModuleOrClass(path, path.length)
+
+    /** If you're looking for a class, pass a type name.
+     *  If a module, a term name.
+     *
+     *  Unlike `getModuleOrClass`, this function
+     *  loads unqualified names from the empty package.
+     */
+    private def staticModuleOrClass(path: Name): Symbol = {
+      val isPackageless = path.pos('.') == path.length
+      if (isPackageless) EmptyPackageClass.info decl path
+      else getModuleOrClass(path)
+    }
+
     protected def mirrorMissingHook(owner: Symbol, name: Name): Symbol = NoSymbol
 
     protected def universeMissingHook(owner: Symbol, name: Name): Symbol = self.missingHook(owner, name)
@@ -67,8 +78,8 @@ trait Mirrors extends api.Mirrors {
 
     /************************ loaders of class symbols ************************/
 
-    private def getClassImpl(fullname: TypeName, tryPackageless: Boolean): ClassSymbol = {
-      var result = getModuleOrClass(fullname.toTypeName, tryPackageless)
+    private def ensureClassSymbol(fullname: String, sym: Symbol): ClassSymbol = {
+      var result = sym
       while (result.isAliasType) result = result.info.typeSymbol
       result match {
         case x: ClassSymbol => x
@@ -81,7 +92,7 @@ trait Mirrors extends api.Mirrors {
       getClassByName(fullname)
 
     def getClassByName(fullname: Name): ClassSymbol =
-      getClassImpl(fullname.toTypeName, tryPackageless = false)
+      ensureClassSymbol(fullname.toString, getModuleOrClass(fullname.toTypeName))
 
     def getRequiredClass(fullname: String): ClassSymbol =
       getClassByName(newTypeNameCached(fullname))
@@ -90,33 +101,33 @@ trait Mirrors extends api.Mirrors {
       getRequiredClass(erasureName[T])
 
     def getClassIfDefined(fullname: String): Symbol =
-      getClassIfDefined(newTypeName(fullname))
+      getClassIfDefined(newTypeNameCached(fullname))
 
     def getClassIfDefined(fullname: Name): Symbol =
       wrapMissing(getClassByName(fullname.toTypeName))
 
-    /** Unlike getClassByName/getRequiredClass this function can also load packageless symbols.
+    /** @inheritdoc
+     *
+     *  Unlike getClassByName/getRequiredClass this function can also load packageless symbols.
      *  Compiler might ignore them, but they should be loadable with macros.
      */
     override def staticClass(fullname: String): ClassSymbol =
-      getClassImpl(newTypeNameCached(fullname), tryPackageless = true)
+      ensureClassSymbol(fullname, staticModuleOrClass(newTypeNameCached(fullname)))
 
     /************************ loaders of module symbols ************************/
 
-    private def getModuleImpl(fullname: TermName, tryPackageless: Boolean): ModuleSymbol = {
-      var result = getModuleOrClass(fullname, tryPackageless)
-      result match {
-        case x: ModuleSymbol => x
-        case _               => MissingRequirementError.notFound("object " + fullname)
+    private def ensureModuleSymbol(fullname: String, sym: Symbol, allowPackages: Boolean): ModuleSymbol =
+      sym match {
+        case x: ModuleSymbol if allowPackages || !x.isPackage => x
+        case _                                                => MissingRequirementError.notFound("object " + fullname)
       }
-    }
 
     @deprecated("Use getModuleByName", "2.10.0")
     def getModule(fullname: Name): ModuleSymbol =
       getModuleByName(fullname)
 
     def getModuleByName(fullname: Name): ModuleSymbol =
-      getModuleImpl(fullname.toTermName, tryPackageless = false)
+      ensureModuleSymbol(fullname.toString, getModuleOrClass(fullname.toTermName), allowPackages = true)
 
     def getRequiredModule(fullname: String): ModuleSymbol =
       getModule(newTermNameCached(fullname))
@@ -132,41 +143,47 @@ trait Mirrors extends api.Mirrors {
       getRequiredModule(erasureName[T] stripSuffix "$")
 
     def getModuleIfDefined(fullname: String): Symbol =
-      getModuleIfDefined(newTermName(fullname))
+      getModuleIfDefined(newTermNameCached(fullname))
 
     def getModuleIfDefined(fullname: Name): Symbol =
       wrapMissing(getModule(fullname.toTermName))
 
-    /** Unlike getModule/getRequiredModule this function can also load packageless symbols.
+    /** @inheritdoc
+     *
+     *  Unlike getModule/getRequiredModule this function can also load packageless symbols.
      *  Compiler might ignore them, but they should be loadable with macros.
      */
     override def staticModule(fullname: String): ModuleSymbol =
-      getModuleImpl(newTermNameCached(fullname), tryPackageless = true)
+      ensureModuleSymbol(fullname, staticModuleOrClass(newTermNameCached(fullname)), allowPackages = false)
 
     /************************ loaders of package symbols ************************/
 
-    def getPackage(fullname: Name): ModuleSymbol = getModule(fullname)
+    private def ensurePackageSymbol(fullname: String, sym: Symbol, allowModules: Boolean): ModuleSymbol =
+      sym match {
+        case x: ModuleSymbol if allowModules || x.isPackage => x
+        case _                                              => MissingRequirementError.notFound("package " + fullname)
+      }
+
+    def getPackage(fullname: Name): ModuleSymbol =
+      ensurePackageSymbol(fullname.toString, getModuleOrClass(fullname), allowModules = true)
 
     def getRequiredPackage(fullname: String): ModuleSymbol =
       getPackage(newTermNameCached(fullname))
 
     def getPackageObject(fullname: String): ModuleSymbol =
-      (getModule(newTermName(fullname)).info member nme.PACKAGE) match {
+      (getPackage(newTermName(fullname)).info member nme.PACKAGE) match {
         case x: ModuleSymbol => x
         case _               => MissingRequirementError.notFound("package object " + fullname)
       }
 
-    def getPackageObjectIfDefined(fullname: String): Symbol = {
-      val module = getModuleIfDefined(newTermName(fullname))
-      if (module == NoSymbol) NoSymbol
-      else {
-        val packageObject = module.info member nme.PACKAGE
-        packageObject match {
-          case x: ModuleSymbol => x
-          case _               => NoSymbol
-        }
-      }
-    }
+    def getPackageObjectIfDefined(fullname: String): Symbol =
+      getPackageObjectIfDefined(newTermNameCached(fullname))
+
+    def getPackageObjectIfDefined(fullname: Name): Symbol =
+      wrapMissing(getPackageObject(fullname.toTermName))
+
+    override def staticPackage(fullname: String): ModuleSymbol =
+      ensurePackageSymbol(fullname.toString, getModuleOrClass(newTermNameCached(fullname)), allowModules = false)
 
     /************************ helpers ************************/
 
diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala
index c8a2424118..e5d0e96d9c 100644
--- a/src/reflect/scala/reflect/internal/StdNames.scala
+++ b/src/reflect/scala/reflect/internal/StdNames.scala
@@ -754,6 +754,7 @@ trait StdNames {
     val splice: NameType               = "splice"
     val staticClass : NameType         = "staticClass"
     val staticModule : NameType        = "staticModule"
+    val staticPackage : NameType       = "staticPackage"
     val synchronized_ : NameType       = "synchronized"
     val tail: NameType                 = "tail"
     val `then` : NameType              = "then"
diff --git a/src/reflect/scala/reflect/runtime/JavaMirrors.scala b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
index 9f9f79058d..0878801715 100644
--- a/src/reflect/scala/reflect/runtime/JavaMirrors.scala
+++ b/src/reflect/scala/reflect/runtime/JavaMirrors.scala
@@ -72,6 +72,18 @@ trait JavaMirrors extends internal.SymbolTable with api.JavaUniverse { self: Sym
       override def complete(sym: Symbol) = sym setInfo new LazyPackageType
     }
 
+    // reflective mirrors can't know the exhaustive list of available packages
+    // (that's because compiler mirrors are based on directories and reflective mirrors are based on classloaders,
+    // and unlike directories classloaders might make up stuff on the fly)
+    // hence we need to be optimistic and create packages out of thin air
+    // the same thing is done by the `missingHook` below
+    override def staticPackage(fullname: String): ModuleSymbol =
+      try super.staticPackage(fullname)
+      catch {
+        case _: MissingRequirementError =>
+          makeScalaPackage(fullname)
+      }
+
 // ----------- Caching ------------------------------------------------------------------
 
     // [Eugene++ to Martin] not weak? why?