Skip to content

Commit 0bddf1b

Browse files
committed
Emit strict pattern binding warnings by default
Emit warnings by default for refutable pattern bindings. Keep the existing desugaring of for generators (using `withFilter`). In the future, the warnings will become errors, and `case` will be required to get filtering for generators.
1 parent 2964880 commit 0bddf1b

File tree

18 files changed

+197
-202
lines changed

18 files changed

+197
-202
lines changed

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1553,7 +1553,7 @@ object desugar {
15531553
Function(derivedValDef(gen.pat, named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
15541554
case _ =>
15551555
val matchCheckMode =
1556-
if (gen.checkMode == GenCheckMode.Check) MatchCheck.IrrefutableGenFrom
1556+
if (gen.checkMode == GenCheckMode.Check || gen.checkMode == GenCheckMode.CheckAndFilter) MatchCheck.IrrefutableGenFrom
15571557
else MatchCheck.None
15581558
makeCaseLambda(CaseDef(gen.pat, EmptyTree, body) :: Nil, matchCheckMode)
15591559
}
@@ -1640,13 +1640,11 @@ object desugar {
16401640
case IdPattern(_) => true
16411641
case _ => false
16421642

1643-
def needsNoFilter(gen: GenFrom): Boolean =
1644-
if (gen.checkMode == GenCheckMode.FilterAlways) // pattern was prefixed by `case`
1645-
false
1646-
else
1647-
gen.checkMode != GenCheckMode.FilterNow
1648-
|| isVarBinding(gen.pat)
1649-
|| isIrrefutable(gen.pat, gen.expr)
1643+
def needsNoFilter(gen: GenFrom): Boolean = gen.checkMode match
1644+
case GenCheckMode.FilterAlways => false // pattern was prefixed by `case`
1645+
case GenCheckMode.FilterNow | GenCheckMode.CheckAndFilter => isVarBinding(gen.pat) || isIrrefutable(gen.pat, gen.expr)
1646+
case GenCheckMode.Check => true
1647+
case GenCheckMode.Ignore => true
16501648

16511649
/** rhs.name with a pattern filter on rhs unless `pat` is irrefutable when
16521650
* matched against `rhs`.

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
168168
enum GenCheckMode {
169169
case Ignore // neither filter nor check since filtering was done before
170170
case Check // check that pattern is irrefutable
171-
case FilterNow // filter out non-matching elements since we are not yet in 3.x
171+
case CheckAndFilter // both check and filter (transitional period starting with 3.2)
172+
case FilterNow // filter out non-matching elements if we are not in 3.2 or later
172173
case FilterAlways // filter out non-matching elements since pattern is prefixed by `case`
173174
}
174175

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2512,9 +2512,10 @@ object Parsers {
25122512
def generatorRest(pat: Tree, casePat: Boolean): GenFrom =
25132513
atSpan(startOffset(pat), accept(LARROW)) {
25142514
val checkMode =
2515-
if (casePat) GenCheckMode.FilterAlways
2516-
else if sourceVersion.isAtLeast(future) then GenCheckMode.Check
2517-
else GenCheckMode.FilterNow // filter for now, to keep backwards compat
2515+
if casePat then GenCheckMode.FilterAlways
2516+
else if sourceVersion.isAtLeast(`3.2`) then GenCheckMode.CheckAndFilter
2517+
else if sourceVersion.isAtLeast(`future`) then GenCheckMode.Check
2518+
else GenCheckMode.FilterNow // filter on source version < 3.2, for backward compat
25182519
GenFrom(pat, subExpr(), checkMode)
25192520
}
25202521

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -835,23 +835,24 @@ trait Checking {
835835
case NonConforming => sel.srcPos
836836
case RefutableExtractor => pat.source.atSpan(pat.span union sel.span)
837837
else pat.srcPos
838-
def rewriteMsg = Message.rewriteNotice("This patch", `future-migration`)
839-
report.warning(
838+
def rewriteMsg = Message.rewriteNotice("This patch", `3.2-migration`)
839+
report.gradualErrorOrMigrationWarning(
840840
em"""$message
841841
|
842842
|If $usage is intentional, this can be communicated by $fix,
843843
|which $addendum.$rewriteMsg""",
844-
pos)
844+
pos, warnFrom = `3.2`, errorFrom = `future`)
845845
false
846846
}
847847

848848
def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt, Reason.NonConforming)
849849

850850
def recur(pat: Tree, pt: Type): Boolean =
851-
!sourceVersion.isAtLeast(future) || // only for 3.x for now since mitigations work only after this PR
852-
pt.hasAnnotation(defn.UncheckedAnnot) || {
851+
!sourceVersion.isAtLeast(`3.2`)
852+
|| pt.hasAnnotation(defn.UncheckedAnnot)
853+
|| {
853854
patmatch.println(i"check irrefutable $pat: ${pat.tpe} against $pt")
854-
pat match {
855+
pat match
855856
case Bind(_, pat1) =>
856857
recur(pat1, pt)
857858
case UnApply(fn, _, pats) =>
@@ -869,7 +870,6 @@ trait Checking {
869870
case _ =>
870871
check(pat, pt)
871872
}
872-
}
873873

874874
recur(pat, pt)
875875
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1619,7 +1619,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
16191619
tree.selector.removeAttachment(desugar.CheckIrrefutable) match {
16201620
case Some(checkMode) if !sel.tpe.hasAnnotation(defn.UncheckedAnnot) =>
16211621
val isPatDef = checkMode == desugar.MatchCheck.IrrefutablePatDef
1622-
if !checkIrrefutable(sel, pat, isPatDef) && sourceVersion == `future-migration` then
1622+
if !checkIrrefutable(sel, pat, isPatDef) && sourceVersion.isAtLeast(`3.2`) && sourceVersion.isMigrating then
16231623
if isPatDef then uncheckedBrackets(tree.selector) match
16241624
case None =>
16251625
patch(Span(tree.selector.span.end), ": @unchecked")

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ class CompilationTests {
7373
aggregateTests(
7474
compileFile("tests/rewrites/rewrites.scala", scala2CompatMode.and("-rewrite", "-indent")),
7575
compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
76-
compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
77-
compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
76+
compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
77+
compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
7878
compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")),
7979
compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")),
8080
compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")),
@@ -203,7 +203,6 @@ class CompilationTests {
203203
compileFile("tests/run-custom-args/typeclass-derivation1.scala", defaultOptions.without(yCheckOptions: _*)),
204204
compileFile("tests/run-custom-args/tuple-cons.scala", allowDeepSubtypes),
205205
compileFile("tests/run-custom-args/i5256.scala", allowDeepSubtypes),
206-
compileFile("tests/run-custom-args/fors.scala", defaultOptions.and("-source", "future")),
207206
compileFile("tests/run-custom-args/no-useless-forwarders.scala", defaultOptions and "-Xmixin-force-forwarders:false"),
208207
compileFile("tests/run-custom-args/defaults-serizaliable-no-forwarders.scala", defaultOptions and "-Xmixin-force-forwarders:false"),
209208
compileFilesInDir("tests/run-custom-args/erased", defaultOptions.and("-language:experimental.erasedDefinitions")),

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ movedTo: https://docs.scala-lang.org/scala3/reference/changed-features/pattern-b
77
In Scala 2, pattern bindings in `val` definitions and `for` expressions are
88
loosely typed. Potentially failing matches are still accepted at compile-time,
99
but may influence the program's runtime behavior.
10-
From Scala 3.1 on, type checking rules will be tightened so that warnings are reported at compile-time instead.
10+
From Scala 3.2 on, type checking rules will be tightened so that warnings are reported at compile-time instead.
1111

1212
## Bindings in Pattern Definitions
1313

@@ -16,7 +16,7 @@ val xs: List[Any] = List(1, 2, 3)
1616
val (x: String) :: _ = xs // error: pattern's type String is more specialized
1717
// than the right-hand side expression's type Any
1818
```
19-
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:
19+
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:
2020
```scala
2121
val pair = (1, true)
2222
val (x, y) = pair
@@ -25,7 +25,7 @@ Sometimes one wants to decompose data anyway, even though the pattern is refutab
2525
```scala
2626
val first :: rest = elems // error
2727
```
28-
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:
28+
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:
2929
```scala
3030
val first :: rest = elems: @unchecked // OK
3131
```
@@ -40,7 +40,7 @@ val elems: List[Any] = List((1, 2), "hello", (3, 4))
4040
for (x, y) <- elems yield (y, x) // error: pattern's type (Any, Any) is more specialized
4141
// than the right-hand side expression's type Any
4242
```
43-
This code gives a compile-time warning in Scala 3.1 whereas in Scala 2 the list `elems`
43+
This code gives a compile-time warning in Scala 3.2 whereas in Scala 2 the list `elems`
4444
is filtered to retain only the elements of tuple type that match the pattern `(x, y)`.
4545
The filtering functionality can be obtained in Scala 3 by prefixing the pattern with `case`:
4646
```scala
@@ -56,4 +56,4 @@ Generator ::= [‘case’] Pattern1 ‘<-’ Expr
5656

5757
## Migration
5858

59-
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.
59+
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.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// These tests should fail under -Xfatal-warnings with source version source version 3.2 or later
2+
import language.`3.2`
3+
4+
object Test:
5+
// from filtering-fors.scala
6+
val xs: List[AnyRef] = ???
7+
8+
for ((x: String) <- xs) do () // error
9+
for (y@ (x: String) <- xs) do () // error
10+
for ((x, y) <- xs) do () // error
11+
12+
for ((x: String) <- xs if x.isEmpty) do () // error
13+
for ((x: String) <- xs; y = x) do () // error
14+
for ((x: String) <- xs; (y, z) <- xs) do () // error // error
15+
for (case (x: String) <- xs; (y, z) <- xs) do () // error
16+
for ((x: String) <- xs; case (y, z) <- xs) do () // error
17+
18+
val pairs: List[AnyRef] = List((1, 2), "hello", (3, 4))
19+
for ((x, y) <- pairs) yield (y, x) // error
20+
21+
// from unchecked-patterns.scala
22+
val y :: ys = List(1, 2, 3) // error
23+
val (1, c) = (1, 2) // error
24+
val 1 *: cs = 1 *: Tuple() // error
25+
26+
val (_: Int | _: AnyRef) = ??? : AnyRef // error
27+
28+
val 1 = 2 // error
29+
30+
object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
31+
object Always1 { def unapply(i: Int): Some[Int] = Some(i) }
32+
object Pair { def unapply(t: (Int, Int)): t.type = t }
33+
object Triple { def unapply(t: (Int, Int, Int)): (Int, Int, Int) = t }
34+
35+
val Positive(p) = 5 // error
36+
val Some(s1) = Option(1) // error

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,44 +5,44 @@
55
|
66
| If this usage is intentional, this can be communicated by adding the `case` keyword before the full pattern,
77
| which will result in a filtering for expression (using `withFilter`).
8-
| This patch can be rewritten automatically under -rewrite -source future-migration.
8+
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
99
-- Error: tests/neg/refutable-pattern-binding-messages.scala:11:11 -----------------------------------------------------
1010
11 | for ((x: String) <- xs) do () // error: pattern type more specialized
1111
| ^^^^^^
1212
| pattern's type String is more specialized than the right hand side expression's type AnyRef
1313
|
1414
| If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
1515
| which will result in a filtering for expression (using `withFilter`).
16-
| This patch can be rewritten automatically under -rewrite -source future-migration.
16+
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
1717
-- Error: tests/neg/refutable-pattern-binding-messages.scala:15:13 -----------------------------------------------------
1818
15 | for none @ None <- ys do () // error: pattern type does not match
1919
| ^^^^
2020
| pattern's type None.type does not match the right hand side expression's type (x$1 : Option[?])
2121
|
2222
| If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
2323
| which will result in a filtering for expression (using `withFilter`).
24-
| This patch can be rewritten automatically under -rewrite -source future-migration.
24+
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
2525
-- Error: tests/neg/refutable-pattern-binding-messages.scala:5:14 ------------------------------------------------------
2626
5 | val Positive(p) = 5 // error: refutable extractor
2727
| ^^^^^^^^^^^^^^^
2828
| pattern binding uses refutable extractor `Test.Positive`
2929
|
3030
| If this usage is intentional, this can be communicated by adding `: @unchecked` after the expression,
3131
| which may result in a MatchError at runtime.
32-
| This patch can be rewritten automatically under -rewrite -source future-migration.
32+
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
3333
-- Error: tests/neg/refutable-pattern-binding-messages.scala:10:20 -----------------------------------------------------
3434
10 | val i :: is = List(1, 2, 3) // error: pattern type more specialized
3535
| ^^^^^^^^^^^^^
3636
| pattern's type ::[Int] is more specialized than the right hand side expression's type List[Int]
3737
|
3838
| If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
3939
| which may result in a MatchError at runtime.
40-
| This patch can be rewritten automatically under -rewrite -source future-migration.
40+
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
4141
-- Error: tests/neg/refutable-pattern-binding-messages.scala:16:10 -----------------------------------------------------
4242
16 | val 1 = 2 // error: pattern type does not match
4343
| ^
4444
| pattern's type (1 : Int) does not match the right hand side expression's type (2 : Int)
4545
|
4646
| If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
4747
| which may result in a MatchError at runtime.
48-
| This patch can be rewritten automatically under -rewrite -source future-migration.
48+
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// scalac: -source:future -Werror
1+
// scalac: -Werror
22
object Test {
33
// refutable extractor
44
object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// These tests should pass under -Xfatal-warnings with source version less than 3.2
2+
import language.`3.0-migration`
3+
4+
object Test:
5+
// from filtering-fors.scala
6+
val xs: List[AnyRef] = ???
7+
8+
for ((x: String) <- xs) do ()
9+
for (y@ (x: String) <- xs) do ()
10+
for ((x, y) <- xs) do ()
11+
12+
for ((x: String) <- xs if x.isEmpty) do ()
13+
for ((x: String) <- xs; y = x) do ()
14+
for ((x: String) <- xs; (y, z) <- xs) do ()
15+
for (case (x: String) <- xs; (y, z) <- xs) do ()
16+
for ((x: String) <- xs; case (y, z) <- xs) do ()
17+
18+
val pairs: List[AnyRef] = List((1, 2), "hello", (3, 4))
19+
for ((x, y) <- pairs) yield (y, x)
20+
21+
// from unchecked-patterns.scala
22+
val y :: ys = List(1, 2, 3)
23+
val (1, c) = (1, 2)
24+
val 1 *: cs = 1 *: Tuple()
25+
26+
val (_: Int | _: AnyRef) = ??? : AnyRef
27+
28+
val 1 = 2
29+
30+
object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
31+
object Always1 { def unapply(i: Int): Some[Int] = Some(i) }
32+
object Pair { def unapply(t: (Int, Int)): t.type = t }
33+
object Triple { def unapply(t: (Int, Int, Int)): (Int, Int, Int) = t }
34+
35+
val Positive(p) = 5
36+
val Some(s1) = Option(1)
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// These tests should pass under -Xfatal-warnings with source version less than 3.2
2+
import language.`3.1`
3+
4+
object Test:
5+
// from filtering-fors.scala
6+
val xs: List[AnyRef] = ???
7+
8+
for ((x: String) <- xs) do ()
9+
for (y@ (x: String) <- xs) do ()
10+
for ((x, y) <- xs) do ()
11+
12+
for ((x: String) <- xs if x.isEmpty) do ()
13+
for ((x: String) <- xs; y = x) do ()
14+
for ((x: String) <- xs; (y, z) <- xs) do ()
15+
for (case (x: String) <- xs; (y, z) <- xs) do ()
16+
for ((x: String) <- xs; case (y, z) <- xs) do ()
17+
18+
val pairs: List[AnyRef] = List((1, 2), "hello", (3, 4))
19+
for ((x, y) <- pairs) yield (y, x)
20+
21+
// from unchecked-patterns.scala
22+
val y :: ys = List(1, 2, 3)
23+
val (1, c) = (1, 2)
24+
val 1 *: cs = 1 *: Tuple()
25+
26+
val (_: Int | _: AnyRef) = ??? : AnyRef
27+
28+
val 1 = 2
29+
30+
object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
31+
object Always1 { def unapply(i: Int): Some[Int] = Some(i) }
32+
object Pair { def unapply(t: (Int, Int)): t.type = t }
33+
object Triple { def unapply(t: (Int, Int, Int)): (Int, Int, Int) = t }
34+
35+
val Positive(p) = 5
36+
val Some(s1) = Option(1)

tests/run-custom-args/fors.check

Lines changed: 0 additions & 46 deletions
This file was deleted.

0 commit comments

Comments
 (0)