Disallow private methods "overriding" non-private ones.

Java requires this. Scala 2 is even stricter: it disallows all provite
members to override, no matter whether they are methods or not.

Author: odersky

compiler/src/dotty/tools/dotc/core/Flags.scala

@@ -552,6 +552,7 @@ object Flags {
   val JavaProtected: FlagSet                 = JavaDefined | Protected
   val MethodOrLazy: FlagSet                  = Lazy | Method
   val MutableOrLazy: FlagSet                 = Lazy | Mutable
+  val MethodOrLazyOrMutable: FlagSet         = Lazy | Method | Mutable
   val LiftedMethod: FlagSet                  = Lifted | Method
   val LocalParam: FlagSet                    = Local | Param
   val LocalParamAccessor: FlagSet            = Local | ParamAccessor | Private

compiler/src/dotty/tools/dotc/typer/RefChecks.scala

@@ -866,6 +866,30 @@ object RefChecks {
     }
   }
 
+  /** Check that we do not "override" anything with a private method
+   *  or something that becomes a private method. According to the Scala
+   *  modeling this is non-sensical since private members don't override.
+   *  But Java and the JVM disagree, if the private member is a method.
+   *  A test case is neg/i7926b.scala.
+   *  Note: The compiler could possibly silently rename the offending private
+   *  instead of flagging it as an error. But that might mean we see some
+   *  surprising names at runtime. E.g. in neg/i4564a.scala, a private
+   *  case class `apply` method would have to be renamed to something else.
+   */
+  def checkNoPrivateOverrides(tree: Tree)(using ctx: Context): Unit =
+    val sym = tree.symbol
+    if sym.owner.isClass
+       && sym.is(Private)
+       && (sym.isOneOf(MethodOrLazyOrMutable) || !sym.is(Local)) // in these cases we'll produce a getter later
+       && !sym.isConstructor
+    then
+      val cls = sym.owner.asClass
+      for bc <- cls.baseClasses.tail do
+        val other = sym.matchingDecl(bc, cls.thisType)
+        if other.exists then
+          ctx.error(i"private $sym cannot override ${other.showLocated}", sym.sourcePos)
+  end checkNoPrivateOverrides
+
   type LevelAndIndex = immutable.Map[Symbol, (LevelInfo, Int)]
 
   class OptLevelInfo {
@@ -957,6 +981,7 @@ class RefChecks extends MiniPhase { thisPhase =>
     else ctx
 
   override def transformValDef(tree: ValDef)(implicit ctx: Context): ValDef = {
+    checkNoPrivateOverrides(tree)
     checkDeprecatedOvers(tree)
     val sym = tree.symbol
     if (sym.exists && sym.owner.isTerm) {
@@ -976,6 +1001,7 @@ class RefChecks extends MiniPhase { thisPhase =>
   }
 
   override def transformDefDef(tree: DefDef)(implicit ctx: Context): DefDef = {
+    checkNoPrivateOverrides(tree)
     checkDeprecatedOvers(tree)
     tree
   }

tests/neg/i4564a.scala

@@ -0,0 +1,6 @@
+
+class BaseCP[T] {
+  def apply(x: T): ClashPoly = if (???) ClashPoly(1) else ???
+}
+object ClashPoly extends BaseCP[Int]
+case class ClashPoly private(x: Int) // private method apply cannot override method apply in class BaseCP

tests/neg/i7926b.scala

@@ -0,0 +1,26 @@
+trait A {
+  def p: Int
+  def getP = p
+}
+class B extends A {
+  def p: Int = 22
+}
+class C extends B {
+  private def p: Int = super.p  // error
+}
+class D extends B {
+  private val p: Int = super.p  // OK
+}
+class E extends B {
+  private var p: Int = 0  // error
+}
+class F extends B {
+  private lazy val p: Int = 0  // error
+}
+object Test {
+  def main(args: Array[String]): Unit =
+    println(new C().getP) // would give illegal access error at runtime if C compiled
+    println(new D().getP) // OK
+    println(new E().getP) // would give illegal access error at runtime if E compiled
+    println(new F().getP) // would give illegal access error at runtime if F compiled
+}

tests/neg/i8005.scala

@@ -8,5 +8,5 @@ abstract class Buffer {
 class ByteBuffer extends Buffer {  // error: ByteBufer needs to be abstract since `bar` is not defined
   private[nio] val isReadOnly: Boolean = false // error
   protected def foo(): Unit = ()  // error
-  private def bar(): Unit = ()
+  private def bar(): Unit = () // error
 }

tests/pos/i4564.scala

@@ -46,17 +46,9 @@ object NoClashOverload {
 case class NoClashOverload private (x: Int)
 
 class BaseNCP[T] {
-  // error: overloaded method apply needs result type
   def apply(x: T): NoClashPoly = if (???) NoClashPoly(1) else ???
 }
 
 object NoClashPoly extends BaseNCP[Boolean]
 case class NoClashPoly (x: Int)
 
-
-class BaseCP[T] {
-  // error: overloaded method apply needs result type
-  def apply(x: T): ClashPoly = if (???) ClashPoly(1) else ???
-}
-object ClashPoly extends BaseCP[Int]
-case class ClashPoly private(x: Int)

tests/pos/typers.scala

@@ -99,7 +99,7 @@ object typers {
     }
 
     class B extends A {
-      private def x: Int = 1
+      private val x: Int = 1
     }
 
     val b: B = new B