Improve diagnostic for refutable extractors in pattern bindings

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -795,26 +795,55 @@ trait Checking {
 
   /** Check that pattern `pat` is irrefutable for scrutinee type `sel.tpe`.
    *  This means `sel` is either marked @unchecked or `sel.tpe` conforms to the
-   *  pattern's type. If pattern is an UnApply, do the check recursively.
+   *  pattern's type. If pattern is an UnApply, also check that the extractor is
+   *  irrefutable, and do the check recursively.
    */
   def checkIrrefutable(sel: Tree, pat: Tree, isPatDef: Boolean)(using Context): Boolean = {
     val pt = sel.tpe
 
-    def fail(pat: Tree, pt: Type): Boolean = {
-      var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
-      if (!pat.tpe.isSingleton) reportedPt = reportedPt.widen
-      val problem = if (pat.tpe <:< reportedPt) "is more specialized than" else "does not match"
-      val fix = if (isPatDef) "adding `: @unchecked` after the expression" else "writing `case ` before the full pattern"
-      val pos = if (isPatDef) sel.srcPos else pat.srcPos
+    enum Reason:
+      case NonConforming, RefutableExtractor
+
+    def fail(pat: Tree, pt: Type, reason: Reason): Boolean = {
+      import Reason._
+      val message = reason match
+        case NonConforming =>
+          var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
+          if !pat.tpe.isSingleton then reportedPt = reportedPt.widen
+          val problem = if pat.tpe <:< reportedPt then "is more specialized than" else "does not match"
+          ex"pattern's type ${pat.tpe} $problem the right hand side expression's type $reportedPt"
+        case RefutableExtractor =>
+          val extractor =
+            val UnApply(fn, _, _) = pat: @unchecked
+            fn match
+              case Select(id, _) => id
+              case TypeApply(Select(id, _), _) => id
+          em"pattern binding uses refutable extractor `$extractor`"
+
+      val fix =
+        if isPatDef then "adding `: @unchecked` after the expression"
+        else "adding the `case` keyword before the full pattern"
+      val addendum =
+        if isPatDef then "may result in a MatchError at runtime"
+        else "will result in a filtering for expression (using `withFilter`)"
+      val usage = reason match
+        case NonConforming => "the narrowing"
+        case RefutableExtractor => "this usage"
+      val pos =
+        if isPatDef then reason match
+          case NonConforming => sel.srcPos
+          case RefutableExtractor => pat.source.atSpan(pat.span union sel.span)
+        else pat.srcPos
       report.warning(
-        ex"""pattern's type ${pat.tpe} $problem the right hand side expression's type $reportedPt
+        em"""$message
             |
-            |If the narrowing is intentional, this can be communicated by $fix.${err.rewriteNotice}""",
+            |If $usage is intentional, this can be communicated by $fix,
+            |which $addendum.${err.rewriteNotice}""",
           pos)
       false
     }
 
-    def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt)
+    def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt, Reason.NonConforming)
 
     def recur(pat: Tree, pt: Type): Boolean =
       !sourceVersion.isAtLeast(future) || // only for 3.x for now since mitigations work only after this PR
@@ -825,7 +854,7 @@ trait Checking {
             recur(pat1, pt)
           case UnApply(fn, _, pats) =>
             check(pat, pt) &&
-            (isIrrefutable(fn, pats.length) || fail(pat, pt)) && {
+            (isIrrefutable(fn, pats.length) || fail(pat, pt, Reason.RefutableExtractor)) && {
               val argPts = unapplyArgs(fn.tpe.widen.finalResultType, fn, pats, pat.srcPos)
               pats.corresponds(argPts)(recur)
             }

tests/neg/refutable-pattern-binding-messages.check

@@ -0,0 +1,42 @@
+-- Error: tests/neg/refutable-pattern-binding-messages.scala:6:14 ------------------------------------------------------
+6 |  for Positive(i) <- List(1, 2, 3) do ()  // error: refutable extractor
+  |      ^^^^^^^^^^^
+  |      pattern binding uses refutable extractor `Test.Positive`
+  |
+  |      If this usage is intentional, this can be communicated by adding the `case` keyword before the full pattern,
+  |      which will result in a filtering for expression (using `withFilter`).
+-- Error: tests/neg/refutable-pattern-binding-messages.scala:11:11 -----------------------------------------------------
+11 |  for ((x: String) <- xs) do ()           // error: pattern type more specialized
+   |           ^^^^^^
+   | pattern's type String is more specialized than the right hand side expression's type AnyRef
+   |
+   | If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
+   | which will result in a filtering for expression (using `withFilter`).
+-- Error: tests/neg/refutable-pattern-binding-messages.scala:15:13 -----------------------------------------------------
+15 |  for none @ None <- ys do ()             // error: pattern type does not match
+   |             ^^^^
+   | pattern's type None.type does not match the right hand side expression's type (x$1 : Option[?])
+   |
+   | If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
+   | which will result in a filtering for expression (using `withFilter`).
+-- Error: tests/neg/refutable-pattern-binding-messages.scala:5:14 ------------------------------------------------------
+5 |  val Positive(p) = 5                     // error: refutable extractor
+  |      ^^^^^^^^^^^^^^^
+  |      pattern binding uses refutable extractor `Test.Positive`
+  |
+  |      If this usage is intentional, this can be communicated by adding `: @unchecked` after the expression,
+  |      which may result in a MatchError at runtime.
+-- Error: tests/neg/refutable-pattern-binding-messages.scala:10:20 -----------------------------------------------------
+10 |  val i :: is = List(1, 2, 3)             // error: pattern type more specialized
+   |                ^^^^^^^^^^^^^
+   |        pattern's type ::[Int] is more specialized than the right hand side expression's type List[Int]
+   |
+   |        If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
+   |        which may result in a MatchError at runtime.
+-- Error: tests/neg/refutable-pattern-binding-messages.scala:16:10 -----------------------------------------------------
+16 |  val 1 = 2                               // error: pattern type does not match
+   |          ^
+   |        pattern's type (1 : Int) does not match the right hand side expression's type (2 : Int)
+   |
+   |        If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
+   |        which may result in a MatchError at runtime.

tests/neg/refutable-pattern-binding-messages.scala

@@ -0,0 +1,17 @@
+// scalac: -source:future -Werror
+object Test {
+  // refutable extractor
+  object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
+  val Positive(p) = 5                     // error: refutable extractor
+  for Positive(i) <- List(1, 2, 3) do ()  // error: refutable extractor
+
+  // more specialized
+  val xs: List[AnyRef] = ???
+  val i :: is = List(1, 2, 3)             // error: pattern type more specialized
+  for ((x: String) <- xs) do ()           // error: pattern type more specialized
+
+  // does not match
+  val ys: List[Option[?]] = ???
+  for none @ None <- ys do ()             // error: pattern type does not match
+  val 1 = 2                               // error: pattern type does not match
+}