Allow discarding "Discarded non-Unit" warnings with : Unit

Co-Authored-By: nmc.borst <[email protected]> Co-Authored-By: Jędrzej Rochala <[email protected]>
scala · Nov 11, 2024 · c21b967 · c21b967
1 parent 1947828
commit c21b967
Show file tree

Hide file tree

Showing 14 changed files with 86 additions and 47 deletions.
diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala
@@ -2469,7 +2469,7 @@ class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol)
 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 msg(using Context) = i"Discarded non-Unit value of type ${tpe.widen}. Add `: Unit` to discard silently."
   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.
@@ -3173,7 +3173,7 @@ class InlinedAnonClassWarning()(using Context)
 class ValueDiscarding(tp: Type)(using Context)
   extends Message(ValueDiscardingID):
     def kind = MessageKind.PotentialIssue
-    def msg(using Context) = i"discarded non-Unit value of type $tp"
+    def msg(using Context) = i"discarded non-Unit value of type ${tp.widen}. Add `: Unit` to discard silently."
     def explain(using Context) = ""
 
 class UnusedNonUnitValue(tp: Type)(using Context)

diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -81,6 +81,9 @@ object Typer {
   /** Indicates that a definition was copied over from the parent refinements */
   val RefinementFromParent = new Property.StickyKey[Unit]
 
+  /** Indicates that an expression is explicitly ascribed to [[Unit]] type. */
+  val AscribedToUnit = new Property.StickyKey[Unit]
+
   /** An attachment on a Select node with an `apply` field indicating that the `apply`
    *  was inserted by the Typer.
    */
@@ -1193,7 +1196,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         else tpt
       val expr1 =
         if isWildcard then tree.expr.withType(underlyingTreeTpe.tpe)
-        else typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem)
+        else
+          if underlyingTreeTpe.tpe.isRef(defn.UnitClass) then
+            untpd.unsplice(tree.expr).putAttachment(AscribedToUnit, ())
+          typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem)
       assignType(cpy.Typed(tree)(expr1, tpt), underlyingTreeTpe)
         .withNotNullInfo(expr1.notNullInfo)
     }
@@ -3377,7 +3383,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
       else if (ctx.mode.is(Mode.Pattern))
         typedUnApply(cpy.Apply(tree)(op, l :: r :: Nil), pt)
       else {
-        val app = typedApply(desugar.binop(l, op, r), pt)
+        val app = typedApply(desugar.binop(l, op, r).withAttachmentsFrom(tree), pt)
         if op.name.isRightAssocOperatorName && !ctx.mode.is(Mode.QuotedExprPattern) then
           val defs = new mutable.ListBuffer[Tree]
           def lift(app: Tree): Tree = (app: @unchecked) match
@@ -4581,9 +4587,14 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         // so will take the code path that decides on inlining
         val tree1 = adapt(tree, WildcardType, locked)
         checkStatementPurity(tree1)(tree, ctx.owner, isUnitExpr = true)
-        if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.Whas.valueDiscard && !isThisTypeResult(tree)) {
+
+        if ctx.settings.Whas.valueDiscard
+           && !ctx.isAfterTyper
+           && !tree.isInstanceOf[Inlined]
+           && !isThisTypeResult(tree)
+           && !tree.hasAttachment(AscribedToUnit) then
           report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
-        }
+
         return tpd.Block(tree1 :: Nil, unitLiteral)
       }
 
@@ -4839,6 +4850,9 @@ 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 tree.hasAttachment(AscribedToUnit) =>
+        // The tree was ascribed to `Unit` explicitly to silence the warning.
+        ()
       case _ if isUnitExpr =>
         report.warning(PureUnitExpression(original, tree.tpe), original.srcPos)
       case _ =>

diff --git a/tests/neg-custom-args/captures/real-try.check b/tests/neg-custom-args/captures/real-try.check
@@ -1,7 +1,7 @@
 -- [E190] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:38:4 ----------------------------------
 38 |  b.x
    |  ^^^
-   |  Discarded non-Unit value of type () -> Unit. You may want to use `()`.
+   |  Discarded non-Unit value of type () -> Unit. Add `: Unit` to discard silently.
    |
    | longer explanation available when compiling with `-explain`
 -- Error: tests/neg-custom-args/captures/real-try.scala:14:2 -----------------------------------------------------------

diff --git a/tests/neg/i13091.check b/tests/neg/i13091.check
@@ -1,15 +1,9 @@
--- [E190] Potential Issue Warning: tests/neg/i13091.scala:7:17 ---------------------------------------------------------
-7 |def test: Unit = new Foo // error: class Foo is marked @experimental ...
-  |                 ^^^^^^^
-  |                 Discarded non-Unit value of type Foo. You may want to use `()`.
+-- Error: tests/neg/i13091.scala:7:16 ----------------------------------------------------------------------------------
+7 |def test = (new Foo): Unit // error: class Foo is marked @experimental ...
+  |                ^^^
+  |                class Foo is marked @experimental
   |
-  | longer explanation available when compiling with `-explain`
--- Error: tests/neg/i13091.scala:7:21 ----------------------------------------------------------------------------------
-7 |def test: Unit = new Foo // error: class Foo is marked @experimental ...
-  |                     ^^^
-  |                     class Foo is marked @experimental
-  |
-  |                     Experimental definition may only be used under experimental mode:
-  |                       1. in a definition marked as @experimental, or
-  |                       2. an experimental feature is imported at the package level, or
-  |                       3. compiling with the -experimental compiler flag.
+  |                Experimental definition may only be used under experimental mode:
+  |                  1. in a definition marked as @experimental, or
+  |                  2. an experimental feature is imported at the package level, or
+  |                  3. compiling with the -experimental compiler flag.
diff --git a/tests/neg/i13091.scala b/tests/neg/i13091.scala
@@ -4,4 +4,4 @@ import annotation.experimental
 
 @experimental class Foo
 
-def test: Unit = new Foo // error: class Foo is marked @experimental ...
+def test = (new Foo): Unit // error: class Foo is marked @experimental ...
diff --git a/tests/neg/i18408a.check b/tests/neg/i18408a.check
@@ -7,7 +7,7 @@
 -- [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 `()`.
+  |               Discarded non-Unit value of type Int. Add `: Unit` to discard silently.
   |
   | longer explanation available when compiling with `-explain`
 -- [E129] Potential Issue Warning: tests/neg/i18408a.scala:4:16 --------------------------------------------------------

diff --git a/tests/neg/i18408b.check b/tests/neg/i18408b.check
@@ -7,7 +7,7 @@
 -- [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 `()`.
+  |               Discarded non-Unit value of type Int. Add `: Unit` to discard silently.
   |
   | longer explanation available when compiling with `-explain`
 -- [E129] Potential Issue Warning: tests/neg/i18408b.scala:4:16 --------------------------------------------------------

diff --git a/tests/neg/i18408c.check b/tests/neg/i18408c.check
@@ -7,7 +7,7 @@
 -- [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 `()`.
+  |               Discarded non-Unit value of type Int. Add `: Unit` to discard silently.
   |
   | longer explanation available when compiling with `-explain`
 -- [E129] Potential Issue Warning: tests/neg/i18408c.scala:4:16 --------------------------------------------------------

diff --git a/tests/neg/spaces-vs-tabs.check b/tests/neg/spaces-vs-tabs.check
@@ -28,9 +28,3 @@
    |	  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
--- [E190] Potential Issue Warning: tests/neg/spaces-vs-tabs.scala:13:7 -------------------------------------------------
-13 |	      1
-   |	      ^
-   |	      Discarded non-Unit value of type Int. You may want to use `()`.
-   |
-   | longer explanation available when compiling with `-explain`
diff --git a/tests/neg/spaces-vs-tabs.scala b/tests/neg/spaces-vs-tabs.scala
@@ -10,6 +10,6 @@ object Test:
 	object Test2:
 
 	    if true then
-	      1
+	      ()
 	  else 2 // error
 
diff --git a/tests/warn/21557.check b/tests/warn/21557.check
@@ -0,0 +1,18 @@
+-- [E190] Potential Issue Warning: tests/warn/21557.scala:9:16 ---------------------------------------------------------
+9 |  val x: Unit = 1 + 1 // warn
+  |                ^^^^^
+  |                Discarded non-Unit value of type Int. Add `: Unit` to discard silently.
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E176] Potential Issue Warning: tests/warn/21557.scala:10:2 ---------------------------------------------------------
+10 |  1 + 1 // warn
+   |  ^^^^^
+   |  unused value of type (2 : Int)
+-- [E175] Potential Issue Warning: tests/warn/21557.scala:15:52 --------------------------------------------------------
+15 |  val x1: Unit = new Assertion("another").shouldPass() // warn (enabled by -Wvalue-discard)
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                 discarded non-Unit value of type Assertion. Add `: Unit` to discard silently.
+-- [E176] Potential Issue Warning: tests/warn/21557.scala:16:41 --------------------------------------------------------
+16 |  new Assertion("yet another").shouldPass() // warn (enabled by -Wnonunit-statement)
+   |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |  unused value of type Assertion
diff --git a/tests/warn/21557.scala b/tests/warn/21557.scala
@@ -0,0 +1,19 @@
+//> using options -Wvalue-discard -Wnonunit-statement
+
+class Assertion(assert: => Any):
+  def shouldPass(): Assertion = ???
+
+def test: Unit =
+  1 + 1: Unit
+  (1 + 1): Unit
+  val x: Unit = 1 + 1 // warn
+  1 + 1 // warn
+  val y: Int = 1 + 1
+
+  new Assertion("").shouldPass(): Unit
+  (new Assertion("").shouldPass()): Unit
+  val x1: Unit = new Assertion("another").shouldPass() // warn (enabled by -Wvalue-discard)
+  new Assertion("yet another").shouldPass() // warn (enabled by -Wnonunit-statement)
+  val y1: Assertion = new Assertion("another other").shouldPass()
+
+  ()
diff --git a/tests/warn/nonunit-statement.check b/tests/warn/nonunit-statement.check
@@ -27,31 +27,31 @@
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:58:19 --------------------------------------------
 58 |    if (!isEmpty) f(a)      // warn (if)
    |                  ^^^^
-   |                  discarded non-Unit value of type U
+   |                  discarded non-Unit value of type U. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:62:7 ---------------------------------------------
 62 |      f(a)                  // warn (if)
    |      ^^^^
-   |      discarded non-Unit value of type Boolean
+   |      discarded non-Unit value of type Boolean. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:73:25 --------------------------------------------
 73 |    if (!fellback) action(z)  // warn (if)
    |                   ^^^^^^^^^
-   |                   discarded non-Unit value of type U
+   |                   discarded non-Unit value of type U. Add `: Unit` to discard silently.
 -- [E176] Potential Issue Warning: tests/warn/nonunit-statement.scala:79:6 ---------------------------------------------
 79 |      g   // warn block statement
    |      ^
    |      unused value of type (g : => Int)
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:81:6 ---------------------------------------------
 81 |      g   // warn (if)
    |      ^
-   |      discarded non-Unit value of type (g : => Int)
+   |      discarded non-Unit value of type Int. Add `: Unit` to discard silently.
 -- [E176] Potential Issue Warning: tests/warn/nonunit-statement.scala:84:6 ---------------------------------------------
 84 |      g   // warn
    |      ^
    |      unused value of type (g : => Int)
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:86:6 ---------------------------------------------
 86 |      g   // warn
    |      ^
-   |      discarded non-Unit value of type (g : => Int)
+   |      discarded non-Unit value of type Int. Add `: Unit` to discard silently.
 -- [E176] Potential Issue Warning: tests/warn/nonunit-statement.scala:96:4 ---------------------------------------------
  96 |    if (b) {          // warn, at least one branch looks interesting
     |    ^
@@ -70,20 +70,20 @@
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:126:37 -------------------------------------------
 126 |    if (start.length != 0) jsb.append(start) // warn (value-discard)
     |                           ^^^^^^^^^^^^^^^^^
-    |                           discarded non-Unit value of type StringBuilder
+    |                           discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:132:18 -------------------------------------------
 132 |        jsb.append(it.next()) // warn (value-discard)
     |        ^^^^^^^^^^^^^^^^^^^^^
-    |        discarded non-Unit value of type StringBuilder
+    |        discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:135:35 -------------------------------------------
 135 |    if (end.length != 0) jsb.append(end) // warn (value-discard)
     |                         ^^^^^^^^^^^^^^^
-    |                         discarded non-Unit value of type StringBuilder
+    |                         discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:141:14 -------------------------------------------
 141 |      b.append(it.next()) // warn (value-discard)
     |      ^^^^^^^^^^^^^^^^^^^
-    |      discarded non-Unit value of type StringBuilder
+    |      discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:146:30 -------------------------------------------
 146 |    while (it.hasNext) it.next()  // warn
     |                       ^^^^^^^^^
-    |                       discarded non-Unit value of type String
+    |                       discarded non-Unit value of type String. Add `: Unit` to discard silently.
diff --git a/tests/warn/warn-value-discard.check b/tests/warn/warn-value-discard.check
@@ -1,20 +1,20 @@
 -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:27:36 -------------------------------------------
 27 |    mutable.Set.empty[String].remove("") // warn
    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |    discarded non-Unit value of type Boolean
+   |    discarded non-Unit value of type Boolean. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:39:41 -------------------------------------------
 39 |    mutable.Set.empty[String].subtractOne("") // warn
    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |    discarded non-Unit value of type scala.collection.mutable.Set[String]
+   |    discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:59:4 --------------------------------------------
 59 |    mutable.Set.empty[String] += "" // warn
    |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |    discarded non-Unit value of type scala.collection.mutable.Set[String]
+   |    discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:15:35 -------------------------------------------
 15 |  firstThing().map(_ => secondThing()) // warn
    |                        ^^^^^^^^^^^^^
-   |                        discarded non-Unit value of type Either[Failed, Unit]
+   |                        discarded non-Unit value of type Either[Failed, Unit]. Add `: Unit` to discard silently.
 -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:18:35 -------------------------------------------
 18 |  firstThing().map(_ => secondThing()) // warn
    |                        ^^^^^^^^^^^^^
-   |                        discarded non-Unit value of type Either[Failed, Unit]
+   |                        discarded non-Unit value of type Either[Failed, Unit]. Add `: Unit` to discard silently.
Original file line number	Diff line number	Diff line change
Expand Up		@@ -4,4 +4,4 @@ import annotation.experimental

		@experimental class Foo

		def test: Unit = new Foo // error: class Foo is marked @experimental ...
		def test = (new Foo): Unit // error: class Foo is marked @experimental ...