Improve exhaustivity/reachability checking for sub-cases

compiler/src/dotty/tools/dotc/ast/Trees.scala

@@ -627,8 +627,6 @@ object Trees {
   case class CaseDef[+T <: Untyped] private[ast] (pat: Tree[T], guard: Tree[T], body: Tree[T])(implicit @constructorOnly src: SourceFile)
     extends Tree[T] {
     type ThisTree[+T <: Untyped] = CaseDef[T]
-    /** Should this case be considered partial for exhaustivity and unreachability checking */
-    def maybePartial(using Context): Boolean = !guard.isEmpty || body.isInstanceOf[SubMatch[T]]
   }
 
   /** label[tpt]: { expr } */

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

@@ -930,12 +930,117 @@ object SpaceEngine {
     case _                                          => tp
   })
 
+  /** Check if the SubMatch selector references the variable bound by the outer pattern.
+   *
+   *  case x @ _ if x match
+   *       ^ pat    ^ selector
+   *
+   */
+  private object SelectorBoundVar:
+    def unapply(args: (Tree, Tree))(using Context): Boolean =
+      val (selector, pat) = args
+      pat match
+        case b: Bind => selector.symbol == b.symbol
+        case _       => false
+
+  /** Find the index of the parameter in an outer UnApply pattern that directly binds the selector symbol.
+   *
+   *  case Wrapper(c) if c match
+   *               ^ returns Some(0)
+   *
+   */
+  private object SelectorParamIndex:
+    def unapply(args: (Tree, Tree))(using Context): Option[Int] =
+      val (selector, pat) = args
+      unbind(pat) match
+        case UnApply(_, _, pats) =>
+          val idx = pats.indexWhere {
+            case b: Bind => b.symbol == selector.symbol
+            case _ => false
+          }
+          Option.when(idx >= 0)(idx)
+        case _ => None
+
+  /** Find the constructor parameter index corresponding to a field access on the outer pattern's bound var.
+   *
+   *  case x if x.version match     -- returns Some(1) for Document(title, version)
+   *            ^^^^^^^^^ selector
+   *
+   */
+  private object SelectorFieldIndex:
+    def unapply(args: (Tree, Tree))(using Context): Option[Int] =
+      args match
+        case (Select(qual, fieldName), b: Bind) if b.symbol == qual.symbol =>
+          val cls = toUnderlying(qual.tpe).classSymbol
+          if cls.is(CaseClass) && !cls.isOneOf(AbstractOrTrait) then
+            val idx = cls.caseAccessors.indexWhere(_.name == fieldName)
+            Option.when(idx >= 0)(idx)
+          else None
+        case _ => None
+
+  private def narrowProdParam(patSpace: Space, idx: Int, subSpace: Space)(using Context): Option[Space] =
+    def narrow(prod: Prod): Option[Space] =
+      val Prod(tp, unappTp, params) = prod
+      if idx >= params.length then None
+      else
+        val narrowedParam = simplify(intersect(params(idx), subSpace))
+        Some(simplify(Prod(tp, unappTp, params.updated(idx, narrowedParam))))
+    patSpace match
+      case prod @ Prod(tp, unappTp1, _) =>
+        expandCaseClass(tp) match
+          case null => None
+          case Prod(_, unappTp2, _) if isSameUnapply(unappTp1, unappTp2) => narrow(prod)
+      case Typ(tp, _) =>
+        expandCaseClass(tp) match
+          case null    => None
+          case prod    => narrow(prod)
+      case _ => None
+
+  private def projectSubMatch(pat: Tree, sm: SubMatch)(using Context): Option[Space] =
+    val Match(selector, cases) = sm
+
+    val subSpace = Or(cases.map(projectCaseDef))
+    if simplify(subSpace) == Empty then return None  // all sub-cases are guarded or empty; treat outer case as partial
+    def selTyp = toUnderlying(selector.tpe)
+    def patSpace = project(pat)
+
+    (selector, pat) match
+      case SelectorBoundVar()      =>
+        Some(simplify(intersect(patSpace, subSpace)))
+      case SelectorParamIndex(idx) =>
+        narrowProdParam(patSpace, idx, subSpace)
+      case SelectorFieldIndex(idx) =>
+        narrowProdParam(patSpace, idx, subSpace)
+      case _ if simplify(minus(project(selTyp), subSpace)) == Empty =>
+        Some(patSpace)
+      case _ => None
+
+  /** Resolve the space covered by a case and whether it may be partial.
+   *  @return (space, maybePartial) where maybePartial is true when the case
+   *          may not fully cover its pattern space (due to a guard or unresolvable SubMatch).
+   */
+  private def resolveCaseDef(c: CaseDef, projectPat: Tree => Space)(using Context): (Space, Boolean) =
+    def patSpace = projectPat(c.pat)
+
+    if !c.guard.isEmpty then (patSpace, true)
+    else c.body match
+      case sm: SubMatch =>
+        projectSubMatch(c.pat, sm) match
+          case Some(space) => (space, false)
+          case None => (patSpace, true)
+      case _ => (patSpace, false)
+
+  /** Project a single CaseDef to the space it definitely covers */
+  private def projectCaseDef(c: CaseDef)(using Context): Space =
+    val (space, maybePartial) = resolveCaseDef(c, project)
+    if maybePartial then Empty else space
+
   def checkExhaustivity(m: Match)(using Context): Unit = trace(i"checkExhaustivity($m)") {
     val selTyp = toUnderlying(m.selector.tpe.stripUnsafeNulls()).dealias
     val targetSpace = trace(i"targetSpace($selTyp)")(project(selTyp))
 
     val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) =>
-      val space = if x.maybePartial then Empty else trace(i"project(${x.pat})")(project(x.pat))
+      val space = trace(i"projectCaseDef(${x.pat})")(projectCaseDef(x))
       space :: acc
     })
 
@@ -978,7 +1083,7 @@ object SpaceEngine {
       cases match
         case Nil =>
         case (c @ CaseDef(pat, _, _)) :: rest =>
-          val curr = trace(i"project($pat)")(projectPat(pat))
+          val (curr, maybePartial) = resolveCaseDef(c, projectPat)
           val covered = trace("covered")(simplify(intersect(curr, targetSpace)))
           val prev = trace("prev")(simplify(Or(prevs)))
           if prev == Empty && covered == Empty then // defer until a case is reachable
@@ -1003,8 +1108,8 @@ object SpaceEngine {
                 hadNullOnly = true
                 report.warning(MatchCaseOnlyNullWarning(), pat.srcPos)
 
-            // in redundancy check, take guard as false (or potential sub cases as partial) for a sound approximation
-            val newPrev = if c.maybePartial then prevs else covered :: prevs
+            // in redundancy check, take guard as false for a sound approximation
+            val newPrev = if maybePartial then prevs else covered :: prevs
             recur(rest, newPrev, Nil)
 
     recur(m.cases, Nil, Nil)

compiler/test/dotc/scoverage-ignore.excludelist

@@ -80,6 +80,7 @@ quote-function-applied-to.scala
 skolems2.scala
 spurious-overload.scala
 sub-cases-exhaustivity.scala
+sub-cases-reachability.scala
 tailrec.scala
 traitParams.scala
 i25460.scala

tests/pos/sub-cases-exhaustivity.scala

@@ -0,0 +1,117 @@
+//> using options -Werror
+import scala.language.experimental.subCases
+
+case class NotFound(id: String)
+enum AcceptError:
+  case IsCancelled(id: String)
+  case Denial
+case object CatastrophicError
+type Error = NotFound | AcceptError | CatastrophicError.type
+
+import AcceptError.*
+
+def errorToString: Error => String =
+  case NotFound(id) => s"NotFound: $id"
+  case CatastrophicError => s"It is all doom"
+  case ae if ae match
+    case Denial => s"In Denial"
+    case IsCancelled(id) => s"IsCancelled: $id"
+
+def errorToString2: Error => String =
+  case NotFound(id) => s"NotFound: $id"
+  case CatastrophicError => s"It is all doom"
+  case ae if ae match
+    case Denial => s"In Denial"
+  case ea if ea match
+    case IsCancelled(id) => s"IsCancelled: $id"
+
+enum Color:
+  case Red, Green, Blue
+
+def colorName(c: Color): String = c match
+  case c1 if c1 match
+    case Color.Red => "red"
+    case Color.Green => "green"
+    case Color.Blue => "blue"
+
+case class Wrapper(c: Color)
+
+def wrappedColorName(w: Wrapper): String = w match
+  case Wrapper(c) if c match
+    case Color.Red => "red"
+    case Color.Green => "green"
+    case Color.Blue => "blue"
+
+def wrappedColorName2(w: Wrapper): String = w match
+  case Wrapper(c) if c match
+    case Color.Red => "red"
+    case Color.Green => "green"
+  case Wrapper(c) if c match
+    case Color.Blue => "blue"
+
+def colorNameAlt(c: Color): String = c match
+  case c1 if c1 match
+    case Color.Red | Color.Green => "warm"
+    case Color.Blue => "cool"
+
+def wrappedColorBind(w: Wrapper): String = w match
+  case x @ Wrapper(c) if c match
+    case Color.Red => "red"
+    case Color.Green => "green"
+    case Color.Blue => "blue"
+
+def nestedSubcases(c: Color): String = c match
+  case c1 if c1 match
+    case Color.Red => "red"
+    case c2 if c2 match
+      case Color.Green => "green"
+      case Color.Blue => "blue"
+
+enum Version:
+  case Legacy
+  case Stable(major: Int, minor: Int)
+
+case class Document(title: String, version: Version)
+
+def getVersion(d: Option[Document]): String = d match
+  case Some(x) if x.version match
+    case Version.Stable(m, n) => s"$m.$n"
+    case Version.Legacy => "legacy"
+  case None => "none"
+
+sealed trait Shape
+case class Circle(r: Double) extends Shape
+case class Rectangle(w: Double, h: Double) extends Shape
+
+def tupleFirstExhaustive(pair: (Color, Color)): String = pair match
+  case (a, b) if a match
+    case Color.Red   => "red first"
+    case Color.Green => "green first"
+    case Color.Blue  => "blue first"
+
+def tupleSecondExhaustive(pair: (Color, Color)): String = pair match
+  case (a, b) if b match
+    case Color.Red   => "red second"
+    case Color.Green => "green second"
+    case Color.Blue  => "blue second"
+
+def typedBoundExhaustive(s: Shape): String = s match
+  case x: Circle if x match
+    case Circle(r) => s"circle r=$r"
+  case _: Rectangle => "rectangle"
+
+def typedGuardedFallback(s: Shape): String = s match
+  case x: Circle if x match
+    case Circle(r) if r > 0 => "positive circle"
+  case _: Circle    => "other circle"
+  case _: Rectangle => "rectangle"
+
+def fieldAccessExhaustive(d: Document): String = d match
+  case x if x.version match
+    case Version.Stable(m, n) => s"$m.$n"
+    case Version.Legacy => "legacy"
+
+def fieldAccessTypedBound(d: Document): String = d match
+  case x: Document if x.version match
+    case Version.Stable(m, n) => s"$m.$n"
+    case Version.Legacy => "legacy"

tests/pos/sub-cases-reachability.scala

@@ -0,0 +1,29 @@
+//> using options -Werror
+import scala.language.experimental.subCases
+
+enum Color:
+  case Red, Green, Blue
+
+def guardMakesPartial(c: Color, flag: Boolean): String = c match
+  case c1 if flag if c1 match
+    case Color.Red => "red"
+    case Color.Green => "green"
+    case Color.Blue => "blue"
+  case _ => "fallback"
+
+def literalFallback(n: Int): String = n match
+  case x if x match
+    case 1 => "one"
+    case 2 => "two"
+  case _ => "other"
+
+enum Version:
+  case Legacy
+  case Stable
+
+case class Document(version: Version)
+
+def partialFieldAccess(d: Document): String = d match
+  case x if x.version match
+    case Version.Legacy => "legacy"
+  case _ => "other"

tests/warn/sub-cases-exhaustivity.check

@@ -14,7 +14,59 @@
    |  It would fail on pattern case: E.B(_)
    |
    | longer explanation available when compiling with `-explain`
--- [E030] Match case Unreachable Warning: tests/warn/sub-cases-exhaustivity.scala:32:9 ---------------------------------
-32 |    case A(_) => 3 // warn: unreacheable
-   |         ^^^^
-   |         Unreachable case
+-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:40:2 -----------------------------
+40 |  w match // warn: match may not be exhaustive: It would fail on pattern case: Wrapper(Red)
+   |  ^
+   |  match may not be exhaustive.
+   |
+   |  It would fail on pattern case: Wrapper(Red)
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:46:2 -----------------------------
+46 |  c match // warn: match may not be exhaustive: It would fail on pattern case: Color.Blue
+   |  ^
+   |  match may not be exhaustive.
+   |
+   |  It would fail on pattern case: Blue
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:53:2 -----------------------------
+53 |  c match // warn: match may not be exhaustive: It would fail on pattern case: Color.Blue
+   |  ^
+   |  match may not be exhaustive.
+   |
+   |  It would fail on pattern case: Blue
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:61:54 ----------------------------
+61 |def tupleFirstMissing(pair: (Color, Color)): String = pair match // warn
+   |                                                      ^^^^
+   |                                                      match may not be exhaustive.
+   |
+   |                                                      It would fail on pattern case: (Blue, _)
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:66:55 ----------------------------
+66 |def tupleSecondMissing(pair: (Color, Color)): String = pair match // warn
+   |                                                       ^^^^
+   |                                                       match may not be exhaustive.
+   |
+   |                                                       It would fail on pattern case: (_, Blue)
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:71:45 ----------------------------
+71 |def typedGuardedSubcases(s: Shape): String = s match // warn
+   |                                             ^
+   |                                             match may not be exhaustive.
+   |
+   |                                             It would fail on pattern case: Circle(_)
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E029] Pattern Match Exhaustivity Warning: tests/warn/sub-cases-exhaustivity.scala:82:46 ----------------------------
+82 |def fieldAccessMissing(d: Document): String = d match // warn
+   |                                              ^
+   |                                              match may not be exhaustive.
+   |
+   |                                              It would fail on pattern case: Document(_, Legacy)
+   |
+   | longer explanation available when compiling with `-explain`