Add Apply.map2Eval and allow traverse laziness

core/src/main/scala/cats/Apply.scala

@@ -33,15 +33,46 @@ trait Apply[F[_]] extends Functor[F] with Cartesian[F] with ApplyArityFunctions[
   def map2[A, B, Z](fa: F[A], fb: F[B])(f: (A, B) => Z): F[Z] =
     map(product(fa, fb)) { case (a, b) => f(a, b) }
 
+  /**
+   * Similar to [[map2]] but uses [[Eval]] to allow for laziness in the `F[B]`
+   * argument. This can allow for "short-circuiting" of computations.
+   *
+   * NOTE: the default implementation of `map2Eval` does does not short-circuit
+   * computations. For data structures that can benefit from laziness, [[Apply]]
+   * instances should override this method.
+   *
+   * In the following example, `x.map2(bomb)(_ + _)` would result in an error,
+   * but `map2Eval` "short-circuits" the computation. `x` is `None` and thus the
+   * result of `bomb` doesn't even need to be evaluated in order to determine
+   * that the result of `map2Eval` should be `None`.
+   *
+   * {{{
+   * scala> import cats.{Eval, Later}
+   * scala> import cats.implicits._
+   * scala> val bomb: Eval[Option[Int]] = Later(sys.error("boom"))
+   * scala> val x: Option[Int] = None
+   * scala> x.map2Eval(bomb)(_ + _).value
+   * res0: Option[Int] = None
+   * }}}
+   */
+  def map2Eval[A, B, Z](fa: F[A], fb: Eval[F[B]])(f: (A, B) => Z): Eval[F[Z]] =
+    fb.map(fb => map2(fa, fb)(f))
+
   /**
    * Two sequentially dependent Applys can be composed.
    *
    * The composition of Applys `F` and `G`, `F[G[x]]`, is also an Apply.
    *
-   * val ap = Apply[Option].compose[List]
-   * val x = Some(List(1, 2))
-   * val y = Some(List(10, 20))
-   * ap.map2(x, y)(_ + _) == Some(List(11, 12, 21, 22))
+   * Example:
+   * {{{
+   * scala> import cats.Apply
+   * scala> import cats.implicits._
+   * scala> val ap = Apply[Option].compose[List]
+   * scala> val x: Option[List[Int]] = Some(List(1, 2))
+   * scala> val y: Option[List[Int]] = Some(List(10, 20))
+   * scala> ap.map2(x, y)(_ + _)
+   * res0: Option[List[Int]] = Some(List(11, 21, 12, 22))
+   * }}}
    */
   def compose[G[_]](implicit GG: Apply[G]): Apply[Lambda[X => F[G[X]]]] =
     new CompositeApply[F, G] {

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

@@ -107,9 +107,9 @@ import simulacrum.typeclass
    * needed.
    */
   def traverse_[G[_], A, B](fa: F[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] =
-    foldLeft(fa, G.pure(())) { (acc, a) =>
-      G.map2(acc, f(a)) { (_, _) => () }
-    }
+    foldRight(fa, Always(G.pure(()))) { (a, acc) =>
+      G.map2Eval(f(a), acc) { (_, _) => () }
+    }.value
 
   /**
    * Behaves like traverse_, but uses [[Unapply]] to find the

core/src/main/scala/cats/Reducible.scala

@@ -83,19 +83,26 @@ import simulacrum.typeclass
    * `A` values will be mapped into `G[B]` and combined using
    * `Applicative#map2`.
    *
-   * This method does the same thing as `Foldable#traverse_`.  The
-   * difference is that we only need `Apply[G]` here, since we don't
-   * need to call `Applicative#pure` for a starting value.
+   * This method is similar to [[Foldable.traverse_]]. There are two
+   * main differences:
+   *
+   * 1. We only need an [[Apply]] instance for `G` here, since we
+   * don't need to call [[Applicative.pure]] for a starting value.
+   * 2. This performs a strict left-associative traversal and thus
+   * must always traverse the entire data structure. Prefer
+   * [[Foldable.traverse_]] if you have an [[Applicative]] instance
+   * available for `G` and want to take advantage of short-circuiting
+   * the traversal.
    */
   def traverse1_[G[_], A, B](fa: F[A])(f: A => G[B])(implicit G: Apply[G]): G[Unit] =
     G.map(reduceLeftTo(fa)(f)((x, y) => G.map2(x, f(y))((_, b) => b)))(_ => ())
 
   /**
    * Sequence `F[G[A]]` using `Apply[G]`.
    *
-   * This method is similar to `Foldable#sequence_`. The difference is
-   * that we only need `Apply[G]` here, since we don't need to call
-   * `Applicative#pure` for a starting value.
+   * This method is similar to [[Foldable.sequence_]] but requires only
+   * an [[Apply]] instance for `G` instead of [[Applicative]]. See the
+   * [[traverse1_]] documentation for a description of the differences.
    */
   def sequence1_[G[_], A](fga: F[G[A]])(implicit G: Apply[G]): G[Unit] =
     G.map(reduceLeft(fga)((x, y) => G.map2(x, y)((_, b) => b)))(_ => ())

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

@@ -170,9 +170,7 @@ trait OneAndLowPriority2 extends OneAndLowPriority1 {
   implicit def oneAndTraverse[F[_]](implicit F: Traverse[F]): Traverse[OneAnd[F, ?]] =
     new Traverse[OneAnd[F, ?]] {
       def traverse[G[_], A, B](fa: OneAnd[F, A])(f: (A) => G[B])(implicit G: Applicative[G]): G[OneAnd[F, B]] = {
-        val tail = F.traverse(fa.tail)(f)
-        val head = f(fa.head)
-        G.ap2[B, F[B], OneAnd[F, B]](G.pure(OneAnd(_, _)))(head, tail)
+        G.map2Eval(f(fa.head), Always(F.traverse(fa.tail)(f)))(OneAnd(_, _)).value
       }
 
       def foldLeft[A, B](fa: OneAnd[F, A], b: B)(f: (B, A) => B): B = {

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

@@ -90,6 +90,12 @@ sealed abstract class Xor[+A, +B] extends Product with Serializable {
     case Xor.Right(b)    => Xor.Right(f(b))
   }
 
+  def map2Eval[AA >: A, C, Z](fc: Eval[AA Xor C])(f: (B, C) => Z): Eval[AA Xor Z] =
+    this match {
+      case l @ Xor.Left(_) => Now(l)
+      case Xor.Right(b) => fc.map(_.map(f(b, _)))
+    }
+
   def leftMap[C](f: A => C): C Xor B = this match {
     case Xor.Left(a)      => Xor.Left(f(a))
     case r @ Xor.Right(_) => r
@@ -211,6 +217,8 @@ private[data] sealed abstract class XorInstances extends XorInstances1 {
         }
       def raiseError[B](e: A): Xor[A, B] = Xor.left(e)
       override def map[B, C](fa: A Xor B)(f: B => C): A Xor C = fa.map(f)
+      override def map2Eval[B, C, Z](fb: A Xor B, fc: Eval[A Xor C])(f: (B, C) => Z): Eval[A Xor Z] =
+        fb.map2Eval(fc)(f)
       override def attempt[B](fab: A Xor B): A Xor (A Xor B) = Xor.right(fab)
       override def recover[B](fab: A Xor B)(pf: PartialFunction[A, B]): A Xor B =
         fab recover pf

core/src/main/scala/cats/std/either.scala

@@ -33,6 +33,15 @@ trait EitherInstances extends EitherInstances1 {
       override def map[B, C](fa: Either[A, B])(f: B => C): Either[A, C] =
         fa.right.map(f)
 
+      override def map2Eval[B, C, Z](fb: Either[A, B], fc: Eval[Either[A, C]])(f: (B, C) => Z): Eval[Either[A, Z]] =
+        fb match {
+          // This should be safe, but we are forced to use `asInstanceOf`,
+          // because `Left[+A, +B]` extends Either[A, B] instead of
+          // `Either[A, Nothing]`
+          case l @ Left(_) => Now(l.asInstanceOf[Either[A, Z]])
+          case Right(b) => fc.map(_.right.map(f(b, _)))
+        }
+
       def traverse[F[_], B, C](fa: Either[A, B])(f: B => F[C])(implicit F: Applicative[F]): F[Either[A, C]] =
         fa.fold(
           a => F.pure(Left(a)),

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

@@ -47,11 +47,10 @@ trait ListInstances extends cats.kernel.std.ListInstances {
         Eval.defer(loop(fa))
       }
 
-      def traverse[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: Applicative[G]): G[List[B]] = {
-        val gba = G.pure(Vector.empty[B])
-        val gbb = fa.foldLeft(gba)((buf, a) => G.map2(buf, f(a))(_ :+ _))
-        G.map(gbb)(_.toList)
-      }
+      def traverse[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: Applicative[G]): G[List[B]] =
+        foldRight[A, G[List[B]]](fa, Always(G.pure(List.empty))){ (a, lglb) =>
+          G.map2Eval(f(a), lglb)(_ :: _)
+        }.value
 
       override def exists[A](fa: List[A])(p: A => Boolean): Boolean =
         fa.exists(p)

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

@@ -14,12 +14,11 @@ trait MapInstances extends cats.kernel.std.MapInstances {
   implicit def mapInstance[K]: Traverse[Map[K, ?]] with FlatMap[Map[K, ?]] =
     new Traverse[Map[K, ?]] with FlatMap[Map[K, ?]] {
 
-      def traverse[G[_] : Applicative, A, B](fa: Map[K, A])(f: (A) => G[B]): G[Map[K, B]] = {
-        val G = Applicative[G]
-        val gba = G.pure(Map.empty[K, B])
-        val gbb = fa.foldLeft(gba) { (buf, a) =>
-          G.map2(buf, f(a._2))({ case(x, y) => x + (a._1 -> y)})
-        }
+      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) =>
+          G.map2Eval(f(kv._2), lbuf)({ (b, buf) => buf + (kv._1 -> b)})
+        }.value
         G.map(gbb)(_.toMap)
       }
 

core/src/main/scala/cats/std/option.scala

@@ -21,6 +21,12 @@ trait OptionInstances extends cats.kernel.std.OptionInstances {
       override def map2[A, B, Z](fa: Option[A], fb: Option[B])(f: (A, B) => Z): Option[Z] =
         fa.flatMap(a => fb.map(b => f(a, b)))
 
+      override def map2Eval[A, B, Z](fa: Option[A], fb: Eval[Option[B]])(f: (A, B) => Z): Eval[Option[Z]] =
+        fa match {
+          case None => Now(None)
+          case Some(a) => fb.map(_.map(f(a, _)))
+        }
+
       def coflatMap[A, B](fa: Option[A])(f: Option[A] => B): Option[B] =
         if (fa.isDefined) Some(f(fa)) else None
 

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

@@ -41,11 +41,8 @@ trait StreamInstances extends cats.kernel.std.StreamInstances {
         // We use foldRight to avoid possible stack overflows. Since
         // we don't want to return a Eval[_] instance, we call .value
         // at the end.
-        //
-        // (We don't worry about internal laziness because traverse
-        // has to evaluate the entire stream anyway.)
         foldRight(fa, Later(init)) { (a, lgsb) =>
-          lgsb.map(gsb => G.map2(f(a), gsb)(_ #:: _))
+          G.map2Eval(f(a), lgsb)(_ #:: _)
         }.value
       }
 

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

@@ -45,7 +45,9 @@ trait VectorInstances extends cats.kernel.std.VectorInstances {
       }
 
       def traverse[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Vector[B]] =
-        fa.foldLeft(G.pure(Vector.empty[B]))((buf, a) => G.map2(buf, f(a))(_ :+ _))
+      foldRight[A, G[Vector[B]]](fa, Always(G.pure(Vector.empty))){ (a, lgvb) =>
+        G.map2Eval(f(a), lgvb)(_ +: _)
+      }.value
 
       override def exists[A](fa: Vector[A])(p: A => Boolean): Boolean =
         fa.exists(p)

tests/src/test/scala/cats/tests/EitherTests.scala

@@ -49,4 +49,10 @@ class EitherTests extends CatsSuite {
       show.show(e).nonEmpty should === (true)
     }
   }
+
+  test("map2Eval is lazy") {
+    val bomb: Eval[Either[String, Int]] = Later(sys.error("boom"))
+    val x: Either[String, Int] = Left("l")
+    x.map2Eval(bomb)(_ + _).value should === (x)
+  }
 }

tests/src/test/scala/cats/tests/OptionTests.scala

@@ -83,4 +83,9 @@ class OptionTests extends CatsSuite {
     // can't use `s.some should === (Some(null))` here, because it leads to NullPointerException
     s.some.exists(_ == null) should ===(true)
   }
+
+  test("map2Eval is lazy") {
+    val bomb: Eval[Option[Int]] = Later(sys.error("boom"))
+    none[Int].map2Eval(bomb)(_ + _).value should === (None)
+  }
 }

tests/src/test/scala/cats/tests/RegressionTests.scala

@@ -1,7 +1,7 @@
 package cats
 package tests
 
-import cats.data.Const
+import cats.data.{Const, NonEmptyList, Xor}
 
 import scala.collection.mutable
 
@@ -83,4 +83,44 @@ class RegressionTests extends CatsSuite {
       List(1,2,3).traverseU(i => Const(List(i))).getConst == List(1,2,3).foldMap(List(_))
     )
   }
+
+  test("#513: traverse short circuits - Xor") {
+    var count = 0
+    def validate(i: Int): Xor[String, Int] = {
+      count = count + 1
+      if (i < 5) Xor.right(i) else Xor.left(s"$i is greater than 5")
+    }
+
+    def checkAndResetCount(expected: Int): Unit = {
+      count should === (expected)
+      count = 0
+    }
+
+    List(1,2,6,8).traverseU(validate) should === (Xor.left("6 is greater than 5"))
+    // shouldn't have ever evaluted validate(8)
+    checkAndResetCount(3)
+
+    Stream(1,2,6,8).traverseU(validate) should === (Xor.left("6 is greater than 5"))
+    checkAndResetCount(3)
+
+    type StringMap[A] = Map[String, A]
+    val intMap: StringMap[Int] = Map("one" -> 1, "two" -> 2, "six" -> 6, "eight" -> 8)
+    intMap.traverseU(validate) should === (Xor.left("6 is greater than 5"))
+    checkAndResetCount(3)
+
+    NonEmptyList(1,2,6,8).traverseU(validate) should === (Xor.left("6 is greater than 5"))
+    checkAndResetCount(3)
+
+    NonEmptyList(6,8).traverseU(validate) should === (Xor.left("6 is greater than 5"))
+    checkAndResetCount(1)
+
+    List(1,2,6,8).traverseU_(validate) should === (Xor.left("6 is greater than 5"))
+    checkAndResetCount(3)
+
+    NonEmptyList(1,2,6,7,8).traverseU_(validate) should === (Xor.left("6 is greater than 5"))
+    checkAndResetCount(3)
+
+    NonEmptyList(6,7,8).traverseU_(validate) should === (Xor.left("6 is greater than 5"))
+    checkAndResetCount(1)
+  }
 }

tests/src/test/scala/cats/tests/XorTests.scala

@@ -235,4 +235,10 @@ class XorTests extends CatsSuite {
     }
   }
 
+  test("map2Eval is lazy") {
+    val bomb: Eval[String Xor Int] = Later(sys.error("boom"))
+    val x = Xor.left[String, Int]("l")
+    x.map2Eval(bomb)(_ + _).value should === (x)
+  }
+
 }