Merge pull request #1882 from JamesIry/SI-5954_2.10.x

SI-5954 Adds implementation restriction preventing companions in package...

11 files changed, 97 insertions, 0 deletions

diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
index cfc7f14210..517b91dca8 100644
--- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
@@ -174,6 +174,7 @@ trait ScalaSettings extends AbsScalaSettings
   val etaExpandKeepsStar = BooleanSetting ("-Yeta-expand-keeps-star", "Eta-expand varargs methods to T* rather than Seq[T].  This is a temporary option to ease transition.")
   val Yinvalidate     = StringSetting     ("-Yinvalidate", "classpath-entry", "Invalidate classpath entry before run", "")
   val noSelfCheck     = BooleanSetting    ("-Yno-self-type-checks", "Suppress check for self-type conformance among inherited members.")
+  val companionsInPkgObjs = BooleanSetting("-Ycompanions-in-pkg-objs", "Allow companion objects and case classes in package objects. See issue SI-5954.")
   val YvirtClasses    = false // too embryonic to even expose as a -Y //BooleanSetting    ("-Yvirtual-classes", "Support virtual classes")
 
   val exposeEmptyPackage = BooleanSetting("-Yexpose-empty-package", "Internal only: expose the empty package.").internalOnly()
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
index 386eec207a..e3fd83f388 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1891,7 +1891,33 @@ trait Typers extends Modes with Adaptations with Tags {
         })
       }
       val impl2  = finishMethodSynthesis(impl1, clazz, context)
+      
+      // SI-5954. On second compile of a companion class contained in a package object we end up
+      // with some confusion of names which leads to having two symbols with the same name in the
+      // same owner. Until that can be straightened out we can't allow companion objects in package
+      // objects. But this code also tries to be friendly by distinguishing between case classes and
+      // user written companion pairs
+      def restrictPackageObjectMembers(mdef : ModuleDef) = for (m <- mdef.symbol.info.members) {
+        // ignore synthetic objects, because the "companion" object to a case class is synthetic and
+        // we only want one error per case class
+        if (!m.isSynthetic) {
+          // can't handle case classes in package objects
+          if (m.isCaseClass) pkgObjectRestriction(m, mdef, "case")
+          // can't handle companion class/object pairs in package objects
+          else if ((m.isClass && m.companionModule != NoSymbol && !m.companionModule.isSynthetic) || 
+                   (m.isModule && m.companionClass != NoSymbol && !m.companionClass.isSynthetic)) 
+                     pkgObjectRestriction(m, mdef, "companion")
+        }
+
+        def pkgObjectRestriction(m : Symbol, mdef : ModuleDef, restricted : String) = {
+          val pkgName = mdef.symbol.ownerChain find (_.isPackage) map (_.decodedName) getOrElse mdef.symbol.toString
+          context.error(if (m.pos.isDefined) m.pos else mdef.pos, s"implementation restriction: package object ${pkgName} cannot contain ${restricted} ${m}. Instead, ${m} should be placed directly in package ${pkgName}.")
+        }          
+      }
 
+      if (!settings.companionsInPkgObjs.value && mdef.symbol.isPackageObject)
+        restrictPackageObjectMembers(mdef)
+      
       treeCopy.ModuleDef(mdef, typedMods, mdef.name, impl2) setType NoType
     }
     /** In order to override this in the TreeCheckers Typer so synthetics aren't re-added
diff --git a/src/reflect/scala/reflect/internal/settings/MutableSettings.scala b/src/reflect/scala/reflect/internal/settings/MutableSettings.scala
index 81368df7a6..ec3501d5bc 100644
--- a/src/reflect/scala/reflect/internal/settings/MutableSettings.scala
+++ b/src/reflect/scala/reflect/internal/settings/MutableSettings.scala
@@ -47,4 +47,6 @@ abstract class MutableSettings extends AbsSettings {
   def XoldPatmat: BooleanSetting
   def XnoPatmatAnalysis: BooleanSetting
   def XfullLubs: BooleanSetting
+  def companionsInPkgObjs: BooleanSetting
+  
 }
diff --git a/src/reflect/scala/reflect/runtime/Settings.scala b/src/reflect/scala/reflect/runtime/Settings.scala
index 0e0cf3fc40..2d5b76f094 100644
--- a/src/reflect/scala/reflect/runtime/Settings.scala
+++ b/src/reflect/scala/reflect/runtime/Settings.scala
@@ -43,6 +43,7 @@ private[reflect] class Settings extends MutableSettings {
   val printtypes        = new BooleanSetting(false)
   val uniqid            = new BooleanSetting(false)
   val verbose           = new BooleanSetting(false)
+  val companionsInPkgObjs = new BooleanSetting(false)
 
   val Yrecursion        = new IntSetting(0)
   val maxClassfileName  = new IntSetting(255)
diff --git a/test/files/neg/t5954.check b/test/files/neg/t5954.check
new file mode 100644
index 0000000000..3ca47cd430
--- /dev/null
+++ b/test/files/neg/t5954.check
@@ -0,0 +1,16 @@
+t5954.scala:36: error: implementation restriction: package object A cannot contain case class D. Instead, class D should be placed directly in package A.
+  case class D()
+             ^
+t5954.scala:35: error: implementation restriction: package object A cannot contain companion object C. Instead, object C should be placed directly in package A.
+  object C
+         ^
+t5954.scala:34: error: implementation restriction: package object A cannot contain companion trait C. Instead, trait C should be placed directly in package A.
+  trait C
+        ^
+t5954.scala:33: error: implementation restriction: package object A cannot contain companion object B. Instead, object B should be placed directly in package A.
+  object B
+         ^
+t5954.scala:32: error: implementation restriction: package object A cannot contain companion class B. Instead, class B should be placed directly in package A.
+  class B
+        ^
+5 errors found
diff --git a/test/files/neg/t5954.scala b/test/files/neg/t5954.scala
new file mode 100644
index 0000000000..9e6f5392c7
--- /dev/null
+++ b/test/files/neg/t5954.scala
@@ -0,0 +1,46 @@
+// if you ever think you've fixed the underlying reason for the implementation restrictions
+// imposed by SI-5954, then here's a test that should pass with two "succes"es
+//
+//import scala.tools.partest._
+//
+//object Test extends DirectTest {
+//  def code = ???
+//
+//  def problemCode = """
+//    package object A {
+//      class B
+//      object B
+//      case class C()
+//    }
+//  """
+//
+//  def compileProblemCode() = {
+//    val classpath = List(sys.props("partest.lib"), testOutput.path) mkString sys.props("path.separator")
+//    compileString(newCompiler("-cp", classpath, "-d", testOutput.path))(problemCode)
+//  }
+//
+//  def show() : Unit = {
+//    for (i <- 0 until 2) {
+//      compileProblemCode()
+//      println(s"success ${i + 1}")
+//    }
+//  }
+//}
+
+package object A {
+  // these should be prevented by the implementation restriction
+  class B
+  object B
+  trait C
+  object C
+  case class D()
+  // all the rest of these should be ok
+  class E
+  object F
+  val g = "omg"
+  var h = "wtf"
+  def i = "lol"
+  type j = String
+  class K(val k : Int) extends AnyVal
+  implicit class L(val l : Int)
+}
diff --git a/test/files/pos/package-case.flags b/test/files/pos/package-case.flags
new file mode 100644
index 0000000000..2f174c4732
--- /dev/null
+++ b/test/files/pos/package-case.flags
@@ -0,0 +1 @@
+-Ycompanions-in-pkg-objs
diff --git a/test/files/pos/t2130-1.flags b/test/files/pos/t2130-1.flags
new file mode 100644
index 0000000000..2f174c4732
--- /dev/null
+++ b/test/files/pos/t2130-1.flags
@@ -0,0 +1 @@
+-Ycompanions-in-pkg-objs
diff --git a/test/files/pos/t2130-2.flags b/test/files/pos/t2130-2.flags
new file mode 100644
index 0000000000..2f174c4732
--- /dev/null
+++ b/test/files/pos/t2130-2.flags
@@ -0,0 +1 @@
+-Ycompanions-in-pkg-objs
diff --git a/test/files/pos/t3999b.flags b/test/files/pos/t3999b.flags
new file mode 100644
index 0000000000..2f174c4732
--- /dev/null
+++ b/test/files/pos/t3999b.flags
@@ -0,0 +1 @@
+-Ycompanions-in-pkg-objs
diff --git a/test/files/pos/t4052.flags b/test/files/pos/t4052.flags
new file mode 100644
index 0000000000..2f174c4732
--- /dev/null
+++ b/test/files/pos/t4052.flags
@@ -0,0 +1 @@
+-Ycompanions-in-pkg-objs