Skip to content

Commit 3ddbf8e

Browse files
griggtgriggt
committed
Improve diagnostic for refutable extractors in pattern bindings
Avoids nonsensical messages such as: pattern's type Int is more specialized than the right hand side expression's type Int when the underlying cause is that the extractor is refutable.
1 parent 6540ad9 commit 3ddbf8e

File tree

3 files changed

+99
-11
lines changed

3 files changed

+99
-11
lines changed

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

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -795,26 +795,55 @@ trait Checking {
795795

796796
/** Check that pattern `pat` is irrefutable for scrutinee type `sel.tpe`.
797797
* This means `sel` is either marked @unchecked or `sel.tpe` conforms to the
798-
* pattern's type. If pattern is an UnApply, do the check recursively.
798+
* pattern's type. If pattern is an UnApply, also check that the extractor is
799+
* irrefutable, and do the check recursively.
799800
*/
800801
def checkIrrefutable(sel: Tree, pat: Tree, isPatDef: Boolean)(using Context): Boolean = {
801802
val pt = sel.tpe
802803

803-
def fail(pat: Tree, pt: Type): Boolean = {
804-
var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
805-
if (!pat.tpe.isSingleton) reportedPt = reportedPt.widen
806-
val problem = if (pat.tpe <:< reportedPt) "is more specialized than" else "does not match"
807-
val fix = if (isPatDef) "adding `: @unchecked` after the expression" else "writing `case ` before the full pattern"
808-
val pos = if (isPatDef) sel.srcPos else pat.srcPos
804+
enum Reason:
805+
case NonConforming, RefutableExtractor
806+
807+
def fail(pat: Tree, pt: Type, reason: Reason): Boolean = {
808+
import Reason._
809+
val message = reason match
810+
case NonConforming =>
811+
var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
812+
if !pat.tpe.isSingleton then reportedPt = reportedPt.widen
813+
val problem = if pat.tpe <:< reportedPt then "is more specialized than" else "does not match"
814+
ex"pattern's type ${pat.tpe} $problem the right hand side expression's type $reportedPt"
815+
case RefutableExtractor =>
816+
val extractor =
817+
val UnApply(fn, _, _) = pat: @unchecked
818+
fn match
819+
case Select(id, _) => id
820+
case TypeApply(Select(id, _), _) => id
821+
em"pattern binding uses refutable extractor `$extractor`"
822+
823+
val fix =
824+
if isPatDef then "adding `: @unchecked` after the expression"
825+
else "adding the `case` keyword before the full pattern"
826+
val addendum =
827+
if isPatDef then "may result in a MatchError at runtime"
828+
else "will result in a filtering for expression (using `withFilter`)"
829+
val usage = reason match
830+
case NonConforming => "the narrowing"
831+
case RefutableExtractor => "this usage"
832+
val pos =
833+
if isPatDef then reason match
834+
case NonConforming => sel.srcPos
835+
case RefutableExtractor => pat.source.atSpan(pat.span union sel.span)
836+
else pat.srcPos
809837
report.warning(
810-
ex"""pattern's type ${pat.tpe} $problem the right hand side expression's type $reportedPt
838+
em"""$message
811839
|
812-
|If the narrowing is intentional, this can be communicated by $fix.${err.rewriteNotice}""",
840+
|If $usage is intentional, this can be communicated by $fix,
841+
|which $addendum.${err.rewriteNotice}""",
813842
pos)
814843
false
815844
}
816845

817-
def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt)
846+
def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt, Reason.NonConforming)
818847

819848
def recur(pat: Tree, pt: Type): Boolean =
820849
!sourceVersion.isAtLeast(future) || // only for 3.x for now since mitigations work only after this PR
@@ -825,7 +854,7 @@ trait Checking {
825854
recur(pat1, pt)
826855
case UnApply(fn, _, pats) =>
827856
check(pat, pt) &&
828-
(isIrrefutable(fn, pats.length) || fail(pat, pt)) && {
857+
(isIrrefutable(fn, pats.length) || fail(pat, pt, Reason.RefutableExtractor)) && {
829858
val argPts = unapplyArgs(fn.tpe.widen.finalResultType, fn, pats, pat.srcPos)
830859
pats.corresponds(argPts)(recur)
831860
}

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
-- Error: tests/neg/refutable-pattern-binding-messages.scala:6:14 ------------------------------------------------------
2+
6 | for Positive(i) <- List(1, 2, 3) do () // error: refutable extractor
3+
| ^^^^^^^^^^^
4+
| pattern binding uses refutable extractor `Test.Positive`
5+
|
6+
| If this usage is intentional, this can be communicated by adding the `case` keyword before the full pattern,
7+
| which will result in a filtering for expression (using `withFilter`).
8+
-- Error: tests/neg/refutable-pattern-binding-messages.scala:11:11 -----------------------------------------------------
9+
11 | for ((x: String) <- xs) do () // error: pattern type more specialized
10+
| ^^^^^^
11+
| pattern's type String is more specialized than the right hand side expression's type AnyRef
12+
|
13+
| If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
14+
| which will result in a filtering for expression (using `withFilter`).
15+
-- Error: tests/neg/refutable-pattern-binding-messages.scala:15:13 -----------------------------------------------------
16+
15 | for none @ None <- ys do () // error: pattern type does not match
17+
| ^^^^
18+
| pattern's type None.type does not match the right hand side expression's type (x$1 : Option[?])
19+
|
20+
| If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
21+
| which will result in a filtering for expression (using `withFilter`).
22+
-- Error: tests/neg/refutable-pattern-binding-messages.scala:5:14 ------------------------------------------------------
23+
5 | val Positive(p) = 5 // error: refutable extractor
24+
| ^^^^^^^^^^^^^^^
25+
| pattern binding uses refutable extractor `Test.Positive`
26+
|
27+
| If this usage is intentional, this can be communicated by adding `: @unchecked` after the expression,
28+
| which may result in a MatchError at runtime.
29+
-- Error: tests/neg/refutable-pattern-binding-messages.scala:10:20 -----------------------------------------------------
30+
10 | val i :: is = List(1, 2, 3) // error: pattern type more specialized
31+
| ^^^^^^^^^^^^^
32+
| pattern's type ::[Int] is more specialized than the right hand side expression's type List[Int]
33+
|
34+
| If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
35+
| which may result in a MatchError at runtime.
36+
-- Error: tests/neg/refutable-pattern-binding-messages.scala:16:10 -----------------------------------------------------
37+
16 | val 1 = 2 // error: pattern type does not match
38+
| ^
39+
| pattern's type (1 : Int) does not match the right hand side expression's type (2 : Int)
40+
|
41+
| If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
42+
| which may result in a MatchError at runtime.

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// scalac: -source:future -Werror
2+
object Test {
3+
// refutable extractor
4+
object Positive { def unapply(i: Int): Option[Int] = Some(i).filter(_ > 0) }
5+
val Positive(p) = 5 // error: refutable extractor
6+
for Positive(i) <- List(1, 2, 3) do () // error: refutable extractor
7+
8+
// more specialized
9+
val xs: List[AnyRef] = ???
10+
val i :: is = List(1, 2, 3) // error: pattern type more specialized
11+
for ((x: String) <- xs) do () // error: pattern type more specialized
12+
13+
// does not match
14+
val ys: List[Option[?]] = ???
15+
for none @ None <- ys do () // error: pattern type does not match
16+
val 1 = 2 // error: pattern type does not match
17+
}

0 commit comments

Comments
 (0)