-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add support for @deprecatedInheritance #19082
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -32,10 +32,16 @@ class CrossVersionChecks extends MiniPhase: | |||||
checkMigration(sym, pos, xMigrationValue) | ||||||
end checkUndesiredProperties | ||||||
|
||||||
/** If @deprecated is present, and the point of reference is not enclosed | ||||||
* in either a deprecated member or a scala bridge method, issue a warning. | ||||||
*/ | ||||||
private def checkDeprecated(sym: Symbol, pos: SrcPos)(using Context): Unit = | ||||||
/**Skip warnings for synthetic members of case classes during declaration and | ||||||
* scan the chain of outer declaring scopes from the current context | ||||||
* a deprecation warning will be skipped if one the following holds | ||||||
* for a given declaring scope: | ||||||
* - the symbol associated with the scope is also deprecated. | ||||||
* - if and only if `sym` is an enum case, the scope is either | ||||||
* a module that declares `sym`, or the companion class of the | ||||||
* module that declares `sym`. | ||||||
*/ | ||||||
def skipWarning(sym: Symbol)(using Context): Boolean = | ||||||
|
||||||
/** is the owner an enum or its companion and also the owner of sym */ | ||||||
def isEnumOwner(owner: Symbol)(using Context) = | ||||||
|
@@ -46,26 +52,22 @@ class CrossVersionChecks extends MiniPhase: | |||||
|
||||||
def isDeprecatedOrEnum(owner: Symbol)(using Context) = | ||||||
// pre: sym is an enumcase | ||||||
owner.isDeprecated | ||||||
|| isEnumOwner(owner) | ||||||
|
||||||
/**Skip warnings for synthetic members of case classes during declaration and | ||||||
* scan the chain of outer declaring scopes from the current context | ||||||
* a deprecation warning will be skipped if one the following holds | ||||||
* for a given declaring scope: | ||||||
* - the symbol associated with the scope is also deprecated. | ||||||
* - if and only if `sym` is an enum case, the scope is either | ||||||
* a module that declares `sym`, or the companion class of the | ||||||
* module that declares `sym`. | ||||||
*/ | ||||||
def skipWarning(using Context): Boolean = | ||||||
(ctx.owner.is(Synthetic) && sym.is(CaseClass)) | ||||||
|| ctx.owner.ownersIterator.exists(if sym.isEnumCase then isDeprecatedOrEnum else _.isDeprecated) | ||||||
owner.isDeprecated || isEnumOwner(owner) | ||||||
|
||||||
(ctx.owner.is(Synthetic) && sym.is(CaseClass)) | ||||||
|| ctx.owner.ownersIterator.exists(if sym.isEnumCase then isDeprecatedOrEnum else _.isDeprecated) | ||||||
end skipWarning | ||||||
|
||||||
|
||||||
/** If @deprecated is present, and the point of reference is not enclosed | ||||||
* in either a deprecated member or a scala bridge method, issue a warning. | ||||||
*/ | ||||||
private def checkDeprecated(sym: Symbol, pos: SrcPos)(using Context): Unit = | ||||||
|
||||||
// Also check for deprecation of the companion class for synthetic methods | ||||||
val toCheck = sym :: (if sym.isAllOf(SyntheticMethod) then sym.owner.companionClass :: Nil else Nil) | ||||||
for sym <- toCheck; annot <- sym.getAnnotation(defn.DeprecatedAnnot) do | ||||||
if !skipWarning then | ||||||
if !skipWarning(sym) then | ||||||
val msg = annot.argumentConstant(0).map(": " + _.stringValue).getOrElse("") | ||||||
val since = annot.argumentConstant(1).map(" since " + _.stringValue).getOrElse("") | ||||||
report.deprecationWarning(em"${sym.showLocated} is deprecated${since}${msg}", pos) | ||||||
|
@@ -107,6 +109,18 @@ class CrossVersionChecks extends MiniPhase: | |||||
} | ||||||
} | ||||||
|
||||||
/** ??? */ | ||||||
def checkDeprecatedInheritance(parents: List[Tree])(using Context): Unit = { | ||||||
for parent <- parents | ||||||
psym = parent.tpe.classSymbol | ||||||
annot <- psym.getAnnotation(defn.DeprecatedInheritanceAnnot) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's cheaper to check the annotation than to walk owners checking whether to skipWarning, also the annotation is rare, so it's more natural to reverse the lines. It's a matter of style (?) whether to have trailing if in the for comprehension or move it to the body. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've actually put the if after but I agree, I will switch them |
||||||
if !skipWarning(psym) | ||||||
do | ||||||
val msg = annot.argumentConstantString(0).map(msg => s": $msg").getOrElse("") | ||||||
val since = annot.argumentConstantString(1).map(version => s" (since: $version)").getOrElse("") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, I don't know what dotty is doing with the annotation. It has a default arg that is empty string. I won't press the matter here, but I wonder what happens to the string. In scala 2, we must handle the empty string to avoid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We don't transform annotations in Dotty, therefore, it does not have the knowledge of the default parameter in case we are using named arguments. The only way to have the message |
||||||
report.deprecationWarning(em"inheritance from $psym is deprecated$since$msg", parent.srcPos) | ||||||
} | ||||||
|
||||||
override def transformValDef(tree: ValDef)(using Context): ValDef = | ||||||
checkDeprecatedOvers(tree) | ||||||
checkExperimentalAnnots(tree.symbol) | ||||||
|
@@ -122,6 +136,10 @@ class CrossVersionChecks extends MiniPhase: | |||||
checkExperimentalAnnots(tree.symbol) | ||||||
tree | ||||||
|
||||||
override def transformTemplate(tree: tpd.Template)(using Context): tpd.Tree = | ||||||
checkDeprecatedInheritance(tree.parents) | ||||||
tree | ||||||
|
||||||
override def transformIdent(tree: Ident)(using Context): Ident = { | ||||||
checkUndesiredProperties(tree.symbol, tree.srcPos) | ||||||
tree | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
import scala.collection.mutable | ||
|
||
class Translater: | ||
val count = new mutable.HashMap[Int, Int] { | ||
override def default(key: Int) = 0 | ||
} | ||
val count = new mutable.HashMap[Int, Int].withDefaultValue(0) | ||
count.get(10) | ||
val n = 10 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
-- Deprecation Warning: tests/warn/i19002.scala:5:20 ------------------------------------------------------------------- | ||
5 |class TBar1 extends TFoo // warn | ||
| ^^^^ | ||
| inheritance from trait TFoo is deprecated (since: FooLib 12.0): this class will be made final | ||
-- Deprecation Warning: tests/warn/i19002.scala:6:20 ------------------------------------------------------------------- | ||
6 |trait TBar2 extends TFoo // warn | ||
| ^^^^ | ||
| inheritance from trait TFoo is deprecated (since: FooLib 12.0): this class will be made final | ||
-- Deprecation Warning: tests/warn/i19002.scala:11:20 ------------------------------------------------------------------ | ||
11 |class CBar1 extends CFoo // warn | ||
| ^^^^ | ||
| inheritance from class CFoo is deprecated (since: FooLib 11.0) | ||
-- Deprecation Warning: tests/warn/i19002.scala:12:20 ------------------------------------------------------------------ | ||
12 |trait CBar2 extends CFoo // warn | ||
| ^^^^ | ||
| inheritance from class CFoo is deprecated (since: FooLib 11.0) | ||
-- Deprecation Warning: tests/warn/i19002.scala:17:20 ------------------------------------------------------------------ | ||
17 |class ABar1 extends AFoo // warn | ||
| ^^^^ | ||
| inheritance from class AFoo is deprecated: this class will be made final | ||
-- Deprecation Warning: tests/warn/i19002.scala:18:20 ------------------------------------------------------------------ | ||
18 |trait ABar2 extends AFoo // warn | ||
| ^^^^ | ||
| inheritance from class AFoo is deprecated: this class will be made final | ||
-- Deprecation Warning: tests/warn/i19002.scala:7:16 ------------------------------------------------------------------- | ||
7 |def TBar3 = new TFoo {} // warn | ||
| ^^^^ | ||
| inheritance from trait TFoo is deprecated (since: FooLib 12.0): this class will be made final | ||
-- Deprecation Warning: tests/warn/i19002.scala:13:16 ------------------------------------------------------------------ | ||
13 |def CBar3 = new CFoo {} // warn | ||
| ^^^^ | ||
| inheritance from class CFoo is deprecated (since: FooLib 11.0) | ||
-- Deprecation Warning: tests/warn/i19002.scala:19:16 ------------------------------------------------------------------ | ||
19 |def ABar3 = new AFoo {} // warn | ||
| ^^^^ | ||
| inheritance from class AFoo is deprecated: this class will be made final |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
//> using options -deprecation | ||
|
||
@deprecatedInheritance("this class will be made final", "FooLib 12.0") | ||
trait TFoo | ||
class TBar1 extends TFoo // warn | ||
trait TBar2 extends TFoo // warn | ||
def TBar3 = new TFoo {} // warn | ||
|
||
@deprecatedInheritance(since = "FooLib 11.0") | ||
class CFoo | ||
class CBar1 extends CFoo // warn | ||
trait CBar2 extends CFoo // warn | ||
def CBar3 = new CFoo {} // warn | ||
|
||
@deprecatedInheritance(message = "this class will be made final") | ||
abstract class AFoo | ||
class ABar1 extends AFoo // warn | ||
trait ABar2 extends AFoo // warn | ||
def ABar3 = new AFoo {} // warn | ||
|
||
@deprecated | ||
class DeprecatedFoo: | ||
class Foo extends AFoo // it shoudln't warn here (in deprecated context) |
Uh oh!
There was an error while loading. Please reload this page.