Skip to content

Support pattern matching on NonEmptyVector #1265

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.

Sign up for GitHub

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
kailuowang merged 1 commit into from
Aug 5, 2016
Merged

Support pattern matching on NonEmptyVector #1265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/src/main/scala/cats/data/NonEmptyVector.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ object NonEmptyVector extends NonEmptyVectorInstances {
new NonEmptyVector(buf.result)
}

def unapply[A](nev: NonEmptyVector[A]): Some[(A, Vector[A])] = Some((nev.head, nev.tail))

@ceedubs ceedubs Aug 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a foreseeable use-case where having the static return type as Some instead of Option will cause inference (or similar) issues?

@travisbrown travisbrown Aug 4, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ceedubs There's some reason to prefer Some as the return type for extractors that can't fail—the compiler actually uses that information in some way, but I can't remember the details. I'll try to look up a reference…

@larsrh larsrh Aug 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The compiler can check for exhaustiveness if it returns Some. It can't when it returns Option.

@dwijnand dwijnand Aug 4, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@larsrh Got a code snippet showing that?

AFAIK Some can't check exhaustiveness any more than Option as it's not sealed (sealedness isn't transitive nor does final imply sealed, double sadly).

@larsrh larsrh Aug 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scala> object MyCons1 { def unapply[A](xs: scala.collection.immutable.::[A]): Some[(A, List[A])] = Some(xs.head, xs.tail) }
defined object MyCons1

scala> object MyCons2 { def unapply[A](xs: scala.collection.immutable.::[A]): Option[(A, List[A])] = Some(xs.head, xs.tail) }
defined object MyCons2

scala> def x = List(1, 2) match { case MyCons1(_, _) => }
<console>:12: warning: match may not be exhaustive.
It would fail on the following input: Nil
       def x = List(1, 2) match { case MyCons1(_, _) => }
                   ^
x: Unit

scala> def x = List(1, 2) match { case MyCons2(_, _) => }
x: Unit

@larsrh larsrh Aug 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AFAICT it's unspecced behaviour, but I think it makes a lot of sense for the pattern matcher to behave that way.

@ceedubs ceedubs Aug 4, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh neat. Thanks, @larsrh!


def fromVector[A](vector: Vector[A]): Option[NonEmptyVector[A]] =
if (vector.isEmpty) None else Some(new NonEmptyVector(vector))

Expand Down
10 changes: 10 additions & 0 deletions tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,16 @@ class NonEmptyVectorTests extends CatsSuite {
NonEmptyVector(1, Vector.empty).toVector.toString should === ("Vector(1)")
}

test("NonEmptyVector.unapply supports pattern matching") {
forAll { (nonEmptyVector: NonEmptyVector[Int]) =>
nonEmptyVector match {
case NonEmptyVector(head, tail) =>
head should === (nonEmptyVector.head)
tail should === (nonEmptyVector.tail)
}
}
}

test("Cannot create a new NonEmptyVector from constructor") {
if(Platform.isJvm) {
if (!Properties.versionNumberString.startsWith("2.10")) {
Expand Down