Skip to content

Commit

Permalink
Always check exhaustivity of custom extractors/guards
Browse files Browse the repository at this point in the history
  • Loading branch information
dwijnand committed Aug 27, 2020
1 parent f271f5e commit f053dfe
Show file tree
Hide file tree
Showing 13 changed files with 78 additions and 42 deletions.
2 changes: 0 additions & 2 deletions src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
Expand Up @@ -174,8 +174,6 @@ trait ScalaSettings extends AbsScalaSettings
def isAtLeastJunit = isTruthy || XmixinForceForwarders.value == "junit"
}

val XstrictPatmatAnalysis = BooleanSetting ("-Xstrict-patmat-analysis", "Assume pattern guards are false for the purposes of exhaustivity analysis")

// XML parsing options
object XxmlSettings extends MultiChoiceEnumeration {
val coalescing = Choice("coalescing", "Convert PCData to Text and coalesce sibling nodes")
Expand Down
Expand Up @@ -529,7 +529,6 @@ trait MatchAnalysis extends MatchApproximation {
// - back off (to avoid crying exhaustive too often) in unhandled cases
val start = if (StatisticsStatics.areSomeColdStatsEnabled) statistics.startTimer(statistics.patmatAnaExhaust) else null
var backoff = false
val strict = settings.XstrictPatmatAnalysis.value

val approx = new TreeMakersToProps(prevBinder)
val symbolicCases = approx.approximateMatch(cases, approx.onUnknown { tm =>
Expand All @@ -538,8 +537,9 @@ trait MatchAnalysis extends MatchApproximation {
case ExtractorTreeMaker(_, _, _)
| ProductExtractorTreeMaker(_, _)
| GuardTreeMaker(_) =>
if (strict) False else True
case _ => // debug.patmat("backing off due to "+ tm)
False
case _ =>
debug.patmat("backing off due to "+ tm)
backoff = true
False
})
Expand Down Expand Up @@ -904,7 +904,7 @@ trait MatchAnalysis extends MatchApproximation {
// then we can safely ignore these counter examples since we will eventually encounter
// both counter examples separately
case _ if inSameDomain =>
if (settings.XstrictPatmatAnalysis.value) Some(WildcardExample) else None
Some(WildcardExample)

// not a valid counter-example, possibly since we have a definite type but there was a field mismatch
// TODO: improve reasoning -- in the mean time, a false negative is better than an annoying false positive
Expand Down
30 changes: 23 additions & 7 deletions test/files/neg/t5365.check
@@ -1,15 +1,31 @@
t5365.scala:2: warning: match may not be exhaustive.
It would fail on the following input: None
t5365.scala:3: warning: match may not be exhaustive.
It would fail on the following inputs: None, Some(_)
def nonExhautiveIfWeAssumeGuardsTrueOrFalse(x: Option[Int]): Int = x match {
^
t5365.scala:17: warning: match may not be exhaustive.
It would fail on the following input: None
t5365.scala:7: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def nonExhautiveIfWeAssumeGuardsFalse(x: Option[Int]): Int = x match {
^
t5365.scala:12: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def inverseGuards(x: Option[Int]): Int = x match {
^
t5365.scala:18: warning: match may not be exhaustive.
It would fail on the following inputs: None, Some(_)
def extractor(x: Option[Int]) = x match {
^
t5365.scala:20: warning: match may not be exhaustive.
It would fail on the following input: None
t5365.scala:21: warning: match may not be exhaustive.
It would fail on the following inputs: None, Some(_)
def repeatedExtractor(x: Option[Int]) = x match {
^
t5365.scala:24: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def extractorStrict(x: Option[Int]) = x match {
^
t5365.scala:28: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def repeatedExtractorStrict(x: Option[Int]) = x match {
^
error: No warnings can be incurred under -Xfatal-warnings.
three warnings found
7 warnings found
one error found
14 changes: 7 additions & 7 deletions test/files/neg/t5365b.check
@@ -1,28 +1,28 @@
t5365b.scala:2: warning: match may not be exhaustive.
t5365b.scala:3: warning: match may not be exhaustive.
It would fail on the following inputs: None, Some(_)
def nonExhautiveIfWeAssumeGuardsTrueOrFalse(x: Option[Int]): Int = x match {
^
t5365b.scala:6: warning: match may not be exhaustive.
t5365b.scala:7: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def nonExhautiveIfWeAssumeGuardsFalse(x: Option[Int]): Int = x match {
^
t5365b.scala:11: warning: match may not be exhaustive.
t5365b.scala:12: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def inverseGuards(x: Option[Int]): Int = x match {
^
t5365b.scala:17: warning: match may not be exhaustive.
t5365b.scala:18: warning: match may not be exhaustive.
It would fail on the following inputs: None, Some(_)
def extractor(x: Option[Int]) = x match {
^
t5365b.scala:20: warning: match may not be exhaustive.
t5365b.scala:21: warning: match may not be exhaustive.
It would fail on the following inputs: None, Some(_)
def repeatedExtractor(x: Option[Int]) = x match {
^
t5365b.scala:23: warning: match may not be exhaustive.
t5365b.scala:24: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def extractorStrict(x: Option[Int]) = x match {
^
t5365b.scala:27: warning: match may not be exhaustive.
t5365b.scala:28: warning: match may not be exhaustive.
It would fail on the following input: Some(_)
def repeatedExtractorStrict(x: Option[Int]) = x match {
^
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t5365b.scala
@@ -1,4 +1,4 @@
// scalac: -Xfatal-warnings -Xstrict-patmat-analysis
// scalac: -Xfatal-warnings
class C {
def nonExhautiveIfWeAssumeGuardsTrueOrFalse(x: Option[Int]): Int = x match {
case Some(n) if n % 2 == 0 => n
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t5365c.check
@@ -1,4 +1,4 @@
t5365c.scala:5: warning: match may not be exhaustive.
t5365c.scala:6: warning: match may not be exhaustive.
It would fail on the following input: (x: C.Z forSome x not in C.Q)
def unsealedTrait(z: Z) = z match {
^
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t5365c.scala
@@ -1,4 +1,4 @@
// scalac: -Xfatal-warnings -Xstrict-patmat-analysis -Xlint:strict-unsealed-patmat
// scalac: -Xfatal-warnings -Xlint:strict-unsealed-patmat
object C {
trait Z
final case class Q(i: Int) extends Z
Expand Down
4 changes: 2 additions & 2 deletions test/files/neg/t5365d.check
@@ -1,8 +1,8 @@
t5365d.scala:10: warning: match may not be exhaustive.
t5365d.scala:11: warning: match may not be exhaustive.
It would fail on the following input: _
def extractorOnly(t: T): Unit = t match {
^
t5365d.scala:14: warning: match may not be exhaustive.
t5365d.scala:15: warning: match may not be exhaustive.
It would fail on the following input: D(_)
def extractorAndClass(t: T): Unit = t match {
^
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t5365d.scala
@@ -1,4 +1,4 @@
// scalac: -Xfatal-warnings -Xstrict-patmat-analysis
// scalac: -Xfatal-warnings
object D {
sealed trait T
final case class C(i: Int) extends T
Expand Down
22 changes: 13 additions & 9 deletions test/files/neg/t5365e.check
@@ -1,23 +1,27 @@
t5365e.scala:7: warning: match may not be exhaustive.
It would fail on the following input: Bar(_)
t5365e.scala:8: warning: match may not be exhaustive.
It would fail on the following inputs: Bar(_), Foo(_)
def f0(x: Exh) = x match { case Foo() => () } // don't back off
^
t5365e.scala:8: warning: match may not be exhaustive.
It would fail on the following input: Bar(_)
t5365e.scala:9: warning: match may not be exhaustive.
It would fail on the following inputs: Bar(_), Foo(_)
def f1(x: Exh) = x match { case Foo(x) => x } // don't back off
^
t5365e.scala:9: warning: match may not be exhaustive.
It would fail on the following input: Bar(_)
t5365e.scala:10: warning: match may not be exhaustive.
It would fail on the following inputs: Bar(_), Foo(_)
def f2(x: Exh) = x match { case Foo(x, y) => x + y } // don't back off
^
t5365e.scala:10: warning: match may not be exhaustive.
t5365e.scala:11: warning: match may not be exhaustive.
It would fail on the following input: Bar(_)
def fX(x: Exh) = x match { case Foo(xs @ _*) => xs } // don't back off
^
t5365e.scala:11: warning: match may not be exhaustive.
t5365e.scala:12: warning: match may not be exhaustive.
It would fail on the following input: Foo(_)
def b1(x: Exh) = x match { case Bar(x) => x } // inexhaustive
^
t5365e.scala:13: warning: match may not be exhaustive.
It would fail on the following input: Foo(_)
def fb(x: Exh) = x match { case Foo(x) => x case Bar(x) => x } // pessimistically inexhaustive
^
error: No warnings can be incurred under -Xfatal-warnings.
5 warnings found
6 warnings found
one error found
8 changes: 1 addition & 7 deletions test/files/neg/t5365e.scala
Expand Up @@ -10,11 +10,5 @@ class Main {
def f2(x: Exh) = x match { case Foo(x, y) => x + y } // don't back off
def fX(x: Exh) = x match { case Foo(xs @ _*) => xs } // don't back off
def b1(x: Exh) = x match { case Bar(x) => x } // inexhaustive
def fb(x: Exh) = x match { case Foo(x) => x case Bar(x) => x } // optimistically exhaustive
// ^ under -Xstrict-patmat-analysis pessimistically approximates case Foo(x) as inexhaustive:
// test/files/neg/t5365e.scala:12: warning: match may not be exhaustive.
// It would fail on the following input: Foo(_)
// def fb(x: Exh) = x match { case Foo(x) => x case Bar(x) => x } // optimistically exhaustive
// ^
// ... and the counter-example needs work -.- ...
def fb(x: Exh) = x match { case Foo(x) => x case Bar(x) => x } // pessimistically inexhaustive
}
16 changes: 16 additions & 0 deletions test/files/run/patmatnew.check
@@ -1,6 +1,22 @@
patmatnew.scala:670: warning: This catches all Throwables. If this is really intended, use `case e : Throwable` to clear this warning.
case e => {
^
patmatnew.scala:175: warning: match may not be exhaustive.
It would fail on the following input: _
def doMatch2(xs: List[String]): List[String] = xs match {
^
patmatnew.scala:277: warning: match may not be exhaustive.
It would fail on the following input: _
def doMatch1(xs: List[Char]) = xs match {
^
patmatnew.scala:280: warning: match may not be exhaustive.
It would fail on the following input: _
def doMatch2(xs: List[Char]) = xs match {
^
patmatnew.scala:315: warning: match may not be exhaustive.
It would fail on the following input: Cons()
stream match {
^
patmatnew.scala:489: warning: unreachable code
case _ if false =>
^
Expand Down
8 changes: 8 additions & 0 deletions test/files/run/virtpatmat_unapplyprod.check
@@ -1,3 +1,11 @@
virtpatmat_unapplyprod.scala:11: warning: match may not be exhaustive.
It would fail on the following input: _
FooSeq(2, "3") match {
^
virtpatmat_unapplyprod.scala:15: warning: match may not be exhaustive.
It would fail on the following input: _
FooSeq(2, "3", true, false, true) match {
^
virtpatmat_unapplyprod.scala:20: warning: match may not be exhaustive.
It would fail on the following inputs: FooSeq((x: Int forSome x not in 1), "a", _), FooSeq((x: Int forSome x not in 1), (x: String forSome x not in "a"), _), FooSeq(1, (x: String forSome x not in "a"), _)
FooSeq(1, "a", true, false, true) match {
Expand Down

0 comments on commit f053dfe

Please sign in to comment.