From 46cd2565e1bc9d5b660bfc0218cdfc5834fa934e Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Mon, 11 Nov 2024 22:10:30 +0100 Subject: [PATCH] Allow discarding "Discarded non-Unit" warnings with `: Unit` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: nmc.borst <1698211+nmcb@users.noreply.github.com> Co-Authored-By: Jędrzej Rochala <48657087+rochala@users.noreply.github.com> --- .../dotty/tools/dotc/reporting/messages.scala | 4 ++-- .../src/dotty/tools/dotc/typer/Typer.scala | 22 +++++++++++++++---- tests/neg-custom-args/captures/real-try.check | 2 +- tests/neg/i13091.check | 22 +++++++------------ tests/neg/i13091.scala | 2 +- tests/neg/i18408a.check | 2 +- tests/neg/i18408b.check | 2 +- tests/neg/i18408c.check | 2 +- tests/neg/spaces-vs-tabs.check | 6 ----- tests/neg/spaces-vs-tabs.scala | 2 +- tests/warn/21557.check | 18 +++++++++++++++ tests/warn/21557.scala | 19 ++++++++++++++++ tests/warn/i18722.check | 8 +++---- tests/warn/nonunit-statement.check | 20 ++++++++--------- tests/warn/warn-value-discard.check | 10 ++++----- 15 files changed, 90 insertions(+), 51 deletions(-) create mode 100644 tests/warn/21557.check create mode 100644 tests/warn/21557.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 3b7fba1cb52d..1126e78ea62a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2469,7 +2469,7 @@ class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol) class PureUnitExpression(stat: untpd.Tree, tpe: Type)(using Context) extends Message(PureUnitExpressionID) { def kind = MessageKind.PotentialIssue - def msg(using Context) = i"Discarded non-Unit value of type ${tpe.widen}. You may want to use `()`." + def msg(using Context) = i"Discarded non-Unit value of type ${tpe.widen}. Add `: Unit` to discard silently." def explain(using Context) = i"""As this expression is not of type Unit, it is desugared into `{ $stat; () }`. |Here the `$stat` expression is a pure statement that can be discarded. @@ -3173,7 +3173,7 @@ class InlinedAnonClassWarning()(using Context) class ValueDiscarding(tp: Type)(using Context) extends Message(ValueDiscardingID): def kind = MessageKind.PotentialIssue - def msg(using Context) = i"discarded non-Unit value of type $tp" + def msg(using Context) = i"discarded non-Unit value of type ${tp.widen}. Add `: Unit` to discard silently." def explain(using Context) = "" class UnusedNonUnitValue(tp: Type)(using Context) diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index bbd78b5dcc2e..3c32e9aea765 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -81,6 +81,9 @@ object Typer { /** Indicates that a definition was copied over from the parent refinements */ val RefinementFromParent = new Property.StickyKey[Unit] + /** Indicates that an expression is explicitly ascribed to [[Unit]] type. */ + val AscribedToUnit = new Property.StickyKey[Unit] + /** An attachment on a Select node with an `apply` field indicating that the `apply` * was inserted by the Typer. */ @@ -1193,7 +1196,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer else tpt val expr1 = if isWildcard then tree.expr.withType(underlyingTreeTpe.tpe) - else typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem) + else + if underlyingTreeTpe.tpe.isRef(defn.UnitClass) then + untpd.unsplice(tree.expr).putAttachment(AscribedToUnit, ()) + typed(tree.expr, underlyingTreeTpe.tpe.widenSkolem) assignType(cpy.Typed(tree)(expr1, tpt), underlyingTreeTpe) .withNotNullInfo(expr1.notNullInfo) } @@ -3377,7 +3383,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer else if (ctx.mode.is(Mode.Pattern)) typedUnApply(cpy.Apply(tree)(op, l :: r :: Nil), pt) else { - val app = typedApply(desugar.binop(l, op, r), pt) + val app = typedApply(desugar.binop(l, op, r).withAttachmentsFrom(tree), pt) if op.name.isRightAssocOperatorName && !ctx.mode.is(Mode.QuotedExprPattern) then val defs = new mutable.ListBuffer[Tree] def lift(app: Tree): Tree = (app: @unchecked) match @@ -4581,9 +4587,14 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // so will take the code path that decides on inlining val tree1 = adapt(tree, WildcardType, locked) checkStatementPurity(tree1)(tree, ctx.owner, isUnitExpr = true) - if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.Whas.valueDiscard && !isThisTypeResult(tree)) { + + if ctx.settings.Whas.valueDiscard + && !ctx.isAfterTyper + && !tree.isInstanceOf[Inlined] + && !isThisTypeResult(tree) + && !tree.hasAttachment(AscribedToUnit) then report.warning(ValueDiscarding(tree.tpe), tree.srcPos) - } + return tpd.Block(tree1 :: Nil, unitLiteral) } @@ -4839,6 +4850,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer // sometimes we do not have the original anymore and use the transformed tree instead. // But taken together, the two criteria are quite accurate. missingArgs(tree, tree.tpe.widen) + case _ if tree.hasAttachment(AscribedToUnit) => + // The tree was ascribed to `Unit` explicitly to silence the warning. + () case _ if isUnitExpr => report.warning(PureUnitExpression(original, tree.tpe), original.srcPos) case _ => diff --git a/tests/neg-custom-args/captures/real-try.check b/tests/neg-custom-args/captures/real-try.check index 7f8ab50bc222..6df092885384 100644 --- a/tests/neg-custom-args/captures/real-try.check +++ b/tests/neg-custom-args/captures/real-try.check @@ -1,7 +1,7 @@ -- [E190] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:38:4 ---------------------------------- 38 | b.x | ^^^ - | Discarded non-Unit value of type () -> Unit. You may want to use `()`. + | Discarded non-Unit value of type () -> Unit. Add `: Unit` to discard silently. | | longer explanation available when compiling with `-explain` -- Error: tests/neg-custom-args/captures/real-try.scala:14:2 ----------------------------------------------------------- diff --git a/tests/neg/i13091.check b/tests/neg/i13091.check index 5cd793a9cfcb..bf8207d85e51 100644 --- a/tests/neg/i13091.check +++ b/tests/neg/i13091.check @@ -1,15 +1,9 @@ --- [E190] Potential Issue Warning: tests/neg/i13091.scala:7:17 --------------------------------------------------------- -7 |def test: Unit = new Foo // error: class Foo is marked @experimental ... - | ^^^^^^^ - | Discarded non-Unit value of type Foo. You may want to use `()`. +-- Error: tests/neg/i13091.scala:7:16 ---------------------------------------------------------------------------------- +7 |def test = (new Foo): Unit // error: class Foo is marked @experimental ... + | ^^^ + | class Foo is marked @experimental | - | longer explanation available when compiling with `-explain` --- Error: tests/neg/i13091.scala:7:21 ---------------------------------------------------------------------------------- -7 |def test: Unit = new Foo // error: class Foo is marked @experimental ... - | ^^^ - | class Foo is marked @experimental - | - | Experimental definition may only be used under experimental mode: - | 1. in a definition marked as @experimental, or - | 2. an experimental feature is imported at the package level, or - | 3. compiling with the -experimental compiler flag. + | Experimental definition may only be used under experimental mode: + | 1. in a definition marked as @experimental, or + | 2. an experimental feature is imported at the package level, or + | 3. compiling with the -experimental compiler flag. diff --git a/tests/neg/i13091.scala b/tests/neg/i13091.scala index 549fdf6d0fae..8ee77efa37c1 100644 --- a/tests/neg/i13091.scala +++ b/tests/neg/i13091.scala @@ -4,4 +4,4 @@ import annotation.experimental @experimental class Foo -def test: Unit = new Foo // error: class Foo is marked @experimental ... +def test = (new Foo): Unit // error: class Foo is marked @experimental ... diff --git a/tests/neg/i18408a.check b/tests/neg/i18408a.check index ff278e6fe5cb..2fb701f90712 100644 --- a/tests/neg/i18408a.check +++ b/tests/neg/i18408a.check @@ -7,7 +7,7 @@ -- [E190] Potential Issue Warning: tests/neg/i18408a.scala:3:15 -------------------------------------------------------- 3 |def test1 = fa(42) | ^^ - | Discarded non-Unit value of type Int. You may want to use `()`. + | Discarded non-Unit value of type Int. Add `: Unit` to discard silently. | | longer explanation available when compiling with `-explain` -- [E129] Potential Issue Warning: tests/neg/i18408a.scala:4:16 -------------------------------------------------------- diff --git a/tests/neg/i18408b.check b/tests/neg/i18408b.check index 7c72833fe5ad..069c8345742b 100644 --- a/tests/neg/i18408b.check +++ b/tests/neg/i18408b.check @@ -7,7 +7,7 @@ -- [E190] Potential Issue Warning: tests/neg/i18408b.scala:3:15 -------------------------------------------------------- 3 |def test1 = fa(42) | ^^ - | Discarded non-Unit value of type Int. You may want to use `()`. + | Discarded non-Unit value of type Int. Add `: Unit` to discard silently. | | longer explanation available when compiling with `-explain` -- [E129] Potential Issue Warning: tests/neg/i18408b.scala:4:16 -------------------------------------------------------- diff --git a/tests/neg/i18408c.check b/tests/neg/i18408c.check index 078f42bb0006..994e6b499371 100644 --- a/tests/neg/i18408c.check +++ b/tests/neg/i18408c.check @@ -7,7 +7,7 @@ -- [E190] Potential Issue Warning: tests/neg/i18408c.scala:3:15 -------------------------------------------------------- 3 |def test1 = fa(42) | ^^ - | Discarded non-Unit value of type Int. You may want to use `()`. + | Discarded non-Unit value of type Int. Add `: Unit` to discard silently. | | longer explanation available when compiling with `-explain` -- [E129] Potential Issue Warning: tests/neg/i18408c.scala:4:16 -------------------------------------------------------- diff --git a/tests/neg/spaces-vs-tabs.check b/tests/neg/spaces-vs-tabs.check index f8374618f0fd..ce55267b0cce 100644 --- a/tests/neg/spaces-vs-tabs.check +++ b/tests/neg/spaces-vs-tabs.check @@ -28,9 +28,3 @@ | The start of this line does not match any of the previous indentation widths. | Indentation width of current line : 1 tab, 2 spaces | This falls between previous widths: 1 tab and 1 tab, 4 spaces --- [E190] Potential Issue Warning: tests/neg/spaces-vs-tabs.scala:13:7 ------------------------------------------------- -13 | 1 - | ^ - | Discarded non-Unit value of type Int. You may want to use `()`. - | - | longer explanation available when compiling with `-explain` diff --git a/tests/neg/spaces-vs-tabs.scala b/tests/neg/spaces-vs-tabs.scala index 4f48d784eb7d..eafc5209ca47 100644 --- a/tests/neg/spaces-vs-tabs.scala +++ b/tests/neg/spaces-vs-tabs.scala @@ -10,6 +10,6 @@ object Test: object Test2: if true then - 1 + () else 2 // error diff --git a/tests/warn/21557.check b/tests/warn/21557.check new file mode 100644 index 000000000000..1753960b852a --- /dev/null +++ b/tests/warn/21557.check @@ -0,0 +1,18 @@ +-- [E190] Potential Issue Warning: tests/warn/21557.scala:9:16 --------------------------------------------------------- +9 | val x: Unit = 1 + 1 // warn + | ^^^^^ + | Discarded non-Unit value of type Int. Add `: Unit` to discard silently. + | + | longer explanation available when compiling with `-explain` +-- [E176] Potential Issue Warning: tests/warn/21557.scala:10:2 --------------------------------------------------------- +10 | 1 + 1 // warn + | ^^^^^ + | unused value of type (2 : Int) +-- [E175] Potential Issue Warning: tests/warn/21557.scala:15:52 -------------------------------------------------------- +15 | val x1: Unit = new Assertion("another").shouldPass() // warn (enabled by -Wvalue-discard) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | discarded non-Unit value of type Assertion. Add `: Unit` to discard silently. +-- [E176] Potential Issue Warning: tests/warn/21557.scala:16:41 -------------------------------------------------------- +16 | new Assertion("yet another").shouldPass() // warn (enabled by -Wnonunit-statement) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | unused value of type Assertion diff --git a/tests/warn/21557.scala b/tests/warn/21557.scala new file mode 100644 index 000000000000..ed7028b6a48a --- /dev/null +++ b/tests/warn/21557.scala @@ -0,0 +1,19 @@ +//> using options -Wvalue-discard -Wnonunit-statement + +class Assertion(assert: => Any): + def shouldPass(): Assertion = ??? + +def test: Unit = + 1 + 1: Unit + (1 + 1): Unit + val x: Unit = 1 + 1 // warn + 1 + 1 // warn + val y: Int = 1 + 1 + + new Assertion("").shouldPass(): Unit + (new Assertion("").shouldPass()): Unit + val x1: Unit = new Assertion("another").shouldPass() // warn (enabled by -Wvalue-discard) + new Assertion("yet another").shouldPass() // warn (enabled by -Wnonunit-statement) + val y1: Assertion = new Assertion("another other").shouldPass() + + () diff --git a/tests/warn/i18722.check b/tests/warn/i18722.check index 4fae07e15204..4b32cb418167 100644 --- a/tests/warn/i18722.check +++ b/tests/warn/i18722.check @@ -1,7 +1,7 @@ -- [E190] Potential Issue Warning: tests/warn/i18722.scala:3:15 -------------------------------------------------------- 3 |def f1: Unit = null // warn | ^^^^ - | Discarded non-Unit value of type Null. You may want to use `()`. + | Discarded non-Unit value of type Null. Add `: Unit` to discard silently. |--------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -12,7 +12,7 @@ -- [E190] Potential Issue Warning: tests/warn/i18722.scala:4:15 -------------------------------------------------------- 4 |def f2: Unit = 1 // warn | ^ - | Discarded non-Unit value of type Int. You may want to use `()`. + | Discarded non-Unit value of type Int. Add `: Unit` to discard silently. |--------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -23,7 +23,7 @@ -- [E190] Potential Issue Warning: tests/warn/i18722.scala:5:15 -------------------------------------------------------- 5 |def f3: Unit = "a" // warn | ^^^ - | Discarded non-Unit value of type String. You may want to use `()`. + | Discarded non-Unit value of type String. Add `: Unit` to discard silently. |--------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -34,7 +34,7 @@ -- [E190] Potential Issue Warning: tests/warn/i18722.scala:7:15 -------------------------------------------------------- 7 |def f4: Unit = i // warn | ^ - | Discarded non-Unit value of type Int. You may want to use `()`. + | Discarded non-Unit value of type Int. Add `: Unit` to discard silently. |--------------------------------------------------------------------------------------------------------------------- | Explanation (enabled by `-explain`) |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/tests/warn/nonunit-statement.check b/tests/warn/nonunit-statement.check index 742a9fe911e8..7e7939e1c163 100644 --- a/tests/warn/nonunit-statement.check +++ b/tests/warn/nonunit-statement.check @@ -27,15 +27,15 @@ -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:58:19 -------------------------------------------- 58 | if (!isEmpty) f(a) // warn (if) | ^^^^ - | discarded non-Unit value of type U + | discarded non-Unit value of type U. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:62:7 --------------------------------------------- 62 | f(a) // warn (if) | ^^^^ - | discarded non-Unit value of type Boolean + | discarded non-Unit value of type Boolean. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:73:25 -------------------------------------------- 73 | if (!fellback) action(z) // warn (if) | ^^^^^^^^^ - | discarded non-Unit value of type U + | discarded non-Unit value of type U. Add `: Unit` to discard silently. -- [E176] Potential Issue Warning: tests/warn/nonunit-statement.scala:79:6 --------------------------------------------- 79 | g // warn block statement | ^ @@ -43,7 +43,7 @@ -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:81:6 --------------------------------------------- 81 | g // warn (if) | ^ - | discarded non-Unit value of type (g : => Int) + | discarded non-Unit value of type Int. Add `: Unit` to discard silently. -- [E176] Potential Issue Warning: tests/warn/nonunit-statement.scala:84:6 --------------------------------------------- 84 | g // warn | ^ @@ -51,7 +51,7 @@ -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:86:6 --------------------------------------------- 86 | g // warn | ^ - | discarded non-Unit value of type (g : => Int) + | discarded non-Unit value of type Int. Add `: Unit` to discard silently. -- [E176] Potential Issue Warning: tests/warn/nonunit-statement.scala:96:4 --------------------------------------------- 96 | if (b) { // warn, at least one branch looks interesting | ^ @@ -70,20 +70,20 @@ -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:126:37 ------------------------------------------- 126 | if (start.length != 0) jsb.append(start) // warn (value-discard) | ^^^^^^^^^^^^^^^^^ - | discarded non-Unit value of type StringBuilder + | discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:132:18 ------------------------------------------- 132 | jsb.append(it.next()) // warn (value-discard) | ^^^^^^^^^^^^^^^^^^^^^ - | discarded non-Unit value of type StringBuilder + | discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:135:35 ------------------------------------------- 135 | if (end.length != 0) jsb.append(end) // warn (value-discard) | ^^^^^^^^^^^^^^^ - | discarded non-Unit value of type StringBuilder + | discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:141:14 ------------------------------------------- 141 | b.append(it.next()) // warn (value-discard) | ^^^^^^^^^^^^^^^^^^^ - | discarded non-Unit value of type StringBuilder + | discarded non-Unit value of type StringBuilder. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/nonunit-statement.scala:146:30 ------------------------------------------- 146 | while (it.hasNext) it.next() // warn | ^^^^^^^^^ - | discarded non-Unit value of type String + | discarded non-Unit value of type String. Add `: Unit` to discard silently. diff --git a/tests/warn/warn-value-discard.check b/tests/warn/warn-value-discard.check index ca6fedb29053..dcd6d62c00e0 100644 --- a/tests/warn/warn-value-discard.check +++ b/tests/warn/warn-value-discard.check @@ -1,20 +1,20 @@ -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:27:36 ------------------------------------------- 27 | mutable.Set.empty[String].remove("") // warn | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | discarded non-Unit value of type Boolean + | discarded non-Unit value of type Boolean. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:39:41 ------------------------------------------- 39 | mutable.Set.empty[String].subtractOne("") // warn | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | discarded non-Unit value of type scala.collection.mutable.Set[String] + | discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:59:4 -------------------------------------------- 59 | mutable.Set.empty[String] += "" // warn | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | discarded non-Unit value of type scala.collection.mutable.Set[String] + | discarded non-Unit value of type scala.collection.mutable.Set[String]. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:15:35 ------------------------------------------- 15 | firstThing().map(_ => secondThing()) // warn | ^^^^^^^^^^^^^ - | discarded non-Unit value of type Either[Failed, Unit] + | discarded non-Unit value of type Either[Failed, Unit]. Add `: Unit` to discard silently. -- [E175] Potential Issue Warning: tests/warn/warn-value-discard.scala:18:35 ------------------------------------------- 18 | firstThing().map(_ => secondThing()) // warn | ^^^^^^^^^^^^^ - | discarded non-Unit value of type Either[Failed, Unit] + | discarded non-Unit value of type Either[Failed, Unit]. Add `: Unit` to discard silently.