Skip to content

Commit b982f8f

Browse files
WojciechMazurWojciechMazur
committed
Improve message for discarded pure non-Unit values
Fixes #18408 Fixes #18722 [Cherry-picked 7451a18][modified]
1 parent b26ae8e commit b982f8f

File tree

13 files changed

+150
-19
lines changed

13 files changed

+150
-19
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2386,6 +2386,16 @@ class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol)
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

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

Lines changed: 3 additions & 3 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 ----------------------------------
2-
30 | b.x
1+
-- [E190] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:36:4 ----------------------------------
2+
36 | 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/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; you may be omitting necessary parentheses
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; you may be omitting necessary parentheses
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; you may be omitting necessary parentheses
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/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)