Improve explicit Unit check

scala · Jun 9, 2022 · 6b5adcc · 6b5adcc
1 parent bde3157
commit 6b5adcc
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 18 deletions.
diff --git a/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala b/src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
@@ -1780,7 +1780,7 @@ abstract class RefChecks extends Transform {
         && !isUninterestingType(t.tpe)            // bottom types, Unit, Any
         && !treeInfo.isThisTypeResult(t)          // buf += x
         && !treeInfo.isSuperConstrCall(t)         // just a thing
-        && !explicitlyUnit(t)                     // suppressed by explicit expr: Unit
+        && !treeInfo.hasExplicitUnit(t)           // suppressed by explicit expr: Unit
         && !isJavaApplication(t)                  // Java methods are inherently side-effecting
       )
       // begin checkInterestingResultInStatement
@@ -1912,13 +1912,6 @@ abstract class RefChecks extends Transform {
             tree
 
           case blk @ Block(stats, expr) =>
-            // if block is value discard, copy any TypedExpectingUnitAttachment to outermost Apply in case there were implicit args
-            blk match {
-              case Block((inner @ treeInfo.Applied(_, _, _)) :: Nil, Literal(Constant(()))) =>
-                if (inner.exists(explicitlyUnit))
-                  inner.updateAttachment(TypedExpectingUnitAttachment)
-              case _ =>
-            }
             // diagnostic info
             val (count, result0, adapted) =
               expr match {
@@ -1929,7 +1922,7 @@ abstract class RefChecks extends Transform {
             val isMultiline = stats.lengthCompare(1 - count) > 0
 
             def checkPure(t: Tree, supple: Boolean): Unit =
-              if (!explicitlyUnit(t) && treeInfo.isPureExprForWarningPurposes(t)) {
+              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 ""

diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -1112,7 +1112,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
           @inline def tpd(transformed: Tree)    = typed(transformed, mode, pt)
 
           @inline def warnValueDiscard(): Unit =
-            if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !explicitlyUnit(tree))
+            if (!isPastTyper && settings.warnValueDiscard.value && !treeInfo.isThisTypeResult(tree) && !treeInfo.hasExplicitUnit(tree))
               context.warning(tree.pos, "discarded non-Unit value", WarningCategory.WFlagValueDiscard)
           @inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit =
             if (!isPastTyper) {

diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala
@@ -779,6 +779,16 @@ abstract class TreeInfo {
     ((sym ne null) && sym.initialize.isTrait)
   }
 
+  def hasExplicitUnit(tree: Tree): Boolean =
+    explicitlyUnit(tree) || {
+      tree match {
+        case Apply(f, _)           => hasExplicitUnit(f)
+        case TypeApply(f, _)       => hasExplicitUnit(f)
+        case AppliedTypeTree(f, _) => hasExplicitUnit(f)
+        case _                     => false
+      }
+    }
+
   /** Applications in Scala can have one of the following shapes:
    *
    *    1) naked core: Ident(_) or Select(_, _) or basically anything else

diff --git a/test/files/neg/nonunit-if.check b/test/files/neg/nonunit-if.check
@@ -1,6 +1,3 @@
-nonunit-if.scala:53: warning: discarded non-Unit value
-  Future(42): Unit        // nowarn { F(42)(ctx) }: Unit where annot is on F(42), TODO spurious -Wvalue-discard
-        ^
 nonunit-if.scala:60: warning: discarded non-Unit value
     if (!isEmpty) f(a)      // warn
                    ^
@@ -56,5 +53,5 @@ nonunit-if.scala:149: warning: unused value
     while (it.hasNext) it.next()  // warn
                               ^
 error: No warnings can be incurred under -Werror.
-19 warnings
+18 warnings
 1 error
diff --git a/test/files/neg/nonunit-statement.check b/test/files/neg/nonunit-statement.check
@@ -1,6 +1,3 @@
-nonunit-statement.scala:54: warning: discarded non-Unit value
-  Future(42): Unit        // nowarn { F(42)(ctx) }: Unit where annot is on F(42), TODO spurious -Wvalue-discard
-        ^
 nonunit-statement.scala:14: warning: unused value
     improved              // warn
     ^
@@ -38,5 +35,5 @@ nonunit-statement.scala:150: warning: unused value
     while (it.hasNext) it.next()  // warn
                               ^
 error: No warnings can be incurred under -Werror.
-13 warnings
+12 warnings
 1 error