Skip to content

Commit b64aa8e

Browse files
nicolasstuckinicolasstucki
committed
Improve message for discarded pure non-Unit values
Fixes #18722
1 parent f62429b commit b64aa8e

File tree

8 files changed

+84
-18
lines changed

8 files changed

+84
-18
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
202202
case ImplausiblePatternWarningID // erorNumber: 186
203203
case SynchronizedCallOnBoxedClassID // errorNumber: 187
204204
case VarArgsParamCannotBeGivenID // erorNumber: 188
205+
case PureUnitExpressionID // erorNumber: 189
205206

206207
def errorNumber = ordinal - 1
207208

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2391,6 +2391,16 @@ class PureExpressionInStatementPosition(stat: untpd.Tree, val exprOwner: Symbol)
23912391
|It can be removed without changing the semantics of the program. This may indicate an error."""
23922392
}
23932393

2394+
class PureUnitExpression(stat: untpd.Tree, tpe: Type)(using Context)
2395+
extends Message(PureUnitExpressionID) {
2396+
def kind = MessageKind.PotentialIssue
2397+
def msg(using Context) = i"Discarded non-Unit value of type ${tpe.widen}. You may want to use `()`."
2398+
def explain(using Context) =
2399+
i"""As this expression is not of type Unit, it is desugared into `{ $stat; () }`.
2400+
|Here the `$stat` expression is a pure statement that can be discarded.
2401+
|Therefore the expression is effectively equivalent to `()`."""
2402+
}
2403+
23942404
class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Context)
23952405
extends Message(UnqualifiedCallToAnyRefMethodID) {
23962406
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
@@ -4212,7 +4212,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
42124212
// local adaptation makes sure every adapted tree conforms to its pt
42134213
// so will take the code path that decides on inlining
42144214
val tree1 = adapt(tree, WildcardType, locked)
4215-
checkStatementPurity(tree1)(tree, ctx.owner)
4215+
checkStatementPurity(tree1)(tree, ctx.owner, isUnitExpr = true)
42164216
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) {
42174217
report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
42184218
}
@@ -4418,7 +4418,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
44184418
typedExpr(cmp, defn.BooleanType)
44194419
case _ =>
44204420

4421-
private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol)(using Context): Unit =
4421+
private def checkStatementPurity(tree: tpd.Tree)(original: untpd.Tree, exprOwner: Symbol, isUnitExpr: Boolean = false)(using Context): Unit =
44224422
if !tree.tpe.isErroneous
44234423
&& !ctx.isAfterTyper
44244424
&& !tree.isInstanceOf[Inlined]
@@ -4436,6 +4436,8 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
44364436
// sometimes we do not have the original anymore and use the transformed tree instead.
44374437
// But taken together, the two criteria are quite accurate.
44384438
missingArgs(tree, tree.tpe.widen)
4439+
case _ if isUnitExpr =>
4440+
report.warning(PureUnitExpression(original, tree.tpe), original.srcPos)
44394441
case _ =>
44404442
report.warning(PureExpressionInStatementPosition(original, exprOwner), original.srcPos)
44414443

‎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:36:4 ----------------------------------
1+
-- [E189] Potential Issue Warning: tests/neg-custom-args/captures/real-try.scala:36:4 ----------------------------------
22
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
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/real-try.scala:19:4 --------------------------------------

‎tests/neg/i18722.check

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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+
-- [E189] 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)