Skip to content

Commit 6e62644

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 119e3d7 commit 6e62644

File tree

3 files changed

+87
-11
lines changed

3 files changed

+87
-11
lines changed

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

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -794,26 +794,49 @@ trait Checking {
794794

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

802-
def fail(pat: Tree, pt: Type): Boolean = {
803-
var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
804-
if (!pat.tpe.isSingleton) reportedPt = reportedPt.widen
805-
val problem = if (pat.tpe <:< reportedPt) "is more specialized than" else "does not match"
806-
val fix = if (isPatDef) "adding `: @unchecked` after the expression" else "writing `case ` before the full pattern"
807-
val pos = if (isPatDef) sel.srcPos else pat.srcPos
803+
enum Reason:
804+
case NonConforming, RefutableExtractor
805+
806+
def fail(pat: Tree, pt: Type, reason: Reason): Boolean = {
807+
import Reason._
808+
val message = reason match
809+
case NonConforming =>
810+
var reportedPt = pt.dropAnnot(defn.UncheckedAnnot)
811+
if !pat.tpe.isSingleton then reportedPt = reportedPt.widen
812+
val problem = if pat.tpe <:< reportedPt then "is more specialized than" else "does not match"
813+
ex"pattern's type ${pat.tpe} $problem the right hand side expression's type $reportedPt"
814+
case RefutableExtractor =>
815+
val extractor =
816+
val UnApply(fn, _, _) = pat: @unchecked
817+
fn match
818+
case Select(id, _) => id
819+
case TypeApply(Select(id, _), _) => id
820+
em"pattern binding uses refutable extractor `$extractor`"
821+
822+
val fix = if isPatDef then "adding `: @unchecked` after the expression" else "writing `case ` before the full pattern"
823+
val usage = reason match
824+
case NonConforming => "the narrowing"
825+
case RefutableExtractor => "this usage"
826+
val pos =
827+
if isPatDef then reason match
828+
case NonConforming => sel.srcPos
829+
case RefutableExtractor => pat.source.atSpan(pat.span union sel.span)
830+
else pat.srcPos
808831
report.warning(
809-
ex"""pattern's type ${pat.tpe} $problem the right hand side expression's type $reportedPt
832+
em"""$message
810833
|
811-
|If the narrowing is intentional, this can be communicated by $fix.${err.rewriteNotice}""",
834+
|If $usage is intentional, this can be communicated by $fix.${err.rewriteNotice}""",
812835
pos)
813836
false
814837
}
815838

816-
def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt)
839+
def check(pat: Tree, pt: Type): Boolean = (pt <:< pat.tpe) || fail(pat, pt, Reason.NonConforming)
817840

818841
def recur(pat: Tree, pt: Type): Boolean =
819842
!sourceVersion.isAtLeast(future) || // only for 3.x for now since mitigations work only after this PR
@@ -824,7 +847,7 @@ trait Checking {
824847
recur(pat1, pt)
825848
case UnApply(fn, _, pats) =>
826849
check(pat, pt) &&
827-
(isIrrefutable(fn, pats.length) || fail(pat, pt)) && {
850+
(isIrrefutable(fn, pats.length) || fail(pat, pt, Reason.RefutableExtractor)) && {
828851
val argPts = unapplyArgs(fn.tpe.widen.finalResultType, fn, pats, pat.srcPos)
829852
pats.corresponds(argPts)(recur)
830853
}

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

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

‎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)