Skip to content

Commit

Permalink
Improve warning for wildcard matching non-nullable types (#21577)
Browse files Browse the repository at this point in the history
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 #21577
  • Loading branch information
HarrisL2 committed Oct 3, 2024
1 parent ba9e59a commit 97faede
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 4 deletions.
7 changes: 7 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,13 @@ extends PatternMatchMsg(MatchCaseOnlyNullWarningID) {
def explain(using Context) = ""
}

class MatchCaseNonNullableWildcardWarning()(using Context)
extends Message(MatchCaseUnreachableID) {
def kind = MessageKind.MatchCaseUnreachable
def msg(using Context) = i"""Unreachable case (if expression is expected to be nullable, consider giving it a nullable type annotation)."""
def explain(using Context) = ""
}

class MatchableWarning(tp: Type, pattern: Boolean)(using Context)
extends TypeMsg(MatchableWarningID) {
def msg(using Context) =
Expand Down
17 changes: 13 additions & 4 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,8 @@ object SpaceEngine {
then project(OrType(selTyp, ConstantType(Constant(null)), soft = false))
else project(selTyp)
)
// println(s"${toUnderlying(m.selector.tpe)}")
// println(s"${selTyp} \n${isNullable} \n${targetSpace}")

var i = 0
val len = cases.length
Expand All @@ -925,11 +927,18 @@ object SpaceEngine {
report.warning(MatchCaseUnreachable(), pat.srcPos)
if pat != EmptyTree // rethrow case of catch uses EmptyTree
&& !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
&& isSubspace(covered, prev)
&& isSubspace(covered, Or(List(prev, Typ(defn.NullType))))
then {
val nullOnly = isNullable && i == len - 1 && isWildcardArg(pat)
val msg = if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
report.warning(msg, pat.srcPos)
if isSubspace(covered, prev) then {
val nullOnly = (isNullable
|| (defn.NullType <:< selTyp)
) && i == len - 1 && isWildcardArg(pat)
val msg =
if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
report.warning(msg, pat.srcPos)
} else {
report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
}
}
deferred = Nil
}
Expand Down
5 changes: 5 additions & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ class CompilationTests {
)
}.checkCompile()

@Test def explicitNullsWarn: Unit = {
implicit val testGroup: TestGroup = TestGroup("explicitNullsWarn")
compileFilesInDir("tests/explicit-nulls/warn", explicitNullsOptions)
}.checkWarnings()

@Test def explicitNullsRun: Unit = {
implicit val testGroup: TestGroup = TestGroup("explicitNullsRun")
compileFilesInDir("tests/explicit-nulls/run", explicitNullsOptions)
Expand Down
12 changes: 12 additions & 0 deletions tests/explicit-nulls/warn/i21577.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:5:9 --------------------------------------------
5 | case _ => println(2) // warn
| ^
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:12:11 ------------------------------------------
12 | case _ => println(2) // warn
| ^
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
-- [E121] Pattern Match Warning: tests/explicit-nulls/warn/i21577.scala:18:11 ------------------------------------------
18 | case _ => println(2) // warn
| ^
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
24 changes: 24 additions & 0 deletions tests/explicit-nulls/warn/i21577.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
def f(s: String) =
val s2 = s.trim()
s2 match
case s3: String => println(1)
case _ => println(2) // warn


def f2(s: String | Null) =
val s2 = s.nn.trim()
s2 match
case s3: String => println(1)
case _ => println(2) // warn

def f3(s: String | Null) =
val s2 = s
s2 match
case s3: String => println(1)
case _ => println(2) // warn

def f4(s: String | Int) =
val s2 = s
s2 match
case s3: String => println(1)
case _ => println(2)

0 comments on commit 97faede

Please sign in to comment.