Merge pull request #15432 from dotty-staging/add-deprecated-overriding

Add deprecated overriding checks

Author: odersky

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

@@ -951,6 +951,7 @@ class Definitions {
   @tu lazy val ContextResultCountAnnot: ClassSymbol = requiredClass("scala.annotation.internal.ContextResultCount")
   @tu lazy val ProvisionalSuperClassAnnot: ClassSymbol = requiredClass("scala.annotation.internal.ProvisionalSuperClass")
   @tu lazy val DeprecatedAnnot: ClassSymbol = requiredClass("scala.deprecated")
+  @tu lazy val DeprecatedOverridingAnnot: ClassSymbol = requiredClass("scala.deprecatedOverriding")
   @tu lazy val ImplicitAmbiguousAnnot: ClassSymbol = requiredClass("scala.annotation.implicitAmbiguous")
   @tu lazy val ImplicitNotFoundAnnot: ClassSymbol = requiredClass("scala.annotation.implicitNotFound")
   @tu lazy val InlineParamAnnot: ClassSymbol = requiredClass("scala.annotation.internal.InlineParam")

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

@@ -316,7 +316,7 @@ object RefChecks {
     /* Check that all conditions for overriding `other` by `member`
        * of class `clazz` are met.
        */
-    def checkOverride(member: Symbol, other: Symbol): Unit = {
+    def checkOverride(member: Symbol, other: Symbol): Unit =
       def memberTp(self: Type) =
         if (member.isClass) TypeAlias(member.typeRef.EtaExpand(member.typeParams))
         else self.memberInfo(member)
@@ -373,6 +373,11 @@ object RefChecks {
         if trueMatch && noErrorType then
           emitOverrideError(overrideErrorMsg(msg, compareTypes))
 
+      def overrideDeprecation(what: String, member: Symbol, other: Symbol, fix: String): Unit =
+        report.deprecationWarning(
+          s"overriding $what${infoStringWithLocation(other)} is deprecated;\n  ${infoString(member)} should be $fix.",
+          if member.owner == clazz then member.srcPos else clazz.srcPos)
+
       def autoOverride(sym: Symbol) =
         sym.is(Synthetic) && (
           desugar.isDesugaredCaseClassMethodName(member.name) || // such names are added automatically, can't have an override preset.
@@ -423,6 +428,7 @@ object RefChecks {
         def otherIsJavaProtected = other.isAllOf(JavaProtected)               // or o is Java defined and protected (see #3946)
         memberIsPublic || protectedOK && (accessBoundaryOK || otherIsJavaProtected)
       end isOverrideAccessOK
+
       if !member.hasTargetName(other.targetName) then
         overrideTargetNameError()
       else if !isOverrideAccessOK then
@@ -509,22 +515,10 @@ object RefChecks {
           overrideError("cannot have a @targetName annotation since external names would be different")
       else if !other.isExperimental && member.hasAnnotation(defn.ExperimentalAnnot) then // (1.12)
         overrideError("may not override non-experimental member")
-      else
-        checkOverrideDeprecated()
-    }
+      else if other.hasAnnotation(defn.DeprecatedOverridingAnnot) then
+        overrideDeprecation("", member, other, "removed or renamed")
     end checkOverride
 
-    /* TODO enable; right now the annotation is scala-private, so cannot be seen
-         * here.
-         */
-    def checkOverrideDeprecated() = { /*
-          if (other.hasDeprecatedOverridingAnnotation) {
-            val suffix = other.deprecatedOverridingMessage map (": " + _) getOrElse ""
-            val msg = s"overriding ${other.fullLocationString} is deprecated$suffix"
-            unit.deprecationWarning(member.pos, msg)
-          }*/
-    }
-
     OverridingPairsChecker(clazz, self).checkAll(checkOverride)
     printMixinOverrideErrors()
 

tests/neg-strict/deprecated-override.scala

@@ -0,0 +1,14 @@
+trait A:
+  def f: Int
+
+class B extends A:
+  @deprecatedOverriding def f = 1
+
+class C extends B:
+  override def f = 2  // error
+
+trait D extends A:
+  override def f = 3
+
+object E extends B, D  // error
+