Fix warning message for matching on redundant nulls

scala · Oct 28, 2024 · 6ba394e · 6ba394e
1 parent a3786a5
commit 6ba394e
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 2 deletions.
diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
@@ -912,6 +912,12 @@ object SpaceEngine {
     && !sel.tpe.widen.isRef(defn.QuotedExprClass)
     && !sel.tpe.widen.isRef(defn.QuotedTypeClass)
 
+  def mayCoverNull(tp: Space)(using Context): Boolean = tp match
+    case Empty => false
+    case Prod(_, _, _) => false
+    case Typ(tp, decomposed) => tp == ConstantType(Constant(null)) 
+    case Or(ss) => ss.exists(mayCoverNull)
+
   def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)"):
     val selTyp = toUnderlying(m.selector.tpe).dealias
     val isNullable = selTyp.isInstanceOf[FlexibleType] || selTyp.classSymbol.isNullableClass
@@ -937,12 +943,16 @@ object SpaceEngine {
                 && !pat.symbol.isAllOf(SyntheticCase, butNot=Method) // ExpandSAMs default cases use SyntheticCase
                 && isSubspace(covered, prev)
             then
-              val nullOnly = isNullable && rest.isEmpty && isWildcardArg(pat)
+              val nullOnly = isNullable && isWildcardArg(pat) && !mayCoverNull(prev)
               val msg = if nullOnly then MatchCaseOnlyNullWarning() else MatchCaseUnreachable()
               report.warning(msg, pat.srcPos)
 
             // in redundancy check, take guard as false in order to soundly approximate
-            val newPrev = if guard.isEmpty then covered :: prevs else prevs
+            val newPrev = if (guard.isEmpty)
+              then if (isWildcardArg(pat)) 
+                then Typ(ConstantType(Constant(null))) :: covered :: prevs
+                else covered :: prevs
+              else prevs
             recur(rest, newPrev, Nil)
 
     recur(m.cases, Nil, Nil)

diff --git a/tests/warn/redundant-null.check b/tests/warn/redundant-null.check
@@ -0,0 +1,52 @@
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:10:7 -----------------------------------------
+10 |  case _: n.type => // warn
+   |       ^^^^^^^^^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:12:7 -----------------------------------------
+12 |  case _ => // warn
+   |       ^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:13:7 -----------------------------------------
+13 |  case _ => // warn
+   |       ^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:18:7 -----------------------------------------
+18 |  case _ => 3 // warn
+   |       ^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:23:7 -----------------------------------------
+23 |  case _: B => // warn
+   |       ^^^^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:24:7 -----------------------------------------
+24 |  case _ => // warn
+   |       ^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:25:7 -----------------------------------------
+25 |  case null => // warn
+   |       ^^^^
+   |       Unreachable case
+-- [E121] Pattern Match Warning: tests/warn/redundant-null.scala:30:7 --------------------------------------------------
+30 |  case _ => // warn
+   |       ^
+   |       Unreachable case except for null (if this is intentional, consider writing case null => instead).
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:31:7 -----------------------------------------
+31 |  case null => // warn
+   |       ^^^^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:32:7 -----------------------------------------
+32 |  case _ => // warn
+   |       ^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:33:7 -----------------------------------------
+33 |  case _ => // warn
+   |       ^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:37:7 -----------------------------------------
+37 |  case _ => println("unreachable") // warn
+   |       ^
+   |       Unreachable case
+-- [E030] Match case Unreachable Warning: tests/warn/redundant-null.scala:41:7 -----------------------------------------
+41 |  case _ => // warn
+   |       ^
+   |       Unreachable case
diff --git a/tests/warn/redundant-null.scala b/tests/warn/redundant-null.scala
@@ -0,0 +1,41 @@
+class A
+class B
+class C
+
+val n = null
+
+def f(s: A) = s match
+  case _: n.type =>
+  case _: A =>
+  case _: n.type => // warn
+  case null =>
+  case _ => // warn
+  case _ => // warn
+
+def f2(s: A | B | C) = s match
+  case _: A => 0
+  case _: C | null | _: B => 1
+  case _ => 3 // warn
+
+def f3(s: A | B) = s match
+  case _: A =>
+  case _ =>
+  case _: B => // warn
+  case _ => // warn
+  case null => // warn
+
+def f4(s: String | Int) = s match
+  case _: Int =>
+  case _: String =>
+  case _ => // warn
+  case null => // warn
+  case _ => // warn
+  case _ => // warn
+
+def f5(x: String) = x match
+  case x => println("catch all")
+  case _ => println("unreachable") // warn
+
+def test(s: String | Null) = s match    
+  case ss => 
+  case _ => // warn