Skip to content

Commit ceea464

Browse files
mbovelmbovel
committed
Warn when calling Any/AnyRef methods on Predef or synthetic file objects
1 parent b806daa commit ceea464

File tree

7 files changed

+149
-55
lines changed

7 files changed

+149
-55
lines changed

‎compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,8 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
194194
case MissingArgumentListID // errorNumber: 178
195195
case MatchTypeScrutineeCannotBeHigherKindedID // errorNumber: 179
196196
case AmbiguousExtensionMethodID // errorNumber 180
197-
case CallToPredefSynchronizedID // errorNumber: 181
198-
case CallToPackageObjectSynchronizedID // errorNumber: 182
197+
case CallToAnyRefMethodOnPredefID // errorNumber: 181
198+
case CallToAnyRefMethodOnPackageObjectID // errorNumber: 182
199199

200200
def errorNumber = ordinal - 1
201201

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

+11-12
Original file line numberDiff line numberDiff line change
@@ -2284,24 +2284,23 @@ class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol)
22842284
|It can be removed without changing the semantics of the program. This may indicate an error."""
22852285
}
22862286

2287-
class CallToPredefSynchronized(stat: untpd.Tree)(using Context)
2288-
extends Message(CallToPredefSynchronizedID) {
2287+
class CallToAnyRefMethodOnPredef(stat: untpd.Tree, method: Symbol)(using Context)
2288+
extends Message(CallToAnyRefMethodOnPredefID) {
22892289
def kind = MessageKind.PotentialIssue
2290-
def msg(using Context) = i"Suspicious call to ${hl("Predef.synchronized")}"
2290+
def msg(using Context) = i"Suspicious call to ${hl("Predef." + method.name)}"
22912291
def explain(using Context) =
2292-
i"""Top-level unqualifed calls to ${hl("synchronized")} are resolved to ${hl("Predef.synchronized")}.
2293-
|Therefore, the call above will globally lock using the ${hl("Predef")} object as a monitor.
2294-
|This might not be what you intented ."""
2292+
i"""Top-level unqualified calls to ${hl("AnyRef")} or ${hl("Any")} methods such as ${hl(method.name.toString)} are
2293+
|resolved to calls on ${hl("Predef")}. This might not be what you intented."""
22952294
}
22962295

2297-
class CallToPackageObjectSynchronized(stat: untpd.Tree)(using Context)
2298-
extends Message(CallToPackageObjectSynchronizedID) {
2296+
class CallToAnyRefMethodOnPackageObject(stat: untpd.Tree, method: Symbol)(using Context)
2297+
extends Message(CallToAnyRefMethodOnPackageObjectID) {
22992298
def kind = MessageKind.PotentialIssue
2300-
def msg(using Context) = i"Suspicious top-level call to ${hl("this.synchronized")}"
2299+
def msg(using Context) = i"Suspicious top-level call to ${hl("this." + method.name)}"
23012300
def explain(using Context) =
2302-
i"""Top-level calls to ${hl("this.synchronized")} are resolved to ${hl("<package object>.synchronized")}.
2303-
|Therefore, the call above will lock using the generated package object as a monitor.
2304-
|This might not be what you intented ."""
2301+
i"""Top-level calls to ${hl("AnyRef")} or ${hl("Any")} methods are resolved to calls on the
2302+
|synthetic package object generated for the current file. This might not be
2303+
|what you intented."""
23052304
}
23062305

23072306
class TraitCompanionWithMutableStatic()(using Context)

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -1091,14 +1091,14 @@ object RefChecks {
10911091
end checkImplicitNotFoundAnnotation
10921092

10931093
def checkSynchronizedCall(tree: Tree, symbol: Symbol)(using Context) =
1094-
if symbol == defn.Object_synchronized then
1094+
if symbol.exists && (symbol.owner == defn.ObjectClass || symbol.owner == defn.AnyClass) then
10951095
tree.tpe match
10961096
case tp: NamedType =>
10971097
val prefixSymbol = tp.prefix.typeSymbol
10981098
if prefixSymbol == defn.ScalaPredefModuleClass then
1099-
report.warning(CallToPredefSynchronized(tree), tree)
1099+
report.warning(CallToAnyRefMethodOnPredef(tree, symbol), tree)
11001100
if prefixSymbol.isPackageObject && !tp.symbol.isConstructor then
1101-
report.warning(CallToPackageObjectSynchronized(tree), tree)
1101+
report.warning(CallToAnyRefMethodOnPackageObject(tree, symbol), tree)
11021102
case _ => ()
11031103

11041104
}

‎tests/neg-custom-args/explain/i17266.check

-22
This file was deleted.

‎tests/neg-custom-args/explain/i17266.scala

-16
This file was deleted.

‎tests/neg/i17266.check

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
-- [E181] Potential Issue Error: tests/neg/i17266.scala:4:2 ------------------------------------------------------------
2+
4 | synchronized { // error
3+
| ^^^^^^^^^^^^
4+
| Suspicious call to Predef.synchronized
5+
|---------------------------------------------------------------------------------------------------------------------
6+
| Explanation (enabled by `-explain`)
7+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
8+
| Top-level unqualified calls to AnyRef or Any methods such as synchronized are
9+
| resolved to calls on Predef. This might not be what you intented.
10+
---------------------------------------------------------------------------------------------------------------------
11+
-- [E182] Potential Issue Error: tests/neg/i17266.scala:9:7 ------------------------------------------------------------
12+
9 | this.synchronized { // error
13+
| ^^^^^^^^^^^^^^^^^
14+
| Suspicious top-level call to this.synchronized
15+
|---------------------------------------------------------------------------------------------------------------------
16+
| Explanation (enabled by `-explain`)
17+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
18+
| Top-level calls to AnyRef or Any methods are resolved to calls on the
19+
| synthetic package object generated for the current file. This might not be
20+
| what you intented.
21+
---------------------------------------------------------------------------------------------------------------------
22+
-- [E181] Potential Issue Error: tests/neg/i17266.scala:22:2 -----------------------------------------------------------
23+
22 | wait() // error
24+
| ^^^^
25+
| Suspicious call to Predef.wait
26+
|--------------------------------------------------------------------------------------------------------------------
27+
| Explanation (enabled by `-explain`)
28+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
29+
| Top-level unqualified calls to AnyRef or Any methods such as wait are
30+
| resolved to calls on Predef. This might not be what you intented.
31+
--------------------------------------------------------------------------------------------------------------------
32+
-- [E182] Potential Issue Error: tests/neg/i17266.scala:25:7 -----------------------------------------------------------
33+
25 | this.wait() // error
34+
| ^^^^^^^^^
35+
| Suspicious top-level call to this.wait
36+
|--------------------------------------------------------------------------------------------------------------------
37+
| Explanation (enabled by `-explain`)
38+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
39+
| Top-level calls to AnyRef or Any methods are resolved to calls on the
40+
| synthetic package object generated for the current file. This might not be
41+
| what you intented.
42+
--------------------------------------------------------------------------------------------------------------------
43+
-- [E181] Potential Issue Error: tests/neg/i17266.scala:32:2 -----------------------------------------------------------
44+
32 | wait(10) // error
45+
| ^^^^
46+
| Suspicious call to Predef.wait
47+
|--------------------------------------------------------------------------------------------------------------------
48+
| Explanation (enabled by `-explain`)
49+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
50+
| Top-level unqualified calls to AnyRef or Any methods such as wait are
51+
| resolved to calls on Predef. This might not be what you intented.
52+
--------------------------------------------------------------------------------------------------------------------
53+
-- [E182] Potential Issue Error: tests/neg/i17266.scala:35:7 -----------------------------------------------------------
54+
35 | this.wait(10) // error
55+
| ^^^^^^^^^
56+
| Suspicious top-level call to this.wait
57+
|--------------------------------------------------------------------------------------------------------------------
58+
| Explanation (enabled by `-explain`)
59+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
60+
| Top-level calls to AnyRef or Any methods are resolved to calls on the
61+
| synthetic package object generated for the current file. This might not be
62+
| what you intented.
63+
--------------------------------------------------------------------------------------------------------------------
64+
-- [E181] Potential Issue Error: tests/neg/i17266.scala:42:2 -----------------------------------------------------------
65+
42 | hashCode() // error
66+
| ^^^^^^^^
67+
| Suspicious call to Predef.hashCode
68+
|--------------------------------------------------------------------------------------------------------------------
69+
| Explanation (enabled by `-explain`)
70+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
71+
| Top-level unqualified calls to AnyRef or Any methods such as hashCode are
72+
| resolved to calls on Predef. This might not be what you intented.
73+
--------------------------------------------------------------------------------------------------------------------
74+
-- [E182] Potential Issue Error: tests/neg/i17266.scala:45:7 -----------------------------------------------------------
75+
45 | this.hashCode() // error
76+
| ^^^^^^^^^^^^^
77+
| Suspicious top-level call to this.hashCode
78+
|--------------------------------------------------------------------------------------------------------------------
79+
| Explanation (enabled by `-explain`)
80+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
81+
| Top-level calls to AnyRef or Any methods are resolved to calls on the
82+
| synthetic package object generated for the current file. This might not be
83+
| what you intented.
84+
--------------------------------------------------------------------------------------------------------------------

‎tests/neg/i17266.scala

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// scalac: -Werror -explain
2+
3+
def test1 =
4+
synchronized { // error
5+
println("hello")
6+
}
7+
8+
def test2 =
9+
this.synchronized { // error
10+
println("hello")
11+
}
12+
13+
object MyLib
14+
15+
def test3 =
16+
import MyLib.*
17+
synchronized { // not an error (should be?)
18+
println("hello")
19+
}
20+
21+
def test4 =
22+
wait() // error
23+
24+
def test5 =
25+
this.wait() // error
26+
27+
def test6 =
28+
import MyLib.*
29+
wait() // not an error (should be?)
30+
31+
def test7 =
32+
wait(10) // error
33+
34+
def test8 =
35+
this.wait(10) // error
36+
37+
def test9 =
38+
import MyLib.*
39+
wait(10) // not an error (should be?)
40+
41+
def test10 =
42+
hashCode() // error
43+
44+
def test11 =
45+
this.hashCode() // error
46+
47+
def test12 =
48+
import MyLib.*
49+
hashCode() // not an error (should be?)

0 commit comments

Comments
 (0)