From a9a0879e30c318143d34c817150437ee192c74e5 Mon Sep 17 00:00:00 2001 From: John Passaro Date: Sun, 20 Nov 2022 03:55:10 -0500 Subject: [PATCH 1/6] restrict `traverse_` and friends to require `Unit` When using `traverse_` or `sequence_` to evaluate some applicative effect `G[B]` within the context of a foldable structure, any `B` value is thrown away. Per the scaladoc for `traverse_`, these functions expect that the `G[B]` is primarily a side effect or action. The scala convention for expressing this expectation is to require a `Unit` parameter. Teach `traverse_`, `sequence_`, and their parallel and nonempty analogues to follow this convention by accepting only `G[Unit]` instead of simply any `G[B]`. Furthermore, update the implementations of `traverse_` and `nonEmptyTraverse` to use `foldMapA` and `reduceMapA` respectively. Since requiring Unit gives us a (trivial) `Monoid` for free, it turns out that in the presence of such a monoid, the `***MapA` functions are doing exactly the same thing as the existing implementations of `traverse_` and `nonEmptyTraverse_`. --- core/src/main/scala/cats/Foldable.scala | 37 ++++++++----------- core/src/main/scala/cats/Parallel.scala | 14 +++---- core/src/main/scala/cats/Reducible.scala | 18 ++++----- core/src/main/scala/cats/instances/list.scala | 7 +--- .../main/scala/cats/instances/vector.scala | 7 +--- .../src/main/scala/cats/syntax/foldable.scala | 3 +- .../src/main/scala/cats/syntax/parallel.scala | 6 +-- docs/nomenclature.md | 4 +- docs/typeclasses/foldable.md | 6 +-- docs/typeclasses/reducible.md | 2 +- .../main/scala/cats/laws/ReducibleLaws.scala | 4 +- .../discipline/NonEmptyTraverseTests.scala | 6 +-- .../cats/laws/discipline/ReducibleTests.scala | 8 ++-- .../cats/tests/ScalaVersionSpecific.scala | 2 +- .../test/scala/cats/tests/ParallelSuite.scala | 18 ++++----- .../scala/cats/tests/ReducibleSuite.scala | 2 +- .../scala/cats/tests/RegressionSuite.scala | 12 +++--- .../test/scala/cats/tests/SyntaxSuite.scala | 10 +++-- .../test/scala/cats/tests/TraverseSuite.scala | 2 +- 19 files changed, 79 insertions(+), 89 deletions(-) diff --git a/core/src/main/scala/cats/Foldable.scala b/core/src/main/scala/cats/Foldable.scala index 3392031344..1f4ce95881 100644 --- a/core/src/main/scala/cats/Foldable.scala +++ b/core/src/main/scala/cats/Foldable.scala @@ -569,36 +569,31 @@ trait Foldable[F[_]] extends UnorderedFoldable[F] with FoldableNFunctions[F] { s /** * Traverse `F[A]` using `Applicative[G]`. * - * `A` values will be mapped into `G[B]` and combined using + * `A` values will be mapped into `G[Unit]` and combined using * `Applicative#map2`. * * For example: * * {{{ * scala> import cats.implicits._ - * scala> def parseInt(s: String): Option[Int] = Either.catchOnly[NumberFormatException](s.toInt).toOption + * scala> def checkInt(s: String): Option[Unit] = Either.catchOnly[NumberFormatException]{ s.toInt ; () }.toOption * scala> val F = Foldable[List] - * scala> F.traverse_(List("333", "444"))(parseInt) + * scala> F.traverse_(List("333", "444"))(checkInt) * res0: Option[Unit] = Some(()) - * scala> F.traverse_(List("333", "zzz"))(parseInt) + * scala> F.traverse_(List("333", "zzz"))(checkInt) * res1: Option[Unit] = None * }}} * * This method is primarily useful when `G[_]` represents an action - * or effect, and the specific `A` aspect of `G[A]` is not otherwise - * needed. + * or effect. It is equivalent to `foldMapA` using the Unit monoid. */ - def traverse_[G[_], A, B](fa: F[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = - foldRight(fa, Always(G.pure(()))) { (a, acc) => - G.map2Eval(f(a), acc) { (_, _) => - () - } - }.value + def traverse_[G[_], A](fa: F[A])(f: A => G[Unit])(implicit G: Applicative[G]): G[Unit] = + foldMapA(fa)(f) /** - * Sequence `F[G[A]]` using `Applicative[G]`. + * Sequence `F[G[Unit]]` using `Applicative[G]`. * - * This is similar to `traverse_` except it operates on `F[G[A]]` + * This is similar to `traverse_` except it operates on `F[G[Unit]]` * values, so no additional functions are needed. * * For example: @@ -606,13 +601,13 @@ trait Foldable[F[_]] extends UnorderedFoldable[F] with FoldableNFunctions[F] { s * {{{ * scala> import cats.implicits._ * scala> val F = Foldable[List] - * scala> F.sequence_(List(Option(1), Option(2), Option(3))) + * scala> F.sequence_(List(Option(()), Option(()), Option(()))) * res0: Option[Unit] = Some(()) - * scala> F.sequence_(List(Option(1), None, Option(3))) + * scala> F.sequence_(List(Option(()), None, Option(()))) * res1: Option[Unit] = None * }}} */ - def sequence_[G[_]: Applicative, A](fga: F[G[A]]): G[Unit] = + def sequence_[G[_]: Applicative](fga: F[G[Unit]]): G[Unit] = traverse_(fga)(identity) /** @@ -1051,10 +1046,10 @@ object Foldable { typeClassInstance.foldMapM[G, A, B](self)(f)(G, B) def foldMapA[G[_], B](f: A => G[B])(implicit G: Applicative[G], B: Monoid[B]): G[B] = typeClassInstance.foldMapA[G, A, B](self)(f)(G, B) - def traverse_[G[_], B](f: A => G[B])(implicit G: Applicative[G]): G[Unit] = - typeClassInstance.traverse_[G, A, B](self)(f)(G) - def sequence_[G[_], B](implicit ev$1: A <:< G[B], ev$2: Applicative[G]): G[Unit] = - typeClassInstance.sequence_[G, B](self.asInstanceOf[F[G[B]]]) + def traverse_[G[_]](f: A => G[Unit])(implicit G: Applicative[G]): G[Unit] = + typeClassInstance.traverse_[G, A](self)(f)(G) + def sequence_[G[_]](implicit ev$1: A <:< G[Unit], ev$2: Applicative[G]): G[Unit] = + typeClassInstance.sequence_[G](self.asInstanceOf[F[G[Unit]]]) def foldK[G[_], B](implicit ev$1: A <:< G[B], G: MonoidK[G]): G[B] = typeClassInstance.foldK[G, B](self.asInstanceOf[F[G[B]]])(G) def find(f: A => Boolean): Option[A] = typeClassInstance.find[A](self)(f) diff --git a/core/src/main/scala/cats/Parallel.scala b/core/src/main/scala/cats/Parallel.scala index ac47c5bd23..aaa12e9458 100644 --- a/core/src/main/scala/cats/Parallel.scala +++ b/core/src/main/scala/cats/Parallel.scala @@ -263,7 +263,7 @@ object Parallel extends ParallelArityFunctions2 { * Like `Foldable[A].sequence_`, but uses the applicative instance * corresponding to the Parallel instance instead. */ - def parSequence_[T[_]: Foldable, M[_], A](tma: T[M[A]])(implicit P: Parallel[M]): M[Unit] = { + def parSequence_[T[_]: Foldable, M[_]](tma: T[M[Unit]])(implicit P: Parallel[M]): M[Unit] = { val fu: P.F[Unit] = Foldable[T].traverse_(tma)(P.parallel.apply(_))(P.applicative) P.sequential(fu) } @@ -272,9 +272,9 @@ object Parallel extends ParallelArityFunctions2 { * Like `Foldable[A].traverse_`, but uses the applicative instance * corresponding to the Parallel instance instead. */ - def parTraverse_[T[_]: Foldable, M[_], A, B]( + def parTraverse_[T[_]: Foldable, M[_], A]( ta: T[A] - )(f: A => M[B])(implicit P: Parallel[M]): M[Unit] = { + )(f: A => M[Unit])(implicit P: Parallel[M]): M[Unit] = { val gtb: P.F[Unit] = Foldable[T].traverse_(ta)(a => P.parallel(f(a)))(P.applicative) P.sequential(gtb) } @@ -348,8 +348,8 @@ object Parallel extends ParallelArityFunctions2 { * Like `Reducible[A].nonEmptySequence_`, but uses the apply instance * corresponding to the Parallel instance instead. */ - def parNonEmptySequence_[T[_]: Reducible, M[_], A]( - tma: T[M[A]] + def parNonEmptySequence_[T[_]: Reducible, M[_]]( + tma: T[M[Unit]] )(implicit P: NonEmptyParallel[M]): M[Unit] = { val fu: P.F[Unit] = Reducible[T].nonEmptyTraverse_(tma)(P.parallel.apply(_))(P.apply) P.sequential(fu) @@ -359,9 +359,9 @@ object Parallel extends ParallelArityFunctions2 { * Like `Reducible[A].nonEmptyTraverse_`, but uses the apply instance * corresponding to the Parallel instance instead. */ - def parNonEmptyTraverse_[T[_]: Reducible, M[_], A, B]( + def parNonEmptyTraverse_[T[_]: Reducible, M[_], A]( ta: T[A] - )(f: A => M[B])(implicit P: NonEmptyParallel[M]): M[Unit] = { + )(f: A => M[Unit])(implicit P: NonEmptyParallel[M]): M[Unit] = { val gtb: P.F[Unit] = Reducible[T].nonEmptyTraverse_(ta)(a => P.parallel(f(a)))(P.apply) P.sequential(gtb) } diff --git a/core/src/main/scala/cats/Reducible.scala b/core/src/main/scala/cats/Reducible.scala index 18bdedd02c..d7d1410614 100644 --- a/core/src/main/scala/cats/Reducible.scala +++ b/core/src/main/scala/cats/Reducible.scala @@ -200,7 +200,7 @@ trait Reducible[F[_]] extends Foldable[F] { self => * Traverse `F[A]` using `Apply[G]`. * * `A` values will be mapped into `G[B]` and combined using - * `Apply#map2`. + * `Apply#map2`. This is equivalent to `reduceMapA`. * * This method is similar to [[Foldable.traverse_]]. There are two * main differences: @@ -213,10 +213,8 @@ trait Reducible[F[_]] extends Foldable[F] { self => * available for `G` and want to take advantage of short-circuiting * the traversal. */ - def nonEmptyTraverse_[G[_], A, B](fa: F[A])(f: A => G[B])(implicit G: Apply[G]): G[Unit] = { - val f1 = f.andThen(G.void) - reduceRightTo(fa)(f1)((x, y) => G.map2Eval(f1(x), y)((_, b) => b)).value - } + def nonEmptyTraverse_[G[_], A](fa: F[A])(f: A => G[Unit])(implicit G: Apply[G]): G[Unit] = + reduceMapA(fa)(f) /** * Sequence `F[G[A]]` using `Apply[G]`. @@ -225,7 +223,7 @@ trait Reducible[F[_]] extends Foldable[F] { self => * an [[Apply]] instance for `G` instead of [[Applicative]]. See the * [[nonEmptyTraverse_]] documentation for a description of the differences. */ - def nonEmptySequence_[G[_], A](fga: F[G[A]])(implicit G: Apply[G]): G[Unit] = + def nonEmptySequence_[G[_]](fga: F[G[Unit]])(implicit G: Apply[G]): G[Unit] = nonEmptyTraverse_(fga)(identity) def toNonEmptyList[A](fa: F[A]): NonEmptyList[A] = @@ -399,10 +397,10 @@ object Reducible { typeClassInstance.reduceMapM[G, A, B](self)(f)(G, B) def reduceRightTo[B](f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[B] = typeClassInstance.reduceRightTo[A, B](self)(f)(g) - def nonEmptyTraverse_[G[_], B](f: A => G[B])(implicit G: Apply[G]): G[Unit] = - typeClassInstance.nonEmptyTraverse_[G, A, B](self)(f)(G) - def nonEmptySequence_[G[_], B](implicit ev$1: A <:< G[B], G: Apply[G]): G[Unit] = - typeClassInstance.nonEmptySequence_[G, B](self.asInstanceOf[F[G[B]]])(G) + def nonEmptyTraverse_[G[_]](f: A => G[Unit])(implicit G: Apply[G]): G[Unit] = + typeClassInstance.nonEmptyTraverse_[G, A](self)(f)(G) + def nonEmptySequence_[G[_]](implicit ev$1: A <:< G[Unit], G: Apply[G]): G[Unit] = + typeClassInstance.nonEmptySequence_[G](self.asInstanceOf[F[G[Unit]]])(G) def toNonEmptyList: NonEmptyList[A] = typeClassInstance.toNonEmptyList[A](self) def minimum(implicit A: Order[A]): A = typeClassInstance.minimum[A](self)(A) def maximum(implicit A: Order[A]): A = typeClassInstance.maximum[A](self)(A) diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index f4d01cda86..a54fcb13ad 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -129,7 +129,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { /** * This avoids making a very deep stack by building a tree instead */ - override def traverse_[G[_], A, B](fa: List[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { + override def traverse_[G[_], A](fa: List[A])(f: A => G[Unit])(implicit G: Applicative[G]): G[Unit] = { // the cost of this is O(size log size) // c(n) = n + 2 * c(n/2) = n + 2(n/2 log (n/2)) = n + n (logn - 1) = n log n // invariant: size >= 1 @@ -155,10 +155,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances { // failed. We do not use laziness to avoid // traversing fa, which we will do fully // in all cases. - Eval.always { - val gb = f(a) - G.void(gb) - } + Eval.always { f(a) } } val len = fa.length diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index 88db57d2b4..fc73d39768 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -137,7 +137,7 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { /** * This avoids making a very deep stack by building a tree instead */ - override def traverse_[G[_], A, B](fa: Vector[A])(f: A => G[B])(implicit G: Applicative[G]): G[Unit] = { + override def traverse_[G[_], A](fa: Vector[A])(f: A => G[Unit])(implicit G: Applicative[G]): G[Unit] = { // the cost of this is O(size) // c(n) = 1 + 2 * c(n/2) // invariant: size >= 1 @@ -161,10 +161,7 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { // failed. We do not use laziness to avoid // traversing fa, which we will do fully // in all cases. - Eval.always { - val gb = f(a) - G.void(gb) - } + Eval.always { f(a) } } val len = fa.length diff --git a/core/src/main/scala/cats/syntax/foldable.scala b/core/src/main/scala/cats/syntax/foldable.scala index 861d61d1e9..e82feca348 100644 --- a/core/src/main/scala/cats/syntax/foldable.scala +++ b/core/src/main/scala/cats/syntax/foldable.scala @@ -47,7 +47,8 @@ private[syntax] trait FoldableSyntaxBinCompat1 { } final class NestedFoldableOps[F[_], G[_], A](private val fga: F[G[A]]) extends AnyVal { - def sequence_(implicit F: Foldable[F], G: Applicative[G]): G[Unit] = F.sequence_(fga) + def sequence_(implicit F: Foldable[F], G: Applicative[G], ev: A <:< Unit): G[Unit] = + F.sequence_(fga.asInstanceOf[F[G[Unit]]]) /** * @see [[Foldable.foldK]]. diff --git a/core/src/main/scala/cats/syntax/parallel.scala b/core/src/main/scala/cats/syntax/parallel.scala index 3624946c1b..828188fdcf 100644 --- a/core/src/main/scala/cats/syntax/parallel.scala +++ b/core/src/main/scala/cats/syntax/parallel.scala @@ -111,7 +111,7 @@ trait ParallelTraverseSyntax { implicit final def catsSyntaxParallelTraverse_[T[_]: Foldable, A](ta: T[A]): ParallelTraversable_Ops[T, A] = new ParallelTraversable_Ops(ta) - implicit final def catsSyntaxParallelSequence_[T[_]: Foldable, M[_], A](tma: T[M[A]]): ParallelSequence_Ops[T, M, A] = + implicit final def catsSyntaxParallelSequence_[T[_]: Foldable, M[_]](tma: T[M[Unit]]): ParallelSequence_Ops[T, M] = new ParallelSequence_Ops(tma) } @@ -189,7 +189,7 @@ final class ParallelSequenceFilterOps[T[_], M[_], A](private val tmoa: T[M[Optio } final class ParallelTraversable_Ops[T[_], A](private val ta: T[A]) extends AnyVal { - def parTraverse_[M[_], B](f: A => M[B])(implicit T: Foldable[T], P: Parallel[M]): M[Unit] = + def parTraverse_[M[_]](f: A => M[Unit])(implicit T: Foldable[T], P: Parallel[M]): M[Unit] = Parallel.parTraverse_(ta)(f) } @@ -217,7 +217,7 @@ final class ParallelSequenceOps1[T[_], M[_], A](private val tma: T[M[A]]) extend Parallel.parSequence(tma) } -final class ParallelSequence_Ops[T[_], M[_], A](private val tma: T[M[A]]) extends AnyVal { +final class ParallelSequence_Ops[T[_], M[_]](private val tma: T[M[Unit]]) extends AnyVal { def parSequence_(implicit T: Foldable[T], P: Parallel[M]): M[Unit] = Parallel.parSequence_(tma) } diff --git a/docs/nomenclature.md b/docs/nomenclature.md index efc810b0a7..b7cedbdd31 100644 --- a/docs/nomenclature.md +++ b/docs/nomenclature.md @@ -115,8 +115,8 @@ Like the previous section, we use the `E` for the error parameter type. | `F[A] => (A => G[B]) => G[B]` | `foldMapM` | `G: Monad` and `B: Monoid` | `F[A] => (A => B) => Option[B]` | `collectFirst` | The `A => B` is a `PartialFunction` | `F[A] => (A => Option[B]) => Option[B]` | `collectFirstSome` | -| `F[A] => (A => G[B]) => G[Unit]` | `traverse_` | `G: Applicative` -| `F[G[A]] => G[Unit]` | `sequence_` | `G: Applicative` +| `F[A] => (A => G[Unit]) => G[Unit]` | `traverse_` | `G: Applicative` +| `F[G[Unit]] => G[Unit]` | `sequence_` | `G: Applicative` | `F[A] => (A => Either[B, C] => (F[B], F[C])` | `partitionEither` | `G: Applicative` ### Reducible diff --git a/docs/typeclasses/foldable.md b/docs/typeclasses/foldable.md index 9256ca58e6..fccf7e3c0b 100644 --- a/docs/typeclasses/foldable.md +++ b/docs/typeclasses/foldable.md @@ -52,10 +52,10 @@ Foldable[List].nonEmpty(List(1,2)) Foldable[Option].toList(Option(1)) Foldable[Option].toList(None) -def parseInt(s: String): Option[Int] = scala.util.Try(Integer.parseInt(s)).toOption +def checkInt(s: String): Option[Unit] = scala.util.Try{ Integer.parseInt(s) ; () }.toOption -Foldable[List].traverse_(List("1", "2"))(parseInt) -Foldable[List].traverse_(List("1", "A"))(parseInt) +Foldable[List].traverse_(List("1", "2"))(checkInt) +Foldable[List].traverse_(List("1", "A"))(checkInt) Foldable[List].sequence_(List(Option(1), Option(2))) Foldable[List].sequence_(List(Option(1), None)) diff --git a/docs/typeclasses/reducible.md b/docs/typeclasses/reducible.md index 77feaa08d3..3e6ee977b5 100644 --- a/docs/typeclasses/reducible.md +++ b/docs/typeclasses/reducible.md @@ -38,7 +38,7 @@ Reducible[NonEmptyList].reduceLeftTo(NonEmptyList.of(1,2,3,4))(_.toString)((s,i) Reducible[NonEmptyList].reduceRightTo(NonEmptyList.of(1,2,3,4))(_.toString)((i,s) => Later(s.value + i)).value Reducible[NonEmptyList].nonEmptyIntercalate(NonEmptyList.of("a", "b", "c"), ", ") -def countChars(s: String) = s.toCharArray.groupBy(identity).view.mapValues(_.length).toMap +def countChars(s: String) = s.toCharArray.groupBy(identity).view.mapValues(x => { x.length ; () }).toMap Reducible[NonEmptyList].nonEmptyTraverse_(NonEmptyList.of("Hello", "World"))(countChars) Reducible[NonEmptyVector].nonEmptyTraverse_(NonEmptyVector.of("Hello", ""))(countChars) diff --git a/laws/src/main/scala/cats/laws/ReducibleLaws.scala b/laws/src/main/scala/cats/laws/ReducibleLaws.scala index 89da4be3b1..2a9e2d7c36 100644 --- a/laws/src/main/scala/cats/laws/ReducibleLaws.scala +++ b/laws/src/main/scala/cats/laws/ReducibleLaws.scala @@ -53,10 +53,10 @@ trait ReducibleLaws[F[_]] extends FoldableLaws[F] { def reduceReduceLeftConsistent[B](fa: F[B])(implicit B: Semigroup[B]): IsEq[B] = fa.reduce <-> fa.reduceLeft(B.combine) - def traverseConsistent[G[_]: Applicative, A, B](fa: F[A], f: A => G[B]): IsEq[G[Unit]] = + def traverseConsistent[G[_]: Applicative, A](fa: F[A], f: A => G[Unit]): IsEq[G[Unit]] = fa.nonEmptyTraverse_(f) <-> fa.traverse_(f) - def sequenceConsistent[G[_]: Applicative, A](fa: F[G[A]]): IsEq[G[Unit]] = + def sequenceConsistent[G[_]: Applicative](fa: F[G[Unit]]): IsEq[G[Unit]] = fa.nonEmptySequence_ <-> fa.sequence_ def sizeConsistent[A](fa: F[A]): IsEq[Long] = diff --git a/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala b/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala index 6df6cc7877..e3d2de1f66 100644 --- a/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala @@ -44,13 +44,11 @@ trait NonEmptyTraverseTests[F[_]] extends TraverseTests[F] with ReducibleTests[F ArbYB: Arbitrary[Y[B]], ArbYC: Arbitrary[Y[C]], ArbFB: Arbitrary[F[B]], - ArbFM: Arbitrary[F[M]], ArbXM: Arbitrary[X[M]], ArbYM: Arbitrary[Y[M]], - ArbFGA: Arbitrary[F[G[A]]], + ArbGU: Arbitrary[G[Unit]], + ArbFGU: Arbitrary[F[G[Unit]]], ArbFXM: Arbitrary[F[X[M]]], - ArbGB: Arbitrary[G[B]], - ArbGM: Arbitrary[G[M]], CogenA: Cogen[A], CogenB: Cogen[B], CogenC: Cogen[C], diff --git a/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala b/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala index 2de4e20681..592fa56b23 100644 --- a/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala @@ -35,8 +35,8 @@ trait ReducibleTests[F[_]] extends FoldableTests[F] { def reducible[G[_]: Applicative, A: Arbitrary, B: Arbitrary](implicit ArbFA: Arbitrary[F[A]], ArbFB: Arbitrary[F[B]], - ArbFGA: Arbitrary[F[G[A]]], - ArbGB: Arbitrary[G[B]], + ArbFGU: Arbitrary[F[G[Unit]]], + ArbGU: Arbitrary[G[Unit]], CogenA: Cogen[A], CogenB: Cogen[B], EqG: Eq[G[Unit]], @@ -58,8 +58,8 @@ trait ReducibleTests[F[_]] extends FoldableTests[F] { forAll(laws.reduceRightConsistentWithReduceRightOption[A] _), "reduce consistent with reduceLeft" -> forAll(laws.reduceReduceLeftConsistent[B] _), - "nonEmptyTraverse_ consistent with traverse_" -> forAll(laws.traverseConsistent[G, A, B] _), - "nonEmptySequence_ consistent with sequence_" -> forAll(laws.sequenceConsistent[G, A] _), + "nonEmptyTraverse_ consistent with traverse_" -> forAll(laws.traverseConsistent[G, A] _), + "nonEmptySequence_ consistent with sequence_" -> forAll(laws.sequenceConsistent[G] _), "size consistent with reduceMap" -> forAll(laws.sizeConsistent[A] _) ) } diff --git a/tests/shared/src/test/scala-2.13+/cats/tests/ScalaVersionSpecific.scala b/tests/shared/src/test/scala-2.13+/cats/tests/ScalaVersionSpecific.scala index 1efbd376a0..0b87e67f9d 100644 --- a/tests/shared/src/test/scala-2.13+/cats/tests/ScalaVersionSpecific.scala +++ b/tests/shared/src/test/scala-2.13+/cats/tests/ScalaVersionSpecific.scala @@ -179,7 +179,7 @@ trait ScalaVersionSpecificRegressionSuite { self: RegressionSuite => // shouldn't have ever evaluated validate(8) checkAndResetCount(3) - assert(LazyList(1, 2, 6, 8).traverse_(validate) === (Either.left("6 is greater than 5"))) + assert(LazyList(1, 2, 6, 8).traverse_(validate(_).void) === (Either.left("6 is greater than 5"))) checkAndResetCount(3) } } diff --git a/tests/shared/src/test/scala/cats/tests/ParallelSuite.scala b/tests/shared/src/test/scala/cats/tests/ParallelSuite.scala index 869b2611b5..d4f9ae14cf 100644 --- a/tests/shared/src/test/scala/cats/tests/ParallelSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/ParallelSuite.scala @@ -79,23 +79,23 @@ class ParallelSuite } test("ParTraverse_ identity should be equivalent to parSequence_") { - forAll { (es: SortedSet[Either[String, Int]]) => - assert(Parallel.parTraverse_(es)(identity) === (Parallel.parSequence_[SortedSet, Either[String, *], Int](es))) + forAll { (es: SortedSet[Either[String, Unit]]) => + assert(Parallel.parTraverse_(es)(identity) === (Parallel.parSequence_[SortedSet, Either[String, *]](es))) } } test("ParTraverse_ syntax should be equivalent to Parallel.parTraverse_") { - forAll { (es: SortedSet[Either[String, Int]]) => + forAll { (es: SortedSet[Either[String, Unit]]) => assert( - Parallel.parTraverse_[SortedSet, Either[String, *], Either[String, Int], Int](es)(identity) === (es + Parallel.parTraverse_[SortedSet, Either[String, *], Either[String, Unit]](es)(identity) === (es .parTraverse_(identity)) ) } } test("ParSequence_ syntax should be equivalent to Parallel.parSequence_") { - forAll { (es: SortedSet[Either[String, Int]]) => - assert(Parallel.parSequence_[SortedSet, Either[String, *], Int](es) === (es.parSequence_)) + forAll { (es: SortedSet[Either[String, Unit]]) => + assert(Parallel.parSequence_[SortedSet, Either[String, *]](es) === (es.parSequence_)) } } @@ -106,7 +106,7 @@ class ParallelSuite } test("ParNonEmptyTraverse_ identity should be equivalent to parNonEmptySequence_") { - forAll { (es: NonEmptyList[Either[String, Int]]) => + forAll { (es: NonEmptyList[Either[String, Unit]]) => assert(Parallel.parNonEmptyTraverse_(es)(identity) === (Parallel.parNonEmptySequence_(es))) } } @@ -311,8 +311,8 @@ class ParallelSuite } test("ParReplicateA_ should be equivalent to fill parSequence_") { - forAll(Gen.choose(1, 20), Arbitrary.arbitrary[Either[String, String]]) { - (repetitions: Int, e: Either[String, String]) => + forAll(Gen.choose(1, 20), Arbitrary.arbitrary[Either[String, Unit]]) { + (repetitions: Int, e: Either[String, Unit]) => assert(Parallel.parReplicateA_(repetitions, e) === Parallel.parSequence_(List.fill(repetitions)(e))) } } diff --git a/tests/shared/src/test/scala/cats/tests/ReducibleSuite.scala b/tests/shared/src/test/scala/cats/tests/ReducibleSuite.scala index 01dd07fd02..4d05f85630 100644 --- a/tests/shared/src/test/scala/cats/tests/ReducibleSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/ReducibleSuite.scala @@ -238,7 +238,7 @@ abstract class ReducibleSuite[F[_]: Reducible](name: String)(implicit val notAllEven = fromValues(2, 4, 6, 9, 10, 12, 14) val out = mutable.ListBuffer[Int]() - notAllEven.nonEmptyTraverse_ { a => out += a; if (a % 2 == 0) Some(a) else None } + notAllEven.nonEmptyTraverse_ { a => out += a; if (a % 2 == 0) Some(()) else None } assert(out.toList === List(2, 4, 6, 9)) } diff --git a/tests/shared/src/test/scala/cats/tests/RegressionSuite.scala b/tests/shared/src/test/scala/cats/tests/RegressionSuite.scala index b326f39c3b..0298af14f4 100644 --- a/tests/shared/src/test/scala/cats/tests/RegressionSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/RegressionSuite.scala @@ -162,23 +162,25 @@ class RegressionSuite extends CatsSuite with ScalaVersionSpecificRegressionSuite assert(Vector(1, 2, 6, 8).traverse(validate) === (Either.left("6 is greater than 5"))) checkAndResetCount(3) - assert(List(1, 2, 6, 8).traverse_(validate) === (Either.left("6 is greater than 5"))) + def validateVoid(i: Int): Either[String, Unit] = validate(i).void + + assert(List(1, 2, 6, 8).traverse_(validateVoid) === (Either.left("6 is greater than 5"))) checkAndResetCount(3) { @annotation.nowarn("cat=deprecation") - val obtained = Stream(1, 2, 6, 8).traverse_(validate) + val obtained = Stream(1, 2, 6, 8).traverse_(validateVoid) assert(obtained === Either.left("6 is greater than 5")) } checkAndResetCount(3) - assert(Vector(1, 2, 6, 8).traverse_(validate) === (Either.left("6 is greater than 5"))) + assert(Vector(1, 2, 6, 8).traverse_(validateVoid) === (Either.left("6 is greater than 5"))) checkAndResetCount(3) - assert(NonEmptyList.of(1, 2, 6, 7, 8).traverse_(validate) === (Either.left("6 is greater than 5"))) + assert(NonEmptyList.of(1, 2, 6, 7, 8).traverse_(validateVoid) === (Either.left("6 is greater than 5"))) checkAndResetCount(3) - assert(NonEmptyList.of(6, 7, 8).traverse_(validate) === (Either.left("6 is greater than 5"))) + assert(NonEmptyList.of(6, 7, 8).traverse_(validateVoid) === (Either.left("6 is greater than 5"))) checkAndResetCount(1) } diff --git a/tests/shared/src/test/scala/cats/tests/SyntaxSuite.scala b/tests/shared/src/test/scala/cats/tests/SyntaxSuite.scala index 79abb123d9..bfdc6abfb3 100644 --- a/tests/shared/src/test/scala/cats/tests/SyntaxSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/SyntaxSuite.scala @@ -137,11 +137,12 @@ object SyntaxSuite { val f3 = mock[Z => A] val a1: A = fz.foldMap(f3) - val f4 = mock[A => G[B]] + val f4 = mock[A => G[Unit]] val gu0: G[Unit] = fa.traverse_(f4) + val fgu = mock[F[G[Unit]]] val fga = mock[F[G[A]]] - val gu1: G[Unit] = fga.sequence_ + val gu1: G[Unit] = fgu.sequence_ val ga: G[A] = fga.foldK val f5 = mock[A => Boolean] @@ -313,10 +314,11 @@ object SyntaxSuite { val f6 = mock[(A, Eval[B]) => Eval[B]] val lb: Eval[B] = fa.reduceRightTo(f4)(f6) - val f7 = mock[A => G[B]] + val f7 = mock[A => G[Unit]] val gu1: G[Unit] = fa.nonEmptyTraverse_(f7) - val gu2: G[Unit] = fga.nonEmptySequence_ + val fgu = mock[F[G[Unit]]] + val gu2: G[Unit] = fgu.nonEmptySequence_ } def testFunctor[F[_]: Functor, A, B]: Unit = { diff --git a/tests/shared/src/test/scala/cats/tests/TraverseSuite.scala b/tests/shared/src/test/scala/cats/tests/TraverseSuite.scala index 94fffaea5e..9db7a1c1d6 100644 --- a/tests/shared/src/test/scala/cats/tests/TraverseSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/TraverseSuite.scala @@ -85,7 +85,7 @@ abstract class TraverseSuite[F[_]: Traverse](name: String)(implicit ArbFInt: Arb test(s"Traverse[$name].traverse matches traverse_ with Option") { forAll { (fa: F[Int], fn: Int => Option[Int]) => - assert(Applicative[Option].void(fa.traverse(fn)) == fa.traverse_(fn)) + assert(Applicative[Option].void(fa.traverse(fn)) == fa.traverse_(fn.andThen(Applicative[Option].void))) } } From 85e3d15845dce786f400b1c90d6c933d59198795 Mon Sep 17 00:00:00 2001 From: John Passaro Date: Sun, 20 Nov 2022 04:30:23 -0500 Subject: [PATCH 2/6] fix compile err in docs --- docs/typeclasses/foldable.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/typeclasses/foldable.md b/docs/typeclasses/foldable.md index fccf7e3c0b..87b04f2edf 100644 --- a/docs/typeclasses/foldable.md +++ b/docs/typeclasses/foldable.md @@ -56,8 +56,8 @@ def checkInt(s: String): Option[Unit] = scala.util.Try{ Integer.parseInt(s) ; () Foldable[List].traverse_(List("1", "2"))(checkInt) Foldable[List].traverse_(List("1", "A"))(checkInt) -Foldable[List].sequence_(List(Option(1), Option(2))) -Foldable[List].sequence_(List(Option(1), None)) +Foldable[List].sequence_(List(Option(()), Option(()))) +Foldable[List].sequence_(List(Option(()), None)) Foldable[List].forallM(List(1, 2, 3))(i => if (i < 2) Some(i % 2 == 0) else None) Foldable[List].existsM(List(1, 2, 3))(i => if (i < 2) Some(i % 2 == 0) else None) From 9f60b5cd2fda3f77cae57f851bf638f2ccb39306 Mon Sep 17 00:00:00 2001 From: John Passaro Date: Sun, 20 Nov 2022 04:54:12 -0500 Subject: [PATCH 3/6] attempt to fix binary compatibility issue Co-authored-by: Arman Bilge --- binCompatTest/src/main/scala-2/catsBC/MimaExceptions.scala | 3 ++- core/src/main/scala/cats/Foldable.scala | 6 +++++- core/src/main/scala/cats/Reducible.scala | 6 ++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/binCompatTest/src/main/scala-2/catsBC/MimaExceptions.scala b/binCompatTest/src/main/scala-2/catsBC/MimaExceptions.scala index bbfadaf8c6..046050a6c0 100644 --- a/binCompatTest/src/main/scala-2/catsBC/MimaExceptions.scala +++ b/binCompatTest/src/main/scala-2/catsBC/MimaExceptions.scala @@ -54,7 +54,8 @@ object MimaExceptions { ( cats.Bimonad[cats.data.NonEmptyChain], cats.NonEmptyTraverse[cats.data.NonEmptyChain], - cats.SemigroupK[cats.data.NonEmptyChain] + cats.SemigroupK[cats.data.NonEmptyChain], + List("foo", "bar", "baz").traverse_(x => Either.right(x)) ) ) } diff --git a/core/src/main/scala/cats/Foldable.scala b/core/src/main/scala/cats/Foldable.scala index 1f4ce95881..1153e199ff 100644 --- a/core/src/main/scala/cats/Foldable.scala +++ b/core/src/main/scala/cats/Foldable.scala @@ -588,7 +588,11 @@ trait Foldable[F[_]] extends UnorderedFoldable[F] with FoldableNFunctions[F] { s * or effect. It is equivalent to `foldMapA` using the Unit monoid. */ def traverse_[G[_], A](fa: F[A])(f: A => G[Unit])(implicit G: Applicative[G]): G[Unit] = - foldMapA(fa)(f) + foldRight(fa, Always(G.pure(()))) { (a, acc) => + G.map2Eval(f(a), acc) { (_, _) => + () + } + }.value /** * Sequence `F[G[Unit]]` using `Applicative[G]`. diff --git a/core/src/main/scala/cats/Reducible.scala b/core/src/main/scala/cats/Reducible.scala index d7d1410614..6f58548411 100644 --- a/core/src/main/scala/cats/Reducible.scala +++ b/core/src/main/scala/cats/Reducible.scala @@ -213,8 +213,10 @@ trait Reducible[F[_]] extends Foldable[F] { self => * available for `G` and want to take advantage of short-circuiting * the traversal. */ - def nonEmptyTraverse_[G[_], A](fa: F[A])(f: A => G[Unit])(implicit G: Apply[G]): G[Unit] = - reduceMapA(fa)(f) + def nonEmptyTraverse_[G[_], A](fa: F[A])(f: A => G[Unit])(implicit G: Apply[G]): G[Unit] = { + val f1 = f.andThen(G.void) + reduceRightTo(fa)(f1)((x, y) => G.map2Eval(f1(x), y)((_, b) => b)).value + } /** * Sequence `F[G[A]]` using `Apply[G]`. From b5ddca02558b434a470813a82127aa1877a19ab1 Mon Sep 17 00:00:00 2001 From: John Passaro Date: Sun, 20 Nov 2022 05:23:08 -0500 Subject: [PATCH 4/6] restore list/vector traverse_ implementations for bincompat --- core/src/main/scala/cats/instances/list.scala | 5 ++++- core/src/main/scala/cats/instances/vector.scala | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/cats/instances/list.scala b/core/src/main/scala/cats/instances/list.scala index a54fcb13ad..e478d75eaa 100644 --- a/core/src/main/scala/cats/instances/list.scala +++ b/core/src/main/scala/cats/instances/list.scala @@ -155,7 +155,10 @@ trait ListInstances extends cats.kernel.instances.ListInstances { // failed. We do not use laziness to avoid // traversing fa, which we will do fully // in all cases. - Eval.always { f(a) } + Eval.always { + val fb = f(a) + G.void(fb) + } } val len = fa.length diff --git a/core/src/main/scala/cats/instances/vector.scala b/core/src/main/scala/cats/instances/vector.scala index fc73d39768..d9f8e37fc7 100644 --- a/core/src/main/scala/cats/instances/vector.scala +++ b/core/src/main/scala/cats/instances/vector.scala @@ -161,7 +161,10 @@ trait VectorInstances extends cats.kernel.instances.VectorInstances { // failed. We do not use laziness to avoid // traversing fa, which we will do fully // in all cases. - Eval.always { f(a) } + Eval.always { + val fb = f(a) + G.void(fb) + } } val len = fa.length From d35c6ea8e395f69a3e07116f0c878d6c0ce81b00 Mon Sep 17 00:00:00 2001 From: John Passaro Date: Sun, 20 Nov 2022 05:30:02 -0500 Subject: [PATCH 5/6] attempt to fix bincompat issues w/ implicit params in laws --- .../cats/laws/discipline/NonEmptyTraverseTests.scala | 10 +++++++--- .../scala/cats/laws/discipline/ReducibleTests.scala | 8 +++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala b/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala index e3d2de1f66..4594945670 100644 --- a/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/NonEmptyTraverseTests.scala @@ -44,11 +44,13 @@ trait NonEmptyTraverseTests[F[_]] extends TraverseTests[F] with ReducibleTests[F ArbYB: Arbitrary[Y[B]], ArbYC: Arbitrary[Y[C]], ArbFB: Arbitrary[F[B]], + ArbFM: Arbitrary[F[M]], ArbXM: Arbitrary[X[M]], ArbYM: Arbitrary[Y[M]], - ArbGU: Arbitrary[G[Unit]], - ArbFGU: Arbitrary[F[G[Unit]]], + ArbFGA: Arbitrary[F[G[A]]], ArbFXM: Arbitrary[F[X[M]]], + ArbGB: Arbitrary[G[B]], + ArbGM: Arbitrary[G[M]], CogenA: Cogen[A], CogenB: Cogen[B], CogenC: Cogen[C], @@ -69,7 +71,9 @@ trait NonEmptyTraverseTests[F[_]] extends TraverseTests[F] with ReducibleTests[F EqXFM: Eq[X[F[M]]], EqYFB: Eq[Y[F[B]]], EqYFM: Eq[Y[F[M]]], - EqOptionA: Eq[Option[A]] + EqOptionA: Eq[Option[A]], + ArbGU: Arbitrary[G[Unit]], + ArbFGU: Arbitrary[F[G[Unit]]] ): RuleSet = new RuleSet { def name: String = "nonEmptyTraverse" def bases: Seq[(String, RuleSet)] = Nil diff --git a/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala b/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala index 592fa56b23..a909a18dc6 100644 --- a/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala @@ -35,8 +35,8 @@ trait ReducibleTests[F[_]] extends FoldableTests[F] { def reducible[G[_]: Applicative, A: Arbitrary, B: Arbitrary](implicit ArbFA: Arbitrary[F[A]], ArbFB: Arbitrary[F[B]], - ArbFGU: Arbitrary[F[G[Unit]]], - ArbGU: Arbitrary[G[Unit]], + ArbFGA: Arbitrary[F[G[A]]], + ArbGB: Arbitrary[G[B]], CogenA: Cogen[A], CogenB: Cogen[B], EqG: Eq[G[Unit]], @@ -45,7 +45,9 @@ trait ReducibleTests[F[_]] extends FoldableTests[F] { EqFA: Eq[F[A]], EqOptionA: Eq[Option[A]], MonoidA: CommutativeMonoid[A], - MonoidB: CommutativeMonoid[B] + MonoidB: CommutativeMonoid[B], + ArbFGU: Arbitrary[F[G[Unit]]], + ArbGU: Arbitrary[G[Unit]] ): RuleSet = new DefaultRuleSet( name = "reducible", From b96d97136cba46c0b324a7db47af6b19105a3ea6 Mon Sep 17 00:00:00 2001 From: John Passaro Date: Sun, 20 Nov 2022 05:49:05 -0500 Subject: [PATCH 6/6] update docs compile error --- docs/typeclasses/reducible.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/typeclasses/reducible.md b/docs/typeclasses/reducible.md index 3e6ee977b5..7e5ea7abc6 100644 --- a/docs/typeclasses/reducible.md +++ b/docs/typeclasses/reducible.md @@ -42,7 +42,7 @@ def countChars(s: String) = s.toCharArray.groupBy(identity).view.mapValues(x => Reducible[NonEmptyList].nonEmptyTraverse_(NonEmptyList.of("Hello", "World"))(countChars) Reducible[NonEmptyVector].nonEmptyTraverse_(NonEmptyVector.of("Hello", ""))(countChars) -Reducible[NonEmptyList].nonEmptySequence_(NonEmptyList.of(Map(1 -> 'o'), Map(1 -> 'o'))) +Reducible[NonEmptyList].nonEmptySequence_(NonEmptyList.of(Map(1 -> ()), Map(1 -> ()))) ```