Skip to content

Warn on unqualified top-level calls to Any or AnyRef methods #17449

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

Merged
merged 2 commits into from
May 11, 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
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case MissingArgumentListID // errorNumber: 178
case MatchTypeScrutineeCannotBeHigherKindedID // errorNumber: 179
case AmbiguousExtensionMethodID // errorNumber 180
case UnqualifiedCallToAnyRefMethodID // errorNumber: 181

def errorNumber = ordinal - 1

Expand Down
10 changes: 10 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2284,6 +2284,16 @@ class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol)
|It can be removed without changing the semantics of the program. This may indicate an error."""
}

class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Context)
extends Message(UnqualifiedCallToAnyRefMethodID) {
def kind = MessageKind.PotentialIssue
def msg(using Context) = i"Suspicious top-level unqualified call to ${hl(method.name.toString)}"
def explain(using Context) =
i"""Top-level unqualified calls to ${hl("AnyRef")} or ${hl("Any")} methods such as ${hl(method.name.toString)} are
|resolved to calls on ${hl("Predef")} or on imported methods. This might not be what
|you intended."""
}

class TraitCompanionWithMutableStatic()(using Context)
extends SyntaxMsg(TraitCompanionWithMutableStaticID) {
def msg(using Context) = i"Companion of traits cannot define mutable @static fields"
Expand Down
11 changes: 11 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,12 @@ object RefChecks {

end checkImplicitNotFoundAnnotation

def checkAnyRefMethodCall(tree: Tree)(using Context) =
if tree.symbol.exists
&& defn.topClasses.contains(tree.symbol.owner)
&& (!ctx.owner.enclosingClass.exists || ctx.owner.enclosingClass.isPackageObject) then
report.warning(UnqualifiedCallToAnyRefMethod(tree, tree.symbol), tree)

}
import RefChecks._

Expand Down Expand Up @@ -1168,6 +1174,11 @@ class RefChecks extends MiniPhase { thisPhase =>
report.error(ex, tree.srcPos)
tree
}

override def transformIdent(tree: Ident)(using Context): Tree =
checkAnyRefMethodCall(tree)
tree

}

/* todo: rewrite and re-enable
Expand Down
4 changes: 3 additions & 1 deletion compiler/test/dotty/tools/utils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import scala.util.control.{ControlThrowable, NonFatal}

import dotc.config.CommandLineParser

object Dummy

def scripts(path: String): Array[File] = {
val dir = new File(this.getClass.getResource(path).getPath)
val dir = new File(Dummy.getClass.getResource(path).getPath)
assert(dir.exists && dir.isDirectory, "Couldn't load scripts dir")
dir.listFiles.filter { f =>
val path = if f.isDirectory then f.getPath + "/" else f.getPath
Expand Down
88 changes: 88 additions & 0 deletions tests/neg/i17266.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
-- [E181] Potential Issue Error: tests/neg/i17266.scala:4:2 ------------------------------------------------------------
4 | synchronized { // error
| ^^^^^^^^^^^^
| Suspicious top-level unqualified call to synchronized
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as synchronized are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
---------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:17:2 -----------------------------------------------------------
17 | synchronized { // error
| ^^^^^^^^^^^^
| Suspicious top-level unqualified call to synchronized
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as synchronized are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
--------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:108:2 ----------------------------------------------------------
108 | wait() // error
| ^^^^
| Suspicious top-level unqualified call to wait
|-------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as wait are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
-------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:115:2 ----------------------------------------------------------
115 | wait() // error
| ^^^^
| Suspicious top-level unqualified call to wait
|-------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as wait are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
-------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:121:2 ----------------------------------------------------------
121 | wait(10) // error
| ^^^^
| Suspicious top-level unqualified call to wait
|-------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as wait are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
-------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:128:2 ----------------------------------------------------------
128 | wait(10) // error
| ^^^^
| Suspicious top-level unqualified call to wait
|-------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as wait are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
-------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:134:2 ----------------------------------------------------------
134 | hashCode() // error
| ^^^^^^^^
| Suspicious top-level unqualified call to hashCode
|-------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as hashCode are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
-------------------------------------------------------------------------------------------------------------------
-- [E181] Potential Issue Error: tests/neg/i17266.scala:141:2 ----------------------------------------------------------
141 | hashCode() // error
| ^^^^^^^^
| Suspicious top-level unqualified call to hashCode
|-------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Top-level unqualified calls to AnyRef or Any methods such as hashCode are
| resolved to calls on Predef or on imported methods. This might not be what
| you intended.
-------------------------------------------------------------------------------------------------------------------
144 changes: 144 additions & 0 deletions tests/neg/i17266.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
// scalac: -Werror -explain

def test1 =
synchronized { // error
println("hello")
}

def test2 =
this.synchronized { // not an error (should be?)
println("hello")
}

object MyLib

def test3 =
import MyLib.*
synchronized { // error
println("hello")
}

def test4 =
1.synchronized { // not an error (should be?)
println("hello")
}

object Test4:
synchronized { // not an error
println("hello")
}

object Test5:
def test5 =
synchronized { // not an error
println("hello")
}

object Test6:
import MyLib.*
synchronized { // not an error
println("hello")
}

object Test7:
import MyLib.*
def test7 =
synchronized { // not an error
println("hello")
}

/*
object Test7b:
def test8 =
import MyLib.*
synchronized { // already an error: Reference to synchronized is ambiguous.
println("hello")
}
*/

class Test8:
synchronized { // not an error
println("hello")
}

class Test9:
def test5 =
synchronized { // not an error
println("hello")
}

class Test10:
import MyLib.*
synchronized { // not an error
println("hello")
}

class Test11:
import MyLib.*
def test7 =
synchronized { // not an error
println("hello")
}

trait Test12:
synchronized { // not an error
println("hello")
}

trait Test13:
def test5 =
synchronized { // not an error
println("hello")
}

trait Test14:
import MyLib.*
synchronized { // not an error
println("hello")
}

trait Test15:
import MyLib.*
def test7 =
synchronized { // not an error
println("hello")
}

def test16 =
wait() // error

def test17 =
this.wait() // not an error (should be?)

def test18 =
import MyLib.*
wait() // error

def test19 =
1.wait() // not an error (should be?)

def test20 =
wait(10) // error

def test21 =
this.wait(10) // not an error (should be?)

def test22 =
import MyLib.*
wait(10) // error

def test23 =
1.wait(10) // not an error (should be?)

def test24 =
hashCode() // error

def test25 =
this.hashCode() // not an error (should be?)

def test26 =
import MyLib.*
hashCode() // error

def test27 =
1.hashCode()// not an error (should be? probably not)