Merge pull request #9462 from dwijnand/patmat-unrelated-unapply

src/compiler/scala/tools/nsc/typechecker/PatternTypers.scala

@@ -82,6 +82,21 @@ trait PatternTypers {
       val member      = unapplyMember(fun.tpe)
       def resultType  = (fun.tpe memberType member).finalResultType
       def isEmptyType = resultOfIsEmpty(resultType)
+
+      def useConstructor = (
+        // Dueling test cases: pos/overloaded-unapply.scala, run/case-class-23.scala, pos/t5022.scala
+        // Use the case class constructor if (after canElide + isCase) the unapply method:
+        // (1) doesn't exist, e.g. case classes with 23+ params. run/case-class-23.scala
+        // (2) is the synthetic case class one, i.e. not user redefined. pos/t11252.scala
+        // (3a) is overloaded and the synthetic case class one is still present (i.e. not suppressed) pos/t12250.scala
+        // (3b) the scrutinee type is the case class (not a subtype). pos/overloaded-unapply.scala vs pos/t12250b.scala
+        canElide && caseClass.isCase && (
+             member == NoSymbol                                                  // (1)
+          || member.isSynthetic                                                  // (2)
+          || (member.alternatives.exists(_.isSynthetic) && caseClass.tpe =:= pt) // (3a)(3b)
+        )
+      )
+
       def isOkay      = (
            resultType.isErroneous
         || (resultType <:< BooleanTpe)
@@ -93,12 +108,7 @@ trait PatternTypers {
       // if we're already failing, no need to emit another error here
       if (fun.tpe.isErroneous)
         fun
-      // Dueling test cases: pos/overloaded-unapply.scala, run/case-class-23.scala, pos/t5022.scala
-      // A case class with 23+ params has no unapply method.
-      // A case class constructor may be overloaded with unapply methods in the companion.
-      // A case class maybe have its own custom unapply (so non-synthetic) scala/bug#11252
-      // Unapply methods aren't `isCaseApplyOrUnapply` in Scala 3 tasty/run/src-2/tastytest/TestColour.scala
-      else if (canElide && caseClass.isCase && !member.isOverloaded && (member == NoSymbol || member.isSynthetic))
+      else if (useConstructor)
         logResult(s"convertToCaseConstructor($fun, $caseClass, pt=$pt)")(convertToCaseConstructor(fun, caseClass, pt))
       else if (!reallyExists(member))
         CaseClassConstructorError(fun, s"${fun.symbol} is not a case class, nor does it have a valid unapply/unapplySeq member")

test/files/neg/case-class-23-unrelated-unapply.check

@@ -0,0 +1,7 @@
+case-class-23-unrelated-unapply.scala:42: error: too many arguments for unapply pattern, maximum = 22
+  val TwentyThree(a, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b) = x
+      ^
+case-class-23-unrelated-unapply.scala:45: error: too many arguments for unapply pattern, maximum = 22
+  val TwentyThree(a2, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b2) = arr
+      ^
+2 errors

test/files/neg/case-class-23-unrelated-unapply.scala

@@ -0,0 +1,47 @@
+case class TwentyThree(
+  _1: Int,
+  _2: Int,
+  _3: Int,
+  _4: Int,
+  _5: Int,
+  _6: Int,
+  _7: Int,
+  _8: Int,
+  _9: Int,
+  _10: Int,
+  _11: Int,
+  _12: Int,
+  _13: Int,
+  _14: Int,
+  _15: Int,
+  _16: Int,
+  _17: Int,
+  _18: Int,
+  _19: Int,
+  _20: Int,
+  _21: Int,
+  _22: Int,
+  _23: Int
+)
+
+object TwentyThree {
+  def unapply(a: Array[Int]): Option[TwentyThree] = {
+    Option.when(a.length == 23) {
+      TwentyThree(
+        a(0), a(1), a(2), a(3), a(4), a(5), a(6), a(7), a(8), a(9),
+        a(10), a(11), a(12), a(13), a(14), a(15), a(16), a(17), a(18), a(19),
+        a(20), a(21), a(22),
+      )
+    }
+  }
+}
+
+// This is borked.. but I'm fairly certain it's borked since before I started meddling with it..
+object Test extends App {
+  val x = new TwentyThree(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23)
+  val TwentyThree(a, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b) = x
+  println((a, b))
+  val arr = Array(0 , 1 , 2 , 3 , 4 , 5 , 6 , 7 , 8 , 9 , 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22)
+  val TwentyThree(a2, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, b2) = arr
+  println((a2, b2))
+}

test/files/pos/t12250.scala

@@ -0,0 +1,11 @@
+// scalac: -Werror
+final case class Foo(value: String)
+
+object Foo {
+  def unapply(str: String): Option[Foo] = Some(Foo(str))
+
+  def extract(id: Foo): String =
+    id match {
+      case Foo(a) => a
+    }
+}

test/files/pos/t12250b.scala

@@ -0,0 +1,23 @@
+// scalac: -Werror
+
+sealed case class Sub1(str: String)
+final  case class Sup1(str: String) extends Sup0
+
+final  class Sub2 extends Sub1("")
+sealed trait Sup0 { def str: String }
+
+// both of these unapplies are overloads of the synthetic unapply
+// i.e. it isn't suppressed
+object Sub1 { def unapply(x: Sub2): Some[String] = Some(x.str) }
+object Sup1 { def unapply(x: Sup0): Some[String] = Some(x.str) }
+
+object Test {
+  // these seek the original unapplies and should be converted to use their constructors
+  def testSub1(x: Sub1) = x match { case Sub1(str) => str }
+  def testSup1(x: Sup1) = x match { case Sup1(str) => str }
+
+  // these seek the user-defined alternative unapplies
+  // thus they shouldn't accidentally be converted to use their constructors
+  def testSub2(x: Sub2) = x match { case Sub1(str) => str }
+  def testSup0(x: Sup0) = x match { case Sup1(str) => str }
+}

test/files/run/case-class-23-redefined-unapply.check

@@ -0,0 +1 @@
+(1,3)

test/files/run/case-class-23-redefined-unapply.scala

@@ -0,0 +1,35 @@
+case class TwentyThree(
+  _1: Int,
+  _2: Int,
+  _3: Int,
+  _4: Int,
+  _5: Int,
+  _6: Int,
+  _7: Int,
+  _8: Int,
+  _9: Int,
+  _10: Int,
+  _11: Int,
+  _12: Int,
+  _13: Int,
+  _14: Int,
+  _15: Int,
+  _16: Int,
+  _17: Int,
+  _18: Int,
+  _19: Int,
+  _20: Int,
+  _21: Int,
+  _22: Int,
+  _23: Int
+)
+
+object TwentyThree {
+  def unapply(x: TwentyThree): Some[(Int, Int, Int)] = Some(x._1, x._2, x._3)
+}
+
+object Test extends App {
+  val x = new TwentyThree(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23)
+  val TwentyThree(a, 2, b) = x
+  println((a, b))
+}