Skip to content

Commit 04063ef

Browse files
jan-pieterjan-pieter
committed
Fix deprecation check and add tests
1 parent 7cf9843 commit 04063ef

File tree

7 files changed

+88
-68
lines changed

7 files changed

+88
-68
lines changed

‎compiler/src/dotty/tools/dotc/core/Flags.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,7 @@ object Flags {
604604
val Scala2Trait: FlagSet = Scala2x | Trait
605605
val SyntheticArtifact: FlagSet = Synthetic | Artifact
606606
val SyntheticCase: FlagSet = Synthetic | Case
607+
val SyntheticMethod: FlagSet = Synthetic | Method
607608
val SyntheticModule: FlagSet = Synthetic | Module
608609
val SyntheticOpaque: FlagSet = Synthetic | Opaque
609610
val SyntheticParam: FlagSet = Synthetic | Param

‎compiler/src/dotty/tools/dotc/typer/CrossVersionChecks.scala

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ class CrossVersionChecks extends MiniPhase:
2626
// I assume that's a consequence of some code trying to avoid noise by suppressing
2727
// warnings after the first, but I think it'd be better if we didn't have to
2828
// arbitrarily choose one as more important than the other.
29-
private def checkUndesiredProperties(sym: Symbol, pos: SrcPos, site: String)(using Context): Unit =
30-
checkDeprecated(sym, pos, site)
29+
private def checkUndesiredProperties(sym: Symbol, pos: SrcPos)(using Context): Unit =
30+
checkDeprecated(sym, pos)
3131
checkExperimentalRef(sym, pos)
3232

3333
val xMigrationValue = ctx.settings.Xmigration.value
@@ -38,7 +38,7 @@ class CrossVersionChecks extends MiniPhase:
3838
/** If @deprecated is present, and the point of reference is not enclosed
3939
* in either a deprecated member or a scala bridge method, issue a warning.
4040
*/
41-
private def checkDeprecated(sym: Symbol, pos: SrcPos, site: String)(using Context): Unit =
41+
private def checkDeprecated(sym: Symbol, pos: SrcPos)(using Context): Unit =
4242

4343
/** is the owner an enum or its companion and also the owner of sym */
4444
def isEnumOwner(owner: Symbol)(using Context) =
@@ -52,26 +52,22 @@ class CrossVersionChecks extends MiniPhase:
5252
owner.isDeprecated
5353
|| isEnumOwner(owner)
5454

55-
/**Scan the chain of outer declaring scopes from the current context
55+
/**Skip warnings for synthetic members of case classes during declaration and
56+
* scan the chain of outer declaring scopes from the current context
5657
* a deprecation warning will be skipped if one the following holds
5758
* for a given declaring scope:
5859
* - the symbol associated with the scope is also deprecated.
5960
* - if and only if `sym` is an enum case, the scope is either
6061
* a module that declares `sym`, or the companion class of the
6162
* module that declares `sym`.
6263
*/
63-
def skipWarning(using Context): Boolean = {
64-
val owners = ctx.owner.ownersIterator.toList
65-
val result = //(sym.isConstructor && sym.owner.is(Flags.CaseClass) && ctx.owner.is(Flags.Synthetic)) &&
66-
(ctx.owner.is(Flags.Synthetic) && sym.is(Flags.CaseClass)) ||
67-
owners.exists(owner => if sym.isEnumCase then isDeprecatedOrEnum(owner) else owner.isDeprecated)
68-
println(i"sym: $sym flags: ${sym.flagsString} owner: ${sym.owner} constructor: ${sym.isConstructor}")
69-
println(i"owner: ${ctx.owner} deprecated: ${ctx.owner.isDeprecated} constructor: ${ctx.owner.isConstructor} flags: ${ctx.owner.flagsString}")
70-
println(i"site: $site")
71-
result
72-
}
64+
def skipWarning(using Context): Boolean =
65+
(ctx.owner.is(Synthetic) && sym.is(CaseClass)) ||
66+
ctx.owner.ownersIterator.exists(if sym.isEnumCase then isDeprecatedOrEnum else _.isDeprecated)
7367

74-
for annot <- sym.getAnnotation(defn.DeprecatedAnnot) do
68+
// Also check for deprecation of the companion class for synthetic methods
69+
val toCheck: List[Symbol] = sym :: Option.when(sym.isAllOf(SyntheticMethod))(sym.owner.companionClass).toList
70+
for annot <- toCheck.flatMap(_.getAnnotation(defn.DeprecatedAnnot)) do
7571
if !skipWarning then
7672
val msg = annot.argumentConstant(0).map(": " + _.stringValue).getOrElse("")
7773
val since = annot.argumentConstant(1).map(" since " + _.stringValue).getOrElse("")
@@ -137,28 +133,28 @@ class CrossVersionChecks extends MiniPhase:
137133
tree
138134

139135
override def transformIdent(tree: Ident)(using Context): Ident = {
140-
checkUndesiredProperties(tree.symbol, tree.srcPos, "ident")
136+
checkUndesiredProperties(tree.symbol, tree.srcPos)
141137
tree
142138
}
143139

144140
override def transformSelect(tree: Select)(using Context): Select = {
145-
checkUndesiredProperties(tree.symbol, tree.srcPos, "select")
141+
checkUndesiredProperties(tree.symbol, tree.srcPos)
146142
tree
147143
}
148144

149145
override def transformNew(tree: New)(using Context): New = {
150-
checkUndesiredProperties(tree.tpe.typeSymbol, tree.srcPos, "new")
146+
checkUndesiredProperties(tree.tpe.typeSymbol, tree.srcPos)
151147
tree
152148
}
153149

154150
override def transformTypeTree(tree: TypeTree)(using Context): TypeTree = {
155151
val tpe = tree.tpe
156152
tpe.foreachPart {
157153
case TypeRef(_, sym: Symbol) =>
158-
checkDeprecated(sym, tree.srcPos, "typeTree.typeRef")
154+
checkDeprecated(sym, tree.srcPos)
159155
checkExperimentalRef(sym, tree.srcPos)
160156
case TermRef(_, sym: Symbol) =>
161-
checkDeprecated(sym, tree.srcPos, "typeTree.termRef")
157+
checkDeprecated(sym, tree.srcPos)
162158
checkExperimentalRef(sym, tree.srcPos)
163159
case _ =>
164160
}
@@ -173,7 +169,7 @@ class CrossVersionChecks extends MiniPhase:
173169
override def transformOther(tree: Tree)(using Context): Tree = tree match
174170
case tree: Import =>
175171
tree.foreachSubTree {
176-
case t: RefTree => checkUndesiredProperties(t.symbol, t.srcPos, "other")
172+
case t: RefTree => checkUndesiredProperties(t.symbol, t.srcPos)
177173
case _ =>
178174
}
179175
tree

‎compiler/test/dotty/tools/dotc/CompilationTests.scala

Lines changed: 36 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -30,43 +30,42 @@ class CompilationTests {
3030
@Test def pos: Unit = {
3131
implicit val testGroup: TestGroup = TestGroup("compilePos")
3232
aggregateTests(
33-
// compileFile("tests/pos/nullarify.scala", defaultOptions.and("-Ycheck:nullarify")),
34-
// compileFile("tests/pos-special/utf8encoded.scala", explicitUTF8),
35-
// compileFile("tests/pos-special/utf16encoded.scala", explicitUTF16),
36-
// compileFilesInDir("tests/pos-special/sourcepath/outer", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
37-
// compileFile("tests/pos-special/sourcepath/outer/nested/Test4.scala", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
38-
// compileFilesInDir("tests/pos-special/fatal-warnings", defaultOptions.and("-Xfatal-warnings", "-deprecation", "-feature")),
39-
// compileFilesInDir("tests/pos-special/spec-t5545", defaultOptions),
40-
// compileFilesInDir("tests/pos-special/strawman-collections", allowDeepSubtypes),
41-
// compileFilesInDir("tests/pos-special/isInstanceOf", allowDeepSubtypes.and("-Xfatal-warnings")),
42-
// compileFilesInDir("tests/new", defaultOptions.and("-source", "3.2")), // just to see whether 3.2 works
43-
// compileFilesInDir("tests/pos-scala2", scala2CompatMode),
44-
// compileFilesInDir("tests/pos-custom-args/captures", defaultOptions.and("-language:experimental.captureChecking")),
45-
// compileFilesInDir("tests/pos-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")),
46-
// compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init")),
47-
// // Run tests for legacy lazy vals
48-
// compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init", "-Ylegacy-lazy-vals", "-Ycheck-constraint-deps"), FileFilter.include(TestSources.posLazyValsAllowlist)),
49-
// compileFilesInDir("tests/pos-deep-subtype", allowDeepSubtypes),
50-
// compileFilesInDir("tests/pos-custom-args/no-experimental", defaultOptions.and("-Yno-experimental")),
51-
// compileFilesInDir("tests/pos-custom-args/strict", defaultOptions.and("-source", "future", "-deprecation", "-Xfatal-warnings")),
52-
// compileDir("tests/pos-special/java-param-names", defaultOptions.withJavacOnlyOptions("-parameters")),
53-
// compileFile(
54-
// // succeeds despite -Xfatal-warnings because of -nowarn
55-
// "tests/neg-custom-args/fatal-warnings/xfatalWarnings.scala",
56-
// defaultOptions.and("-nowarn", "-Xfatal-warnings")
57-
// ),
58-
// compileFile("tests/pos-special/typeclass-scaling.scala", defaultOptions.and("-Xmax-inlines", "40")),
59-
// compileFile("tests/pos-special/i7575.scala", defaultOptions.andLanguageFeature("dynamics")),
60-
// compileFile("tests/pos-special/kind-projector.scala", defaultOptions.and("-Ykind-projector")),
61-
// compileFile("tests/pos-special/kind-projector-underscores.scala", defaultOptions.and("-Ykind-projector:underscores")),
62-
// compileFile("tests/run/i5606.scala", defaultOptions.and("-Yretain-trees")),
63-
// compileFile("tests/pos-custom-args/i8875.scala", defaultOptions.and("-Xprint:getters")),
64-
// compileFile("tests/pos-custom-args/i9267.scala", defaultOptions.and("-Ystop-after:erasure")),
65-
// compileFile("tests/pos-special/extend-java-enum.scala", defaultOptions.and("-source", "3.0-migration")),
66-
// compileFile("tests/pos-custom-args/help.scala", defaultOptions.and("-help", "-V", "-W", "-X", "-Y")),
67-
// compileFile("tests/pos-custom-args/i13044.scala", defaultOptions.and("-Xmax-inlines:33")),
68-
// compileFile("tests/pos-custom-args/jdk-8-app.scala", defaultOptions.and("-release:8")),
69-
compileFile("tests/pos-custom-args/i11022.scala", defaultOptions.and("-Xfatal-warnings", "-deprecation")),
33+
compileFile("tests/pos/nullarify.scala", defaultOptions.and("-Ycheck:nullarify")),
34+
compileFile("tests/pos-special/utf8encoded.scala", explicitUTF8),
35+
compileFile("tests/pos-special/utf16encoded.scala", explicitUTF16),
36+
compileFilesInDir("tests/pos-special/sourcepath/outer", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
37+
compileFile("tests/pos-special/sourcepath/outer/nested/Test4.scala", defaultOptions.and("-sourcepath", "tests/pos-special/sourcepath")),
38+
compileFilesInDir("tests/pos-special/fatal-warnings", defaultOptions.and("-Xfatal-warnings", "-deprecation", "-feature")),
39+
compileFilesInDir("tests/pos-special/spec-t5545", defaultOptions),
40+
compileFilesInDir("tests/pos-special/strawman-collections", allowDeepSubtypes),
41+
compileFilesInDir("tests/pos-special/isInstanceOf", allowDeepSubtypes.and("-Xfatal-warnings")),
42+
compileFilesInDir("tests/new", defaultOptions.and("-source", "3.2")), // just to see whether 3.2 works
43+
compileFilesInDir("tests/pos-scala2", scala2CompatMode),
44+
compileFilesInDir("tests/pos-custom-args/captures", defaultOptions.and("-language:experimental.captureChecking")),
45+
compileFilesInDir("tests/pos-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")),
46+
compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init")),
47+
// Run tests for legacy lazy vals
48+
compileFilesInDir("tests/pos", defaultOptions.and("-Ysafe-init", "-Ylegacy-lazy-vals", "-Ycheck-constraint-deps"), FileFilter.include(TestSources.posLazyValsAllowlist)),
49+
compileFilesInDir("tests/pos-deep-subtype", allowDeepSubtypes),
50+
compileFilesInDir("tests/pos-custom-args/no-experimental", defaultOptions.and("-Yno-experimental")),
51+
compileFilesInDir("tests/pos-custom-args/strict", defaultOptions.and("-source", "future", "-deprecation", "-Xfatal-warnings")),
52+
compileDir("tests/pos-special/java-param-names", defaultOptions.withJavacOnlyOptions("-parameters")),
53+
compileFile(
54+
// succeeds despite -Xfatal-warnings because of -nowarn
55+
"tests/neg-custom-args/fatal-warnings/xfatalWarnings.scala",
56+
defaultOptions.and("-nowarn", "-Xfatal-warnings")
57+
),
58+
compileFile("tests/pos-special/typeclass-scaling.scala", defaultOptions.and("-Xmax-inlines", "40")),
59+
compileFile("tests/pos-special/i7575.scala", defaultOptions.andLanguageFeature("dynamics")),
60+
compileFile("tests/pos-special/kind-projector.scala", defaultOptions.and("-Ykind-projector")),
61+
compileFile("tests/pos-special/kind-projector-underscores.scala", defaultOptions.and("-Ykind-projector:underscores")),
62+
compileFile("tests/run/i5606.scala", defaultOptions.and("-Yretain-trees")),
63+
compileFile("tests/pos-custom-args/i8875.scala", defaultOptions.and("-Xprint:getters")),
64+
compileFile("tests/pos-custom-args/i9267.scala", defaultOptions.and("-Ystop-after:erasure")),
65+
compileFile("tests/pos-special/extend-java-enum.scala", defaultOptions.and("-source", "3.0-migration")),
66+
compileFile("tests/pos-custom-args/help.scala", defaultOptions.and("-help", "-V", "-W", "-X", "-Y")),
67+
compileFile("tests/pos-custom-args/i13044.scala", defaultOptions.and("-Xmax-inlines:33")),
68+
compileFile("tests/pos-custom-args/jdk-8-app.scala", defaultOptions.and("-release:8")),
7069
).checkCompile()
7170
}
7271

‎tests/neg-custom-args/deprecation/i11022.check

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
-- Error: tests/neg-custom-args/deprecation/i11022.scala:8:7 -----------------------------------------------------------
2+
8 |val a: CaseClass = CaseClass(42) // error: deprecated type // error: deprecated apply method
3+
| ^^^^^^^^^
4+
| class CaseClass is deprecated: no CaseClass
5+
-- Error: tests/neg-custom-args/deprecation/i11022.scala:8:19 ----------------------------------------------------------
6+
8 |val a: CaseClass = CaseClass(42) // error: deprecated type // error: deprecated apply method
7+
| ^^^^^^^^^
8+
| method apply in object CaseClass is deprecated: no CaseClass
9+
-- Error: tests/neg-custom-args/deprecation/i11022.scala:9:7 -----------------------------------------------------------
10+
9 |val b: CaseClass = new CaseClass(42) // error: deprecated type // error: deprecated class
11+
| ^^^^^^^^^
12+
| class CaseClass is deprecated: no CaseClass
13+
-- Error: tests/neg-custom-args/deprecation/i11022.scala:9:23 ----------------------------------------------------------
14+
9 |val b: CaseClass = new CaseClass(42) // error: deprecated type // error: deprecated class
15+
| ^^^^^^^^^
16+
| class CaseClass is deprecated: no CaseClass
17+
-- Error: tests/neg-custom-args/deprecation/i11022.scala:10:14 ---------------------------------------------------------
18+
10 |val c: Unit = CaseClass(42).magic() // error: deprecated apply method
19+
| ^^^^^^^^^
20+
| method apply in object CaseClass is deprecated: no CaseClass

‎tests/neg-custom-args/deprecation/i11022.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
@deprecated("no CaseClass")
2+
case class CaseClass(rgb: Int):
3+
def magic(): Unit = ()
4+
5+
object CaseClass:
6+
def notDeprecated(): Unit = ()
7+
8+
val a: CaseClass = CaseClass(42) // error: deprecated type // error: deprecated apply method
9+
val b: CaseClass = new CaseClass(42) // error: deprecated type // error: deprecated class
10+
val c: Unit = CaseClass(42).magic() // error: deprecated apply method
11+
val d: Unit = CaseClass.notDeprecated() // compiles

‎tests/pos-custom-args/i11022.scala

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎tests/pos/i11022.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// scalac: -Werror -deprecation
2+
@deprecated("no CaseClass")
3+
case class CaseClass(rgb: Int)

0 commit comments

Comments
 (0)