deprecate nested "overriding" classes with same name

[scabug]

Martin says:

In dotty I propose to no longer allow nested classes with the same name which shadow each other. It's less convenient that way, but a lot safer.

instead of:

trait Core extends Base {
  class Status
}

trait Ext extends Core {
  class Status extends super.Status
}

write:

trait Core extends Base {
  class Status
}

trait Ext extends Core {
  class StatusExt extends Status
}

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-8353?orig=1
Reporter: @adriaanm

[scabug]

@adriaanm said:
See also #7278

[scabug]

@soc said:
I'd prefer either fixing it and make it a full-blown overriding relationship, or just deprecate and remove nested classes/types altogether. Having completely different rules for classes/types and methods/values is just nuts.

[scabug]

@adriaanm said:
Neither of these extremes will happen. A language with virtual classes is research territory, and a language without nested classes/types would not be Scala.

[Blaisorblade]

This came up today on Gitter, where people wondered if Dotty might change things back.

In dotty I propose to no longer allow nested classes with the same name which shadow each other. It's less convenient that way, but a lot safer.

Does anybody know the rationale for that? On Gitter @nafg mentioned the (IMHO most useful) use case:

Now, if it could be made to work the way people often assume... (essentially overriding the type. It could be required to extend the class in the supertype)

which is the only one I've seen used (in the Scala collection views, for encoding virtual classes).

[adriaanm]

Martin mentioned last week that this restriction is no longer in dotty, so we can close this ticket.

[Blaisorblade]

Can I suggest reopening until Dotty actually lifts the restriction and removes http://dotty.epfl.ch/docs/reference/dropped/class-shadowing.html? On latest master (scala/scala3@8feb596):

sbt:dotty> repl -language:Scala2
[warn] Multiple main classes detected.  Run 'show discoveredMainClasses' to see the list
[warn] Multiple main classes detected.  Run 'show discoveredMainClasses' to see the list
[info] Running (fork) dotty.tools.repl.Main -classpath /Users/pgiarrusso/git/dotty/library/target/scala-2.12/dotty-library_2.12-0.8.0-bin-SNAPSHOT-nonbootstrapped.jar -language:Scala2
scala> class Base { class Ops }; class Sub extends Base {class Ops}
1 |class Base { class Ops }; class Sub extends Base {class Ops}
  |                                                        ^
  |class Ops cannot have the same name as class Ops in class Base -- class definitions cannot be overridden

EDIT: opening a dotty issue for this.

[lrytz]

Yes, let's see if it really happens :)

[Blaisorblade]

BTW, the current Dotty docs do give a rationale (which is the most well-known problem):

The issue is that the two Ops classes look like one overrides the other, but classes in Scala cannot be overridden.

I suspect we'd want to warn in Scala2 mode and restrict to overrides in Scala3 mode? But given the amount of code impacted, I'd like some non-completely-ad-hoc policy for making these language changes.

These seem enough questions to reopen this. EDIT1: Whoops had missed Lukas comment!

EDIT2: Dotty seems close to supporting shadowing, but supporting actual overriding is less trivial — if I disable checks stupidly, shadowing works but overriding gives cyclic reference errors.
scala/scala3#4361 (comment)

[Blaisorblade]

Just realized that shadowing still seems inherently unsound unless the shadowing class in fact inherits from the superclass, per example fail2 in #7278. Further discussion continues in scala/scala3#4361 (EDIT: fixed link, thanks @Jasper-M).

[Blaisorblade]

FYI: I've re-closed the Dotty issue for now, since Martin went back (more than a month ago) to the original plan of disallowing shadowing.

[SethTisue]

volunteer to do the deprecation?

[eed3si9n]

Did anyone say deprecation? - scala/scala#8705

[soronpo]

This PR causes a false class shadowing warning.
#11895