Warn when calling synchronized on AnyVal

Co-Authored-By: James You <[email protected]> Co-Authored-By: diogocanut <[email protected]> Co-Authored-By: Nicolas Stucki <[email protected]>
scala · Jul 11, 2023 · d38d6d4 · d38d6d4
1 parent ed319e8
commit d38d6d4
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 1 deletion.
diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala
@@ -200,6 +200,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case MatchTypeNoCasesID // errorNumber: 184
   case UnimportedAndImportedID // errorNumber: 185
   case ImplausiblePatternWarningID // erorNumber: 186
+  case SynchronizedCallOnBoxedClassID // errorNumber: 187
 
   def errorNumber = ordinal - 1
 

diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala
@@ -2345,6 +2345,15 @@ class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Cont
        |you intended."""
 }
 
+class SynchronizedCallOnBoxedClass(stat: tpd.Tree)(using Context)
+  extends Message(SynchronizedCallOnBoxedClassID) {
+  def kind = MessageKind.PotentialIssue
+  def msg(using Context) = i"Suspicious ${hl("synchronized")} call on boxed class"
+  def explain(using Context) =
+    i"""|You called the ${hl("synchronized")} method on a boxed primitive. 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"

diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
@@ -1196,6 +1196,10 @@ class RefChecks extends MiniPhase { thisPhase =>
     checkAnyRefMethodCall(tree)
     tree
 
+  override def transformSelect(tree: tpd.Select)(using Context): tpd.Tree =
+    if defn.ScalaBoxedClasses().contains(tree.qualifier.tpe.typeSymbol) && tree.name == nme.synchronized_ then
+      report.warning(SynchronizedCallOnBoxedClass(tree), tree.srcPos)
+    tree
 }
 
 /* todo: rewrite and re-enable

diff --git a/tests/neg/17284.check b/tests/neg/17284.check
@@ -0,0 +1,30 @@
+-- [E187] Potential Issue Error: tests/neg/17284.scala:4:6 -------------------------------------------------------------
+4 |  451.synchronized {} // error
+  |  ^^^^^^^^^^^^^^^^
+  |  Suspicious synchronized call on boxed class
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | You called the synchronized method on a boxed primitive. This might not be what
+  | you intended.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E187] Potential Issue Error: tests/neg/17284.scala:8:4 -------------------------------------------------------------
+8 |  x.synchronized {} // error
+  |  ^^^^^^^^^^^^^^
+  |  Suspicious synchronized call on boxed class
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | You called the synchronized method on a boxed primitive. This might not be what
+  | you intended.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E187] Potential Issue Error: tests/neg/17284.scala:11:7 ------------------------------------------------------------
+11 |  true.synchronized {} // error
+   |  ^^^^^^^^^^^^^^^^^
+   |  Suspicious synchronized call on boxed class
+   |--------------------------------------------------------------------------------------------------------------------
+   | Explanation (enabled by `-explain`)
+   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+   | You called the synchronized method on a boxed primitive. This might not be what
+   | you intended.
+    --------------------------------------------------------------------------------------------------------------------
diff --git a/tests/neg/17284.scala b/tests/neg/17284.scala
@@ -0,0 +1,14 @@
+// scalac: -Werror -explain
+
+def test =
+  451.synchronized {} // error
+
+def test2 =
+  val x: Integer = 451
+  x.synchronized {} // error
+
+def test3 =
+  true.synchronized {} // error
+
+def test4 =
+  true.hashCode() // success
diff --git a/tests/neg/i17266.check b/tests/neg/i17266.check
@@ -20,6 +20,16 @@
    | resolved to calls on Predef or on imported methods. This might not be what
    | you intended.
     --------------------------------------------------------------------------------------------------------------------
+-- [E187] Potential Issue Error: tests/neg/i17266.scala:22:4 -----------------------------------------------------------
+22 |  1.synchronized { // error
+   |  ^^^^^^^^^^^^^^
+   |  Suspicious synchronized call on boxed class
+   |--------------------------------------------------------------------------------------------------------------------
+   | Explanation (enabled by `-explain`)
+   |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+   | You called the synchronized method on a boxed primitive. This might not be what
+   | you intended.
+    --------------------------------------------------------------------------------------------------------------------
 -- [E181] Potential Issue Error: tests/neg/i17266.scala:108:2 ----------------------------------------------------------
 108 |  wait() // error
     |  ^^^^

diff --git a/tests/neg/i17266.scala b/tests/neg/i17266.scala
@@ -19,7 +19,7 @@ def test3 =
   }
 
 def test4 =
-  1.synchronized { // not an error (should be?)
+  1.synchronized { // error
     println("hello")
   }