In 3.4 make refutable patterns in a for comprehension an error (#18842)

supersedes #16665

Only make refutable patterns in a for comprehension an error, here we
have a clear set in stone solution: put `case` before the pattern.

It is still in the air the ideal solution for pattern val definitions,
see
https://contributors.scala-lang.org/t/pre-sip-replace-non-sensical-unchecked-annotations/6342/85,
so keep those as a warning for now

Author: bishabosha

community-build/community-projects/scala-xml

@@ -2772,7 +2772,7 @@ object Parsers {
       atSpan(startOffset(pat), accept(LARROW)) {
         val checkMode =
           if casePat then GenCheckMode.FilterAlways
-          else if sourceVersion.isAtLeast(`future`) then GenCheckMode.Check
+          else if sourceVersion.isAtLeast(`3.4`) then GenCheckMode.Check
           else if sourceVersion.isAtLeast(`3.2`) then GenCheckMode.CheckAndFilter
           else GenCheckMode.FilterNow  // filter on source version < 3.2, for backward compat
         GenFrom(pat, subExpr(), checkMode)

community-build/community-projects/scalatest

@@ -923,7 +923,13 @@ trait Checking {
               |
               |If $usage is intentional, this can be communicated by $fix,
               |which $addendum.$rewriteMsg"""),
-        pos, warnFrom = `3.2`, errorFrom = `future`)
+        pos,
+        warnFrom = `3.2`,
+        // we tighten for-comprehension without `case` to error in 3.4,
+        // but we keep pat-defs as warnings for now ("@unchecked"),
+        // until we propose an alternative way to assert exhaustivity to the typechecker.
+        errorFrom = if isPatDef then `future` else `3.4`
+      )
       false
     }
 

community-build/community-projects/scalaz

@@ -1493,6 +1493,7 @@ object Build {
         (
           (dir / "shared/src/test/scala" ** (("*.scala": FileFilter)
             -- "ReflectiveCallTest.scala" // uses many forms of structural calls that are not allowed in Scala 3 anymore
+            -- "UTF16Test.scala" // refutable pattern match
             )).get
 
           ++ (dir / "shared/src/test/require-sam" ** "*.scala").get

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -0,0 +1,12 @@
+-- [E008] Not Found Error: tests/neg/irrefutable.scala:27:29 -----------------------------------------------------------
+27 |    for (case Foo(x: Int) <- xs) yield x // error
+   |                             ^^
+   |                             value withFilter is not a member of Lst[Foo[Any]]
+-- Error: tests/neg/irrefutable.scala:30:16 ----------------------------------------------------------------------------
+30 |    for (Foo(x: Int) <- xs) yield x // error
+   |                ^^^
+   | pattern's type Int is more specialized than the right hand side expression's type Any
+   |
+   | 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`).
+   | This patch can be rewritten automatically under -rewrite -source 3.2-migration.

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

@@ -0,0 +1,42 @@
+// This tests that A.f1 is recognized as an irrefutable pattern and A.f2_nocase is not, and therefore A.f2 solves this
+// by adding a case to the pattern, which results in withFilter being inserted.
+// see also: tests/run/irrefutable.scala for an example that exercises the insertion of withFilter.
+
+class Lst[+T](val id: String, val underlying: List[T]) {
+  def map[U](f: T => U): Lst[U] = new Lst(id, underlying.map(f))
+
+  // hide the withFilter so that there is a compile error
+  // def withFilter(f: T => Boolean): Lst.WithFilter[T] = new Lst.WithFilter(this, f)
+}
+
+// object Lst:
+//   class WithFilter[+T](lst: Lst[T], filter: T => Boolean):
+//     def forwardingFilter[T1](filter: T1 => Boolean): T1 => Boolean = t =>
+//       println(s"filtering $t in ${lst.id}")
+//       filter(t)
+
+//     def map[U](f: T => U): Lst[U] = Lst(lst.id, lst.underlying.withFilter(forwardingFilter(filter)).map(f))
+
+case class Foo[T](x: T)
+
+object A {
+  def f1(xs: Lst[Foo[Int]]): Lst[Int] = {
+    for (Foo(x: Int) <- xs) yield x
+  }
+  def f2(xs: Lst[Foo[Any]]): Lst[Int] = {
+    for (case Foo(x: Int) <- xs) yield x // error
+  }
+  def f2_nocase(xs: Lst[Foo[Any]]): Lst[Int] = {
+    for (Foo(x: Int) <- xs) yield x // error
+  }
+}
+
+@main def Test =
+  val xs = new Lst("xs", List(Foo(1), Foo(2), Foo(3)))
+  println("=== mapping xs with A.f1 ===")
+  val xs1 = A.f1(xs)
+  assert(xs1.underlying == List(1, 2, 3))
+  val ys = new Lst("ys", List(Foo(1: Any), Foo(2: Any), Foo(3: Any)))
+  println("=== mapping ys with A.f2 ===")
+  val ys1 = A.f2(ys)
+  assert(ys1.underlying == List(1, 2, 3))

project/Build.scala

@@ -1,11 +1,3 @@
--- 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.
-  |      This patch can be rewritten automatically under -rewrite -source 3.2-migration.
 -- Error: tests/neg/refutable-pattern-binding-messages.scala:6:14 ------------------------------------------------------
 6 |  for Positive(i) <- List(1, 2, 3) do ()  // error: refutable extractor
   |      ^^^^^^^^^^^
@@ -14,14 +6,6 @@
   |      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`).
   |      This patch can be rewritten automatically under -rewrite -source 3.2-migration.
--- 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.
-   |        This patch can be rewritten automatically under -rewrite -source 3.2-migration.
 -- Error: tests/neg/refutable-pattern-binding-messages.scala:11:11 -----------------------------------------------------
 11 |  for ((x: String) <- xs) do ()           // error: pattern type more specialized
    |           ^^^^^^
@@ -38,6 +22,22 @@
    | 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`).
    | This patch can be rewritten automatically under -rewrite -source 3.2-migration.
+-- 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.
+  |      This patch can be rewritten automatically under -rewrite -source 3.2-migration.
+-- 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.
+   |        This patch can be rewritten automatically under -rewrite -source 3.2-migration.
 -- Error: tests/neg/refutable-pattern-binding-messages.scala:16:10 -----------------------------------------------------
 16 |  val 1 = 2                               // error: pattern type does not match
    |          ^

tests/neg/irrefutable.check

@@ -2,7 +2,7 @@
 class A[T]
 class Test1 {
   def x(backing: Map[A[_], Any]) =
-    for( (k: A[kt], v) <- backing)
+    for(case (k: A[kt], v) <- backing)
       yield (k: A[kt])
 }
 

tests/neg/irrefutable.scala

@@ -32,12 +32,12 @@ object Test {
 
   def groupsMatching: Unit = {
     val Date = """(\d+)/(\d+)/(\d+)""".r
-    for (Regex.Groups(a, b, c) <- Date findFirstMatchIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.") {
+    for (case Regex.Groups(a, b, c) <- Date findFirstMatchIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.") {
       assert(a == "1")
       assert(b == "1")
       assert(c == "2001")
     }
-    for (Regex.Groups(a, b, c) <- (Date findAllIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.").matchData) {
+    for (case Regex.Groups(a, b, c) <- (Date findAllIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.").matchData) {
       assert(a == "1" || a == "31")
       assert(b == "1" || b == "12")
       assert(c == "2001" || c == "2000")

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

@@ -0,0 +1,5 @@
+=== mapping xs with A.f1 ===
+=== mapping ys with A.f2 ===
+filtering Foo(1) in ys
+filtering Foo(2) in ys
+filtering Foo(3) in ys

tests/pos/irrefutable.scala

@@ -0,0 +1,36 @@
+// This tests that A.f1 does not filter its inputs, whereas A.f2 does.
+// see also: tests/neg/irrefutable.scala for an example that exercises the requirement to insert case.
+
+class Lst[+T](val id: String, val underlying: List[T]) {
+  def map[U](f: T => U): Lst[U] = new Lst(id, underlying.map(f))
+  def withFilter(f: T => Boolean): Lst.WithFilter[T] = new Lst.WithFilter(this, f)
+}
+
+object Lst:
+  class WithFilter[+T](lst: Lst[T], filter: T => Boolean):
+    def forwardingFilter[T1](filter: T1 => Boolean): T1 => Boolean = t =>
+      println(s"filtering $t in ${lst.id}")
+      filter(t)
+
+    def map[U](f: T => U): Lst[U] = Lst(lst.id, lst.underlying.withFilter(forwardingFilter(filter)).map(f))
+
+case class Foo[T](x: T)
+
+object A {
+  def f1(xs: Lst[Foo[Int]]): Lst[Int] = {
+    for (Foo(x: Int) <- xs) yield x
+  }
+  def f2(xs: Lst[Foo[Any]]): Lst[Int] = {
+    for (case Foo(x: Int) <- xs) yield x
+  }
+}
+
+@main def Test =
+  val xs = new Lst("xs", List(Foo(1), Foo(2), Foo(3)))
+  println("=== mapping xs with A.f1 ===")
+  val xs1 = A.f1(xs)
+  assert(xs1.underlying == List(1, 2, 3))
+  val ys = new Lst("ys", List(Foo(1: Any), Foo(2: Any), Foo(3: Any)))
+  println("=== mapping ys with A.f2 ===")
+  val ys1 = A.f2(ys)
+  assert(ys1.underlying == List(1, 2, 3))

tests/pos/t6205.scala

@@ -1,5 +1,5 @@
 object Test {
-  def f(xs: List[Any]) = for (key @ (dummy: String) <- xs) yield key
+  def f(xs: List[Any]) = for (case key @ (dummy: String) <- xs) yield key
 
   def main(args: Array[String]): Unit = {
     f("abc" :: Nil) foreach println

tests/run/ReplacementMatching.scala

@@ -82,24 +82,24 @@ def typeChecks(g: Gamma)(level: 0 | 1)(term: Term): Option[Type] =
       yield LambdaType(t, res)
     case App(fun, arg) => // T-App
       for
-        LambdaType(t1, t2) <- typeChecks(g)(level)(fun)
+        case LambdaType(t1, t2) <- typeChecks(g)(level)(fun)
         `t1` <- typeChecks(g)(level)(arg)
       yield t2
     case Box(body) if level == 0 => // T-Box
       for t <- typeChecks(g)(1)(body) yield BoxType(t)
     case Lift(body) if level == 0 => // T-Lift
       for NatType <- typeChecks(g)(0)(body) yield BoxType(NatType)
     case Splice(body) if level == 1 => // T-Unbox
-      for BoxType(t) <- typeChecks(g)(0)(body) yield t
+      for case BoxType(t) <- typeChecks(g)(0)(body) yield t
     case Match(scrutinee, pat, thenp, elsep) => // T-Pat
       for
-        BoxType(t1) <- typeChecks(g)(0)(scrutinee)
+        case BoxType(t1) <- typeChecks(g)(0)(scrutinee)
         delta <- typePatChecks(g, t1)(pat)
         t <- typeChecks(g ++ delta)(0)(thenp)
         `t` <- typeChecks(g)(0)(elsep)
       yield t
     case Fix(t) if level == 0 =>
-      for LambdaType(t1, t2) <- typeChecks(g)(0)(t) yield t2 // T-Fix
+      for case LambdaType(t1, t2) <- typeChecks(g)(0)(t) yield t2 // T-Fix
     case _ => None
   if res.isEmpty then
     println(s"Failed to type $term at level $level with environment $g")

tests/run/irrefutable.check

@@ -20,10 +20,10 @@ object Test extends App {
   val t = "Last modified 2011-07-15"
   val p1 = """(\d\d\d\d)-(\d\d)-(\d\d)""".r
   val y1: Option[String] = for {
-    p1(year, month, day) <- p1 findFirstIn t
+    case p1(year, month, day) <- p1 findFirstIn t
   } yield year
   val y2: Option[String] = for {
-    p1(year, month, day) <- p1 findFirstMatchIn t
+    case p1(year, month, day) <- p1 findFirstMatchIn t
   } yield year
   println(s"$y1 $y2")
 

tests/run/irrefutable.scala

@@ -8,9 +8,9 @@ object Test {
     val l = List(PrimaryKey, NoNull, lower)
 
     // withFilter must be generated in these
-    for (option @ NoNull <- l) println("Found " + option)
-    for (option @ `lower` <- l) println("Found " + option)
-    for ((`lower`, i) <- l.zipWithIndex) println("Found " + i)
+    for (case option @ NoNull <- l) println("Found " + option)
+    for (case option @ `lower` <- l) println("Found " + option)
+    for (case (`lower`, i) <- l.zipWithIndex) println("Found " + i)
 
     // no withFilter
     for (X <- List("A single ident is always a pattern")) println(X)

tests/run/patmat-bind-typed.scala

@@ -1,7 +1,7 @@
 object Test {
   def main(args: Array[String]): Unit = {
     val mixedList = List(1,(1,2),4,(3,1),(5,4),6)
-    val as = for((a,b) <- mixedList) yield a
+    val as = for(case (a,b) <- mixedList) yield a
     println(as.mkString(", "))
   }
 }