Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Either lint unused or warn discarded or warn pure #10641

Merged
merged 1 commit into from May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 25 additions & 15 deletions src/compiler/scala/tools/nsc/typechecker/RefChecks.scala
Expand Up @@ -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 {
Expand All @@ -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 = {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 =
expr match {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -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 = {
Expand Down
7 changes: 6 additions & 1 deletion src/reflect/scala/reflect/internal/StdAttachments.scala
Expand Up @@ -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. */
Expand All @@ -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
}
2 changes: 2 additions & 0 deletions src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions 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] _
^
2 changes: 1 addition & 1 deletion test/files/jvm/innerClassAttribute.check
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions 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
13 changes: 13 additions & 0 deletions 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
}
}
9 changes: 9 additions & 0 deletions 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
13 changes: 13 additions & 0 deletions 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
}
}
6 changes: 6 additions & 0 deletions 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
13 changes: 13 additions & 0 deletions 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
}
}
15 changes: 15 additions & 0 deletions 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
13 changes: 13 additions & 0 deletions 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
}
}
38 changes: 16 additions & 22 deletions 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
27 changes: 15 additions & 12 deletions 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
6 changes: 5 additions & 1 deletion 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}
Expand Down Expand Up @@ -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
}