Skip to content

Commit 9fd4000

Browse files
committed
Exhaustivity warnings on nested case classes
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...
1 parent 46d0b05 commit 9fd4000

File tree

5 files changed

+34
-10
lines changed

5 files changed

+34
-10
lines changed

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,8 @@ class SpaceEngine(using Context) extends SpaceLogic {
804804
}
805805

806806
private def exhaustivityCheckable(sel: Tree): Boolean = {
807+
val seen = collection.mutable.Set.empty[Type]
808+
807809
// Possible to check everything, but be compatible with scalac by default
808810
def isCheckable(tp: Type): Boolean =
809811
val tpw = tp.widen.dealias
@@ -815,16 +817,15 @@ class SpaceEngine(using Context) extends SpaceLogic {
815817
isCheckable(and.tp1) || isCheckable(and.tp2)
816818
}) ||
817819
tpw.isRef(defn.BooleanClass) ||
818-
classSym.isAllOf(JavaEnumTrait)
820+
classSym.isAllOf(JavaEnumTrait) ||
821+
classSym.is(Case) && {
822+
if seen.add(tpw) then productSelectorTypes(tpw, sel.srcPos).exists(isCheckable(_))
823+
else true // recursive case class: return true and other members can still fail the check
824+
}
819825

820826
val res = !sel.tpe.hasAnnotation(defn.UncheckedAnnot) && {
821827
ctx.settings.YcheckAllPatmat.value
822828
|| isCheckable(sel.tpe)
823-
|| {
824-
val tpw = sel.tpe.widen.dealias
825-
val classSym = tpw.classSymbol
826-
classSym.is(Case) && productSelectorTypes(tpw, sel.srcPos).exists(isCheckable(_))
827-
}
828829
}
829830

830831
debug.println(s"exhaustivity checkable: ${sel.show} = $res")

compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,26 @@ import java.nio.file.{Path => JPath}
1515
import scala.io.Source._
1616
import org.junit.Test
1717

18+
class PatmatDefaultExhaustivityTest extends PatmatExhaustivityTest {
19+
override val testsDir = "tests/patmat-default"
20+
override val options = super.options.filter(_ != "-Ycheck-all-patmat")
21+
}
22+
1823
class PatmatExhaustivityTest {
1924
val testsDir = "tests/patmat"
2025
// stop-after: patmatexhaust-huge.scala crash compiler
21-
val options = List("-pagewidth", "80", "-color:never", "-Ystop-after:explicitSelf", "-Ycheck-all-patmat", "-classpath", TestConfiguration.basicClasspath)
26+
def options = List("-pagewidth", "80", "-color:never", "-Ystop-after:explicitSelf", "-Ycheck-all-patmat", "-classpath", TestConfiguration.basicClasspath)
2227

2328
private def compile(files: Seq[String]): Seq[String] = {
2429
val stringBuffer = new StringWriter()
25-
val reporter = TestReporter.simplifiedReporter(new PrintWriter(stringBuffer))
30+
val printWriter = new PrintWriter(stringBuffer)
31+
val reporter = TestReporter.simplifiedReporter(printWriter)
2632

2733
try {
2834
Main.process((options ++ files).toArray, reporter, null)
2935
} catch {
3036
case e: Throwable =>
31-
println(s"Compile $files exception:")
32-
e.printStackTrace()
37+
e.printStackTrace(printWriter)
3338
}
3439

3540
stringBuffer.toString.trim.replaceAll("\\s+\n", "\n") match {
File renamed without changes.

tests/patmat-default/i13003.check

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
4: Pattern Match Exhaustivity: One(Two(None))
2+
7: Pattern Match Exhaustivity: Two(None)
3+
10: Pattern Match Exhaustivity: None, Some(None)
4+
13: Pattern Match Exhaustivity: None, Some(None), Some(Some(None))

tests/patmat-default/i13003.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
case class One(two: Two)
2+
case class Two(o: Option[Int])
3+
4+
def matchOneTwo(one: One) = one match
5+
case One(Two(Some(i))) => "match!"
6+
7+
def matchTwo(two: Two) = two match
8+
case Two(Some(i)) => "match!"
9+
10+
def matchOO(oo: Option[Option[Int]]) = oo match
11+
case Some(Some(i)) => "match!"
12+
13+
def matchOOO(ooo: Option[Option[Option[Int]]]) = ooo match
14+
case Some(Some(Some(i))) => "match!"

0 commit comments

Comments
 (0)