Skip to content

Commit

Permalink
Add -Wnonunit-statement
Browse files Browse the repository at this point in the history
Warn about any interesting value in statement position.

Generalizes the current warning for "pure" expressions.
  • Loading branch information
som-snytt committed Jan 5, 2022
1 parent 4b2151c commit abe01d8
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/compiler/scala/tools/nsc/settings/Warnings.scala
Expand Up @@ -113,6 +113,7 @@ trait Warnings {
)
) withAbbreviation "-Ywarn-macros"
val warnDeadCode = BooleanSetting("-Wdead-code", "Warn when dead code is identified.") withAbbreviation "-Ywarn-dead-code"
val warnNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
val warnValueDiscard = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.") withAbbreviation "-Ywarn-value-discard"
val warnNumericWiden = BooleanSetting("-Wnumeric-widen", "Warn when numerics are widened.") withAbbreviation "-Ywarn-numeric-widen"
val warnOctalLiteral = BooleanSetting("-Woctal-literal", "Warn on obsolete octal syntax.") withAbbreviation "-Ywarn-octal-literal"
Expand Down
17 changes: 12 additions & 5 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Expand Up @@ -1723,6 +1723,15 @@ abstract class RefChecks extends Transform {
if (!t.isDef && t.hasSymbolField && t.symbol.isTermMacro)
reporter.error(t.pos, "macro has not been expanded")

private def checkInterestingResultInStatement(t: Tree): Boolean =
settings.warnNonUnitStatement && !t.isDef && !treeInfo.isPureDef(t) &&
!(t.tpe =:= UnitTpe) && !t.tpe.typeSymbol.isBottomClass && !treeInfo.isThisTypeResult(t) &&
(t.symbol == null || !t.symbol.isConstructor) && {
val msg = "expression in statement position is not assigned"
refchecksWarning(t.pos, msg, WarningCategory.OtherPureStatement)
true
}

override def transform(tree: Tree): Tree = {
val savedLocalTyper = localTyper
val savedCurrentApplication = currentApplication
Expand Down Expand Up @@ -1762,14 +1771,12 @@ abstract class RefChecks extends Transform {

case Template(parents, self, body) =>
localTyper = localTyper.atOwner(tree, currentOwner)
for (stat <- body) {
if (treeInfo.isPureExprForWarningPurposes(stat)) {
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 ""
refchecksWarning(stat.pos, s"$msg$clause", WarningCategory.OtherPureStatement)
}
}

validateBaseTypes(currentOwner)
checkOverloadedRestrictions(currentOwner, currentOwner)
// scala/bug#7870 default getters for constructors live in the companion module
Expand Down Expand Up @@ -1857,7 +1864,7 @@ abstract class RefChecks extends Transform {
refchecksWarning(t.pos, text, WarningCategory.OtherPureStatement)
}
// sanity check block for unintended expr placement
stats.foreach(checkPure(_, supple = false))
stats.foreach { t => if (!checkInterestingResultInStatement(t)) checkPure(t, supple = false) }
if (result0.nonEmpty) checkPure(result0, supple = true)

def checkImplicitlyAdaptedBlockResult(t: Tree): Unit =
Expand Down
10 changes: 3 additions & 7 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -1111,13 +1111,9 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
@inline def tpdPos(transformed: Tree) = typedPos(tree.pos, mode, pt)(transformed)
@inline def tpd(transformed: Tree) = typed(transformed, mode, pt)

@inline def warnValueDiscard(): Unit = if (!isPastTyper && settings.warnValueDiscard) {
def isThisTypeResult = (tree, tree.tpe) match {
case (Apply(Select(receiver, _), _), SingleType(_, sym)) => sym == receiver.symbol
case _ => false
}
if (!isThisTypeResult && !explicitlyUnit(tree)) context.warning(tree.pos, "discarded non-Unit value", WarningCategory.WFlagValueDiscard)
}
@inline def warnValueDiscard(): Unit =
if (!isPastTyper && settings.warnValueDiscard && !treeInfo.isThisTypeResult(tree) && !explicitlyUnit(tree))
context.warning(tree.pos, "discarded non-Unit value", WarningCategory.WFlagValueDiscard)
@inline def warnNumericWiden(tpSym: Symbol, ptSym: Symbol): Unit =
if (!isPastTyper) {
val isInharmonic = (
Expand Down
11 changes: 11 additions & 0 deletions src/reflect/scala/reflect/internal/TreeInfo.scala
Expand Up @@ -351,6 +351,17 @@ abstract class TreeInfo {
case _ => false
}

/** Is tree an application with result `this.type`?
*/
def isThisTypeResult(tree: Tree): Boolean = tree match {
case Apply(Select(receiver, _), _) =>
tree.tpe match {
case SingleType(_, sym) => sym == receiver.symbol
case _ => false
}
case _ => false
}

/**
* Named arguments can transform a constructor call into a block, e.g.
* <init>(b = foo, a = bar)
Expand Down
21 changes: 21 additions & 0 deletions test/files/neg/nonunit-statement.check
@@ -0,0 +1,21 @@
nonunit-statement.scala:13: warning: expression in statement position is not assigned
improved // warn
^
nonunit-statement.scala:20: warning: expression in statement position is not assigned
new E().toString // warn
^
nonunit-statement.scala:26: warning: expression in statement position is not assigned
Future(42) // warn
^
nonunit-statement.scala:29: warning: expression in statement position is not assigned
copy() // warn
^
nonunit-statement.scala:36: warning: expression in statement position is not assigned
27 +: xs // warn
^
nonunit-statement.scala:39: warning: a pure expression does nothing in statement position; multiline expressions may require enclosing parentheses
null // warn for purity
^
error: No warnings can be incurred under -Werror.
6 warnings
1 error
41 changes: 41 additions & 0 deletions test/files/neg/nonunit-statement.scala
@@ -0,0 +1,41 @@

// scalac: -Werror -Wnonunit-statement

package tpolecat

import concurrent._

class C {
import ExecutionContext.Implicits._
def c = {
def improved = Future(42)
def stale = Future(27)
improved // warn
stale
}
}
class D {
def d = {
class E
new E().toString // warn
new E().toString * 2
}
}
class F {
import ExecutionContext.Implicits._
Future(42) // warn
}
case class K(s: String) {
copy() // warn
}
class Mutate {
import collection.mutable._
val b = ListBuffer.empty[Int]
b += 42 // nowarn
val xs = List(42)
27 +: xs // warn
}
class WhoCares {
null // warn for purity
??? // nowarn for impurity
}

0 comments on commit abe01d8

Please sign in to comment.