Deprecate nested class shadowing in "override" position

Fixes scala/bug#8353 This deprecates nested class shadowing in "override" position for Dotty compatibilty. In general Scala does not implement override of classes where "new Status" might take on different meaning based on what the subtypes are doing. To avoid the confusion, we are deprecating the nested class shadow another nested class if it was introduced by a parent class.
scala · Feb 12, 2020 · 5c40039 · 5c40039
1 parent d1b3235
commit 5c40039
Show file tree

Hide file tree

Showing 17 changed files with 100 additions and 61 deletions.
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -272,6 +272,7 @@ abstract class RefChecks extends Transform {
      *     that are not implemented in a subclass.
      *  4. Check that every member with an `override` modifier
      *     overrides some other member.
+     *  5. Check that the nested class do not shadow other nested classes from outer class's parent.
      */
     private def checkAllOverrides(clazz: Symbol, typesOnly: Boolean = false): Unit = {
       val self = clazz.thisType
@@ -832,7 +833,21 @@ abstract class RefChecks extends Transform {
           }
           member resetFlag (OVERRIDE | ABSOVERRIDE)  // Any Override
         }
-    }
+
+      // 5. Check that the nested class do not shadow other nested classes from outer class's parent
+      def checkNestedClassShadow(): Unit =
+        if (clazz.isNestedClass) {
+          val overridden = clazz.owner.ancestors
+            .map(a => clazz.matchingSymbol(a, clazz.owner.thisType))
+            .filter(c => c.exists && c.isClass)
+          overridden foreach { sym2 =>
+            def msg(what: String) = s"shadowing a nested class of a parent is $what but class ${clazz.name} shadows $sym2 defined in ${sym2.owner}; rename the class to something else"
+            if (currentRun.isScala300) reporter.error(clazz.pos, msg("unsupported"))
+            else currentRun.reporting.deprecationWarning(clazz.pos, clazz, msg("deprecated"), "2.13.2")
+          }
+        }
+      checkNestedClassShadow()
+    } // end checkAllOverrides
 
   // Basetype Checking --------------------------------------------------------
 

diff --git a/test/files/neg/nested-class-shadowing-removal.check b/test/files/neg/nested-class-shadowing-removal.check
@@ -0,0 +1,4 @@
+nested-class-shadowing-removal.scala:9: error: shadowing a nested class of a parent is unsupported but class Status shadows class Status defined in trait Core; rename the class to something else
+  class Status extends super.Status
+        ^
+1 error
diff --git a/test/files/neg/nested-class-shadowing-removal.scala b/test/files/neg/nested-class-shadowing-removal.scala
@@ -0,0 +1,10 @@
+// scalac: -Werror -Xlint:deprecation -Xsource:3.0
+//
+
+trait Core {
+  class Status
+}
+
+trait Ext extends Core {
+  class Status extends super.Status
+}
diff --git a/test/files/neg/nested-class-shadowing.check b/test/files/neg/nested-class-shadowing.check
@@ -0,0 +1,6 @@
+nested-class-shadowing.scala:9: warning: shadowing a nested class of a parent is deprecated but class Status shadows class Status defined in trait Core; rename the class to something else
+  class Status extends super.Status
+        ^
+error: No warnings can be incurred under -Werror.
+1 warning
+1 error
diff --git a/test/files/neg/nested-class-shadowing.scala b/test/files/neg/nested-class-shadowing.scala
@@ -0,0 +1,10 @@
+// scalac: -Werror -Xlint:deprecation
+//
+
+trait Core {
+  class Status
+}
+
+trait Ext extends Core {
+  class Status extends super.Status
+}
diff --git a/test/files/res/t597.check b/test/files/res/t597.check
@@ -1,4 +1,6 @@
 
-nsc> 
-nsc> 
+nsc> warning: 2 deprecations (since 2.13.2); re-run with -deprecation for details
+
+nsc> warning: 1 deprecation (since 2.13.2); re-run with -deprecation for details
+
 nsc> 
diff --git a/test/files/res/t743.check b/test/files/res/t743.check
@@ -1,4 +1,5 @@
 
-nsc> 
+nsc> warning: 1 deprecation (since 2.13.2); re-run with -deprecation for details
+
 nsc> 
 nsc> 
diff --git a/test/files/res/t831.check b/test/files/res/t831.check
@@ -1,4 +1,5 @@
 
-nsc> 
+nsc> warning: 3 deprecations (since 2.13.2); re-run with -deprecation for details
+
 nsc> 
 nsc> 
diff --git a/test/files/run/bugs.check b/test/files/run/bugs.check
@@ -84,12 +84,6 @@ hello
 <<< bug 328
 >>> bug 328
 
-<<< bug 396
-A
-B
-C
->>> bug 396
-
 <<< bug 399
 a
 >>> bug 399

diff --git a/test/files/run/bugs.scala b/test/files/run/bugs.scala
@@ -1,3 +1,6 @@
+// scalac: -Werror -Xlint:deprecation
+//
+
 //############################################################################
 // Bugs
 //############################################################################
@@ -394,29 +397,6 @@ object Bug328Test {
   def test(args: Array[String]): Unit = test0(args);
 }
 
-//############################################################################
-// Bug 396
-
-trait Bug396A {
-  class I {
-    def run = Console.println("A");
-  }
-}
-trait Bug396B extends Bug396A {
-  class I extends super.I {
-    override def run = { super.run; Console.println("B"); }
-  }
-}
-trait Bug396C extends Bug396A {
-  trait I extends super.I {
-    override def run = { super.run; Console.println("C"); }
-  }
-}
-object Bug396Test extends Bug396B with Bug396C {
-  class I2 extends super[Bug396B].I with super[Bug396C].I;
-  def test(args: Array[String]): Unit = (new I2).run
-}
-
 //############################################################################
 // Bug 399
 
@@ -476,7 +456,6 @@ object Test  {
     test(266, Bug266Test.test(args));
     test(316, Bug316Test.test(args));
     test(328, Bug328Test.test(args));
-    test(396, Bug396Test.test(args));
     test(399, Bug399Test.test(args));
 
     if (errors > 0) {

diff --git a/test/files/run/t6146b.check b/test/files/run/t6146b.check
@@ -1,4 +1,4 @@
-t6146b.scala:15: warning: match may not be exhaustive.
+t6146b.scala:18: warning: match may not be exhaustive.
 It would fail on the following inputs: S2(), S3()
   def foo(f: F[Int]) = f match { case X.S1 => }
                        ^

diff --git a/test/files/run/t6146b.scala b/test/files/run/t6146b.scala
@@ -1,7 +1,10 @@
+// scalac: -Xlint:deprecation
+//
+
 import scala.tools.partest.ReplTest
 
 class A {
-  sealed trait F[A]
+  // sealed trait F[A]
 }
 
 class C[T] extends A {

diff --git a/test/files/run/t657.scala b/test/files/run/t657.scala
@@ -1,53 +1,55 @@
+// scalac: -Werror -Xlint:deprecation
+//
 
 import scala.language.{ implicitConversions }
 abstract class BaseList {
-  type Node <: NodeImpl;
-  implicit def convertNode(ni : NodeImpl) = ni.asInstanceOf[Node];
-  abstract class NodeImpl;
+  type Node <: BaseNodeImpl
+  implicit def convertNode(ni : BaseNodeImpl) = ni.asInstanceOf[Node];
+  abstract class BaseNodeImpl
 }
 abstract class LinkedList extends BaseList {
-  type Node <: NodeImpl;
-  trait NodeImpl extends super.NodeImpl;
+  type Node <: NodeImpl0
+  trait NodeImpl0 extends super.BaseNodeImpl;
 }
 trait OffsetList extends LinkedList {
-  type Node <: NodeImpl;
-  trait NodeImpl extends super.NodeImpl;
+  type Node <: NodeImpl1
+  trait NodeImpl1 extends super.NodeImpl0
 }
 
 trait PriorityTree extends BaseList {
-  type Node <: NodeImpl;
-  trait NodeImpl extends super.NodeImpl {
-    def chop : Node = this;
+  type Node <: NodeImpl2
+  trait NodeImpl2 extends super.BaseNodeImpl {
+    def chop : Node = this
   }
 }
 
 trait PrecedenceParser  extends LinkedList with PriorityTree {
-  type Node <: NodeImpl;
-  trait NodeImpl extends super[LinkedList].NodeImpl with super[PriorityTree].NodeImpl;
+  type Node <: NodeImpl3
+  trait NodeImpl3 extends super[LinkedList].NodeImpl0 with super[PriorityTree].NodeImpl2
 }
 
 trait Matcher extends PrecedenceParser {
-  type Node <: NodeImpl;
-  trait NodeImpl extends super.NodeImpl;
+  type Node <: NodeImpl4
+  trait NodeImpl4 extends super.NodeImpl3
 
-  type Matchable <: Node with MatchableImpl;
-  implicit def convertMatchable(m : MatchableImpl) = m.asInstanceOf[Matchable];
-  trait MatchableImpl extends NodeImpl {
+  type Matchable <: Node with MatchableImpl0
+  implicit def convertMatchable(m : MatchableImpl0) = m.asInstanceOf[Matchable]
+  trait MatchableImpl0 extends NodeImpl4 {
     override def chop : Node = {
       Console.println("passed"); super.chop;
     }
   }
 }
 
 class Test1 extends OffsetList with Matcher {
-  type Node = NodeImpl;
-  trait NodeImpl extends super[OffsetList].NodeImpl with super[Matcher].NodeImpl;
-  class MatchableImpl extends super.MatchableImpl with NodeImpl;
-  type Matchable = MatchableImpl;
+  type Node = NodeImpl5
+  trait NodeImpl5 extends super[OffsetList].NodeImpl1 with super[Matcher].NodeImpl4
+  class MatchableImpl1 extends super.MatchableImpl0 with NodeImpl5
+  type Matchable = MatchableImpl1
 }
 
 object Test extends App {
-  val test = new Test1;
-  val m = new test.MatchableImpl;
-  m.chop;
+  val test = new Test1
+  val m = new test.MatchableImpl1
+  m.chop
 }
diff --git a/test/files/run/t6677b.check b/test/files/run/t6677b.check
@@ -0,0 +1,3 @@
+t6677b.scala:15: warning: class shadowing is deprecated but class X shadows class X defined in trait U1; rename the class to something else
+  class U11 extends U1 { class X extends super.X { foo } } // refer to foo to add $outer pointer
+                               ^
diff --git a/test/files/run/t6677b.scala b/test/files/run/t6677b.scala
@@ -1,3 +1,6 @@
+// scalac: -Xlint:deprecation
+//
+
 trait U {
 	trait U1 {
 		class X

diff --git a/test/files/run/t744.check b/test/files/run/t744.check
@@ -1,3 +1,6 @@
+t744.scala:12: warning: class shadowing is deprecated but class FileImpl shadows trait FileImpl defined in trait Linked; rename the class to something else
+    trait FileImpl extends super.FileImpl {
+          ^
 BEGIN
 Hello from linked
 END
diff --git a/test/files/run/t744.scala b/test/files/run/t744.scala
@@ -1,3 +1,6 @@
+// scalac: -Xlint:deprecation
+//
+
 trait Linked {
   type File <: FileImpl;
   trait FileImpl {