Skip to content

Commit 97faede

Browse files
committed
Improve warning for wildcard matching non-nullable types (scala#21577)
Adds a more detailed warning message when a wildcard case is unreachable on an non-nullable type to suggest adding a nullable type annotation. Fixes scala#21577
1 parent ba9e59a commit 97faede

File tree

5 files changed

+61
-4
lines changed

5 files changed

+61
-4
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,13 @@ extends PatternMatchMsg(MatchCaseOnlyNullWarningID) {
935935
def explain(using Context) = ""
936936
}
937937

938+
class MatchCaseNonNullableWildcardWarning()(using Context)
939+
extends Message(MatchCaseUnreachableID) {
940+
def kind = MessageKind.MatchCaseUnreachable
941+
def msg(using Context) = i"""Unreachable case (if expression is expected to be nullable, consider giving it a nullable type annotation)."""
942+
def explain(using Context) = ""
943+
}
944+
938945
class MatchableWarning(tp: Type, pattern: Boolean)(using Context)
939946
extends TypeMsg(MatchableWarningID) {
940947
def msg(using Context) =

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -903,6 +903,8 @@ object SpaceEngine {
903903
then project(OrType(selTyp, ConstantType(Constant(null)), soft = false))
904904
else project(selTyp)
905905
)
906+
// println(s"${toUnderlying(m.selector.tpe)}")
907+
// println(s"${selTyp} \n${isNullable} \n${targetSpace}")
906908

907909
var i = 0
908910
val len = cases.length
@@ -925,11 +927,18 @@ object SpaceEngine {
925927
report.warning(MatchCaseUnreachable(), pat.srcPos)
926928
if pat != EmptyTree // rethrow case of catch uses EmptyTree
927929
&& !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
928-
&& isSubspace(covered, prev)
930+
&& isSubspace(covered, Or(List(prev, Typ(defn.NullType))))
929931
then {
930-
val nullOnly = isNullable && i == len - 1 && isWildcardArg(pat)
931-
val msg = if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
932-
report.warning(msg, pat.srcPos)
932+
if isSubspace(covered, prev) then {
933+
val nullOnly = (isNullable
934+
|| (defn.NullType <:< selTyp)
935+
) && i == len - 1 && isWildcardArg(pat)
936+
val msg =
937+
if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
938+
report.warning(msg, pat.srcPos)
939+
} else {
940+
report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
941+
}
933942
}
934943
deferred = Nil
935944
}

compiler/test/dotty/tools/dotc/CompilationTests.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,11 @@ class CompilationTests {
212212
)
213213
}.checkCompile()
214214

215+
@Test def explicitNullsWarn: Unit = {
216+
implicit val testGroup: TestGroup = TestGroup("explicitNullsWarn")
217+
compileFilesInDir("tests/explicit-nulls/warn", explicitNullsOptions)
218+
}.checkWarnings()
219+
215220
@Test def explicitNullsRun: Unit = {
216221
implicit val testGroup: TestGroup = TestGroup("explicitNullsRun")
217222
compileFilesInDir("tests/explicit-nulls/run", explicitNullsOptions)
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:5:9 --------------------------------------------
2+
5 | case _ => println(2) // warn
3+
| ^
4+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
5+
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:11 ------------------------------------------
6+
12 | case _ => println(2) // warn
7+
| ^
8+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
9+
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:18:11 ------------------------------------------
10+
18 | case _ => println(2) // warn
11+
| ^
12+
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
def f(s: String) =
2+
val s2 = s.trim()
3+
s2 match
4+
case s3: String => println(1)
5+
case _ => println(2) // warn
6+
7+
8+
def f2(s: String | Null) =
9+
val s2 = s.nn.trim()
10+
s2 match
11+
case s3: String => println(1)
12+
case _ => println(2) // warn
13+
14+
def f3(s: String | Null) =
15+
val s2 = s
16+
s2 match
17+
case s3: String => println(1)
18+
case _ => println(2) // warn
19+
20+
def f4(s: String | Int) =
21+
val s2 = s
22+
s2 match
23+
case s3: String => println(1)
24+
case _ => println(2)

0 commit comments

Comments
 (0)