Skip to content

Commit

Permalink
Exhaustivity warnings on nested case classes
Browse files Browse the repository at this point in the history
Fixes scala#13003, by refixing scala#12485 (PR scala#12488).

Part of the issue is that isCheckable behaves differently under
-Ycheck-all-patmat and our tests only run under that flag.  So for
starters I added a test variant where that flag isn't used.  I'd like to
understand why that flag exists to see if we could remove it from
guarding the logic and the tests.

Also make sure that any patmat test that throws an exception fails the
test...
  • Loading branch information
dwijnand committed Jul 9, 2021
1 parent 46d0b05 commit 9fd4000
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 10 deletions.
13 changes: 7 additions & 6 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,8 @@ class SpaceEngine(using Context) extends SpaceLogic {
}

private def exhaustivityCheckable(sel: Tree): Boolean = {
val seen = collection.mutable.Set.empty[Type]

// Possible to check everything, but be compatible with scalac by default
def isCheckable(tp: Type): Boolean =
val tpw = tp.widen.dealias
Expand All @@ -815,16 +817,15 @@ class SpaceEngine(using Context) extends SpaceLogic {
isCheckable(and.tp1) || isCheckable(and.tp2)
}) ||
tpw.isRef(defn.BooleanClass) ||
classSym.isAllOf(JavaEnumTrait)
classSym.isAllOf(JavaEnumTrait) ||
classSym.is(Case) && {
if seen.add(tpw) then productSelectorTypes(tpw, sel.srcPos).exists(isCheckable(_))
else true // recursive case class: return true and other members can still fail the check
}

val res = !sel.tpe.hasAnnotation(defn.UncheckedAnnot) && {
ctx.settings.YcheckAllPatmat.value
|| isCheckable(sel.tpe)
|| {
val tpw = sel.tpe.widen.dealias
val classSym = tpw.classSymbol
classSym.is(Case) && productSelectorTypes(tpw, sel.srcPos).exists(isCheckable(_))
}
}

debug.println(s"exhaustivity checkable: ${sel.show} = $res")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,26 @@ import java.nio.file.{Path => JPath}
import scala.io.Source._
import org.junit.Test

class PatmatDefaultExhaustivityTest extends PatmatExhaustivityTest {
override val testsDir = "tests/patmat-default"
override val options = super.options.filter(_ != "-Ycheck-all-patmat")
}

class PatmatExhaustivityTest {
val testsDir = "tests/patmat"
// stop-after: patmatexhaust-huge.scala crash compiler
val options = List("-pagewidth", "80", "-color:never", "-Ystop-after:explicitSelf", "-Ycheck-all-patmat", "-classpath", TestConfiguration.basicClasspath)
def options = List("-pagewidth", "80", "-color:never", "-Ystop-after:explicitSelf", "-Ycheck-all-patmat", "-classpath", TestConfiguration.basicClasspath)

private def compile(files: Seq[String]): Seq[String] = {
val stringBuffer = new StringWriter()
val reporter = TestReporter.simplifiedReporter(new PrintWriter(stringBuffer))
val printWriter = new PrintWriter(stringBuffer)
val reporter = TestReporter.simplifiedReporter(printWriter)

try {
Main.process((options ++ files).toArray, reporter, null)
} catch {
case e: Throwable =>
println(s"Compile $files exception:")
e.printStackTrace()
e.printStackTrace(printWriter)
}

stringBuffer.toString.trim.replaceAll("\\s+\n", "\n") match {
Expand Down
File renamed without changes.
4 changes: 4 additions & 0 deletions tests/patmat-default/i13003.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
4: Pattern Match Exhaustivity: One(Two(None))
7: Pattern Match Exhaustivity: Two(None)
10: Pattern Match Exhaustivity: None, Some(None)
13: Pattern Match Exhaustivity: None, Some(None), Some(Some(None))
14 changes: 14 additions & 0 deletions tests/patmat-default/i13003.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
case class One(two: Two)
case class Two(o: Option[Int])

def matchOneTwo(one: One) = one match
case One(Two(Some(i))) => "match!"

def matchTwo(two: Two) = two match
case Two(Some(i)) => "match!"

def matchOO(oo: Option[Option[Int]]) = oo match
case Some(Some(i)) => "match!"

def matchOOO(ooo: Option[Option[Option[Int]]]) = ooo match
case Some(Some(Some(i))) => "match!"

0 comments on commit 9fd4000

Please sign in to comment.