Skip to content

Add deprecated overriding checks #15432

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

Merged
merged 2 commits into from
Jun 15, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/Definitions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
24 changes: 9 additions & 15 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down
14 changes: 14 additions & 0 deletions tests/neg-strict/deprecated-override.scala
Original file line number Diff line number Diff line change
@@ -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