SI-7850 CCE in patmat with invalid isEmpty.

Name-based pattern matcher needed some hardening against
unapply methods with the right name but wrong types. Only
isEmpty methods which return Boolean are acceptable.
Catching it directly rather than indirectly also allowed
for better error messages.

Author: paulp

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

@@ -625,8 +625,7 @@ trait ContextErrors {
         setError(tree)
       }
 
-      def CaseClassConstructorError(tree: Tree) = {
-        val baseMessage = tree.symbol + " is not a case class constructor, nor does it have an unapply/unapplySeq method"
+      def CaseClassConstructorError(tree: Tree, baseMessage: String) = {
         val addendum = directUnapplyMember(tree.symbol.info) match {
           case sym if hasMultipleNonImplicitParamLists(sym) => s"\nNote: ${sym.defString} exists in ${tree.symbol}, but it cannot be used as an extractor due to its second non-implicit parameter list"
           case _                                            => ""

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

@@ -78,18 +78,32 @@ trait PatternTypers {
       // Do some ad-hoc overloading resolution and update the tree's symbol and type
       // do not update the symbol if the tree's symbol's type does not define an unapply member
       // (e.g. since it's some method that returns an object with an unapply member)
-      val fun       = inPlaceAdHocOverloadingResolution(fun0)(hasUnapplyMember)
-      def caseClass = fun.tpe.typeSymbol.linkedClassOfClass
+      val fun         = inPlaceAdHocOverloadingResolution(fun0)(hasUnapplyMember)
+      val caseClass   = fun.tpe.typeSymbol.linkedClassOfClass
+      val member      = unapplyMember(fun.tpe)
+      def resultType  = (fun.tpe memberType member).finalResultType
+      def isEmptyType = resultOfMatchingMethod(resultType, nme.isEmpty)()
+      def isOkay      = (
+           resultType.isErroneous
+        || (resultType <:< BooleanTpe)
+        || (isEmptyType <:< BooleanTpe)
+        || member.isMacro
+        || member.isOverloaded // the whole overloading situation is over the rails
+      )
 
       // 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.
-      if (caseClass.isCase && !unapplyMember(fun.tpe).isOverloaded)
+      if (caseClass.isCase && !member.isOverloaded)
         logResult(s"convertToCaseConstructor($fun, $caseClass, pt=$pt)")(convertToCaseConstructor(fun, caseClass, pt))
-      else if (hasUnapplyMember(fun))
+      else if (!reallyExists(member))
+        CaseClassConstructorError(fun, s"${fun.symbol} is not a case class, nor does it have an unapply/unapplySeq member")
+      else if (isOkay)
         fun
+      else if (isEmptyType == NoType)
+        CaseClassConstructorError(fun, s"an unapply result must have a member `def isEmpty: Boolean")
       else
-        CaseClassConstructorError(fun)
+        CaseClassConstructorError(fun, s"an unapply result must have a member `def isEmpty: Boolean (found: def isEmpty: $isEmptyType)")
     }
 
     def typedArgsForFormals(args: List[Tree], formals: List[Type], mode: Mode): List[Tree] = {

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

@@ -39,8 +39,6 @@ trait Unapplies extends ast.TreeDSL {
    */
   def unapplyMember(tp: Type): Symbol = directUnapplyMember(tp) filter (sym => !hasMultipleNonImplicitParamLists(sym))
 
-  def unapplyMemberType(tree: Tree): Type = tree.tpe memberType unapplyMember(tree.tpe)
-
   object HasUnapply {
     def unapply(tp: Type): Option[Symbol] = unapplyMember(tp).toOption
   }

src/reflect/scala/reflect/internal/StdNames.scala

@@ -691,6 +691,7 @@ trait StdNames {
     val inlinedEquals: NameType        = "inlinedEquals"
     val isArray: NameType              = "isArray"
     val isDefinedAt: NameType          = "isDefinedAt"
+    val isEmpty: NameType              = "isEmpty"
     val isInstanceOf_ : NameType       = "isInstanceOf"
     val isInstanceOf_Ob : NameType     = "$isInstanceOf"
     val java: NameType                 = "java"

test/files/neg/t4425.check

@@ -1,12 +1,12 @@
-t4425.scala:3: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425.scala:3: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: Int)(y: Option[Int]): None.type exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
   42 match { case _ X _ => () }
                     ^
-t4425.scala:8: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425.scala:8: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: Int)(y: Int): Some[(Int, Int)] exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
   42 match { case _ X _ => () }
                     ^
-t4425.scala:13: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425.scala:13: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: String)(y: String): Some[(Int, Int)] exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
   "" match { case _ X _ => () }
                     ^

test/files/neg/t4425b.check

@@ -1,24 +1,24 @@
-t4425b.scala:5: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425b.scala:5: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
     println(      "" match { case _ X _   => "ok" ; case _ => "fail" })
                                     ^
-t4425b.scala:6: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425b.scala:6: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
     println((X: Any) match { case _ X _   => "ok" ; case _ => "fail" })
                                     ^
-t4425b.scala:7: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425b.scala:7: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
     println(      "" match { case X(_)    => "ok" ; case _ => "fail" })
                                   ^
-t4425b.scala:8: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425b.scala:8: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
     println((X: Any) match { case X(_)    => "ok" ; case _ => "fail" })
                                   ^
-t4425b.scala:9: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425b.scala:9: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
     println(      "" match { case X(_, _) => "ok" ; case _ => "fail" })
                                   ^
-t4425b.scala:10: error: object X is not a case class constructor, nor does it have an unapply/unapplySeq method
+t4425b.scala:10: error: object X is not a case class, nor does it have an unapply/unapplySeq member
 Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
     println((X: Any) match { case X(_, _) => "ok" ; case _ => "fail" })
                                   ^

test/files/neg/t7850.check

@@ -0,0 +1,7 @@
+t7850.scala:11: error: an unapply result must have a member `def isEmpty: Boolean (found: def isEmpty: Casey)
+    val Casey(x1) = new Casey(1)
+        ^
+t7850.scala:12: error: an unapply result must have a member `def isEmpty: Boolean
+    val Dingy(x2) = new Dingy(1)
+        ^
+two errors found

test/files/neg/t7850.scala

@@ -0,0 +1,16 @@
+// isEmpty returns non-boolean
+class Casey(a: Int) { def isEmpty = this; def get = this }
+object Casey { def unapply(a: Casey) = a }
+
+// no isEmpty method at all
+class Dingy(a: Int) { def get = this }
+object Dingy { def unapply(a: Dingy) = a }
+
+object Test {
+  def main(args: Array[String]) {
+    val Casey(x1) = new Casey(1)
+    val Dingy(x2) = new Dingy(1)
+    println(s"$x1 $x2")
+  }
+}
+