Skip to content

Fix #12337: Enable exhaustivity check for case classes with checkable components #12377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 11, 2021

Conversation

liufengyun
Copy link
Contributor

Fix #12337: Enable exhaustivity check for case classes with checkable components

This aligns with Scala 2 behavior.

sealed trait Status
object Status {
case object Active extends Status
case object Inactive extends Status
Copy link
Contributor

@kevin-lee kevin-lee May 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can one of these case object be case class instead to make sure it works for case class as well?

e.g.)

sealed trait Status
object Status {
  case class Active(since: Int) extends Status
  case object Inactive extends Status
}

case class Foo(status: Status)
def bar(foo: Foo): Unit = foo match {
  case Foo(Status.Active(2000)) =>
    println("active since 2000")
}
// Expected:
// warning: match may not be exhaustive.
// It would fail on the following inputs: Foo(Active((x: Int forSome x not in 2000))), Foo(Inactive)
// def bar(foo: Foo): Unit = foo match {

Copy link
Contributor

@kevin-lee kevin-lee May 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or probably adding separate cases for case class and case object might be better?

e.g.)

sealed trait Status
object Status {
  case class Active(since: Int) extends Status
  case object Inactive extends Status
}

case class Foo(status: Status)
def bar(foo: Foo): Unit = foo match {
  case Foo(Status.Active(since)) =>
    println(s"active since $since")
}
// Expected:
// warning: match may not be exhaustive.
// It would fail on the following input: Foo(Inactive)
// def bar(foo: Foo): Unit = foo match {

def baz(foo: Foo): Unit = foo match {
  case Foo(Status.Active(2000)) =>
    println("active since 2000")
  case Foo(Status.Inactive) =>
    println("inactive")
}
// Expected:
// warning: match may not be exhaustive.
// It would fail on the following input: Foo(Active((x: Int forSome x not in 2000)))
// def baz(foo: Foo): Unit = foo match {

@liufengyun liufengyun merged commit 591154d into scala:master May 11, 2021
@liufengyun liufengyun deleted the fix-12337 branch May 11, 2021 07:21
@liufengyun
Copy link
Contributor Author

Thanks @dwijnand !

@kevin-lee
Copy link
Contributor

@liufengyun @dwijnand Thank you!

liufengyun added a commit to dotty-staging/dotty that referenced this pull request May 15, 2021
@Kordyjan Kordyjan added this to the 3.0.1 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No warning is triggered for non-exhaustive pattern matching on case class field
5 participants