Disallow overriding val parameters

compiler/src/dotty/tools/dotc/transform/ParamForwarding.scala

@@ -30,7 +30,8 @@ import NameKinds.ParamAccessorName
  *  The aim of this transformation is to avoid redundant parameter accessor fields.
  */
 class ParamForwarding extends MiniPhase with IdentityDenotTransformer:
-  import ast.tpd._
+  import ast.tpd.*
+  import ParamForwarding.inheritedAccessor
 
   private def thisPhase: ParamForwarding = this
 
@@ -39,20 +40,6 @@ class ParamForwarding extends MiniPhase with IdentityDenotTransformer:
   override def description: String = ParamForwarding.description
 
   def transformIfParamAlias(mdef: ValOrDefDef)(using Context): Tree =
-
-    def inheritedAccessor(sym: Symbol)(using Context): Symbol =
-      val candidate = sym.owner.asClass.superClass
-        .info.decl(sym.name).suchThat(_.is(ParamAccessor, butNot = Mutable))
-        .symbol
-      if !candidate.is(Private)  // candidate might be private and accessible if it is in an outer class
-         && candidate.isAccessibleFrom(currentClass.thisType, superAccess = true)
-      then
-        candidate
-      else if candidate.is(SuperParamAlias) then
-        inheritedAccessor(candidate)
-      else
-        NoSymbol
-
     val sym = mdef.symbol.asTerm
     if sym.is(SuperParamAlias) then
       assert(sym.is(ParamAccessor, butNot = Mutable))
@@ -84,3 +71,17 @@ class ParamForwarding extends MiniPhase with IdentityDenotTransformer:
 object ParamForwarding:
   val name: String = "paramForwarding"
   val description: String = "add forwarders for aliases of superclass parameters"
+
+  def inheritedAccessor(sym: Symbol)(using Context): Symbol =
+    val candidate = sym.owner.asClass.superClass
+      .info.decl(sym.name).suchThat(_.is(ParamAccessor, butNot = Mutable))
+      .symbol
+    if !candidate.is(Private)  // candidate might be private and accessible if it is in an outer class
+        && candidate.isAccessibleFrom(currentClass.thisType, superAccess = true)
+    then
+      candidate
+    else if candidate.is(SuperParamAlias) then
+      inheritedAccessor(candidate)
+    else
+      NoSymbol
+end ParamForwarding

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

@@ -15,8 +15,8 @@ import config.Printers.{checks, noPrinter}
 import Decorators._
 import OverridingPairs.isOverridingPair
 import typer.ErrorReporting._
-import config.Feature.{warnOnMigration, migrateTo3}
-import config.SourceVersion.`3.0`
+import config.Feature.{warnOnMigration, migrateTo3, sourceVersion}
+import config.SourceVersion.{`3.0`, `future`}
 import config.Printers.refcheck
 import reporting._
 import Constants.Constant
@@ -264,6 +264,8 @@ object RefChecks {
    *    1.10.  If O is inline (and deferred, otherwise O would be final), M must be inline
    *    1.11.  If O is a Scala-2 macro, M must be a Scala-2 macro.
    *    1.12.  If O is non-experimental, M must be non-experimental.
+   *    1.13   Under -source future, if O is a val parameter, M must be a val parameter
+   *           that passes its on to O.
    *  2. Check that only abstract classes have deferred members
    *  3. Check that concrete classes do not have deferred definitions
    *     that are not implemented in a subclass.
@@ -514,12 +516,26 @@ object RefChecks {
           overrideError(i"needs to be declared with @targetName(${"\""}${other.targetName}${"\""}) so that external names match")
         else
           overrideError("cannot have a @targetName annotation since external names would be different")
+      else if other.is(ParamAccessor) && !isInheritedAccessor(member, other) then // (1.13)
+        if sourceVersion.isAtLeast(`future`) then
+          overrideError(i"cannot override val parameter ${other.showLocated}")
+        else
+          report.deprecationWarning(
+            i"overriding val parameter ${other.showLocated} is deprecated, will be illegal in a future version",
+            member.srcPos)
       else if !other.isExperimental && member.hasAnnotation(defn.ExperimentalAnnot) then // (1.12)
         overrideError("may not override non-experimental member")
       else if other.hasAnnotation(defn.DeprecatedOverridingAnnot) then
         overrideDeprecation("", member, other, "removed or renamed")
     end checkOverride
 
+    def isInheritedAccessor(mbr: Symbol, other: Symbol): Boolean =
+      mbr.is(ParamAccessor)
+      && {
+        val next = ParamForwarding.inheritedAccessor(mbr)
+        next == other || isInheritedAccessor(next, other)
+      }
+
     OverridingPairsChecker(clazz, self).checkAll(checkOverride)
     printMixinOverrideErrors()
 

tests/init/neg/override5.scala

@@ -25,9 +25,3 @@ trait Base {
 
   val message = "hello, " + name
 }
-
-class Derived(val name: String) extends Base
-
-class Derived2 extends Derived("hello") {
-  override val name: String = "ok"         // error
-}

tests/neg-custom-args/deprecation/i11344.scala

@@ -0,0 +1,6 @@
+trait Pet(val name: String, rest: Int):
+   def f(suffix: String) = s"$name$suffix$rest"
+
+class Birdie(override val name: String) extends Pet("huh", 1) // error
+
+

tests/neg-strict/i16092.scala

@@ -0,0 +1,24 @@
+trait X {
+  type T
+  def process(t: T): Unit
+}
+
+class Z(val x: X, val t: x.T) {
+  def process(): Unit = x.process(t)
+}
+class Evil(x1: X, x2: X, t: x1.T) extends Z(x1, t) {
+  override val x: X = x2 // error breaks connection between x and t
+}
+// alarm bells should be ringing by now
+
+// taking it to its conclusion...
+object x1 extends X {
+  override type T = Int
+  override def process(t: T): Unit = println("Int: " + t)
+}
+object x2 extends X {
+  override type T = String
+  override def process(t: T): Unit = println("String: " + t)
+}
+
+@main def Test = new Evil(x1, x2, 42).process() // BOOM: basically did x2.process(42)

tests/neg/i16092-members-only.scala

@@ -0,0 +1,31 @@
+trait X:
+  type T
+  def process(t: T): Unit
+
+abstract class Z:
+  def x1: X
+  val x: X = x1
+  def t: x.T
+  def process(): Unit = x.process(t)
+
+class Evil extends Z:
+  def x2: X
+  override val x: X = x2
+
+// alarm bells should be ringing by now
+
+// taking it to its conclusion...
+object X1 extends X:
+  override type T = Int
+  override def process(t: T): Unit = println("Int: " + t)
+
+object X2 extends X:
+  override type T = String
+  override def process(t: T): Unit = println("String: " + t)
+
+@main def Test =
+  new Evil{
+    val x1 = X1
+    val x2 = X2
+    val t = 42  // error
+  }.process() // BOOM: basically did x2.process(42)

tests/neg/i9460.scala

@@ -1,4 +1,4 @@
-trait A(val s: String) { println(s) }
-trait B extends A { override val s = "B" } // requires override val s
+trait A(s: String) { println(s) }
+trait B extends A { val s = "B" }
 class C extends B  // error
 @main def Test = C()

tests/pos-with-compiler/tasty/test-definitions.scala

@@ -163,9 +163,9 @@ object definitions {
       }
     }
 
-    abstract class LambdaType[ParamInfo, This <: LambdaType[ParamInfo, This]](
+    abstract class LambdaType[ParamInfo, This <: LambdaType[ParamInfo, This]]
+    extends Type {
       val companion: LambdaTypeCompanion[ParamInfo, This]
-    ) extends Type {
       private[Type] var _pinfos: List[ParamInfo]
       private[Type] var _restpe: Type
 
@@ -186,16 +186,21 @@ object definitions {
     }
 
     case class MethodType(paramNames: List[String], private[Type] var _pinfos: List[Type], private[Type] var _restpe: Type)
-    extends LambdaType[Type, MethodType](MethodType) {
+    extends LambdaType[Type, MethodType] {
+      override val companion = MethodType
       def isImplicit = (companion `eq` ImplicitMethodType) || (companion `eq` ErasedImplicitMethodType)
       def isErased = (companion `eq` ErasedMethodType) || (companion `eq` ErasedImplicitMethodType)
     }
 
     case class PolyType(paramNames: List[String], private[Type] var _pinfos: List[TypeBounds], private[Type] var _restpe: Type)
-    extends LambdaType[TypeBounds, PolyType](PolyType)
+    extends LambdaType[TypeBounds, PolyType] {
+      override val companion = PolyType
+    }
 
     case class TypeLambda(paramNames: List[String], private[Type] var _pinfos: List[TypeBounds], private[Type] var _restpe: Type)
-    extends LambdaType[TypeBounds, TypeLambda](TypeLambda)
+    extends LambdaType[TypeBounds, TypeLambda] {
+      override val companion = TypeLambda
+    }
 
     object TypeLambda extends LambdaTypeCompanion[TypeBounds, TypeLambda]
     object PolyType   extends LambdaTypeCompanion[TypeBounds, PolyType]

tests/pos/i16092.scala

@@ -0,0 +1,18 @@
+class A(val x: Int)
+class B(override val x: Int) extends A(x)
+
+class C(x: Int) extends A(x)
+case class D(override val x: Int) extends C(x)
+
+// The following is extracted from akka:
+trait LogEvent {
+  def cause: Throwable
+}
+
+/**
+ * For ERROR Logging
+ */
+case class Error(override val cause: Throwable) extends LogEvent
+class Error2(override val cause: Throwable) extends Error(cause)
+class Error3(override val cause: Throwable) extends Error2(cause)
+

tests/pos/i2051.scala

@@ -4,6 +4,3 @@ class B[T](override val x:T) extends A[T](x)
 class C[T](val x:T, val y: Int, val z: Boolean)
 class D[T](override val x:T, y: Int, z: Boolean) extends C[T](x, y, z)
 
-trait X(val x: Int, y: Int, z: Int)
-trait Y(override val x: Int, y: Int, z: Int) extends X
-class Z(override val x: Int, y: Int, z: Int) extends Y(x, y, z) with X(x, y, z)

tests/run/i16092.scala

@@ -0,0 +1,8 @@
+
+class A(a: Int)
+
+class B extends A(1):
+  val a = 2 // ok
+
+@main def Test =
+  assert(B().a == 2)

tests/run/paramForwarding.scala

@@ -16,8 +16,7 @@ class B(override val theValue: Int) extends A(42) {
 
 // Bz contains a field Bz.theValue$$local accessible using the getter
 // Bz.theValue() which overrides A.theValue()
-class Bz extends A(42) {
-  override val theValue: Int = 10
+class Bz(override val theValue: Int = 10) extends A(42) {
   val theValueInBz = theValue
 }