Improve message for discarded pure non-Unit values

Fixes scala#18408
Fixes scala#18722

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

@@ -202,6 +202,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case ImplausiblePatternWarningID // erorNumber: 186
   case SynchronizedCallOnBoxedClassID // errorNumber: 187
   case VarArgsParamCannotBeGivenID // erorNumber: 188
+  case PureUnitExpressionID // erorNumber: 189
 
   def errorNumber = ordinal - 1
 

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

@@ -2391,6 +2391,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 PureUnitExpression(stat: untpd.Tree, tpe: Type)(using Context)
+  extends Message(PureUnitExpressionID) {
+  def kind = MessageKind.PotentialIssue
+  def msg(using Context) = i"Discarded non-Unit value of type ${tpe.widen}. You may want to use `()`."
+  def explain(using Context) =
+    i"""As this expression is not of type Unit, it is desugared into `{ $stat; () }`.
+       |Here the `$stat` expression is a pure statement that can be discarded.
+       |Therefore the expression is effectively equivalent to `()`."""
+}
+
 class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Context)
   extends Message(UnqualifiedCallToAnyRefMethodID) {
   def kind = MessageKind.PotentialIssue

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

@@ -4212,7 +4212,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         // local adaptation makes sure every adapted tree conforms to its pt
         // so will take the code path that decides on inlining
         val tree1 = adapt(tree, WildcardType, locked)
-        checkStatementPurity(tree1)(tree, ctx.owner)
+        checkStatementPurity(tree1)(tree, ctx.owner, isUnitExpr = true)
         if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) {
           report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
         }
@@ -4418,7 +4418,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         typedExpr(cmp, defn.BooleanType)
       case _ =>
 
-  private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol)(using Context): Unit =
+  private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol, isUnitExpr: Boolean = false)(using Context): Unit =
     if !tree.tpe.isErroneous
       && !ctx.isAfterTyper
       && !tree.isInstanceOf[Inlined]
@@ -4436,6 +4436,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         // sometimes we do not have the original anymore and use the transformed tree instead.
         // But taken together, the two criteria are quite accurate.
         missingArgs(tree, tree.tpe.widen)
+      case _ if isUnitExpr =>
+        report.warning(PureUnitExpression(original, tree.tpe), original.srcPos)
       case _ =>
         report.warning(PureExpressionInStatementPosition(original, exprOwner), original.srcPos)
 

tests/neg-custom-args/captures/real-try.check

@@ -1,7 +1,7 @@
--- [E129] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:36:4 ----------------------------------
+-- [E189] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:36:4 ----------------------------------
 36 |  b.x
    |  ^^^
-   |  A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |  Discarded non-Unit value of type () -> Unit. You may want to use `()`.
    |
    | longer explanation available when compiling with `-explain`
 -- [E007] Type Mismatch Error: tests/neg-custom-args/captures/real-try.scala:19:4 --------------------------------------

tests/neg/i18408a.check

@@ -0,0 +1,18 @@
+-- [E103] Syntax Error: tests/neg/i18408a.scala:2:0 --------------------------------------------------------------------
+2 |fa(42) // error
+  |^^
+  |Illegal start of toplevel definition
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E189] Potential Issue Warning: tests/neg/i18408a.scala:3:15 --------------------------------------------------------
+3 |def test1 = fa(42)
+  |               ^^
+  |               Discarded non-Unit value of type Int. You may want to use `()`.
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E129] Potential Issue Warning: tests/neg/i18408a.scala:4:16 --------------------------------------------------------
+4 |def test2 = fa({42; ()})
+  |                ^^
+  |                A pure expression does nothing in statement position; you may be omitting necessary parentheses
+  |
+  | longer explanation available when compiling with `-explain`

tests/neg/i18408a.scala

@@ -0,0 +1,4 @@
+def fa(f: String ?=> Unit): Unit = ???
+fa(42) // error
+def test1 = fa(42)
+def test2 = fa({42; ()})

tests/neg/i18408b.check

@@ -0,0 +1,18 @@
+-- [E103] Syntax Error: tests/neg/i18408b.scala:2:0 --------------------------------------------------------------------
+2 |fa(42) // error
+  |^^
+  |Illegal start of toplevel definition
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E189] Potential Issue Warning: tests/neg/i18408b.scala:3:15 --------------------------------------------------------
+3 |def test1 = fa(42)
+  |               ^^
+  |               Discarded non-Unit value of type Int. You may want to use `()`.
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E129] Potential Issue Warning: tests/neg/i18408b.scala:4:16 --------------------------------------------------------
+4 |def test2 = fa({42; ()})
+  |                ^^
+  |                A pure expression does nothing in statement position; you may be omitting necessary parentheses
+  |
+  | longer explanation available when compiling with `-explain`

tests/neg/i18408b.scala

@@ -0,0 +1,4 @@
+def fa(f: => Unit): Unit = ???
+fa(42) // error
+def test1 = fa(42)
+def test2 = fa({42; ()})

tests/neg/i18408c.check

@@ -0,0 +1,18 @@
+-- [E103] Syntax Error: tests/neg/i18408c.scala:2:0 --------------------------------------------------------------------
+2 |fa(42) // error
+  |^^
+  |Illegal start of toplevel definition
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E189] Potential Issue Warning: tests/neg/i18408c.scala:3:15 --------------------------------------------------------
+3 |def test1 = fa(42)
+  |               ^^
+  |               Discarded non-Unit value of type Int. You may want to use `()`.
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E129] Potential Issue Warning: tests/neg/i18408c.scala:4:16 --------------------------------------------------------
+4 |def test2 = fa({42; ()})
+  |                ^^
+  |                A pure expression does nothing in statement position; you may be omitting necessary parentheses
+  |
+  | longer explanation available when compiling with `-explain`

tests/neg/i18408c.scala

@@ -0,0 +1,4 @@
+def fa(f: Unit): Unit = ???
+fa(42) // error
+def test1 = fa(42)
+def test2 = fa({42; ()})

tests/neg/i18722.check

@@ -0,0 +1,44 @@
+-- [E189] Potential Issue Error: tests/neg/i18722.scala:3:15 -----------------------------------------------------------
+3 |def f1: Unit = null // error
+  |               ^^^^
+  |               Discarded non-Unit value of type Null. You may want to use `()`.
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | As this expression is not of type Unit, it is desugared into `{ null; () }`.
+  | Here the `null` expression is a pure statement that can be discarded.
+  | Therefore the expression is effectively equivalent to `()`.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E189] Potential Issue Error: tests/neg/i18722.scala:4:15 -----------------------------------------------------------
+4 |def f2: Unit = 1 // error
+  |               ^
+  |               Discarded non-Unit value of type Int. You may want to use `()`.
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | As this expression is not of type Unit, it is desugared into `{ 1; () }`.
+  | Here the `1` expression is a pure statement that can be discarded.
+  | Therefore the expression is effectively equivalent to `()`.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E189] Potential Issue Error: tests/neg/i18722.scala:5:15 -----------------------------------------------------------
+5 |def f3: Unit = "a" // error
+  |               ^^^
+  |               Discarded non-Unit value of type String. You may want to use `()`.
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | As this expression is not of type Unit, it is desugared into `{ "a"; () }`.
+  | Here the `"a"` expression is a pure statement that can be discarded.
+  | Therefore the expression is effectively equivalent to `()`.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E189] Potential Issue Error: tests/neg/i18722.scala:7:15 -----------------------------------------------------------
+7 |def f4: Unit = i // error
+  |               ^
+  |               Discarded non-Unit value of type Int. You may want to use `()`.
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | As this expression is not of type Unit, it is desugared into `{ i; () }`.
+  | Here the `i` expression is a pure statement that can be discarded.
+  | Therefore the expression is effectively equivalent to `()`.
+   ---------------------------------------------------------------------------------------------------------------------

tests/neg/i18722.scala

@@ -0,0 +1,9 @@
+//> using options -Werror -explain
+
+def f1: Unit = null // error
+def f2: Unit = 1 // error
+def f3: Unit = "a" // error
+val i: Int = 1
+def f4: Unit = i // error
+val u: Unit = ()
+def f5: Unit = u

tests/neg/spaces-vs-tabs.check

@@ -28,9 +28,9 @@
    |	  The start of this line does not match any of the previous indentation widths.
    |	  Indentation width of current line : 1 tab, 2 spaces
    |	  This falls between previous widths: 1 tab and 1 tab, 4 spaces
--- [E129] Potential Issue Warning: tests/neg/spaces-vs-tabs.scala:13:7 -------------------------------------------------
+-- [E189] Potential Issue Warning: tests/neg/spaces-vs-tabs.scala:13:7 -------------------------------------------------
 13 |	      1
    |	      ^
-   |	      A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |	      Discarded non-Unit value of type Int. You may want to use `()`.
    |
    | longer explanation available when compiling with `-explain`

tests/neg/syntax-error-recovery.check

@@ -94,45 +94,45 @@
    |          Not found: bam
    |
    | longer explanation available when compiling with `-explain`
--- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:7:2 -------------------------------------------
+-- [E189] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:7:2 -------------------------------------------
 6 |        2
 7 |  }
   |         ^
-  |         A pure expression does nothing in statement position; you may be omitting necessary parentheses
+  |         Discarded non-Unit value of type Null. You may want to use `()`.
   |
   | longer explanation available when compiling with `-explain`
--- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:15:2 ------------------------------------------
+-- [E189] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:15:2 ------------------------------------------
 14 |        2
 15 |  println(baz) // error
    |         ^
-   |         A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |         Discarded non-Unit value of type Null. You may want to use `()`.
    |
    | longer explanation available when compiling with `-explain`
--- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:27:2 ------------------------------------------
+-- [E189] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:27:2 ------------------------------------------
 26 |      2
 27 |  println(bam) // error
    |       ^
-   |       A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |       Discarded non-Unit value of type Null. You may want to use `()`.
    |
    | longer explanation available when compiling with `-explain`
--- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:36:2 ------------------------------------------
+-- [E189] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:36:2 ------------------------------------------
 35 |        2
 36 |  }
    |         ^
-   |         A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |         Discarded non-Unit value of type Null. You may want to use `()`.
    |
    | longer explanation available when compiling with `-explain`
--- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:44:2 ------------------------------------------
+-- [E189] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:44:2 ------------------------------------------
 43 |        2
 44 |  println(baz) // error
    |         ^
-   |         A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |         Discarded non-Unit value of type Null. You may want to use `()`.
    |
    | longer explanation available when compiling with `-explain`
--- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:56:2 ------------------------------------------
+-- [E189] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:56:2 ------------------------------------------
 55 |      2
 56 |  println(bam) // error
    |       ^
-   |       A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |       Discarded non-Unit value of type Null. You may want to use `()`.
    |
    | longer explanation available when compiling with `-explain`