Skip to content

Commit bf50ebd

Browse files
WojciechMazurWojciechMazur
authored
Backport "Improve message for discarded pure non-Unit values" to LTS (#20731)
Backports #18723 to the LTS branch. PR submitted by the release tooling. [skip ci]
2 parents b26ae8e + fb14996 commit bf50ebd

File tree

17 files changed

+155
-24
lines changed

17 files changed

+155
-24
lines changed

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2380,12 +2380,22 @@ class MemberWithSameNameAsStatic()(using Context)
23802380
class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol)(using Context)
23812381
extends Message(PureExpressionInStatementPositionID) {
23822382
def kind = MessageKind.PotentialIssue
2383-
def msg(using Context) = "A pure expression does nothing in statement position; you may be omitting necessary parentheses"
2383+
def msg(using Context) = "A pure expression does nothing in statement position"
23842384
def explain(using Context) =
23852385
i"""The pure expression $stat doesn't have any side effect and its result is not assigned elsewhere.
23862386
|It can be removed without changing the semantics of the program. This may indicate an error."""
23872387
}
23882388

2389+
class PureUnitExpression(stat: untpd.Tree, tpe: Type)(using Context)
2390+
extends Message(PureUnitExpressionID) {
2391+
def kind = MessageKind.PotentialIssue
2392+
def msg(using Context) = i"Discarded non-Unit value of type ${tpe.widen}. You may want to use `()`."
2393+
def explain(using Context) =
2394+
i"""As this expression is not of type Unit, it is desugared into `{ $stat; () }`.
2395+
|Here the `$stat` expression is a pure statement that can be discarded.
2396+
|Therefore the expression is effectively equivalent to `()`."""
2397+
}
2398+
23892399
class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Context)
23902400
extends Message(UnqualifiedCallToAnyRefMethodID) {
23912401
def kind = MessageKind.PotentialIssue

‎compiler/src/dotty/tools/dotc/typer/Typer.scala

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4127,7 +4127,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
41274127
// local adaptation makes sure every adapted tree conforms to its pt
41284128
// so will take the code path that decides on inlining
41294129
val tree1 = adapt(tree, WildcardType, locked)
4130-
checkStatementPurity(tree1)(tree, ctx.owner)
4130+
checkStatementPurity(tree1)(tree, ctx.owner, isUnitExpr = true)
41314131
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) {
41324132
report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
41334133
}
@@ -4424,7 +4424,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
44244424
else false
44254425
}
44264426

4427-
private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol)(using Context): Unit =
4427+
private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol, isUnitExpr: Boolean = false)(using Context): Unit =
44284428
if !tree.tpe.isErroneous
44294429
&& !ctx.isAfterTyper
44304430
&& !tree.isInstanceOf[Inlined]
@@ -4442,6 +4442,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
44424442
// sometimes we do not have the original anymore and use the transformed tree instead.
44434443
// But taken together, the two criteria are quite accurate.
44444444
missingArgs(tree, tree.tpe.widen)
4445+
case _ if isUnitExpr =>
4446+
report.warning(PureUnitExpression(original, tree.tpe), original.srcPos)
44454447
case _ =>
44464448
report.warning(PureExpressionInStatementPosition(original, exprOwner), original.srcPos)
44474449

‎compiler/test-resources/repl/nowarn.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ scala> def f = { 1; 2 }
2727
-- [E129] Potential Issue Warning: ---------------------------------------------
2828
1 | def f = { 1; 2 }
2929
| ^
30-
|A pure expression does nothing in statement position; you may be omitting necessary parentheses
30+
| A pure expression does nothing in statement position
3131
|
3232
| longer explanation available when compiling with `-explain`
3333
def f: Int

‎language-server/test/dotty/tools/languageserver/DiagnosticsTest.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class DiagnosticsTest {
3030
|}"""
3131
.diagnostics(m1,
3232
(m1 to m2,
33-
"A pure expression does nothing in statement position; you may be omitting necessary parentheses",
33+
"A pure expression does nothing in statement position",
3434
Warning, Some(PureExpressionInStatementPositionID)))
3535

3636
@Test def diagnosticWorksheetPureExpression: Unit =

‎tests/neg-custom-args/captures/real-try.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
-- [E129] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:30:4 ----------------------------------
1+
-- [E190] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:30:4 ----------------------------------
22
30 | b.x
33
| ^^^
4-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
4+
| Discarded non-Unit value of type () -> Unit. You may want to use `()`.
55
|
66
| longer explanation available when compiling with `-explain`
77
-- Error: tests/neg-custom-args/captures/real-try.scala:12:2 -----------------------------------------------------------

‎tests/neg-macros/annot-suspend-cycle.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
-- [E129] Potential Issue Warning: tests/neg-macros/annot-suspend-cycle/Macro.scala:7:4 --------------------------------
22
7 | new Foo
33
| ^^^^^^^
4-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
4+
| A pure expression does nothing in statement position
55
|
66
| longer explanation available when compiling with `-explain`
77
Cyclic macro dependencies in tests/neg-macros/annot-suspend-cycle/Test.scala.

‎tests/neg/i18408a.check

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
-- [E103] Syntax Error: tests/neg/i18408a.scala:2:0 --------------------------------------------------------------------
2+
2 |fa(42) // error
3+
|^^
4+
|Illegal start of toplevel definition
5+
|
6+
| longer explanation available when compiling with `-explain`
7+
-- [E190] Potential Issue Warning: tests/neg/i18408a.scala:3:15 --------------------------------------------------------
8+
3 |def test1 = fa(42)
9+
| ^^
10+
| Discarded non-Unit value of type Int. You may want to use `()`.
11+
|
12+
| longer explanation available when compiling with `-explain`
13+
-- [E129] Potential Issue Warning: tests/neg/i18408a.scala:4:16 --------------------------------------------------------
14+
4 |def test2 = fa({42; ()})
15+
| ^^
16+
| A pure expression does nothing in statement position
17+
|
18+
| longer explanation available when compiling with `-explain`

‎tests/neg/i18408a.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
def fa(f: String ?=> Unit): Unit = ???
2+
fa(42) // error
3+
def test1 = fa(42)
4+
def test2 = fa({42; ()})

‎tests/neg/i18408b.check

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
-- [E103] Syntax Error: tests/neg/i18408b.scala:2:0 --------------------------------------------------------------------
2+
2 |fa(42) // error
3+
|^^
4+
|Illegal start of toplevel definition
5+
|
6+
| longer explanation available when compiling with `-explain`
7+
-- [E190] Potential Issue Warning: tests/neg/i18408b.scala:3:15 --------------------------------------------------------
8+
3 |def test1 = fa(42)
9+
| ^^
10+
| Discarded non-Unit value of type Int. You may want to use `()`.
11+
|
12+
| longer explanation available when compiling with `-explain`
13+
-- [E129] Potential Issue Warning: tests/neg/i18408b.scala:4:16 --------------------------------------------------------
14+
4 |def test2 = fa({42; ()})
15+
| ^^
16+
| A pure expression does nothing in statement position
17+
|
18+
| longer explanation available when compiling with `-explain`

‎tests/neg/i18408b.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
def fa(f: => Unit): Unit = ???
2+
fa(42) // error
3+
def test1 = fa(42)
4+
def test2 = fa({42; ()})

‎tests/neg/i18408c.check

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
-- [E103] Syntax Error: tests/neg/i18408c.scala:2:0 --------------------------------------------------------------------
2+
2 |fa(42) // error
3+
|^^
4+
|Illegal start of toplevel definition
5+
|
6+
| longer explanation available when compiling with `-explain`
7+
-- [E190] Potential Issue Warning: tests/neg/i18408c.scala:3:15 --------------------------------------------------------
8+
3 |def test1 = fa(42)
9+
| ^^
10+
| Discarded non-Unit value of type Int. You may want to use `()`.
11+
|
12+
| longer explanation available when compiling with `-explain`
13+
-- [E129] Potential Issue Warning: tests/neg/i18408c.scala:4:16 --------------------------------------------------------
14+
4 |def test2 = fa({42; ()})
15+
| ^^
16+
| A pure expression does nothing in statement position
17+
|
18+
| longer explanation available when compiling with `-explain`

‎tests/neg/i18408c.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
def fa(f: Unit): Unit = ???
2+
fa(42) // error
3+
def test1 = fa(42)
4+
def test2 = fa({42; ()})

‎tests/neg/i18722.check

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
-- [E190] Potential Issue Error: tests/neg/i18722.scala:3:15 -----------------------------------------------------------
2+
3 |def f1: Unit = null // error
3+
| ^^^^
4+
| Discarded non-Unit value of type Null. You may want to use `()`.
5+
|---------------------------------------------------------------------------------------------------------------------
6+
| Explanation (enabled by `-explain`)
7+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
8+
| As this expression is not of type Unit, it is desugared into `{ null; () }`.
9+
| Here the `null` expression is a pure statement that can be discarded.
10+
| Therefore the expression is effectively equivalent to `()`.
11+
---------------------------------------------------------------------------------------------------------------------
12+
-- [E190] Potential Issue Error: tests/neg/i18722.scala:4:15 -----------------------------------------------------------
13+
4 |def f2: Unit = 1 // error
14+
| ^
15+
| Discarded non-Unit value of type Int. You may want to use `()`.
16+
|---------------------------------------------------------------------------------------------------------------------
17+
| Explanation (enabled by `-explain`)
18+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
19+
| As this expression is not of type Unit, it is desugared into `{ 1; () }`.
20+
| Here the `1` expression is a pure statement that can be discarded.
21+
| Therefore the expression is effectively equivalent to `()`.
22+
---------------------------------------------------------------------------------------------------------------------
23+
-- [E190] Potential Issue Error: tests/neg/i18722.scala:5:15 -----------------------------------------------------------
24+
5 |def f3: Unit = "a" // error
25+
| ^^^
26+
| Discarded non-Unit value of type String. You may want to use `()`.
27+
|---------------------------------------------------------------------------------------------------------------------
28+
| Explanation (enabled by `-explain`)
29+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
30+
| As this expression is not of type Unit, it is desugared into `{ "a"; () }`.
31+
| Here the `"a"` expression is a pure statement that can be discarded.
32+
| Therefore the expression is effectively equivalent to `()`.
33+
---------------------------------------------------------------------------------------------------------------------
34+
-- [E190] Potential Issue Error: tests/neg/i18722.scala:7:15 -----------------------------------------------------------
35+
7 |def f4: Unit = i // error
36+
| ^
37+
| Discarded non-Unit value of type Int. You may want to use `()`.
38+
|---------------------------------------------------------------------------------------------------------------------
39+
| Explanation (enabled by `-explain`)
40+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
41+
| As this expression is not of type Unit, it is desugared into `{ i; () }`.
42+
| Here the `i` expression is a pure statement that can be discarded.
43+
| Therefore the expression is effectively equivalent to `()`.
44+
---------------------------------------------------------------------------------------------------------------------

‎tests/neg/i18722.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//> using options -Werror -explain
2+
3+
def f1: Unit = null // error
4+
def f2: Unit = 1 // error
5+
def f3: Unit = "a" // error
6+
val i: Int = 1
7+
def f4: Unit = i // error
8+
val u: Unit = ()
9+
def f5: Unit = u

‎tests/neg/nowarn.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Matching filters for @nowarn or -Wconf:
3232
-- [E129] Potential Issue Warning: tests/neg/nowarn.scala:15:11 --------------------------------------------------------
3333
15 |def t2 = { 1; 2 } // warning (the invalid nowarn doesn't silence anything)
3434
| ^
35-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
35+
| A pure expression does nothing in statement position
3636
|
3737
| longer explanation available when compiling with `-explain`
3838
-- Warning: tests/neg/nowarn.scala:14:8 --------------------------------------------------------------------------------
@@ -43,7 +43,7 @@ Matching filters for @nowarn or -Wconf:
4343
-- [E129] Potential Issue Warning: tests/neg/nowarn.scala:18:12 --------------------------------------------------------
4444
18 |def t2a = { 1; 2 } // warning (invalid nowarn doesn't silence)
4545
| ^
46-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
46+
| A pure expression does nothing in statement position
4747
|
4848
| longer explanation available when compiling with `-explain`
4949
-- Warning: tests/neg/nowarn.scala:17:8 --------------------------------------------------------------------------------

‎tests/neg/spaces-vs-tabs.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@
2828
| The start of this line does not match any of the previous indentation widths.
2929
| Indentation width of current line : 1 tab, 2 spaces
3030
| This falls between previous widths: 1 tab and 1 tab, 4 spaces
31-
-- [E129] Potential Issue Warning: tests/neg/spaces-vs-tabs.scala:13:7 -------------------------------------------------
31+
-- [E190] Potential Issue Warning: tests/neg/spaces-vs-tabs.scala:13:7 -------------------------------------------------
3232
13 | 1
3333
| ^
34-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
34+
| Discarded non-Unit value of type Int. You may want to use `()`.
3535
|
3636
| longer explanation available when compiling with `-explain`

‎tests/neg/syntax-error-recovery.check

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,45 +94,45 @@
9494
| Not found: bam
9595
|
9696
| longer explanation available when compiling with `-explain`
97-
-- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:7:2 -------------------------------------------
97+
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:7:2 -------------------------------------------
9898
6 | 2
9999
7 | }
100100
| ^
101-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
101+
| Discarded non-Unit value of type Null. You may want to use `()`.
102102
|
103103
| longer explanation available when compiling with `-explain`
104-
-- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:15:2 ------------------------------------------
104+
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:15:2 ------------------------------------------
105105
14 | 2
106106
15 | println(baz) // error
107107
| ^
108-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
108+
| Discarded non-Unit value of type Null. You may want to use `()`.
109109
|
110110
| longer explanation available when compiling with `-explain`
111-
-- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:27:2 ------------------------------------------
111+
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:27:2 ------------------------------------------
112112
26 | 2
113113
27 | println(bam) // error
114114
| ^
115-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
115+
| Discarded non-Unit value of type Null. You may want to use `()`.
116116
|
117117
| longer explanation available when compiling with `-explain`
118-
-- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:36:2 ------------------------------------------
118+
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:36:2 ------------------------------------------
119119
35 | 2
120120
36 | }
121121
| ^
122-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
122+
| Discarded non-Unit value of type Null. You may want to use `()`.
123123
|
124124
| longer explanation available when compiling with `-explain`
125-
-- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:44:2 ------------------------------------------
125+
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:44:2 ------------------------------------------
126126
43 | 2
127127
44 | println(baz) // error
128128
| ^
129-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
129+
| Discarded non-Unit value of type Null. You may want to use `()`.
130130
|
131131
| longer explanation available when compiling with `-explain`
132-
-- [E129] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:56:2 ------------------------------------------
132+
-- [E190] Potential Issue Warning: tests/neg/syntax-error-recovery.scala:56:2 ------------------------------------------
133133
55 | 2
134134
56 | println(bam) // error
135135
| ^
136-
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
136+
| Discarded non-Unit value of type Null. You may want to use `()`.
137137
|
138138
| longer explanation available when compiling with `-explain`

0 commit comments

Comments
 (0)