Add support for @deprecatedInheritance #19082
Conversation
|
|
hamzaremmal
force-pushed
the
i19002
branch
2 times, most recently
from
ad66ab5
to
698d90f
Compare
hamzaremmal
force-pushed
the
i19002
branch
2 times, most recently
from
803108b
to
60bca78
Compare
|
|
||
| (ctx.owner.is(Synthetic) && sym.is(CaseClass)) | ||
| || ctx.owner.ownersIterator.exists(if sym.isEnumCase then isDeprecatedOrEnum else _.isDeprecated) | ||
| } |
There was a problem hiding this comment.
I see you are adding braces just for methods. You might prefer end SkipWarning.
| if !skipWarning(psym) then | ||
| val msg = annot.argumentConstantString(0).map(msg => s": $msg").getOrElse("") | ||
| val since = annot.argumentConstantString(1).map(version => s" (since: $version)").getOrElse("") | ||
| report.deprecationWarning(em"inheritance from $psym is deprecated$since$msg", pos) |
There was a problem hiding this comment.
IIRC filtering on empty annot arg avoids deprecated (since: ) which is minor but looks funny. I think because they starting warning (years ago) if you omitted the since arg, you'd see deprecated("no way", "").
There was a problem hiding this comment.
This is already the case, see tests/warn/i19002.check
| for parent <- parents | ||
| psym = parent.tpe.classSymbol | ||
| if !skipWarning(psym) | ||
| annot <- psym.getAnnotation(defn.DeprecatedInheritanceAnnot) |
There was a problem hiding this comment.
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.
I've actually put the if after but I agree, I will switch them
| annot <- psym.getAnnotation(defn.DeprecatedInheritanceAnnot) | ||
| 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.
| val since = annot.argumentConstantString(1).map(version => s" (since: $version)").getOrElse("") | |
| val since = annot.argumentConstantString(1).filter(!_.isEmpty).map(version => s" (since: $version)").getOrElse("") |
There was a problem hiding this comment.
There is no need for the filter (See : tests/warn/i19002.check)
There was a problem hiding this comment.
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 (since: ) text. Sorry I don't already know the answer, I'll check it out later.
There was a problem hiding this comment.
It has a default arg that is empty string. I won't press the matter here, but I wonder what happens to the string.
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 (since: ) is to explicitly write @deprecatedInheritance(since = "").
Backports #19082 to the LTS branch. PR submitted by the release tooling. [skip ci]
Closes #19002