Skip to content

Commit 1c1ce9e

Browse files
mboveldwijnanditielsa
committed
Warn when calling methods of topClasses on Predef or on package objects
Co-Authored-By: Dale Wijnand <[email protected]> Co-Authored-By: itielsa <[email protected]>
1 parent 274babf commit 1c1ce9e

File tree

7 files changed

+178
-2
lines changed

7 files changed

+178
-2
lines changed

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

+2
Original file line numberDiff line numberDiff line change
@@ -194,6 +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 CallToAnyRefMethodOnPredefID // errorNumber: 181
198+
case CallToAnyRefMethodOnPackageObjectID // errorNumber: 182
197199

198200
def errorNumber = ordinal - 1
199201

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

+19
Original file line numberDiff line numberDiff line change
@@ -2284,6 +2284,25 @@ 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 CallToAnyRefMethodOnPredef(stat: untpd.Tree, method: Symbol)(using Context)
2288+
extends Message(CallToAnyRefMethodOnPredefID) {
2289+
def kind = MessageKind.PotentialIssue
2290+
def msg(using Context) = i"Suspicious call to ${hl("Predef." + method.name)}"
2291+
def explain(using Context) =
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 intended."""
2294+
}
2295+
2296+
class CallToAnyRefMethodOnPackageObject(stat: untpd.Tree, method: Symbol)(using Context)
2297+
extends Message(CallToAnyRefMethodOnPackageObjectID) {
2298+
def kind = MessageKind.PotentialIssue
2299+
def msg(using Context) = i"Suspicious top-level call to ${hl("this." + method.name)}"
2300+
def explain(using Context) =
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 intended."""
2304+
}
2305+
22872306
class TraitCompanionWithMutableStatic()(using Context)
22882307
extends SyntaxMsg(TraitCompanionWithMutableStaticID) {
22892308
def msg(using Context) = i"Companion of traits cannot define mutable @static fields"

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

+20
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,17 @@ object RefChecks {
10901090

10911091
end checkImplicitNotFoundAnnotation
10921092

1093+
def checkSynchronizedCall(tree: Tree, symbol: Symbol)(using Context) =
1094+
if symbol.exists && defn.topClasses.contains(symbol.owner) then
1095+
tree.tpe match
1096+
case tp: NamedType =>
1097+
val prefixSymbol = tp.prefix.typeSymbol
1098+
if prefixSymbol == defn.ScalaPredefModuleClass then
1099+
report.warning(CallToAnyRefMethodOnPredef(tree, symbol), tree)
1100+
if prefixSymbol.isPackageObject && !tp.symbol.isConstructor then
1101+
report.warning(CallToAnyRefMethodOnPackageObject(tree, symbol), tree)
1102+
case _ => ()
1103+
10931104
}
10941105
import RefChecks._
10951106

@@ -1168,6 +1179,15 @@ class RefChecks extends MiniPhase { thisPhase =>
11681179
report.error(ex, tree.srcPos)
11691180
tree
11701181
}
1182+
1183+
override def transformSelect(tree: Select)(using Context): Tree =
1184+
checkSynchronizedCall(tree, tree.denot.symbol)
1185+
tree
1186+
1187+
override def transformIdent(tree: Ident)(using Context): Tree =
1188+
checkSynchronizedCall(tree, tree.symbol)
1189+
tree
1190+
11711191
}
11721192

11731193
/* todo: rewrite and re-enable

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class CompilationTests {
145145
compileFilesInDir("tests/neg-custom-args/feature", defaultOptions.and("-Xfatal-warnings", "-feature")),
146146
compileFilesInDir("tests/neg-custom-args/no-experimental", defaultOptions.and("-Yno-experimental")),
147147
compileFilesInDir("tests/neg-custom-args/captures", defaultOptions.and("-language:experimental.captureChecking")),
148-
compileFilesInDir("tests/neg-custom-args/explain", defaultOptions.and("-explain")),
148+
compileFilesInDir("tests/neg-custom-args/explain", defaultOptions.and("-Xfatal-warnings", "-explain")),
149149
compileFile("tests/neg-custom-args/avoid-warn-deprecation.scala", defaultOptions.and("-Xfatal-warnings", "-feature")),
150150
compileFile("tests/neg-custom-args/i3246.scala", scala2CompatMode),
151151
compileFile("tests/neg-custom-args/overrideClass.scala", scala2CompatMode),

‎compiler/test/dotty/tools/utils.scala

+3-1
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ import scala.util.control.{ControlThrowable, NonFatal}
1717

1818
import dotc.config.CommandLineParser
1919

20+
object Dummy
21+
2022
def scripts(path: String): Array[File] = {
21-
val dir = new File(this.getClass.getResource(path).getPath)
23+
val dir = new File(Dummy.getClass.getResource(path).getPath)
2224
assert(dir.exists && dir.isDirectory, "Couldn't load scripts dir")
2325
dir.listFiles.filter { f =>
2426
val path = if f.isDirectory then f.getPath + "/" else f.getPath

‎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 intended.
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 intended.
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 intended.
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 intended.
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 intended.
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 intended.
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 intended.
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 intended.
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)