use of custom unapply breaks exhaustivity checking

[fommil]

In 2.12.4 the following code (correctly) warns on compile due to exhaustivity checking

sealed abstract class Maybe[A]
final case class Empty[A]() extends Maybe[A]
final case class Just[A](a: A) extends Maybe[A]

Just(1) {
  case Just(2) => true
  case Empty() => true
}

but the following, with an alternative encoding of Empty (#10659) compiles without warning

  final case object Empty extends Maybe[Nothing] {
    def apply[A](): Maybe[A] = this.asInstanceOf[Maybe[A]]
    def unapply[A](e: Maybe[A]): Boolean = this == e
  }

Guessing, it looks like the presence of the "custom" unapply causes the more aggressive checkers not to trigger. Perhaps the presence of the custom unapply should only disable the aggressive checking around the Empty, not around the entire ADT Maybe.

// @xuwei-k

[sjrd]

Well of course it breaks exhaustivity checking, because the match might indeed not be exhaustive, if you replace the body of your unapply by false. The compiler has no way to inspect the body of your unapply to make a decision about exhaustivity.

[fommil]

Then it is broken in two ways. The one I care about is that it no longer warns that Just is not covered.

[sjrd]

Replace the body of your unapply by true and now Just is covered.

[fommil]

But that is then broken.

The custom extractor is against Maybe, so perhaps there is no way to fix this.

[sjrd]

But that is then broken.

You know that. The compiler has no idea.

[fommil]

yes, but that's just further evidence that the compiler should be more cautious, not less. It's almost like the exhaustiveness checker is assuming that Boolean unapply is returning true.

[TomasMikula]

So you would like a warning every time the compiler cannot prove exhaustiveness (as opposed to when it can prove non-exhaustiveness)?

[fommil]

That would be pretty good, yes! Effectively forcing pattern matches only for destructuring. I'm aware the motivating example is therefore excluded.

[SethTisue]

I'd love to have an annotation that fails compilation on non-exhaustive matches (somewhat like @tailrec). I looked for an existing ticket on that but failed, anyone know of one?

[fommil]

I did the same search when I wanted to close this as a dupe, and realised this is the closest we have. But my ticket search foo is pretty bad.

[fommil]

I added a custom unapply to the scalaz IList, looking like this

  object :: {
    def unapply[A](xs: ICons[A]): Option[(A, IList[A])] = xs match {
      case ICons(head, tail) => Some(head -> tail)
    }
  }

even though the input type is ICons, it still disables the exhaustivity checker for INil when used e.g.

ilist match {
  case head :: tail => ...
  // missing INil() but no warning!
}

in addition, the definition of an unapplySeq also breaks exhaustivity checking, e.g.

  def unapplySeq[A](ilist: IList[A]): Option[Seq[A]] = Some(ilist.toList)

It is debatable what should and shouldn't disable the matcher. But I feel it shouldn't happen silently (there is a scalafix request for this).

[durban]

@SethTisue See scala/scala#5617.

[LPTK]

In addition to this, it would be very nice if the exhaustive checker could special-case extractors that return Some[T] or the true literal type, and assume that these extractors can never fail.

Conceptually, the unapply shown by @fommil above should really be:

    def unapply[A](xs: ICons[A]): Some[(A, IList[A])] = xs match {
      case ICons(head, tail) => Some(head -> tail)
    }

It compiles, but neither Dotty nor Scalac leverage the additional knowledge. This would address @sjrd's remark that the compiler currently has no clue whether an extractor is fallible or not, although that is arguably an important semantic distinction. Would also probably help with scala/scala3#2575.

Perhaps that should be a separate ticket.

[smarter]

It compiles, but neither Dotty nor Scalac leverage the additional knowledge.

They both do in some situations, for example in:

sealed trait Base
case class One() extends Base
object Foo {
  def unapply(x: Base): Option[Int] =
    Some(0)
}

object Test {
  def test(x: Base) =
    x match {
      case Foo(a) => a
    }
}

You get a "match may not be exhaustive" warning for Dotty, if Option[Int] is replaced by Some[Int], the warning goes away.

[LPTK]

@smarter you're right, they seem to do the right thing for Some-returning extractors!, though Dotty has some weird rough edges. However, Dotty doesn't seem to like extractors with return type true (see the same link).

[smarter]

@LPTK Feel free to open issues for any problem you encounter, it helps!

[TomasMikula]

@LPTK The Scala issue for Some-returning extractors is #10502.

[jcracknell]

I find this issue interesting and felt it was worth mentioning that (as best I can tell) custom unapply for a case class is never used when pattern matching. The changes in 2.12.2 seemingly just allow the definition.

case class Foo(x: Int)
object Foo { def unapply(f: Foo): Option[Int] = ??? }
Foo(42) match { case Foo(i) => i } // 42, b/c obvs yo

[LPTK]

@jcracknell maybe we should open another issue for that. It does seem a little surprising, and I think emitting a warning for this would be a good idea.