Backport "Improve message for discarded pure non-Unit values" to LTS

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

@@ -2380,12 +2380,22 @@ class MemberWithSameNameAsStatic()(using Context)
 class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol)(using Context)
   extends Message(PureExpressionInStatementPositionID) {
   def kind = MessageKind.PotentialIssue
-  def msg(using Context) = "A pure expression does nothing in statement position; you may be omitting necessary parentheses"
+  def msg(using Context) = "A pure expression does nothing in statement position"
   def explain(using Context) =
     i"""The pure expression $stat doesn't have any side effect and its result is not assigned elsewhere.
         |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

@@ -4127,7 +4127,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)
         }
@@ -4424,7 +4424,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
     else false
   }
 
-  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]
@@ -4442,6 +4442,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)
 

compiler/test-resources/repl/nowarn.scala

@@ -27,7 +27,7 @@ scala> def f = { 1; 2 }
 -- [E129] Potential Issue Warning: ---------------------------------------------
 1 | def f = { 1; 2 }
   |           ^
-  |A pure expression does nothing in statement position; you may be omitting necessary parentheses
+  |           A pure expression does nothing in statement position
   |
   | longer explanation available when compiling with `-explain`
 def f: Int

language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala

@@ -30,7 +30,7 @@ class DiagnosticsTest {
           |}"""
      .diagnostics(m1,
        (m1 to m2,
-        "A pure expression does nothing in statement position; you may be omitting necessary parentheses",
+        "A pure expression does nothing in statement position",
         Warning, Some(PureExpressionInStatementPositionID)))
 
   @Test def diagnosticWorksheetPureExpression: Unit =

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

@@ -1,7 +1,7 @@
--- [E129] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:30:4 ----------------------------------
+-- [E190] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:30:4 ----------------------------------
 30 |  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`
 -- Error: tests/neg-custom-args/captures/real-try.scala:12:2 -----------------------------------------------------------

tests/neg-macros/annot-suspend-cycle.check

@@ -1,7 +1,7 @@
 -- [E129] Potential Issue Warning: tests/neg-macros/annot-suspend-cycle/Macro.scala:7:4 --------------------------------
 7 |    new Foo
   |    ^^^^^^^
-  |    A pure expression does nothing in statement position; you may be omitting necessary parentheses
+  |    A pure expression does nothing in statement position
   |
   | longer explanation available when compiling with `-explain`
 Cyclic macro dependencies in tests/neg-macros/annot-suspend-cycle/Test.scala.

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`
+-- [E190] 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
+  |
+  | 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`
+-- [E190] 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
+  |
+  | 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`
+-- [E190] 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
+  |
+  | 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 @@
+-- [E190] 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 `()`.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E190] 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 `()`.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E190] 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 `()`.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E190] 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/nowarn.check

@@ -32,7 +32,7 @@ Matching filters for @nowarn or -Wconf:
 -- [E129] Potential Issue Warning: tests/neg/nowarn.scala:15:11 --------------------------------------------------------
 15 |def t2 = { 1; 2 } // warning (the invalid nowarn doesn't silence anything)
    |           ^
-   |           A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |           A pure expression does nothing in statement position
    |
    | longer explanation available when compiling with `-explain`
 -- Warning: tests/neg/nowarn.scala:14:8 --------------------------------------------------------------------------------
@@ -43,7 +43,7 @@ Matching filters for @nowarn or -Wconf:
 -- [E129] Potential Issue Warning: tests/neg/nowarn.scala:18:12 --------------------------------------------------------
 18 |def t2a = { 1; 2 } // warning (invalid nowarn doesn't silence)
    |            ^
-   |            A pure expression does nothing in statement position; you may be omitting necessary parentheses
+   |            A pure expression does nothing in statement position
    |
    | longer explanation available when compiling with `-explain`
 -- Warning: tests/neg/nowarn.scala:17:8 --------------------------------------------------------------------------------

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 -------------------------------------------------
+-- [E190] 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 -------------------------------------------
+-- [E190] 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 ------------------------------------------
+-- [E190] 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 ------------------------------------------
+-- [E190] 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 ------------------------------------------
+-- [E190] 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 ------------------------------------------
+-- [E190] 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 ------------------------------------------
+-- [E190] 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`