Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate nested class shadowing in "override" position #8705

Merged
merged 1 commit into from Feb 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
lrytz marked this conversation as resolved.
Show resolved Hide resolved
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 --------------------------------------------------------

Expand Down
4 changes: 4 additions & 0 deletions 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
10 changes: 10 additions & 0 deletions 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
}
6 changes: 6 additions & 0 deletions 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
10 changes: 10 additions & 0 deletions 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
}
6 changes: 4 additions & 2 deletions 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>
3 changes: 2 additions & 1 deletion 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>
3 changes: 2 additions & 1 deletion 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>
6 changes: 0 additions & 6 deletions test/files/run/bugs.check
Expand Up @@ -84,12 +84,6 @@ hello
<<< bug 328
>>> bug 328

<<< bug 396
A
B
C
>>> bug 396

<<< bug 399
a
>>> bug 399
Expand Down
27 changes: 3 additions & 24 deletions test/files/run/bugs.scala
@@ -1,3 +1,6 @@
// scalac: -Werror -Xlint:deprecation
//

//############################################################################
// Bugs
//############################################################################
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion 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 => }
^
Expand Down
5 changes: 4 additions & 1 deletion 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 {
Expand Down
50 changes: 26 additions & 24 deletions 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
}
3 changes: 3 additions & 0 deletions 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
^
3 changes: 3 additions & 0 deletions test/files/run/t6677b.scala
@@ -1,3 +1,6 @@
// scalac: -Xlint:deprecation
//

trait U {
trait U1 {
class X
Expand Down
3 changes: 3 additions & 0 deletions 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
3 changes: 3 additions & 0 deletions test/files/run/t744.scala
@@ -1,3 +1,6 @@
// scalac: -Xlint:deprecation
//

trait Linked {
type File <: FileImpl;
trait FileImpl {
Expand Down