Warn when calling synchronized on AnyVal

Co-Authored-By: James You <[email protected]>
Co-Authored-By: diogocanut <[email protected]>

Author: 3 people

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

@@ -196,6 +196,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case AmbiguousExtensionMethodID // errorNumber 180
   case UnqualifiedCallToAnyRefMethodID // errorNumber: 181
   case NotConstantID // errorNumber: 182
+  case SynchronizedCallOnBoxedClassID // errorNumber: 183
 
   def errorNumber = ordinal - 1
 

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

@@ -2298,6 +2298,15 @@ class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Cont
        |you intended."""
 }
 
+class SynchronizedCallOnBoxedClass(stat: untpd.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"

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

@@ -1179,6 +1179,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

tests/neg/17284.check

@@ -0,0 +1,30 @@
+-- [E183] 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.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E183] 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.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E183] 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.
+    --------------------------------------------------------------------------------------------------------------------

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

tests/neg/i17266.check

@@ -20,6 +20,16 @@
    | resolved to calls on Predef or on imported methods. This might not be what
    | you intended.
     --------------------------------------------------------------------------------------------------------------------
+-- [E183] 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
     |  ^^^^

tests/neg/i17266.scala

@@ -19,7 +19,7 @@ def test3 =
   }
 
 def test4 =
-  1.synchronized { // not an error (should be?)
+  1.synchronized { // error
     println("hello")
   }