Merge pull request #10641 from som-snytt/tweak/remove-ad-for-mitigation

Either lint unused or warn discarded or warn pure

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.attachments.containsElement(DiscardedValue) && {
+          t.setAttachments(t.attachments.removeElement(DiscardedValue))
+          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 {
@@ -1919,10 +1926,10 @@ abstract class RefChecks extends Transform {
             }
           case _ => t
         }
-        def msg = s"unused value of type ${where.tpe} (add `: Unit` to discard silently)"
+        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 = {
@@ -1967,7 +1974,7 @@ abstract class RefChecks extends Transform {
             for (stat <- body)
               if (!checkInterestingResultInStatement(stat) && treeInfo.isPureExprForWarningPurposes(stat)) {
                 val msg = "a pure expression does nothing in statement position"
-                val clause = if (body.lengthCompare(1) > 0) "; multiline expressions may require enclosing parentheses" else ""
+                val clause = if (body.lengthCompare(2) > 0) "; multiline expressions may require enclosing parentheses" else ""
                 refchecksWarning(stat.pos, s"$msg$clause", WarningCategory.OtherPureStatement)
               }
             validateBaseTypes(currentOwner)
@@ -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.attachments.containsElement(DiscardedExpr)) {
+                    t.setAttachments(t.attachments.removeElement(DiscardedExpr))
+                    "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 = {
               def loop(t: Tree): Unit =

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/jvm/innerClassAttribute.check

@@ -4,7 +4,7 @@ Classes_1.scala:117: warning: a pure expression does nothing in statement positi
 Classes_1.scala:124: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
     ((x: String) => x + "2")
                  ^
-Classes_1.scala:129: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
+Classes_1.scala:129: warning: a pure expression does nothing in statement position
   (s: String) => {
               ^
 Classes_1.scala:130: warning: a pure expression does nothing in statement position; multiline expressions might require enclosing parentheses

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,57 +1,51 @@
-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] (add `: Unit` to discard silently)
+nonunit-if.scala:13: warning: unused value of type scala.concurrent.Future[Int]
     improved              // warn
     ^
-nonunit-if.scala:20: warning: unused value of type String (add `: Unit` to discard silently)
+nonunit-if.scala:20: warning: unused value of type String
     new E().toString      // warn
             ^
-nonunit-if.scala:26: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently)
+nonunit-if.scala:26: warning: unused value of type scala.concurrent.Future[Int]
   Future(42)              // warn
         ^
-nonunit-if.scala:30: warning: unused value of type K (add `: Unit` to discard silently)
+nonunit-if.scala:30: warning: unused value of type K
   copy()                  // warn
       ^
-nonunit-if.scala:37: warning: unused value of type List[Int] (add `: Unit` to discard silently)
+nonunit-if.scala:37: warning: unused value of type List[Int]
   27 +: xs                // warn
      ^
 nonunit-if.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
   null                    // warn for purity
   ^
-nonunit-if.scala:58: warning: unused value of type U (add `: Unit` to discard silently)
+nonunit-if.scala:58: warning: unused value of type U
     if (!isEmpty) f(a)      // warn, check is on
                    ^
-nonunit-if.scala:62: warning: unused value of type Boolean (add `: Unit` to discard silently)
+nonunit-if.scala:62: warning: unused value of type Boolean
       f(a)                  // warn, check is on
        ^
-nonunit-if.scala:73: warning: unused value of type U (add `: Unit` to discard silently)
+nonunit-if.scala:73: warning: unused value of type U
     if (!fellback) action(z)  // warn, check is on
                          ^
-nonunit-if.scala:81: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-if.scala:81: warning: unused value of type Int
       g   // warn, check is on
       ^
-nonunit-if.scala:79: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-if.scala:79: warning: unused value of type Int
       g   // warn block statement
       ^
-nonunit-if.scala:86: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-if.scala:86: warning: unused value of type Int
       g   // warn
       ^
-nonunit-if.scala:84: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-if.scala:84: warning: unused value of type Int
       g   // warn
       ^
-nonunit-if.scala:96: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-if.scala:96: warning: unused value of type Int
     if (b) {          // warn, at least one branch looks interesting
     ^
-nonunit-if.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A] (add `: Unit` to discard silently)
+nonunit-if.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A]
     set += a     // warn because cannot know whether the `set` was supposed to be consumed or assigned
         ^
-nonunit-if.scala:146: warning: unused value of type String (add `: Unit` to discard silently)
+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/nonunit-statement.check

@@ -1,39 +1,42 @@
-nonunit-statement.scala:13: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently)
+nonunit-statement.scala:13: warning: unused value of type scala.concurrent.Future[Int]
     improved              // warn
     ^
-nonunit-statement.scala:20: warning: unused value of type String (add `: Unit` to discard silently)
+nonunit-statement.scala:20: warning: unused value of type String
     new E().toString      // warn
             ^
-nonunit-statement.scala:26: warning: unused value of type scala.concurrent.Future[Int] (add `: Unit` to discard silently)
+nonunit-statement.scala:26: warning: unused value of type scala.concurrent.Future[Int]
   Future(42)              // warn
         ^
-nonunit-statement.scala:30: warning: unused value of type K (add `: Unit` to discard silently)
+nonunit-statement.scala:30: warning: unused value of type K
   copy()                  // warn
       ^
-nonunit-statement.scala:37: warning: unused value of type List[Int] (add `: Unit` to discard silently)
+nonunit-statement.scala:37: warning: unused value of type List[Int]
   27 +: xs                // warn
      ^
 nonunit-statement.scala:44: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
   null                    // warn for purity
   ^
-nonunit-statement.scala:79: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-statement.scala:79: warning: unused value of type Int
       g   // warn block statement
       ^
-nonunit-statement.scala:86: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-statement.scala:86: warning: unused value of type Int
       g   // warn
       ^
-nonunit-statement.scala:84: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-statement.scala:84: warning: unused value of type Int
       g   // warn
       ^
-nonunit-statement.scala:96: warning: unused value of type Int (add `: Unit` to discard silently)
+nonunit-statement.scala:96: warning: unused value of type Int
     if (b) {          // warn, at least one branch looks interesting
     ^
-nonunit-statement.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A] (add `: Unit` to discard silently)
+nonunit-statement.scala:116: warning: unused value of type scala.collection.mutable.LinkedHashSet[A]
     set += a     // warn because cannot know whether the `set` was supposed to be consumed or assigned
         ^
-nonunit-statement.scala:146: warning: unused value of type String (add `: Unit` to discard silently)
+nonunit-statement.scala:146: warning: unused value of type String
     while (it.hasNext) it.next()  // warn
                               ^
+nonunit-statement.scala:202: warning: a pure expression does nothing in statement position
+  null // warn for purity, but <init> does not cause multiline clause
+  ^
 error: No warnings can be incurred under -Werror.
-12 warnings
+13 warnings
 1 error

test/files/neg/nonunit-statement.scala

@@ -1,4 +1,4 @@
-// scalac: -Werror -Wnonunit-statement -Wnonunit-if:false -Wvalue-discard
+//> using options -Werror -Wnonunit-statement -Wnonunit-if:false -Wvalue-discard
 // debug: -Vprint:refchecks -Yprint-trees:format
 import collection.ArrayOps
 import collection.mutable.{ArrayBuilder, LinkedHashSet, ListBuffer}
@@ -197,3 +197,7 @@ class Depends {
     ()
   }
 }
+// some uninteresting expressions may warn for other reasons
+class NotMultiline {
+  null // warn for purity, but <init> does not cause multiline clause
+}