Emit strict pattern binding warnings by default

Author: griggt

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

@@ -1538,7 +1538,7 @@ object desugar {
           Function(derivedValDef(gen.pat, named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
         case _ =>
           val matchCheckMode =
-            if (gen.checkMode == GenCheckMode.Check) MatchCheck.IrrefutableGenFrom
+            if (gen.checkMode == GenCheckMode.Check || gen.checkMode == GenCheckMode.CheckAndFilter) MatchCheck.IrrefutableGenFrom
             else MatchCheck.None
           makeCaseLambda(CaseDef(gen.pat, EmptyTree, body) :: Nil, matchCheckMode)
       }
@@ -1625,13 +1625,11 @@ object desugar {
         case IdPattern(_) => true
         case _ => false
 
-      def needsNoFilter(gen: GenFrom): Boolean =
-        if (gen.checkMode == GenCheckMode.FilterAlways) // pattern was prefixed by `case`
-          false
-        else
-          gen.checkMode != GenCheckMode.FilterNow
-          || isVarBinding(gen.pat)
-          || isIrrefutable(gen.pat, gen.expr)
+      def needsNoFilter(gen: GenFrom): Boolean = gen.checkMode match
+        case GenCheckMode.FilterAlways => false  // pattern was prefixed by `case`
+        case GenCheckMode.FilterNow | GenCheckMode.CheckAndFilter => isVarBinding(gen.pat) || isIrrefutable(gen.pat, gen.expr)
+        case GenCheckMode.Check => true
+        case GenCheckMode.Ignore => true
 
       /** rhs.name with a pattern filter on rhs unless `pat` is irrefutable when
        *  matched against `rhs`.

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

@@ -168,7 +168,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
   enum GenCheckMode {
     case Ignore       // neither filter nor check since filtering was done before
     case Check        // check that pattern is irrefutable
-    case FilterNow    // filter out non-matching elements since we are not yet in 3.x
+    case CheckAndFilter // both check and filter (transitional period starting with 3.2)
+    case FilterNow    // filter out non-matching elements if we are not in 3.2 or later
     case FilterAlways // filter out non-matching elements since pattern is prefixed by `case`
   }
 

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

@@ -2512,9 +2512,10 @@ object Parsers {
     def generatorRest(pat: Tree, casePat: Boolean): GenFrom =
       atSpan(startOffset(pat), accept(LARROW)) {
         val checkMode =
-          if (casePat) GenCheckMode.FilterAlways
-          else if sourceVersion.isAtLeast(future) then GenCheckMode.Check
-          else GenCheckMode.FilterNow  // filter for now, to keep backwards compat
+          if casePat then GenCheckMode.FilterAlways
+          else if sourceVersion.isAtLeast(`3.2`) then GenCheckMode.CheckAndFilter
+          else if sourceVersion.isAtLeast(`future`) then GenCheckMode.Check
+          else GenCheckMode.FilterNow  // filter on source version < 3.2, for backward compat
         GenFrom(pat, subExpr(), checkMode)
       }
 

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

@@ -823,26 +823,28 @@ trait Checking {
       val usage = reason match
         case NonConforming => "the narrowing"
         case RefutableExtractor => "this usage"
-      def rewriteMsg = Message.rewriteNotice("This patch", `future-migration`)
+      def rewriteMsg = Message.rewriteNotice("This patch", `3.2-migration`)
       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(
+      report.gradualErrorOrMigrationWarning(
         em"""$message
             |
-            |If $usage is intentional, this can be communicated by $fix.$rewriteMsg""", pos)
+            |If $usage is intentional, this can be communicated by $fix.$rewriteMsg""",
+        pos, warnFrom = `3.2`, errorFrom = `future`)
       false
     }
 
     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
-      pt.hasAnnotation(defn.UncheckedAnnot) || {
+      !sourceVersion.isAtLeast(`3.2`)
+      || pt.hasAnnotation(defn.UncheckedAnnot)
+      || {
         patmatch.println(i"check irrefutable $pat: ${pat.tpe} against $pt")
-        pat match {
+        pat match
           case Bind(_, pat1) =>
             recur(pat1, pt)
           case UnApply(fn, _, pats) =>
@@ -860,7 +862,6 @@ trait Checking {
           case _ =>
             check(pat, pt)
         }
-      }
 
     recur(pat, pt)
   }

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

@@ -1620,7 +1620,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
             tree.selector.removeAttachment(desugar.CheckIrrefutable) match {
               case Some(checkMode) if !sel.tpe.hasAnnotation(defn.UncheckedAnnot) =>
                 val isPatDef = checkMode == desugar.MatchCheck.IrrefutablePatDef
-                if !checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `future-migration` then
+                if !checkIrrefutable(sel, pat, isPatDef) && sourceVersion.isAtLeast(`3.2`) && sourceVersion.isMigrating then
                   if isPatDef then uncheckedBrackets(tree.selector) match
                     case None =>
                       patch(Span(tree.selector.span.end), ": @unchecked")

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -73,8 +73,8 @@ class CompilationTests {
     aggregateTests(
       compileFile("tests/rewrites/rewrites.scala", scala2CompatMode.and("-rewrite", "-indent")),
       compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
-      compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
-      compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
+      compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
+      compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
       compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")),
@@ -203,7 +203,6 @@ class CompilationTests {
       compileFile("tests/run-custom-args/typeclass-derivation1.scala", defaultOptions.without(yCheckOptions: _*)),
       compileFile("tests/run-custom-args/tuple-cons.scala", allowDeepSubtypes),
       compileFile("tests/run-custom-args/i5256.scala", allowDeepSubtypes),
-      compileFile("tests/run-custom-args/fors.scala", defaultOptions.and("-source", "future")),
       compileFile("tests/run-custom-args/no-useless-forwarders.scala", defaultOptions and "-Xmixin-force-forwarders:false"),
       compileFile("tests/run-custom-args/defaults-serizaliable-no-forwarders.scala", defaultOptions and "-Xmixin-force-forwarders:false"),
       compileFilesInDir("tests/run-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")),

docs/_docs/reference/changed-features/pattern-bindings.md

@@ -7,7 +7,7 @@ movedTo: https://docs.scala-lang.org/scala3/reference/changed-features/pattern-b
 In Scala 2, pattern bindings in `val` definitions and `for` expressions are
 loosely typed. Potentially failing matches are still accepted at compile-time,
 but may influence the program's runtime behavior.
-From Scala 3.1 on, type checking rules will be tightened so that warnings are reported at compile-time instead.
+From Scala 3.2 on, type checking rules will be tightened so that warnings are reported at compile-time instead.
 
 ## Bindings in Pattern Definitions
 
@@ -16,7 +16,7 @@ val xs: List[Any] = List(1, 2, 3)
 val (x: String) :: _ = xs   // error: pattern's type String is more specialized
                             // than the right-hand side expression's type Any
 ```
-This code gives a compile-time warning in Scala 3.1 (and also in Scala 3.0 under the `-source future` setting) whereas it will fail at runtime with a `ClassCastException` in Scala 2. In Scala 3.1, a pattern binding is only allowed if the pattern is _irrefutable_, that is, if the right-hand side's type conforms to the pattern's type. For instance, the following is OK:
+This code gives a compile-time warning in Scala 3.2 (and also earlier Scala 3.x under the `-source future` setting) whereas it will fail at runtime with a `ClassCastException` in Scala 2. In Scala 3.2, a pattern binding is only allowed if the pattern is _irrefutable_, that is, if the right-hand side's type conforms to the pattern's type. For instance, the following is OK:
 ```scala
 val pair = (1, true)
 val (x, y) = pair
@@ -25,7 +25,7 @@ Sometimes one wants to decompose data anyway, even though the pattern is refutab
 ```scala
 val first :: rest = elems   // error
 ```
-This works in Scala 2. In fact it is a typical use case for Scala 2's rules. But in Scala 3.1 it will give a warning. One can avoid the warning by marking the right-hand side with an [`@unchecked`](https://scala-lang.org/api/3.x/scala/unchecked.html) annotation:
+This works in Scala 2. In fact it is a typical use case for Scala 2's rules. But in Scala 3.2 it will give a warning. One can avoid the warning by marking the right-hand side with an [`@unchecked`](https://scala-lang.org/api/3.x/scala/unchecked.html) annotation:
 ```scala
 val first :: rest = elems: @unchecked   // OK
 ```
@@ -40,7 +40,7 @@ val elems: List[Any] = List((1, 2), "hello", (3, 4))
 for (x, y) <- elems yield (y, x) // error: pattern's type (Any, Any) is more specialized
                                  // than the right-hand side expression's type Any
 ```
-This code gives a compile-time warning in Scala 3.1 whereas in Scala 2 the list `elems`
+This code gives a compile-time warning in Scala 3.2 whereas in Scala 2 the list `elems`
 is filtered to retain only the elements of tuple type that match the pattern `(x, y)`.
 The filtering functionality can be obtained in Scala 3 by prefixing the pattern with `case`:
 ```scala
@@ -56,4 +56,4 @@ Generator      ::=  [‘case’] Pattern1 ‘<-’ Expr ;
 
 ## Migration
 
-The new syntax is supported in Scala 3.0. However, to enable smooth cross compilation between Scala 2 and Scala 3, the changed behavior and additional type checks are only enabled under the `-source future` setting. They will be enabled by default in version 3.1 of the language.
+The new syntax is supported in Scala 3.0. However, to enable smooth cross compilation between Scala 2 and Scala 3, the changed behavior and additional type checks are only enabled under the `-source future` setting. They will be enabled by default in version 3.2 of the language.

tests/neg-strict/filtering-fors.scala renamed to tests/neg-custom-args/fatal-warnings/filtering-fors.scala

@@ -0,0 +1,36 @@
+// These tests should fail under -Xfatal-warnings with source version source version 3.2 or later
+import language.`3.2`
+
+object Test:
+  // from filtering-fors.scala
+  val xs: List[AnyRef] = ???
+
+  for ((x: String) <- xs) do ()    // error
+  for (y@ (x: String) <- xs) do () // error
+  for ((x, y) <- xs) do ()         // error
+
+  for ((x: String) <- xs if x.isEmpty) do ()   // error
+  for ((x: String) <- xs; y = x) do ()          // error
+  for ((x: String) <- xs; (y, z) <- xs) do ()   // error // error
+  for (case (x: String) <- xs; (y, z) <- xs) do () // error
+  for ((x: String) <- xs; case (y, z) <- xs) do () // error
+
+  val pairs: List[AnyRef] = List((1, 2), "hello", (3, 4))
+  for ((x, y) <- pairs) yield (y, x) // error
+
+  // from unchecked-patterns.scala
+  val y :: ys = List(1, 2, 3)        // error
+  val (1, c) = (1, 2)                // error
+  val 1 *: cs = 1 *: Tuple()         // error
+
+  val (_: Int | _: AnyRef) = ??? : AnyRef  // error
+
+  val 1 = 2  // error
+
+  object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
+  object Always1 { def unapply(i: Int): Some[Int] = Some(i) }
+  object Pair { def unapply(t: (Int, Int)): t.type = t }
+  object Triple { def unapply(t: (Int, Int, Int)): (Int, Int, Int) = t }
+
+  val Positive(p) = 5                // error
+  val Some(s1) = Option(1)           // error

tests/neg-custom-args/fatal-warnings/strict-pattern-bindings-3.2.scala

@@ -4,39 +4,39 @@
   |      pattern binding uses refutable extractor `Test.Positive`
   |
   |      If this usage is intentional, this can be communicated by writing `case ` before the full pattern.
-  |      This patch can be rewritten automatically under -rewrite -source future-migration.
+  |      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
    |           ^^^^^^
    |           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 writing `case ` before the full pattern.
-   |           This patch can be rewritten automatically under -rewrite -source future-migration.
+   |           This patch can be rewritten automatically under -rewrite -source 3.2-migration.
 -- 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 writing `case ` before the full pattern.
-   |           This patch can be rewritten automatically under -rewrite -source future-migration.
+   |           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.
-  |      This patch can be rewritten automatically under -rewrite -source future-migration.
+  |      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.
-   |        This patch can be rewritten automatically under -rewrite -source future-migration.
+   |        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
    |          ^
    |        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.
-   |        This patch can be rewritten automatically under -rewrite -source future-migration.
+   |        This patch can be rewritten automatically under -rewrite -source 3.2-migration.

tests/neg-strict/unchecked-patterns.scala renamed to tests/neg-custom-args/fatal-warnings/unchecked-patterns.scala

@@ -1,4 +1,4 @@
-// scalac: -source:future -Werror
+// scalac: -Werror
 object Test {
   // refutable extractor
   object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }

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

@@ -0,0 +1,36 @@
+// These tests should pass under -Xfatal-warnings with source version less than 3.2
+import language.`3.0-migration`
+
+object Test:
+  // from filtering-fors.scala
+  val xs: List[AnyRef] = ???
+
+  for ((x: String) <- xs) do ()
+  for (y@ (x: String) <- xs) do ()
+  for ((x, y) <- xs) do ()
+
+  for ((x: String) <- xs if x.isEmpty) do ()
+  for ((x: String) <- xs; y = x) do ()
+  for ((x: String) <- xs; (y, z) <- xs) do ()
+  for (case (x: String) <- xs; (y, z) <- xs) do ()
+  for ((x: String) <- xs; case (y, z) <- xs) do ()
+
+  val pairs: List[AnyRef] = List((1, 2), "hello", (3, 4))
+  for ((x, y) <- pairs) yield (y, x)
+
+  // from unchecked-patterns.scala
+  val y :: ys = List(1, 2, 3)
+  val (1, c) = (1, 2)
+  val 1 *: cs = 1 *: Tuple()
+
+  val (_: Int | _: AnyRef) = ??? : AnyRef
+
+  val 1 = 2
+
+  object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
+  object Always1 { def unapply(i: Int): Some[Int] = Some(i) }
+  object Pair { def unapply(t: (Int, Int)): t.type = t }
+  object Triple { def unapply(t: (Int, Int, Int)): (Int, Int, Int) = t }
+
+  val Positive(p) = 5
+  val Some(s1) = Option(1)

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

@@ -0,0 +1,36 @@
+// These tests should pass under -Xfatal-warnings with source version less than 3.2
+import language.`3.1`
+
+object Test:
+  // from filtering-fors.scala
+  val xs: List[AnyRef] = ???
+
+  for ((x: String) <- xs) do ()
+  for (y@ (x: String) <- xs) do ()
+  for ((x, y) <- xs) do ()
+
+  for ((x: String) <- xs if x.isEmpty) do ()
+  for ((x: String) <- xs; y = x) do ()
+  for ((x: String) <- xs; (y, z) <- xs) do ()
+  for (case (x: String) <- xs; (y, z) <- xs) do ()
+  for ((x: String) <- xs; case (y, z) <- xs) do ()
+
+  val pairs: List[AnyRef] = List((1, 2), "hello", (3, 4))
+  for ((x, y) <- pairs) yield (y, x)
+
+  // from unchecked-patterns.scala
+  val y :: ys = List(1, 2, 3)
+  val (1, c) = (1, 2)
+  val 1 *: cs = 1 *: Tuple()
+
+  val (_: Int | _: AnyRef) = ??? : AnyRef
+
+  val 1 = 2
+
+  object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
+  object Always1 { def unapply(i: Int): Some[Int] = Some(i) }
+  object Pair { def unapply(t: (Int, Int)): t.type = t }
+  object Triple { def unapply(t: (Int, Int, Int)): (Int, Int, Int) = t }
+
+  val Positive(p) = 5
+  val Some(s1) = Option(1)