From f43228c47c7da40f88142d695d56be2d332f54fd Mon Sep 17 00:00:00 2001 From: Daniel Urban Date: Mon, 21 Aug 2017 20:18:30 +0200 Subject: [PATCH 1/6] Sync NEL and NEV methods (fixes #1832) --- .../main/scala/cats/data/NonEmptyList.scala | 24 ++++++--- .../main/scala/cats/data/NonEmptyVector.scala | 18 +++++++ .../scala/cats/tests/NonEmptyListTests.scala | 14 +++++- .../cats/tests/NonEmptyVectorTests.scala | 49 ++++++++++++++++++- 4 files changed, 94 insertions(+), 11 deletions(-) diff --git a/core/src/main/scala/cats/data/NonEmptyList.scala b/core/src/main/scala/cats/data/NonEmptyList.scala index 4dfc37dc61..a12cd68de3 100644 --- a/core/src/main/scala/cats/data/NonEmptyList.scala +++ b/core/src/main/scala/cats/data/NonEmptyList.scala @@ -66,6 +66,8 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) { */ def size: Int = 1 + tail.size + def length: Int = size + /** * Applies f to all the elements of the structure */ @@ -73,12 +75,24 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) { NonEmptyList(f(head), tail.map(f)) def ++[AA >: A](l: List[AA]): NonEmptyList[AA] = - NonEmptyList(head, tail ++ l) + concat(l) + + def concat[AA >: A](other: List[AA]): NonEmptyList[AA] = + NonEmptyList(head, tail ::: other) + + /** + * Append another NonEmptyList + */ + def concatNel[AA >: A](other: NonEmptyList[AA]): NonEmptyList[AA] = + NonEmptyList(head, tail ::: other.toList) def flatMap[B](f: A => NonEmptyList[B]): NonEmptyList[B] = f(head) ++ tail.flatMap(f andThen (_.toList)) def ::[AA >: A](a: AA): NonEmptyList[AA] = + prepend(a) + + def prepend[AA >: A](a: AA): NonEmptyList[AA] = NonEmptyList(a, head :: tail) /** @@ -137,12 +151,6 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) { } } - /** - * Append another NonEmptyList - */ - def concat[AA >: A](other: NonEmptyList[AA]): NonEmptyList[AA] = - NonEmptyList(head, tail ::: other.toList) - /** * Find the first element matching the predicate, if one exists */ @@ -405,7 +413,7 @@ private[data] sealed abstract class NonEmptyListInstances extends NonEmptyListIn with Monad[NonEmptyList] with NonEmptyTraverse[NonEmptyList] { def combineK[A](a: NonEmptyList[A], b: NonEmptyList[A]): NonEmptyList[A] = - a concat b + a concatNel b override def split[A](fa: NonEmptyList[A]): (A, List[A]) = (fa.head, fa.tail) diff --git a/core/src/main/scala/cats/data/NonEmptyVector.scala b/core/src/main/scala/cats/data/NonEmptyVector.scala index 9fdeebd230..00c8160913 100644 --- a/core/src/main/scala/cats/data/NonEmptyVector.scala +++ b/core/src/main/scala/cats/data/NonEmptyVector.scala @@ -36,6 +36,10 @@ final class NonEmptyVector[+A] private (val toVector: Vector[A]) extends AnyVal def tail: Vector[A] = toVector.tail + def last: A = toVector.last + + def init: Vector[A] = toVector.init + /** * Remove elements not matching the predicate * @@ -60,6 +64,8 @@ final class NonEmptyVector[+A] private (val toVector: Vector[A]) extends AnyVal */ def filterNot(f: A => Boolean): Vector[A] = toVector.filterNot(f) + def collect[B](pf: PartialFunction[A, B]): Vector[B] = toVector.collect(pf) + /** * Alias for [[concat]] */ @@ -198,6 +204,18 @@ final class NonEmptyVector[+A] private (val toVector: Vector[A]) extends AnyVal */ def zipWith[B, C](b: NonEmptyVector[B])(f: (A, B) => C): NonEmptyVector[C] = NonEmptyVector.fromVectorUnsafe((toVector, b.toVector).zipped.map(f)) + + def reverse: NonEmptyVector[A] = + new NonEmptyVector(toVector.reverse) + + def zipWithIndex: NonEmptyVector[(A, Int)] = + new NonEmptyVector(toVector.zipWithIndex) + + def sortBy[B](f: A => B)(implicit B: Order[B]): NonEmptyVector[A] = + new NonEmptyVector(toVector.sortBy(f)(B.toOrdering)) + + def sorted[AA >: A](implicit AA: Order[AA]): NonEmptyVector[AA] = + new NonEmptyVector(toVector.sorted(AA.toOrdering)) } private[data] sealed abstract class NonEmptyVectorInstances { diff --git a/tests/src/test/scala/cats/tests/NonEmptyListTests.scala b/tests/src/test/scala/cats/tests/NonEmptyListTests.scala index 2e1497dcc0..69c7294537 100644 --- a/tests/src/test/scala/cats/tests/NonEmptyListTests.scala +++ b/tests/src/test/scala/cats/tests/NonEmptyListTests.scala @@ -224,6 +224,7 @@ class NonEmptyListTests extends CatsSuite { test(":: consistent with List") { forAll { (nel: NonEmptyList[Int], i: Int) => (i :: nel).toList should === (i :: nel.toList) + nel.prepend(i).toList should === (i :: nel.toList) } } @@ -257,9 +258,10 @@ class NonEmptyListTests extends CatsSuite { } } - test("NonEmptyList#size is consistent with List#size") { + test("NonEmptyList#size and length is consistent with List#size") { forAll { nel: NonEmptyList[Int] => nel.size should === (nel.toList.size) + nel.length should === (nel.toList.size) } } @@ -282,7 +284,15 @@ class NonEmptyListTests extends CatsSuite { } } - test("NonEmptyList#fromFoldable is consistent with NonEmptyList#fromList") { + test("NonEmptyList#concat/concatNel is consistent with List#:::") { + forAll { (nel: NonEmptyList[Int], l: List[Int], n: Int) => + (nel ++ l).toList should === (nel.toList ::: l) + nel.concat(l).toList should === (nel.toList ::: l) + nel.concatNel(NonEmptyList(n, l)).toList should === (nel.toList ::: (n :: l)) + } + } + + test("NonEmptyList#fromFoldabale is consistent with NonEmptyList#fromList") { forAll { (xs: List[Int]) => NonEmptyList.fromList(xs) should === (NonEmptyList.fromFoldable(xs)) } diff --git a/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala b/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala index 93b3284d34..9a417e4631 100644 --- a/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala +++ b/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala @@ -317,12 +317,12 @@ class NonEmptyVectorTests extends CatsSuite { } } - test("NonEmptyVector#zipWith is consistent with Vector#zip and then Vector#map") { forAll { (a: NonEmptyVector[Int], b: NonEmptyVector[Int], f: (Int, Int) => Int) => a.zipWith(b)(f).toVector should ===(a.toVector.zip(b.toVector).map { case (x, y) => f(x, y) }) } } + test("NonEmptyVector#nonEmptyPartition remains sorted") { forAll { (nev: NonEmptyVector[Int], f: Int => Either[String, String]) => @@ -333,6 +333,53 @@ class NonEmptyVectorTests extends CatsSuite { ior.right.map(xs => xs.sorted should === (xs)) } } + + test("NonEmptyVector#last is consistent with Vector#last") { + forAll { nonEmptyVector: NonEmptyVector[Int] => + nonEmptyVector.last should === (nonEmptyVector.toVector.last) + } + } + + test("NonEmptyVector#init is consistent with Vector#init") { + forAll { nonEmptyVector: NonEmptyVector[Int] => + nonEmptyVector.init should === (nonEmptyVector.toVector.init) + } + } + + test("NonEmptyVector#collect is consistent with Vector#collect") { + val pf: PartialFunction[Int, Double] = { + case i if (i % 2 == 0) => i.toDouble + } + forAll { nonEmptyVector: NonEmptyVector[Int] => + nonEmptyVector.collect(pf) should === (nonEmptyVector.toVector.collect(pf)) + } + } + + test("NonEmptyVector#length and size is consistent with Vector#length") { + forAll { nonEmptyVector: NonEmptyVector[Int] => + nonEmptyVector.length should === (nonEmptyVector.toVector.length) + nonEmptyVector.size should === (nonEmptyVector.toVector.length.toLong) + } + } + + test("NonEmptyVector#reverse is consistent with Vector#reverse") { + forAll { nonEmptyVector: NonEmptyVector[Int] => + nonEmptyVector.reverse should === (NonEmptyVector.fromVectorUnsafe(nonEmptyVector.toVector.reverse)) + } + } + + test("NonEmptyVector#zipWithIndex is consistent with Vector#zipWithIndex") { + forAll { nonEmptyVector: NonEmptyVector[Int] => + nonEmptyVector.zipWithIndex should === (NonEmptyVector.fromVectorUnsafe(nonEmptyVector.toVector.zipWithIndex)) + } + } + + test("NonEmptyVector#sorted and sortBy is consistent with Vector#sorted and sortBy") { + forAll { nonEmptyVector: NonEmptyVector[Int] => + nonEmptyVector.sorted should === (NonEmptyVector.fromVectorUnsafe(nonEmptyVector.toVector.sorted)) + nonEmptyVector.sortBy(i => -i) should === (NonEmptyVector.fromVectorUnsafe(nonEmptyVector.toVector.sortBy(i => -i))) + } + } } class ReducibleNonEmptyVectorCheck extends ReducibleCheck[NonEmptyVector]("NonEmptyVector") { From 1bf60ee788cf12009d151677a79de02dcf8fafb1 Mon Sep 17 00:00:00 2001 From: Daniel Urban Date: Wed, 23 Aug 2017 21:09:06 +0200 Subject: [PATCH 2/6] Override Traverse#zipWithIndex for NonEmptyVector --- core/src/main/scala/cats/data/NonEmptyVector.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/scala/cats/data/NonEmptyVector.scala b/core/src/main/scala/cats/data/NonEmptyVector.scala index 00c8160913..0750618411 100644 --- a/core/src/main/scala/cats/data/NonEmptyVector.scala +++ b/core/src/main/scala/cats/data/NonEmptyVector.scala @@ -269,6 +269,9 @@ private[data] sealed abstract class NonEmptyVectorInstances { override def traverse[G[_], A, B](fa: NonEmptyVector[A])(f: (A) => G[B])(implicit G: Applicative[G]): G[NonEmptyVector[B]] = G.map2Eval(f(fa.head), Always(Traverse[Vector].traverse(fa.tail)(f)))(NonEmptyVector(_, _)).value + override def zipWithIndex[A](fa: NonEmptyVector[A]): NonEmptyVector[(A, Int)] = + fa.zipWithIndex + override def foldLeft[A, B](fa: NonEmptyVector[A], b: B)(f: (B, A) => B): B = fa.foldLeft(b)(f) From d21fa9d37bbc9ba50e459945adf0793f0587fe5f Mon Sep 17 00:00:00 2001 From: Daniel Urban Date: Sat, 16 Sep 2017 15:26:53 +0200 Subject: [PATCH 3/6] More tests for NonEmptyVector#zipWithIndex --- .../test/scala/cats/tests/NonEmptyVectorTests.scala | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala b/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala index 9a417e4631..7bd91690d0 100644 --- a/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala +++ b/tests/src/test/scala/cats/tests/NonEmptyVectorTests.scala @@ -323,6 +323,13 @@ class NonEmptyVectorTests extends CatsSuite { } } + test("NonEmptyVector#zipWith is consistent with #zipWithIndex") { + forAll { nev: NonEmptyVector[Int] => + val zw = nev.zipWith(NonEmptyVector.fromVectorUnsafe((0 until nev.length).toVector))(Tuple2.apply) + nev.zipWithIndex should === (zw) + } + } + test("NonEmptyVector#nonEmptyPartition remains sorted") { forAll { (nev: NonEmptyVector[Int], f: Int => Either[String, String]) => @@ -370,7 +377,9 @@ class NonEmptyVectorTests extends CatsSuite { test("NonEmptyVector#zipWithIndex is consistent with Vector#zipWithIndex") { forAll { nonEmptyVector: NonEmptyVector[Int] => - nonEmptyVector.zipWithIndex should === (NonEmptyVector.fromVectorUnsafe(nonEmptyVector.toVector.zipWithIndex)) + val expected = NonEmptyVector.fromVectorUnsafe(nonEmptyVector.toVector.zipWithIndex) + nonEmptyVector.zipWithIndex should === (expected) + Traverse[NonEmptyVector].zipWithIndex(nonEmptyVector) should === (expected) } } From 9a5d4ffcfb9fde43579884368111a501b1e7be21 Mon Sep 17 00:00:00 2001 From: Daniel Urban Date: Fri, 6 Oct 2017 19:07:26 +0200 Subject: [PATCH 4/6] Add deprecated overload of NonEmptyList#concat --- core/src/main/scala/cats/data/NonEmptyList.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/scala/cats/data/NonEmptyList.scala b/core/src/main/scala/cats/data/NonEmptyList.scala index a12cd68de3..946729b011 100644 --- a/core/src/main/scala/cats/data/NonEmptyList.scala +++ b/core/src/main/scala/cats/data/NonEmptyList.scala @@ -80,6 +80,10 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) { def concat[AA >: A](other: List[AA]): NonEmptyList[AA] = NonEmptyList(head, tail ::: other) + @deprecated("Use concatNel", since = "1.0.0-RC1") + def concat[AA >: A](other: NonEmptyList[AA]): NonEmptyList[AA] = + concatNel(other) + /** * Append another NonEmptyList */ From 7a41804cc8a91849b70089aa5a3e81521e95e6e9 Mon Sep 17 00:00:00 2001 From: Daniel Urban Date: Sat, 7 Oct 2017 21:07:06 +0200 Subject: [PATCH 5/6] Try to appease CI --- tests/src/test/scala/cats/tests/NonEmptyListTests.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/test/scala/cats/tests/NonEmptyListTests.scala b/tests/src/test/scala/cats/tests/NonEmptyListTests.scala index 69c7294537..c1774d76f8 100644 --- a/tests/src/test/scala/cats/tests/NonEmptyListTests.scala +++ b/tests/src/test/scala/cats/tests/NonEmptyListTests.scala @@ -12,6 +12,7 @@ import cats.data.{NonEmptyList, NonEmptyVector} import cats.laws.discipline.arbitrary._ import cats.laws.discipline.{ComonadTests, NonEmptyTraverseTests, MonadTests, ReducibleTests, SemigroupKTests, SerializableTests} +@deprecated("to be able to test deprecated methods", since = "1.0.0-RC1") class NonEmptyListTests extends CatsSuite { // Lots of collections here.. telling ScalaCheck to calm down a bit implicit override val generatorDrivenConfig: PropertyCheckConfiguration = @@ -288,6 +289,7 @@ class NonEmptyListTests extends CatsSuite { forAll { (nel: NonEmptyList[Int], l: List[Int], n: Int) => (nel ++ l).toList should === (nel.toList ::: l) nel.concat(l).toList should === (nel.toList ::: l) + nel.concat(NonEmptyList(n, l)).toList should === (nel.toList ::: (n :: l)) nel.concatNel(NonEmptyList(n, l)).toList should === (nel.toList ::: (n :: l)) } } From f18b06ff4de48034f0facae6156a2436ca7a9c92 Mon Sep 17 00:00:00 2001 From: Daniel Urban Date: Wed, 11 Oct 2017 19:26:26 +0200 Subject: [PATCH 6/6] Deprecating separately --- ...nEmptyListTests.scala => nonEmptyListTests.scala} | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) rename tests/src/test/scala/cats/tests/{NonEmptyListTests.scala => nonEmptyListTests.scala} (98%) diff --git a/tests/src/test/scala/cats/tests/NonEmptyListTests.scala b/tests/src/test/scala/cats/tests/nonEmptyListTests.scala similarity index 98% rename from tests/src/test/scala/cats/tests/NonEmptyListTests.scala rename to tests/src/test/scala/cats/tests/nonEmptyListTests.scala index c1774d76f8..225f7a5f0e 100644 --- a/tests/src/test/scala/cats/tests/NonEmptyListTests.scala +++ b/tests/src/test/scala/cats/tests/nonEmptyListTests.scala @@ -12,7 +12,6 @@ import cats.data.{NonEmptyList, NonEmptyVector} import cats.laws.discipline.arbitrary._ import cats.laws.discipline.{ComonadTests, NonEmptyTraverseTests, MonadTests, ReducibleTests, SemigroupKTests, SerializableTests} -@deprecated("to be able to test deprecated methods", since = "1.0.0-RC1") class NonEmptyListTests extends CatsSuite { // Lots of collections here.. telling ScalaCheck to calm down a bit implicit override val generatorDrivenConfig: PropertyCheckConfiguration = @@ -289,7 +288,6 @@ class NonEmptyListTests extends CatsSuite { forAll { (nel: NonEmptyList[Int], l: List[Int], n: Int) => (nel ++ l).toList should === (nel.toList ::: l) nel.concat(l).toList should === (nel.toList ::: l) - nel.concat(NonEmptyList(n, l)).toList should === (nel.toList ::: (n :: l)) nel.concatNel(NonEmptyList(n, l)).toList should === (nel.toList ::: (n :: l)) } } @@ -324,6 +322,16 @@ class NonEmptyListTests extends CatsSuite { } } +@deprecated("to be able to test deprecated methods", since = "1.0.0-RC1") +class DeprecatedNonEmptyListTests extends CatsSuite { + + test("Deprecated NonEmptyList#concat is consistent with List#:::") { + forAll { (nel: NonEmptyList[Int], l: List[Int], n: Int) => + nel.concat(NonEmptyList(n, l)).toList should === (nel.toList ::: (n :: l)) + } + } +} + class ReducibleNonEmptyListCheck extends ReducibleCheck[NonEmptyList]("NonEmptyList") { def iterator[T](nel: NonEmptyList[T]): Iterator[T] = nel.toList.iterator