Make iterateRight sound (typelevel#1980)

kailuowang · Oct 23, 2017 · af58c4f · af58c4f
1 parent 693fde4
commit af58c4f
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 7 deletions.
diff --git a/core/src/main/scala/cats/Foldable.scala b/core/src/main/scala/cats/Foldable.scala
@@ -505,10 +505,11 @@ import simulacrum.typeclass
 }
 
 object Foldable {
-  def iterateRight[A, B](it: Iterator[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = {
-    def loop(): Eval[B] =
-      Eval.defer(if (it.hasNext) f(it.next, loop()) else lb)
-    loop()
+  def iterateRight[A, B](iterable: Iterable[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = {
+    def loop(it: Iterator[A]): Eval[B] =
+      Eval.defer(if (it.hasNext) f(it.next, loop(it)) else lb)
+
+    Eval.always(iterable.iterator).flatMap(loop)
   }
 
 

diff --git a/core/src/main/scala/cats/instances/map.scala b/core/src/main/scala/cats/instances/map.scala
@@ -19,7 +19,7 @@ trait MapInstances extends cats.kernel.instances.MapInstances {
 
       def traverse[G[_], A, B](fa: Map[K, A])(f: A => G[B])(implicit G: Applicative[G]): G[Map[K, B]] = {
         val gba: Eval[G[Map[K, B]]] = Always(G.pure(Map.empty))
-        val gbb = Foldable.iterateRight(fa.iterator, gba){ (kv, lbuf) =>
+        val gbb = Foldable.iterateRight(fa, gba){ (kv, lbuf) =>
           G.map2Eval(f(kv._2), lbuf)({ (b, buf) => buf + (kv._1 -> b)})
         }.value
         G.map(gbb)(_.toMap)
@@ -51,7 +51,7 @@ trait MapInstances extends cats.kernel.instances.MapInstances {
         fa.foldLeft(b) { case (x, (k, a)) => f(x, a)}
 
       def foldRight[A, B](fa: Map[K, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
-        Foldable.iterateRight(fa.values.iterator, lb)(f)
+        Foldable.iterateRight(fa.values, lb)(f)
 
       def tailRecM[A, B](a: A)(f: A => Map[K, Either[A, B]]): Map[K, B] = {
         val bldr = Map.newBuilder[K, B]

diff --git a/core/src/main/scala/cats/instances/set.scala b/core/src/main/scala/cats/instances/set.scala
@@ -18,7 +18,7 @@ trait SetInstances extends cats.kernel.instances.SetInstances {
         fa.foldLeft(b)(f)
 
       def foldRight[A, B](fa: Set[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
-        Foldable.iterateRight(fa.iterator, lb)(f)
+        Foldable.iterateRight(fa, lb)(f)
 
       override def get[A](fa: Set[A])(idx: Long): Option[A] = {
         @tailrec

diff --git a/tests/src/test/scala/cats/tests/FoldableSuite.scala b/tests/src/test/scala/cats/tests/FoldableSuite.scala
@@ -193,6 +193,18 @@ class FoldableSuiteAdditional extends CatsSuite {
     assert(foldMResult.get == expected)
     ()
   }
+  test(s"Foldable.iterateRight") {
+    forAll { (fa: List[Int]) =>
+      val eval = Foldable.iterateRight(fa, Eval.later(0)) { (a, eb) =>
+        Eval.always(a + eb.value)
+      }
+
+      eval.value should === (fa.sum)
+
+      //Repeat here so the result is evaluated again
+      eval.value should === (fa.sum)
+    }
+  }
 
   test("Foldable[List].foldM stack safety") {
     checkFoldMStackSafety[List](_.toList)