remove iteratorFoldM fixes typelevel#1716

core/src/main/scala/cats/Foldable.scala

@@ -414,41 +414,6 @@ object Foldable {
     loop()
   }
 
-  /**
-   * Implementation of [[Foldable.foldM]] which can short-circuit for
-   * structures with an `Iterator`.
-   *
-   * For example we can sum a `Stream` of integers and stop if
-   * the sum reaches 100 (if we reach the end of the `Stream`
-   * before getting to 100 we return the total sum) :
-   *
-   * {{{
-   * scala> import cats.implicits._
-   * scala> type LongOr[A] = Either[Long, A]
-   * scala> def sumStream(s: Stream[Int]): Long =
-   *      |   Foldable.iteratorFoldM[LongOr, Int, Long](s.toIterator, 0L){ (acc, n) =>
-   *      |     val sum = acc + n
-   *      |     if (sum < 100L) Right(sum) else Left(sum)
-   *      |   }.merge
-   *
-   * scala> sumStream(Stream.continually(1))
-   * res0: Long = 100
-   *
-   * scala> sumStream(Stream(1,2,3,4))
-   * res1: Long = 10
-   * }}}
-   *
-   * Note that `Foldable[Stream].foldM` uses this method underneath, so
-   * you wouldn't call this method explicitly like in the example above.
-   */
-  def iteratorFoldM[M[_], A, B](it: Iterator[A], z: B)(f: (B, A) => M[B])(implicit M: Monad[M]): M[B] = {
-    val go: B => M[Either[B, B]] = { b =>
-      if (it.hasNext) M.map(f(b, it.next))(Left(_))
-      else M.pure(Right(b))
-    }
-    M.tailRecM(z)(go)
-  }
-
 
   /**
    * Isomorphic to

core/src/main/scala/cats/data/NonEmptyList.scala

@@ -431,9 +431,6 @@ private[data] sealed trait NonEmptyListInstances extends NonEmptyListInstances0
       override def fold[A](fa: NonEmptyList[A])(implicit A: Monoid[A]): A =
         fa.reduce
 
-      override def foldM[G[_], A, B](fa: NonEmptyList[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
-        Foldable.iteratorFoldM(fa.toList.toIterator, z)(f)
-
       override def find[A](fa: NonEmptyList[A])(f: A => Boolean): Option[A] =
         fa find f
 

core/src/main/scala/cats/data/NonEmptyVector.scala

@@ -264,9 +264,6 @@ private[data] sealed trait NonEmptyVectorInstances {
       override def fold[A](fa: NonEmptyVector[A])(implicit A: Monoid[A]): A =
         fa.reduce
 
-      override def foldM[G[_], A, B](fa: NonEmptyVector[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
-        Foldable.iteratorFoldM(fa.toVector.toIterator, z)(f)
-
       override def find[A](fa: NonEmptyVector[A])(f: A => Boolean): Option[A] =
         fa.find(f)
 

core/src/main/scala/cats/instances/list.scala

@@ -90,9 +90,6 @@ trait ListInstances extends cats.kernel.instances.ListInstances {
 
       override def filter[A](fa: List[A])(f: A => Boolean): List[A] = fa.filter(f)
 
-      override def foldM[G[_], A, B](fa: List[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
-        Foldable.iteratorFoldM(fa.toIterator, z)(f)
-
       override def fold[A](fa: List[A])(implicit A: Monoid[A]): A = A.combineAll(fa)
 
       override def toList[A](fa: List[A]): List[A] = fa

core/src/main/scala/cats/instances/map.scala

@@ -85,9 +85,6 @@ trait MapInstances extends cats.kernel.instances.MapInstances {
 
       override def isEmpty[A](fa: Map[K, A]): Boolean = fa.isEmpty
 
-      override def foldM[G[_], A, B](fa: Map[K, A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
-        Foldable.iteratorFoldM(fa.valuesIterator, z)(f)
-
       override def fold[A](fa: Map[K, A])(implicit A: Monoid[A]): A =
         A.combineAll(fa.values)
 

core/src/main/scala/cats/instances/set.scala

@@ -43,9 +43,6 @@ trait SetInstances extends cats.kernel.instances.SetInstances {
 
       override def isEmpty[A](fa: Set[A]): Boolean = fa.isEmpty
 
-      override def foldM[G[_], A, B](fa: Set[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
-        Foldable.iteratorFoldM(fa.toIterator, z)(f)
-
       override def fold[A](fa: Set[A])(implicit A: Monoid[A]): A = A.combineAll(fa)
 
       override def toList[A](fa: Set[A]): List[A] = fa.toList

core/src/main/scala/cats/instances/stream.scala

@@ -118,9 +118,6 @@ trait StreamInstances extends cats.kernel.instances.StreamInstances {
 
       override def collect[A, B](fa: Stream[A])(f: PartialFunction[A, B]): Stream[B] = fa.collect(f)
 
-      override def foldM[G[_], A, B](fa: Stream[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
-        Foldable.iteratorFoldM(fa.toIterator, z)(f)
-
       override def fold[A](fa: Stream[A])(implicit A: Monoid[A]): A = A.combineAll(fa)
 
       override def toList[A](fa: Stream[A]): List[A] = fa.toList

core/src/main/scala/cats/instances/vector.scala

@@ -90,9 +90,6 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances {
 
       override def collect[A, B](fa: Vector[A])(f: PartialFunction[A, B]): Vector[B] = fa.collect(f)
 
-      override def foldM[G[_], A, B](fa: Vector[A], z: B)(f: (B, A) => G[B])(implicit G: Monad[G]): G[B] =
-        Foldable.iteratorFoldM(fa.toIterator, z)(f)
-
       override def fold[A](fa: Vector[A])(implicit A: Monoid[A]): A = A.combineAll(fa)
 
       override def toList[A](fa: Vector[A]): List[A] = fa.toList

tests/src/test/scala/cats/tests/FoldableTests.scala

@@ -197,12 +197,7 @@ class FoldableTestsAdditional extends CatsSuite {
     }
     assert(result.value)
 
-    // test trampolining
-    val large = Stream((1 to 10000): _*)
-    assert(contains(large, 10000).value)
 
-    // test laziness of foldM
-    dangerous.foldM(0)((acc, a) => if (a < 2) Some(acc + a) else None) should === (None)
   }
 
   test(".foldLeftM short-circuiting") {
@@ -214,17 +209,23 @@ class FoldableTestsAdditional extends CatsSuite {
   }
 
   test(".foldLeftM short-circuiting optimality") {
-    // test that no more elements are evaluated than absolutely necessary
+    // test that no more than 1 elements are evaluated than absolutely necessary
 
     def concatUntil(ss: Stream[String], stop: String): Either[String, String] =
       Foldable[Stream].foldLeftM[Either[String, ?], String, String](ss, "") { (acc, s) =>
         if (s == stop) Left(acc) else Right(acc + s)
       }
 
     def boom: Stream[String] = sys.error("boom")
-    assert(concatUntil("STOP" #:: boom, "STOP") == Left(""))
-    assert(concatUntil("Zero" #:: "STOP" #:: boom, "STOP") == Left("Zero"))
-    assert(concatUntil("Zero" #:: "One" #:: "STOP" #:: boom, "STOP") == Left("ZeroOne"))
+    assert(concatUntil("STOP" #:: "AFTER" #:: boom, "STOP") == Left(""))
+    assert(concatUntil("Zero" #:: "STOP" #:: "AFTER" #:: boom, "STOP") == Left("Zero"))
+    assert(concatUntil("Zero" #:: "One" #:: "STOP" #:: "AFTER" #:: boom, "STOP") == Left("ZeroOne"))
+  }
+
+  test("Foldable[List] doesn't break substitution") {
+    val result  = List.range(0,10).foldM(List.empty[Int])((accum, elt) => Eval.always(elt :: accum))
+
+    assert(result.value == result.value)
   }
 }