Merge pull request #8705 from eed3si9n/wip/deprecate-class-shadowing

Deprecate nested class shadowing in "override" position
scala · Feb 12, 2020 · be9e4d2 · be9e4d2
2 parents f573c4e + c25dfdc
commit be9e4d2
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: shadowing a nested class of a parent 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: shadowing a nested class of a parent 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 {