From 9fd4000e550b8ceca20031e68fd46cc9f63ecb87 Mon Sep 17 00:00:00 2001
From: Dale Wijnand <dale.wijnand@gmail.com>
Date: Thu, 8 Jul 2021 10:00:07 +0100
Subject: [PATCH] Exhaustivity warnings on nested case classes

Fixes #13003, by refixing #12485 (PR #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...
---
 .../dotty/tools/dotc/transform/patmat/Space.scala  | 13 +++++++------
 .../dotc/transform/PatmatExhaustivityTest.scala    | 13 +++++++++----
 tests/{patmat => patmat-default}/i12485.scala      |  0
 tests/patmat-default/i13003.check                  |  4 ++++
 tests/patmat-default/i13003.scala                  | 14 ++++++++++++++
 5 files changed, 34 insertions(+), 10 deletions(-)
 rename tests/{patmat => patmat-default}/i12485.scala (100%)
 create mode 100644 tests/patmat-default/i13003.check
 create mode 100644 tests/patmat-default/i13003.scala

diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
index e3ad8f946000..af02bfed8411 100644
--- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
+++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
@@ -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
@@ -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")
diff --git a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala
index 58e1133180ec..a0c46c829d0d 100644
--- a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala
+++ b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala
@@ -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 {
diff --git a/tests/patmat/i12485.scala b/tests/patmat-default/i12485.scala
similarity index 100%
rename from tests/patmat/i12485.scala
rename to tests/patmat-default/i12485.scala
diff --git a/tests/patmat-default/i13003.check b/tests/patmat-default/i13003.check
new file mode 100644
index 000000000000..19fb6a466549
--- /dev/null
+++ b/tests/patmat-default/i13003.check
@@ -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))
diff --git a/tests/patmat-default/i13003.scala b/tests/patmat-default/i13003.scala
new file mode 100644
index 000000000000..af9158e09c5e
--- /dev/null
+++ b/tests/patmat-default/i13003.scala
@@ -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!"