Skip to content
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

Add support for @deprecatedInheritance #19082

Merged
merged 1 commit into from
Dec 20, 2023
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 @@ -1006,6 +1006,7 @@ class Definitions {
@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 DeprecatedInheritanceAnnot: ClassSymbol = requiredClass("scala.deprecatedInheritance")
@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
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,10 @@ class InteractiveDriver(val settings: List[String]) extends Driver {

private val compiler: Compiler = new InteractiveCompiler

private val myOpenedFiles = new mutable.LinkedHashMap[URI, SourceFile] {
override def default(key: URI) = NoSource
}
private val myOpenedFiles = new mutable.LinkedHashMap[URI, SourceFile].withDefaultValue(NoSource)
def openedFiles: Map[URI, SourceFile] = myOpenedFiles

private val myOpenedTrees = new mutable.LinkedHashMap[URI, List[SourceTree]] {
override def default(key: URI) = Nil
}
private val myOpenedTrees = new mutable.LinkedHashMap[URI, List[SourceTree]].withDefaultValue(Nil)
def openedTrees: Map[URI, List[SourceTree]] = myOpenedTrees

private val myCompilationUnits = new mutable.LinkedHashMap[URI, CompilationUnit]
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/reporting/Message.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ object Message:
def openLambda(tp: LambdaType): Unit =
openedLambdas += tp

val seen = new collection.mutable.HashMap[SeenKey, List[Recorded]]:
override def default(key: SeenKey) = Nil
val seen = new collection.mutable.HashMap[SeenKey, List[Recorded]].withDefaultValue(Nil)

var nonSensical = false

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import dotty.tools.dotc.reporting.CodeAction

/** Handles rewriting of Scala2 files to Dotty */
object Rewrites {
private class PatchedFiles extends mutable.HashMap[SourceFile, Patches]
private type PatchedFiles = mutable.HashMap[SourceFile, Patches]

private case class Patch(span: Span, replacement: String) {
def delta = replacement.length - (span.end - span.start)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ class SemanticSymbolBuilder:
private val locals = mutable.HashMap[Symbol, Int]()

/** The local symbol(s) starting at given offset */
private val symsAtOffset = new mutable.HashMap[Int, Set[Symbol]]():
override def default(key: Int) = Set[Symbol]()
private val symsAtOffset = new mutable.HashMap[Int, Set[Symbol]]().withDefault(_ => Set[Symbol]())


def symbolName(sym: Symbol)(using Context): String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ class CountOuterAccesses extends MiniPhase:
// LambdaLift can create outer paths. These need to be known in this phase.

/** The number of times an outer accessor that might be dropped is accessed */
val outerAccessCount = new mutable.HashMap[Symbol, Int] {
override def default(s: Symbol): Int = 0
}
val outerAccessCount = new mutable.HashMap[Symbol, Int].withDefaultValue(0)

private def markAccessed(tree: RefTree)(using Context): Tree =
val sym = tree.symbol
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -504,9 +504,7 @@ object PatternMatcher {
}

private class RefCounter extends PlanTransform {
val count = new mutable.HashMap[Symbol, Int] {
override def default(key: Symbol) = 0
}
val count = new mutable.HashMap[Symbol, Int].withDefaultValue(0)
}

/** Reference counts for all labels */
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1150,9 +1150,7 @@ trait Checking {

/** Check that class does not declare same symbol twice */
def checkNoDoubleDeclaration(cls: Symbol)(using Context): Unit = {
val seen = new mutable.HashMap[Name, List[Symbol]] {
override def default(key: Name) = Nil
}
val seen = new mutable.HashMap[Name, List[Symbol]].withDefaultValue(Nil)
typr.println(i"check no double declarations $cls")

def checkDecl(decl: Symbol): Unit = {
Expand Down
58 changes: 38 additions & 20 deletions compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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) =
Expand All @@ -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)
Expand Down Expand Up @@ -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)
hamzaremmal marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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("")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val since = annot.argumentConstantString(1).map(version => s" (since: $version)").getOrElse("")
val since = annot.argumentConstantString(1).filter(!_.isEmpty).map(version => s" (since: $version)").getOrElse("")

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for the filter (See : tests/warn/i19002.check)

Copy link
Contributor

Choose a reason for hiding this comment

The 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 (since: ) text. Sorry I don't already know the answer, I'll check it out later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 = "").

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)
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/util/Stats.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ import collection.mutable

@volatile private var stack: List[String] = Nil

val hits: mutable.HashMap[String, Int] = new mutable.HashMap[String, Int] {
override def default(key: String): Int = 0
}
val hits: mutable.Map[String, Int] = new mutable.HashMap[String, Int].withDefaultValue(0)

inline def record(inline fn: String, inline n: Int = 1, inline skip: Boolean = timerOnly): Unit =
if (enabled && !skip) doRecord(fn, n)
Expand Down
4 changes: 1 addition & 3 deletions tests/init/pos/patternMatcher.scala
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
36 changes: 36 additions & 0 deletions tests/warn/i19002.check
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
23 changes: 23 additions & 0 deletions tests/warn/i19002.scala
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)
Loading