Fix short circuiting behaviour in traverse and traverseFilter (#3328

)

* fixed short circuiting behavior in `traverse` and `traverseFilter` for Queue, Set, Vector and LazyList instances

* fixed, broken formatting

* added short-circuiting `map2Eval` implementation for `Chain`

alleycats-core/src/main/scala/alleycats/std/set.scala

@@ -1,10 +1,10 @@
 package alleycats
 package std
 
-import cats.{Applicative, Eval, Foldable, Monad, Monoid, Traverse, TraverseFilter}
+import alleycats.compat.scalaVersionSpecific._
+import cats.{Always, Applicative, Eval, Foldable, Monad, Monoid, Traverse, TraverseFilter}
 
 import scala.annotation.tailrec
-import compat.scalaVersionSpecific._
 
 object set extends SetInstances
 
@@ -36,6 +36,14 @@ trait SetInstances {
       override def map[A, B](fa: Set[A])(f: A => B): Set[B] = fa.map(f)
       def flatMap[A, B](fa: Set[A])(f: A => Set[B]): Set[B] = fa.flatMap(f)
 
+      override def map2[A, B, Z](fa: Set[A], fb: Set[B])(f: (A, B) => Z): Set[Z] =
+        if (fb.isEmpty) Set.empty[Z] // do O(1) work if fb is empty
+        else fa.flatMap(a => fb.map(b => f(a, b))) // already O(1) if fa is empty
+
+      override def map2Eval[A, B, Z](fa: Set[A], fb: Eval[Set[B]])(f: (A, B) => Z): Eval[Set[Z]] =
+        if (fa.isEmpty) Eval.now(Set.empty[Z]) // no need to evaluate fb
+        else fb.map(fb => map2(fa, fb)(f))
+
       def tailRecM[A, B](a: A)(f: (A) => Set[Either[A, B]]): Set[B] = {
         val bldr = Set.newBuilder[B]
 
@@ -74,9 +82,9 @@ trait SetInstances {
 
       def traverse[G[_]: Applicative, A, B](sa: Set[A])(f: A => G[B]): G[Set[B]] = {
         val G = Applicative[G]
-        sa.foldLeft(G.pure(Set.empty[B])) { (buf, a) =>
-          G.map2(buf, f(a))(_ + _)
-        }
+        foldRight[A, G[Set[B]]](sa, Always(G.pure(Set.empty[B]))) { (a, lglb) =>
+          G.map2Eval(f(a), lglb)((b, set) => set + b)
+        }.value
       }
 
       override def get[A](fa: Set[A])(idx: Long): Option[A] = {
@@ -123,11 +131,8 @@ trait SetInstances {
       val traverse: Traverse[Set] = alleyCatsSetTraverse
 
       def traverseFilter[G[_], A, B](fa: Set[A])(f: A => G[Option[B]])(implicit G: Applicative[G]): G[Set[B]] =
-        fa.foldLeft(G.pure(Set.empty[B])) { (gSet, a) =>
-          G.map2(f(a), gSet) {
-            case (Some(b), set) => set + b
-            case (None, set)    => set
-          }
-        }
+        traverse
+          .foldRight(fa, Eval.now(G.pure(Set.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(o + _)))
+          .value
     }
 }

core/src/main/scala-2.13+/cats/instances/lazyList.scala

@@ -163,7 +163,10 @@ trait LazyListInstances extends cats.kernel.instances.LazyListInstances {
     def traverseFilter[G[_], A, B](
       fa: LazyList[A]
     )(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[LazyList[B]] =
-      fa.foldRight(Eval.now(G.pure(LazyList.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
+      traverse
+        .foldRight(fa, Eval.now(G.pure(LazyList.empty[B])))((x, xse) =>
+          G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o))
+        )
         .value
 
     override def filterA[G[_], A](fa: LazyList[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[LazyList[A]] =

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

@@ -774,6 +774,14 @@ sealed abstract private[data] class ChainInstances extends ChainInstances1 {
         acc
       }
 
+      override def map2[A, B, Z](fa: Chain[A], fb: Chain[B])(f: (A, B) => Z): Chain[Z] =
+        if (fb.isEmpty) nil // do O(1) work if fb is empty
+        else fa.flatMap(a => fb.map(b => f(a, b))) // already O(1) if fa is empty
+
+      override def map2Eval[A, B, Z](fa: Chain[A], fb: Eval[Chain[B]])(f: (A, B) => Z): Eval[Chain[Z]] =
+        if (fa.isEmpty) Eval.now(nil) // no need to evaluate fb
+        else fb.map(fb => map2(fa, fb)(f))
+
       override def get[A](fa: Chain[A])(idx: Long): Option[A] = fa.get(idx)
 
       def functor: Functor[Chain] = this

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

@@ -172,7 +172,8 @@ private object QueueInstances {
     override def flattenOption[A](fa: Queue[Option[A]]): Queue[A] = fa.flatten
 
     def traverseFilter[G[_], A, B](fa: Queue[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Queue[B]] =
-      fa.foldRight(Eval.now(G.pure(Queue.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
+      traverse
+        .foldRight(fa, Eval.now(G.pure(Queue.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
         .value
 
     override def filterA[G[_], A](fa: Queue[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Queue[A]] =

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

@@ -26,6 +26,14 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances {
       def flatMap[A, B](fa: Vector[A])(f: A => Vector[B]): Vector[B] =
         fa.flatMap(f)
 
+      override def map2[A, B, Z](fa: Vector[A], fb: Vector[B])(f: (A, B) => Z): Vector[Z] =
+        if (fb.isEmpty) Vector.empty // do O(1) work if either is empty
+        else fa.flatMap(a => fb.map(b => f(a, b))) // already O(1) if fa is empty
+
+      override def map2Eval[A, B, Z](fa: Vector[A], fb: Eval[Vector[B]])(f: (A, B) => Z): Eval[Vector[Z]] =
+        if (fa.isEmpty) Eval.now(Vector.empty) // no need to evaluate fb
+        else fb.map(fb => map2(fa, fb)(f))
+
       def coflatMap[A, B](fa: Vector[A])(f: Vector[A] => B): Vector[B] = {
         @tailrec def loop(builder: VectorBuilder[B], as: Vector[A]): Vector[B] =
           as match {
@@ -166,7 +174,9 @@ private[instances] trait VectorInstancesBinCompat0 {
     override def flattenOption[A](fa: Vector[Option[A]]): Vector[A] = fa.flatten
 
     def traverseFilter[G[_], A, B](fa: Vector[A])(f: (A) => G[Option[B]])(implicit G: Applicative[G]): G[Vector[B]] =
-      fa.foldRight(Eval.now(G.pure(Vector.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o)))
+      traverse
+        .foldRight(fa, Eval.now(G.pure(Vector.empty[B])))((x, xse) => G.map2Eval(f(x), xse)((i, o) => i.fold(o)(_ +: o))
+        )
         .value
 
     override def filterA[G[_], A](fa: Vector[A])(f: (A) => G[Boolean])(implicit G: Applicative[G]): G[Vector[A]] =