From b0ad490b26977c686d743af48221c85b617f8ca3 Mon Sep 17 00:00:00 2001 From: Erik Osheim Date: Tue, 10 Nov 2015 09:11:03 -0500 Subject: [PATCH 1/3] Fix Foldabe#dropWhile_ bug. In addition to fixing this bug, the commit defines several FoldableCheck tests for various types that have foldable instances. Finally, the commit adds takeWhile_. Fixes #563. --- core/src/main/scala/cats/Foldable.scala | 11 ++++++- core/src/main/scala/cats/std/all.scala | 1 + core/src/main/scala/cats/std/iterable.scala | 18 ++++++++++++ core/src/main/scala/cats/std/package.scala | 1 + .../test/scala/cats/tests/FoldableTests.scala | 29 +++++++++++++++++++ 5 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 core/src/main/scala/cats/std/iterable.scala diff --git a/core/src/main/scala/cats/Foldable.scala b/core/src/main/scala/cats/Foldable.scala index d90294e530..7fde6654c1 100644 --- a/core/src/main/scala/cats/Foldable.scala +++ b/core/src/main/scala/cats/Foldable.scala @@ -181,13 +181,22 @@ import simulacrum.typeclass if (p(a)) buf += a else buf }.toList + /** + * Convert F[A] to a List[A], dropping all initial elements which + * match `p`. + */ + def takeWhile_[A](fa: F[A])(p: A => Boolean): List[A] = + foldRight(fa, Now(List.empty[A])) { (a, llst) => + if (p(a)) llst.map(a :: _) else Now(Nil) + }.value + /** * Convert F[A] to a List[A], dropping all initial elements which * match `p`. */ def dropWhile_[A](fa: F[A])(p: A => Boolean): List[A] = foldLeft(fa, mutable.ListBuffer.empty[A]) { (buf, a) => - if (buf.nonEmpty || p(a)) buf += a else buf + if (buf.nonEmpty || !p(a)) buf += a else buf }.toList /** diff --git a/core/src/main/scala/cats/std/all.scala b/core/src/main/scala/cats/std/all.scala index 5983aa0c5d..0591d7ccf8 100644 --- a/core/src/main/scala/cats/std/all.scala +++ b/core/src/main/scala/cats/std/all.scala @@ -10,6 +10,7 @@ trait AllInstances with SetInstances with StreamInstances with VectorInstances + with IterableInstances with AnyValInstances with MapInstances with BigIntInstances diff --git a/core/src/main/scala/cats/std/iterable.scala b/core/src/main/scala/cats/std/iterable.scala new file mode 100644 index 0000000000..74ceeec201 --- /dev/null +++ b/core/src/main/scala/cats/std/iterable.scala @@ -0,0 +1,18 @@ +package cats +package std + +trait IterableInstances { + implicit val iterableInstance: Foldable[Iterable] = + new Foldable[Iterable] { + + def foldLeft[A, B](fa: Iterable[A], b: B)(f: (B, A) => B): B = + fa.foldLeft(b)(f) + + def foldRight[A, B](fa: Iterable[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = { + val it = fa.iterator + def loop(): Eval[B] = + if (it.hasNext) f(it.next, Eval.defer(loop())) else lb + Eval.defer(loop()) + } + } +} diff --git a/core/src/main/scala/cats/std/package.scala b/core/src/main/scala/cats/std/package.scala index 4e298d622c..bdb2e2b83d 100644 --- a/core/src/main/scala/cats/std/package.scala +++ b/core/src/main/scala/cats/std/package.scala @@ -13,6 +13,7 @@ package object std { object vector extends VectorInstances object map extends MapInstances object future extends FutureInstances + object iterable extends IterableInstances object string extends StringInstances object int extends IntInstances diff --git a/tests/src/test/scala/cats/tests/FoldableTests.scala b/tests/src/test/scala/cats/tests/FoldableTests.scala index 2e8a6d8b63..dc400f02f6 100644 --- a/tests/src/test/scala/cats/tests/FoldableTests.scala +++ b/tests/src/test/scala/cats/tests/FoldableTests.scala @@ -5,6 +5,10 @@ import org.scalatest.prop.PropertyChecks import org.scalacheck.{Arbitrary, Gen} import org.scalacheck.Arbitrary.arbitrary +import cats.data.Streaming +import cats.std.all._ +import cats.laws.discipline.arbitrary._ + abstract class FoldableCheck[F[_]: Foldable](name: String)(implicit ArbFInt: Arbitrary[F[Int]]) extends CatsSuite with PropertyChecks { def iterator[T](fa: F[T]): Iterator[T] @@ -26,6 +30,7 @@ abstract class FoldableCheck[F[_]: Foldable](name: String)(implicit ArbFInt: Arb fa.forall(_ > n) should === (iterator(fa).forall(_ > n)) fa.filter_(_ > n) should === (iterator(fa).filter(_ > n).toList) fa.dropWhile_(_ > n) should === (iterator(fa).dropWhile(_ > n).toList) + fa.takeWhile_(_ > n) should === (iterator(fa).takeWhile(_ > n).toList) } } @@ -96,3 +101,27 @@ class FoldableTestsAdditional extends CatsSuite { assert(dangerous.toStreaming.take(3).toList == List(0, 1, 2)) } } + +class FoldableListCheck extends FoldableCheck[List]("list") { + def iterator[T](list: List[T]): Iterator[T] = list.iterator +} + +class FoldableVectorCheck extends FoldableCheck[Vector]("vector") { + def iterator[T](vector: Vector[T]): Iterator[T] = vector.iterator +} + +class FoldableStreamCheck extends FoldableCheck[Stream]("stream") { + def iterator[T](stream: Stream[T]): Iterator[T] = stream.iterator +} + +class FoldableStreamingCheck extends FoldableCheck[Streaming]("streaming") { + def iterator[T](streaming: Streaming[T]): Iterator[T] = streaming.iterator +} + +class FoldableIterableCheck extends FoldableCheck[Iterable]("iterable") { + def iterator[T](iterable: Iterable[T]): Iterator[T] = iterable.iterator +} + +class FoldableMapCheck extends FoldableCheck[Map[Int, ?]]("map") { + def iterator[T](map: Map[Int, T]): Iterator[T] = map.iterator.map(_._2) +} From 3e05426037c44bf2ba52987d02a50ec250a296d2 Mon Sep 17 00:00:00 2001 From: Erik Osheim Date: Tue, 10 Nov 2015 22:53:28 -0500 Subject: [PATCH 2/3] Fix comment bug. --- core/src/main/scala/cats/Foldable.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/Foldable.scala b/core/src/main/scala/cats/Foldable.scala index 7fde6654c1..5ea749a5e6 100644 --- a/core/src/main/scala/cats/Foldable.scala +++ b/core/src/main/scala/cats/Foldable.scala @@ -182,7 +182,7 @@ import simulacrum.typeclass }.toList /** - * Convert F[A] to a List[A], dropping all initial elements which + * Convert F[A] to a List[A], retaining only initial elements which * match `p`. */ def takeWhile_[A](fa: F[A])(p: A => Boolean): List[A] = From b6b2f2b9730da6c28d846cc21530639ffe398688 Mon Sep 17 00:00:00 2001 From: Erik Osheim Date: Thu, 12 Nov 2015 18:15:24 -0500 Subject: [PATCH 3/3] Remove Foldable[Iterable]. Iterable is one of the most useful Scala collection types. It's also impossible to be sure that its .iterator method doesn't side-effect. C'est la vie. --- core/src/main/scala/cats/std/all.scala | 1 - core/src/main/scala/cats/std/iterable.scala | 18 ------------------ core/src/main/scala/cats/std/package.scala | 1 - .../test/scala/cats/tests/FoldableTests.scala | 4 ---- 4 files changed, 24 deletions(-) delete mode 100644 core/src/main/scala/cats/std/iterable.scala diff --git a/core/src/main/scala/cats/std/all.scala b/core/src/main/scala/cats/std/all.scala index 0591d7ccf8..5983aa0c5d 100644 --- a/core/src/main/scala/cats/std/all.scala +++ b/core/src/main/scala/cats/std/all.scala @@ -10,7 +10,6 @@ trait AllInstances with SetInstances with StreamInstances with VectorInstances - with IterableInstances with AnyValInstances with MapInstances with BigIntInstances diff --git a/core/src/main/scala/cats/std/iterable.scala b/core/src/main/scala/cats/std/iterable.scala deleted file mode 100644 index 74ceeec201..0000000000 --- a/core/src/main/scala/cats/std/iterable.scala +++ /dev/null @@ -1,18 +0,0 @@ -package cats -package std - -trait IterableInstances { - implicit val iterableInstance: Foldable[Iterable] = - new Foldable[Iterable] { - - def foldLeft[A, B](fa: Iterable[A], b: B)(f: (B, A) => B): B = - fa.foldLeft(b)(f) - - def foldRight[A, B](fa: Iterable[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = { - val it = fa.iterator - def loop(): Eval[B] = - if (it.hasNext) f(it.next, Eval.defer(loop())) else lb - Eval.defer(loop()) - } - } -} diff --git a/core/src/main/scala/cats/std/package.scala b/core/src/main/scala/cats/std/package.scala index bdb2e2b83d..4e298d622c 100644 --- a/core/src/main/scala/cats/std/package.scala +++ b/core/src/main/scala/cats/std/package.scala @@ -13,7 +13,6 @@ package object std { object vector extends VectorInstances object map extends MapInstances object future extends FutureInstances - object iterable extends IterableInstances object string extends StringInstances object int extends IntInstances diff --git a/tests/src/test/scala/cats/tests/FoldableTests.scala b/tests/src/test/scala/cats/tests/FoldableTests.scala index dc400f02f6..2fb1eff709 100644 --- a/tests/src/test/scala/cats/tests/FoldableTests.scala +++ b/tests/src/test/scala/cats/tests/FoldableTests.scala @@ -118,10 +118,6 @@ class FoldableStreamingCheck extends FoldableCheck[Streaming]("streaming") { def iterator[T](streaming: Streaming[T]): Iterator[T] = streaming.iterator } -class FoldableIterableCheck extends FoldableCheck[Iterable]("iterable") { - def iterator[T](iterable: Iterable[T]): Iterator[T] = iterable.iterator -} - class FoldableMapCheck extends FoldableCheck[Map[Int, ?]]("map") { def iterator[T](map: Map[Int, T]): Iterator[T] = map.iterator.map(_._2) }