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

Author: mbovel

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

@@ -194,8 +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 CallToAnyRefMethodOnPredefID // errorNumber: 181
-  case CallToAnyRefMethodOnPackageObjectID // errorNumber: 182
+  case UnqualifiedCallToAnyRefMethodID // errorNumber: 181
 
   def errorNumber = ordinal - 1
 

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

@@ -2288,23 +2288,14 @@ 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 CallToAnyRefMethodOnPredef(stat: untpd.Tree, method: Symbol)(using Context)
-  extends Message(CallToAnyRefMethodOnPredefID) {
+class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Context)
+  extends Message(UnqualifiedCallToAnyRefMethodID) {
   def kind = MessageKind.PotentialIssue
-  def msg(using Context) = i"Suspicious call to ${hl("Predef." + method.name)}"
+  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")}. This might not be what you intended."""
-}
-
-class CallToAnyRefMethodOnPackageObject(stat: untpd.Tree, method: Symbol)(using Context)
-  extends Message(CallToAnyRefMethodOnPackageObjectID) {
-  def kind = MessageKind.PotentialIssue
-  def msg(using Context) = i"Suspicious top-level call to ${hl("this." + method.name)}"
-  def explain(using Context) =
-    i"""Top-level calls to ${hl("AnyRef")} or ${hl("Any")} methods are resolved to calls on the
-       |synthetic package object generated for the current file. This might not be
-       |what you intended."""
+       |resolved to calls on ${hl("Predef")} or on imported methods. This might not be what
+       |you intended."""
 }
 
 class TraitCompanionWithMutableStatic()(using Context)

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

@@ -1090,16 +1090,11 @@ object RefChecks {
 
   end checkImplicitNotFoundAnnotation
 
-  def checkSynchronizedCall(tree: Tree, symbol: Symbol)(using Context) =
-    if symbol.exists && defn.topClasses.contains(symbol.owner) then
-      tree.tpe match
-        case tp: NamedType =>
-          val prefixSymbol = tp.prefix.typeSymbol
-          if prefixSymbol == defn.ScalaPredefModuleClass then
-            report.warning(CallToAnyRefMethodOnPredef(tree, symbol), tree)
-          if prefixSymbol.isPackageObject && !tp.symbol.isConstructor then
-            report.warning(CallToAnyRefMethodOnPackageObject(tree, symbol), tree)
-        case _ => ()
+  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._
@@ -1180,12 +1175,8 @@ class RefChecks extends MiniPhase { thisPhase =>
       tree
   }
 
-  override def transformSelect(tree: Select)(using Context): Tree =
-    checkSynchronizedCall(tree, tree.denot.symbol)
-    tree
-
   override def transformIdent(tree: Ident)(using Context): Tree =
-    checkSynchronizedCall(tree, tree.symbol)
+    checkAnyRefMethodCall(tree)
     tree
 
 }

tests/neg/i17266.check

@@ -1,84 +1,88 @@
 -- [E181] Potential Issue Error: tests/neg/i17266.scala:4:2 ------------------------------------------------------------
 4 |  synchronized { // error
   |  ^^^^^^^^^^^^
-  |  Suspicious call to Predef.synchronized
+  |  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. This might not be what you intended.
+  | resolved to calls on Predef or on imported methods. This might not be what
+  | you intended.
    ---------------------------------------------------------------------------------------------------------------------
--- [E182] Potential Issue Error: tests/neg/i17266.scala:9:7 ------------------------------------------------------------
-9 |  this.synchronized { // error
-  |  ^^^^^^^^^^^^^^^^^
-  |  Suspicious top-level call to this.synchronized
-  |---------------------------------------------------------------------------------------------------------------------
-  | Explanation (enabled by `-explain`)
-  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-  | Top-level calls to AnyRef or Any methods are resolved to calls on the
-  | synthetic package object generated for the current file. This might not be
-  | what you intended.
-   ---------------------------------------------------------------------------------------------------------------------
--- [E181] Potential Issue Error: tests/neg/i17266.scala:22:2 -----------------------------------------------------------
-22 |  wait() // error
-   |  ^^^^
-   |  Suspicious call to Predef.wait
-   |--------------------------------------------------------------------------------------------------------------------
-   | Explanation (enabled by `-explain`)
-   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-   | Top-level unqualified calls to AnyRef or Any methods such as wait are
-   | resolved to calls on Predef. This might not be what you intended.
-    --------------------------------------------------------------------------------------------------------------------
--- [E182] Potential Issue Error: tests/neg/i17266.scala:25:7 -----------------------------------------------------------
-25 |  this.wait() // error
-   |  ^^^^^^^^^
-   |  Suspicious top-level call to this.wait
-   |--------------------------------------------------------------------------------------------------------------------
-   | Explanation (enabled by `-explain`)
-   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-   | Top-level calls to AnyRef or Any methods are resolved to calls on the
-   | synthetic package object generated for the current file. This might not be
-   | what you intended.
-    --------------------------------------------------------------------------------------------------------------------
--- [E181] Potential Issue Error: tests/neg/i17266.scala:32:2 -----------------------------------------------------------
-32 |  wait(10) // error
-   |  ^^^^
-   |  Suspicious call to Predef.wait
-   |--------------------------------------------------------------------------------------------------------------------
-   | Explanation (enabled by `-explain`)
-   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-   | Top-level unqualified calls to AnyRef or Any methods such as wait are
-   | resolved to calls on Predef. This might not be what you intended.
-    --------------------------------------------------------------------------------------------------------------------
--- [E182] Potential Issue Error: tests/neg/i17266.scala:35:7 -----------------------------------------------------------
-35 |  this.wait(10) // error
-   |  ^^^^^^^^^
-   |  Suspicious top-level call to this.wait
-   |--------------------------------------------------------------------------------------------------------------------
-   | Explanation (enabled by `-explain`)
-   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-   | Top-level calls to AnyRef or Any methods are resolved to calls on the
-   | synthetic package object generated for the current file. This might not be
-   | what you intended.
-    --------------------------------------------------------------------------------------------------------------------
--- [E181] Potential Issue Error: tests/neg/i17266.scala:42:2 -----------------------------------------------------------
-42 |  hashCode() // error
-   |  ^^^^^^^^
-   |  Suspicious call to Predef.hashCode
-   |--------------------------------------------------------------------------------------------------------------------
-   | Explanation (enabled by `-explain`)
-   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-   | Top-level unqualified calls to AnyRef or Any methods such as hashCode are
-   | resolved to calls on Predef. This might not be what you intended.
-    --------------------------------------------------------------------------------------------------------------------
--- [E182] Potential Issue Error: tests/neg/i17266.scala:45:7 -----------------------------------------------------------
-45 |  this.hashCode() // error
-   |  ^^^^^^^^^^^^^
-   |  Suspicious top-level call to this.hashCode
+-- [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 calls to AnyRef or Any methods are resolved to calls on the
-   | synthetic package object generated for the current file. This might not be
-   | what you intended.
+   | 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.
+     -------------------------------------------------------------------------------------------------------------------

tests/neg/i17266.scala

@@ -6,44 +6,139 @@ def test1 =
   }
 
 def test2 =
-  this.synchronized { // error
+  this.synchronized { // not an error (should be?)
     println("hello")
   }
 
 object MyLib
 
 def test3 =
   import MyLib.*
-  synchronized { // not an error (should be?)
+  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 test5 =
-  this.wait() // error
+def test17 =
+  this.wait() // not an error (should be?)
 
-def test6 =
+def test18 =
   import MyLib.*
-  wait() // not an error (should be?)
+  wait() // error
 
-def test7 =
+def test19 =
+  1.wait() // not an error (should be?)
+
+def test20 =
   wait(10) // error
 
-def test8 =
-  this.wait(10) // error
+def test21 =
+  this.wait(10) // not an error (should be?)
 
-def test9 =
+def test22 =
   import MyLib.*
-  wait(10) // not an error (should be?)
+  wait(10) // error
+
+def test23 =
+  1.wait(10) // not an error (should be?)
 
-def test10 =
+def test24 =
   hashCode() // error
 
-def test11 =
-  this.hashCode() // error
+def test25 =
+  this.hashCode() // not an error (should be?)
 
-def test12 =
+def test26 =
   import MyLib.*
-  hashCode() // not an error (should be?)
+  hashCode() // error
+
+def test27 =
+  1.hashCode()// not an error (should be? probably not)