Fix foldA, reduceA, and reduceMapA short-circuiting

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

@@ -341,7 +341,7 @@ import Foldable.sentinel
     )
 
   /**
-   * Fold implemented using the given Monoid[A] instance.
+   * Fold implemented using the given `Monoid[A]` instance.
    */
   def fold[A](fa: F[A])(implicit A: Monoid[A]): A =
     A.combineAll(toIterable(fa))
@@ -397,8 +397,7 @@ import Foldable.sentinel
   /**
    * Fold implemented using the given `Applicative[G]` and `Monoid[A]` instance.
    *
-   * This method is identical to fold, except that we use `Applicative[G]` and `Monoid[A]`
-   * to combine a's inside an applicative G.
+   * This method is similar to [[fold]], but may short-circuit.
    *
    * For example:
    *
@@ -409,10 +408,10 @@ import Foldable.sentinel
    * res0: Either[String, Int] = Right(3)
    * }}}
    *
-   * `noop` usage description [[https://github.com/typelevel/simulacrum/issues/162 here]]
+   * See [[https://github.com/typelevel/simulacrum/issues/162 this issue]] for an explanation of `@noop` usage.
    */
   @noop def foldA[G[_], A](fga: F[G[A]])(implicit G: Applicative[G], A: Monoid[A]): G[A] =
-    fold(fga)(Applicative.monoid)
+    foldMapA(fga)(identity)
 
   /**
    * Fold implemented by mapping `A` values into `B` in a context `G` and then
@@ -440,7 +439,7 @@ import Foldable.sentinel
    * Monadic folding on `F` by mapping `A` values to `G[B]`, combining the `B`
    * values using the given `Monoid[B]` instance.
    *
-   * Similar to [[foldM]], but using a `Monoid[B]`.
+   * Similar to [[foldM]], but using a `Monoid[B]`. Will typically be more efficient than [[foldMapA]].
    *
    * {{{
    * scala> import cats.Foldable
@@ -458,9 +457,22 @@ import Foldable.sentinel
     foldM(fa, B.empty)((b, a) => G.map(f(a))(B.combine(b, _)))
 
   /**
-   * Equivalent to foldMapM.
-   * The difference is that foldMapA only requires G to be an Applicative
-   * rather than a Monad. It is also slower due to use of Eval.
+   * Fold in an [[Applicative]] context by mapping the `A` values to `G[B]`. combining
+   * the `B` values using the given `Monoid[B]` instance.
+   *
+   * Similar to [[foldMapM]], but will typically be less efficient.
+   *
+   * {{{
+   * scala> import cats.Foldable
+   * scala> import cats.implicits._
+   * scala> val evenNumbers = List(2,4,6,8,10)
+   * scala> val evenOpt: Int => Option[Int] =
+   *      |   i => if (i % 2 == 0) Some(i) else None
+   * scala> Foldable[List].foldMapA(evenNumbers)(evenOpt)
+   * res0: Option[Int] = Some(30)
+   * scala> Foldable[List].foldMapA(evenNumbers :+ 11)(evenOpt)
+   * res1: Option[Int] = None
+   * }}}
    */
   def foldMapA[G[_], A, B](fa: F[A])(f: A => G[B])(implicit G: Applicative[G], B: Monoid[B]): G[B] =
     foldRight(fa, Eval.now(G.pure(B.empty)))((a, egb) => G.map2Eval(f(a), egb)(B.combine)).value

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

@@ -77,7 +77,7 @@ import simulacrum.{noop, typeclass}
   def reduceLeftTo[A, B](fa: F[A])(f: A => B)(g: (B, A) => B): B
 
   /**
-   *  Monadic variant of [[reduceLeftTo]]
+   *  Monadic variant of [[reduceLeftTo]].
    */
   def reduceLeftM[G[_], A, B](fa: F[A])(f: A => G[B])(g: (B, A) => G[B])(implicit G: FlatMap[G]): G[B] =
     reduceLeftTo(fa)(f)((gb, a) => G.flatMap(gb)(g(_, a)))
@@ -86,23 +86,43 @@ import simulacrum.{noop, typeclass}
    * Reduce a `F[G[A]]` value using `Applicative[G]` and `Semigroup[A]`, a universal
    * semigroup for `G[_]`.
    *
-   * `noop` usage description [[https://github.com/typelevel/simulacrum/issues/162 here]]
+   * This method is similar to [[reduce]], but may short-circuit.
+   *
+   * See [[https://github.com/typelevel/simulacrum/issues/162 this issue]] for an explanation of `@noop` usage.
    */
   @noop def reduceA[G[_], A](fga: F[G[A]])(implicit G: Apply[G], A: Semigroup[A]): G[A] =
-    reduce(fga)(Apply.semigroup)
+    reduceMapA(fga)(identity)
 
   /**
-   * Apply `f` to each `a` of `fa` and combine the result into Apply[G] using the
-   * given `Semigroup[B]`.
+   * Reduce in an [[Apply]] context by mapping the `A` values to `G[B]`. combining
+   * the `B` values using the given `Semigroup[B]` instance.
+   *
+   * Similar to [[reduceMapM]], but may be less efficient.
+   *
+   * {{{
+   * scala> import cats.Reducible
+   * scala> import cats.data.NonEmptyList
+   * scala> import cats.implicits._
+   * scala> val evenOpt: Int => Option[Int] =
+   *      |   i => if (i % 2 == 0) Some(i) else None
+   * scala> val allEven = NonEmptyList.of(2,4,6,8,10)
+   * allEven: cats.data.NonEmptyList[Int] = NonEmptyList(2, 4, 6, 8, 10)
+   * scala> val notAllEven = allEven ++ List(11)
+   * notAllEven: cats.data.NonEmptyList[Int] = NonEmptyList(2, 4, 6, 8, 10, 11)
+   * scala> Reducible[NonEmptyList].reduceMapA(allEven)(evenOpt)
+   * res0: Option[Int] = Some(30)
+   * scala> Reducible[NonEmptyList].reduceMapA(notAllEven)(evenOpt)
+   * res1: Option[Int] = None
+   * }}}
    */
   def reduceMapA[G[_], A, B](fa: F[A])(f: A => G[B])(implicit G: Apply[G], B: Semigroup[B]): G[B] =
-    reduceLeftTo(fa)(f)((gb, a) => G.map2(gb, f(a))(B.combine))
+    reduceRightTo(fa)(f)((a, egb) => G.map2Eval(f(a), egb)(B.combine)).value
 
   /**
-   * Monadic reducing by mapping the `A` values to `G[B]`. combining
+   * Reduce in an [[FlatMap]] context by mapping the `A` values to `G[B]`. combining
    * the `B` values using the given `Semigroup[B]` instance.
    *
-   * Similar to [[reduceLeftM]], but using a `Semigroup[B]`.
+   * Similar to [[reduceLeftM]], but using a `Semigroup[B]`. May be more efficient than [[reduceMapA]].
    *
    * {{{
    * scala> import cats.Reducible

tests/src/test/scala/cats/tests/FoldableSuite.scala

@@ -470,6 +470,13 @@ class FoldableSuiteAdditional extends CatsSuite with ScalaVersionSpecificFoldabl
     assert(ns.foldA == "boom!!!".asLeft[Int])
   }
 
+  test(".foldA short-circuiting") {
+    implicit val F = foldableStreamWithDefaultImpl
+    val ns = Stream.from(1).map(n => if (n >= 100000) Left(n) else Right(n))
+
+    assert(F.foldA(ns) === Left(100000))
+  }
+
   test(".foldLeftM short-circuiting") {
     implicit val F = foldableStreamWithDefaultImpl
     val ns = Stream.continually(1)

tests/src/test/scala/cats/tests/ReducibleSuite.scala

@@ -99,6 +99,20 @@ class ReducibleSuiteAdditional extends CatsSuite {
 
     assert(xs.reduceMapM(i => if (i < n) Right(i) else Left(i)) === Left(n))
   }
+
+  test("reduceMapA should be stack-safe and short-circuiting if reduceRightTo is sufficiently lazy") {
+    val n = 100000
+    val xs = NES(0, Stream.from(1))
+
+    assert(xs.reduceMapA(i => if (i < n) Right(i) else Left(i)) === Left(n))
+  }
+
+  test("reduceA should be stack-safe and short-circuiting if reduceRightTo is sufficiently lazy") {
+    val n = 100000
+    val xs = NES(Right(0), Stream.from(1).map(i => if (i < n) Right(i) else Left(i)))
+
+    assert(xs.reduceA === Left(n))
+  }
 }
 
 abstract class ReducibleSuite[F[_]: Reducible](name: String)(implicit ArbFInt: Arbitrary[F[Int]],