Defer warn value discard to refchecks

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -1908,6 +1908,13 @@ abstract class RefChecks extends Transform {
         && !treeInfo.hasExplicitUnit(t)           // suppressed by explicit expr: Unit
         && !isJavaApplication(t)                  // Java methods are inherently side-effecting
       )
+      def checkDiscardValue(t: Tree): Boolean =
+        t.hasAttachment[DiscardedValue.type] && {
+          t.removeAttachment[DiscardedValue.type]
+          val msg = s"discarded non-Unit value of type ${t.tpe}"
+          refchecksWarning(t.pos, msg, WarningCategory.WFlagValueDiscard)
+          true
+        }
       // begin checkInterestingResultInStatement
       settings.warnNonUnitStatement.value && checkInterestingShapes(t) && {
         val where = t match {
@@ -1922,7 +1929,7 @@ abstract class RefChecks extends Transform {
         def msg = s"unused value of type ${where.tpe}"
         refchecksWarning(where.pos, msg, WarningCategory.OtherPureStatement)
         true
-      }
+      } || checkDiscardValue(t)
     } // end checkInterestingResultInStatement
 
     override def transform(tree: Tree): Tree = {
@@ -2038,27 +2045,30 @@ abstract class RefChecks extends Transform {
 
           case blk @ Block(stats, expr) =>
             // diagnostic info
-            val (count, result0, adapted) =
+            val (count, result0) =
               expr match {
-                case Block(expr :: Nil, Literal(Constant(()))) => (1, expr, true)
-                case Literal(Constant(()))                     => (0, EmptyTree, false)
-                case _                                         => (1, EmptyTree, false)
+                case Block(expr :: Nil, Literal(Constant(()))) => (1, expr)
+                case Literal(Constant(()))                     => (0, EmptyTree)
+                case _                                         => (1, EmptyTree)
               }
             val isMultiline = stats.lengthCompare(1 - count) > 0
 
-            def checkPure(t: Tree, supple: Boolean): Unit =
+            def checkPure(t: Tree): Unit =
               if (!treeInfo.hasExplicitUnit(t) && treeInfo.isPureExprForWarningPurposes(t)) {
-                val msg = "a pure expression does nothing in statement position"
-                val parens = if (isMultiline) "multiline expressions might require enclosing parentheses" else ""
-                val discard = if (adapted) "; a value can be silently discarded when Unit is expected" else ""
+                val msg =
+                  if (t.hasAttachment[DiscardedExpr.type]) {
+                    t.removeAttachment[DiscardedExpr.type]
+                    "discarded pure expression does nothing"
+                  }
+                  else "a pure expression does nothing in statement position"
                 val text =
-                  if (supple) s"$parens$discard"
-                  else if (!parens.isEmpty) s"$msg; $parens" else msg
+                  if (!isMultiline) msg
+                  else s"$msg; multiline expressions might require enclosing parentheses"
                 refchecksWarning(t.pos, text, WarningCategory.OtherPureStatement)
               }
             // check block for unintended "expression in statement position"
-            stats.foreach { t => if (!checkInterestingResultInStatement(t)) checkPure(t, supple = false) }
-            if (result0.nonEmpty) checkPure(result0, supple = true)
+            stats.foreach { t => if (!checkInterestingResultInStatement(t)) checkPure(t) }
+            if (result0.nonEmpty) result0.updateAttachment(DiscardedExpr) // see checkPure on recursion into result
 
             def checkImplicitlyAdaptedBlockResult(t: Tree): Unit =
               expr match {

src/compiler/scala/tools/nsc/typechecker/Typers.scala

@@ -1170,7 +1170,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
 
           @inline def warnValueDiscard(): Unit =
             if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree))
-              context.warning(tree.pos, s"discarded non-Unit value of type ${tree.tpe}", WarningCategory.WFlagValueDiscard)
+              tree.updateAttachment(DiscardedValue)
           @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit = if (!isPastTyper) {
             val targetIsWide = ptSym == FloatClass || ptSym == DoubleClass
             val isInharmonic = {

src/reflect/scala/reflect/internal/StdAttachments.scala

@@ -160,7 +160,7 @@ trait StdAttachments {
   case object RootSelection extends PlainAttachment
 
   /** Marks a Typed tree with Unit tpt. */
-  case object TypedExpectingUnitAttachment
+  case object TypedExpectingUnitAttachment extends PlainAttachment
   def explicitlyUnit(tree: Tree): Boolean = tree.hasAttachment[TypedExpectingUnitAttachment.type]
 
   /** For `val i = 42`, marks field as inferred so accessor (getter) can warn if implicit. */
@@ -176,4 +176,9 @@ trait StdAttachments {
 
   /** Not a named arg in an application. Used for suspicious literal booleans. */
   case object UnnamedArg extends PlainAttachment
+
+  /** Adapted under value discard at typer. */
+  case object DiscardedValue extends PlainAttachment
+  /** Discarded pure expression observed at refchecks. */
+  case object DiscardedExpr extends PlainAttachment
 }

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -88,6 +88,8 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.PermittedSubclassSymbols
     this.NamePos
     this.UnnamedArg
+    this.DiscardedValue
+    this.DiscardedExpr
     this.noPrint
     this.typeDebug
     // inaccessible: this.posAssigner

test/async/jvm/toughtype.check

@@ -1,6 +1,3 @@
-toughtype.scala:175: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
-              identity[A] _
-                      ^
-toughtype.scala:175: warning: a pure expression does nothing in statement position
+toughtype.scala:175: warning: discarded pure expression does nothing
               identity[A] _
                       ^

test/files/neg/discard-advice-a.check

@@ -0,0 +1,12 @@
+discard-advice-a.scala:7: warning: unused value of type scala.concurrent.Future[Int]
+    Future(42)
+          ^
+discard-advice-a.scala:10: warning: unused value of type scala.concurrent.Future[Int]
+    Future(42)
+          ^
+discard-advice-a.scala:11: warning: unused value of type Boolean(true)
+    true
+    ^
+error: No warnings can be incurred under -Werror.
+3 warnings
+1 error

test/files/neg/discard-advice-a.scala

@@ -0,0 +1,13 @@
+//> using options -Werror -Wnonunit-statement -Wvalue-discard
+
+import concurrent._, ExecutionContext.Implicits._
+
+class C {
+  def f(): Unit = {
+    Future(42)
+  }
+  def g(): Unit = {
+    Future(42)
+    true
+  }
+}

test/files/neg/discard-advice-b.check

@@ -0,0 +1,9 @@
+discard-advice-b.scala:7: warning: discarded non-Unit value of type scala.concurrent.Future[Int]
+    Future(42)
+          ^
+discard-advice-b.scala:11: warning: discarded non-Unit value of type Boolean(true)
+    true
+    ^
+error: No warnings can be incurred under -Werror.
+2 warnings
+1 error

test/files/neg/discard-advice-b.scala

@@ -0,0 +1,13 @@
+//> using options -Werror -Wvalue-discard
+
+import concurrent._, ExecutionContext.Implicits._
+
+class C {
+  def f(): Unit = {
+    Future(42)
+  }
+  def g(): Unit = {
+    Future(42)
+    true
+  }
+}

test/files/neg/discard-advice-c.check

@@ -0,0 +1,6 @@
+discard-advice-c.scala:11: warning: discarded pure expression does nothing
+    true
+    ^
+error: No warnings can be incurred under -Werror.
+1 warning
+1 error

test/files/neg/discard-advice-c.scala

@@ -0,0 +1,13 @@
+//> using options -Werror
+
+import concurrent._, ExecutionContext.Implicits._
+
+class C {
+  def f(): Unit = {
+    Future(42)
+  }
+  def g(): Unit = {
+    Future(42)
+    true
+  }
+}

test/files/neg/discard-advice-d.check

@@ -0,0 +1,15 @@
+discard-advice-d.scala:7: warning: unused value of type scala.concurrent.Future[Int]
+Applicable -Wconf / @nowarn filters for this warning: msg=<part of the message>, cat=other-pure-statement, site=C.f
+    Future(42)
+          ^
+discard-advice-d.scala:10: warning: unused value of type scala.concurrent.Future[Int]
+Applicable -Wconf / @nowarn filters for this warning: msg=<part of the message>, cat=other-pure-statement, site=C.g
+    Future(42)
+          ^
+discard-advice-d.scala:11: warning: unused value of type Boolean(true)
+Applicable -Wconf / @nowarn filters for this warning: msg=<part of the message>, cat=other-pure-statement, site=C.g
+    true
+    ^
+error: No warnings can be incurred under -Werror.
+3 warnings
+1 error

test/files/neg/discard-advice-d.scala

@@ -0,0 +1,13 @@
+//> using options -Wconf:any:warning-verbose -Werror -Wnonunit-statement -Wvalue-discard
+
+import concurrent._, ExecutionContext.Implicits._
+
+class C {
+  def f(): Unit = {
+    Future(42)
+  }
+  def g(): Unit = {
+    Future(42)
+    true
+  }
+}

test/files/neg/nonunit-if.check

@@ -1,9 +1,3 @@
-nonunit-if.scala:58: warning: discarded non-Unit value of type U
-    if (!isEmpty) f(a)      // warn, check is on
-                   ^
-nonunit-if.scala:62: warning: discarded non-Unit value of type Boolean
-      f(a)                  // warn, check is on
-       ^
 nonunit-if.scala:13: warning: unused value of type scala.concurrent.Future[Int]
     improved              // warn
     ^
@@ -53,5 +47,5 @@ nonunit-if.scala:146: warning: unused value of type String
     while (it.hasNext) it.next()  // warn
                               ^
 error: No warnings can be incurred under -Werror.
-18 warnings
+16 warnings
 1 error

test/files/neg/t11379a.check

@@ -3,23 +3,4 @@ t11379a.scala:17: error: type mismatch;
  required: Unit => Unit
   def test4: Either[Int, Unit] = Right(()).map(Right(()))
                                                     ^
-t11379a.scala:9: warning: discarded non-Unit value of type scala.util.Right[Nothing,Unit]
-  def test1: Either[Int, Unit] = Right(Right(()))
-                                            ^
-t11379a.scala:10: warning: discarded non-Unit value of type scala.util.Either[Int,Unit]
-  def test2: Either[Int, Unit] = Right(()).map(_ => unitRight[Int])
-                                                             ^
-t11379a.scala:11: warning: discarded non-Unit value of type scala.util.Either[Int,Unit]
-  def test3: Either[Int, Unit] = Right(()).map { case _ => unitRight[Int] }
-                                                                    ^
-t11379a.scala:20: warning: discarded non-Unit value of type scala.util.Either[Nothing,Unit]
-  def test5: Either[Int, Unit] = Right(()).map { case _ => unitRight }
-                                                           ^
-t11379a.scala:21: warning: discarded non-Unit value of type scala.util.Either[Nothing,Unit]
-  def test6: Either[Int, Unit] = Right(()).map { _ => unitRight }
-                                                      ^
-t11379a.scala:22: warning: discarded non-Unit value of type scala.util.Either[Nothing,Unit]
-  def test7: Either[Int, Unit] = Right(()).map(_ => unitRight)
-                                                    ^
-6 warnings
 1 error

test/files/neg/t11379a2.check

@@ -0,0 +1,21 @@
+t11379a2.scala:9: warning: discarded non-Unit value of type scala.util.Right[Nothing,Unit]
+  def test1: Either[Int, Unit] = Right(Right(()))
+                                            ^
+t11379a2.scala:10: warning: discarded non-Unit value of type scala.util.Either[Int,Unit]
+  def test2: Either[Int, Unit] = Right(()).map(_ => unitRight[Int])
+                                                             ^
+t11379a2.scala:11: warning: discarded non-Unit value of type scala.util.Either[Int,Unit]
+  def test3: Either[Int, Unit] = Right(()).map { case _ => unitRight[Int] }
+                                                                    ^
+t11379a2.scala:20: warning: discarded non-Unit value of type scala.util.Either[Nothing,Unit]
+  def test5: Either[Int, Unit] = Right(()).map { case _ => unitRight }
+                                                           ^
+t11379a2.scala:21: warning: discarded non-Unit value of type scala.util.Either[Nothing,Unit]
+  def test6: Either[Int, Unit] = Right(()).map { _ => unitRight }
+                                                      ^
+t11379a2.scala:22: warning: discarded non-Unit value of type scala.util.Either[Nothing,Unit]
+  def test7: Either[Int, Unit] = Right(()).map(_ => unitRight)
+                                                    ^
+error: No warnings can be incurred under -Werror.
+6 warnings
+1 error

test/files/neg/t11379a2.scala

@@ -0,0 +1,23 @@
+// scalac: -Werror -Wvalue-discard
+object UnitOfTrust {
+  import scala.util._
+
+  private def unitRight[A]: Either[A, Unit] = Right(())
+
+  // fails with:
+  //   discarded non-Unit value
+  def test1: Either[Int, Unit] = Right(Right(()))
+  def test2: Either[Int, Unit] = Right(()).map(_ => unitRight[Int])
+  def test3: Either[Int, Unit] = Right(()).map { case _ => unitRight[Int] }
+
+  // fails with:
+  //   error: type mismatch;
+  //   found   : scala.util.Right[Nothing,Unit]
+  //   required: Unit => Unit
+  //def test4: Either[Int, Unit] = Right(()).map(Right(()))
+
+  // was: compiles just fine
+  def test5: Either[Int, Unit] = Right(()).map { case _ => unitRight }
+  def test6: Either[Int, Unit] = Right(()).map { _ => unitRight }
+  def test7: Either[Int, Unit] = Right(()).map(_ => unitRight)
+}

test/files/neg/t12495.check

@@ -1,12 +1,12 @@
-t12495.scala:4: warning: discarded non-Unit value of type (Int, Int)
-  def g = f(42, 27)
-           ^
 t12495.scala:4: warning: adapted the argument list to expected Unit type: arguments will be discarded
         signature: Function1.apply(v1: T1): R
   given arguments: 42, 27
  after adaptation: Function1((42, 27): Unit)
   def g = f(42, 27)
            ^
+t12495.scala:4: warning: discarded non-Unit value of type (Int, Int)
+  def g = f(42, 27)
+           ^
 error: No warnings can be incurred under -Werror.
 2 warnings
 1 error

test/files/neg/t9847.check

@@ -1,6 +1,12 @@
 t9847.scala:6: warning: discarded non-Unit value of type Int(42)
   def f(): Unit = 42
                   ^
+t9847.scala:9: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
+    1
+    ^
+t9847.scala:13: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
+    1
+    ^
 t9847.scala:14: warning: discarded non-Unit value of type Int(1)
   + 1
   ^
@@ -10,21 +16,6 @@ t9847.scala:18: warning: discarded non-Unit value of type Int
 t9847.scala:21: warning: discarded non-Unit value of type Int
   def j(): Unit = x + 1
                     ^
-t9847.scala:6: warning: a pure expression does nothing in statement position
-  def f(): Unit = 42
-                  ^
-t9847.scala:9: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
-    1
-    ^
-t9847.scala:13: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses
-    1
-    ^
-t9847.scala:14: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
-  + 1
-  ^
-t9847.scala:14: warning: a pure expression does nothing in statement position
-  + 1
-  ^
 t9847.scala:23: warning: a pure expression does nothing in statement position
   class C { 42 }
             ^
@@ -35,5 +26,5 @@ t9847.scala:24: warning: a pure expression does nothing in statement position; m
   class D { 42 ; 17 }
                  ^
 error: No warnings can be incurred under -Werror.
-12 warnings
+9 warnings
 1 error

test/files/run/contrib674.check

@@ -1,6 +1,3 @@
-contrib674.scala:15: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
-    1
-    ^
-contrib674.scala:15: warning: a pure expression does nothing in statement position
+contrib674.scala:15: warning: discarded pure expression does nothing
     1
     ^

test/files/run/names-defaults.check

@@ -22,10 +22,7 @@ names-defaults.scala:280: warning: local val v in method foo is never used
 names-defaults.scala:280: warning: local val u in method foo is never used
   class A2489x2 { def foo(): Unit = { val v = 10; def bar(a: Int = 1, b: Int = 2) = a; bar(); val u = 0 } }
                                                                                                   ^
-names-defaults.scala:269: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
-    spawn(b = { val ttt = 1; ttt }, a = 0)
-                             ^
-names-defaults.scala:269: warning: a pure expression does nothing in statement position
+names-defaults.scala:269: warning: discarded pure expression does nothing
     spawn(b = { val ttt = 1; ttt }, a = 0)
                              ^
 1: @

test/files/run/patmatnew.check

@@ -4,16 +4,10 @@ patmatnew.scala:673: warning: This catches all Throwables. If this is really int
 patmatnew.scala:354: warning: a pure expression does nothing in statement position
         case 1 => "OK"
                   ^
-patmatnew.scala:355: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
+patmatnew.scala:355: warning: discarded pure expression does nothing
         case 2 => assert(false); "KO"
                                  ^
-patmatnew.scala:355: warning: a pure expression does nothing in statement position
-        case 2 => assert(false); "KO"
-                                 ^
-patmatnew.scala:356: warning: multiline expressions might require enclosing parentheses; a value can be silently discarded when Unit is expected
-        case 3 => assert(false); "KO"
-                                 ^
-patmatnew.scala:356: warning: a pure expression does nothing in statement position
+patmatnew.scala:356: warning: discarded pure expression does nothing
         case 3 => assert(false); "KO"
                                  ^
 patmatnew.scala:178: warning: match may not be exhaustive.