From 56dede8def4f906d3a060b947b55f86746b81e90 Mon Sep 17 00:00:00 2001 From: Cody Allen Date: Wed, 13 Jan 2016 18:08:14 -0500 Subject: [PATCH] Fix order of effects in FreeApplicative.foldMap Fixes #799 --- .../scala/cats/free/FreeApplicative.scala | 2 +- .../cats/tests/FreeApplicativeTests.scala | 44 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/cats/free/FreeApplicative.scala b/core/src/main/scala/cats/free/FreeApplicative.scala index 204815b0bb..92d68eef6a 100644 --- a/core/src/main/scala/cats/free/FreeApplicative.scala +++ b/core/src/main/scala/cats/free/FreeApplicative.scala @@ -30,7 +30,7 @@ sealed abstract class FreeApplicative[F[_], A] extends Product with Serializable final def foldMap[G[_]](f: F ~> G)(implicit G: Applicative[G]): G[A] = this match { case Pure(a) => G.pure(a) - case Ap(pivot, fn) => G.ap(f(pivot))(fn.foldMap(f)) + case Ap(pivot, fn) => G.map2(f(pivot), fn.foldMap(f))((a, g) => g(a)) } /** Interpret/run the operations using the semantics of `Applicative[F]`. diff --git a/tests/src/test/scala/cats/tests/FreeApplicativeTests.scala b/tests/src/test/scala/cats/tests/FreeApplicativeTests.scala index aae6fdf5ca..3ac915abdd 100644 --- a/tests/src/test/scala/cats/tests/FreeApplicativeTests.scala +++ b/tests/src/test/scala/cats/tests/FreeApplicativeTests.scala @@ -4,8 +4,9 @@ package tests import cats.arrow.NaturalTransformation import cats.free.FreeApplicative import cats.laws.discipline.{MonoidalTests, ApplicativeTests, SerializableTests} -import cats.laws.discipline.eq.tuple3Eq +import cats.laws.discipline.eq.{tuple3Eq, tuple2Eq} import cats.data.Const +import cats.state.State import org.scalacheck.{Arbitrary, Gen} @@ -86,4 +87,45 @@ class FreeApplicativeTests extends CatsSuite { val fli2 = FreeApplicative.lift[List, Int](List.empty) fli2.analyze[G[Int]](countingNT) should ===(List(0)) } + + test("foldMap order of effects - regression check for #799") { + trait Foo[A] { + def getA: A + } + final case class Bar(getA: Int) extends Foo[Int] + final case class Baz(getA: Long) extends Foo[Long] + + type Dsl[A] = FreeApplicative[Foo, A] + + type Tracked[A] = State[String, A] + + val f: Foo ~> Tracked = new (Foo ~> Tracked) { + def apply[A](fa: Foo[A]): Tracked[A] = State[String, A]{ s0 => + (s0 + fa.toString + ";", fa.getA) + } + } + + val x: Dsl[Int] = FreeApplicative.lift(Bar(3)) + val y: Dsl[Long] = FreeApplicative.lift(Baz(5L)) + + val z1: Dsl[Long] = Apply[Dsl].map2(x, y)((x, y) => x.toLong + y) + val z2: Dsl[Long] = Apply[Dsl].map2(y, x)((y, x) => x.toLong + y) + + z1.foldMap(f).run("").value should === (("Bar(3);Baz(5);", 8L)) + z2.foldMap(f).run("").value should === (("Baz(5);Bar(3);", 8L)) + } + + test("analyze order of effects - regression check for #799") { + type Dsl[A] = FreeApplicative[Id, A] + val x: Dsl[String] = FreeApplicative.lift[Id, String]("x") + val y: Dsl[String] = FreeApplicative.lift[Id, String]("y") + + val z = Apply[Dsl].map2(x, y)((_, _) => ()) + + val asString: Id ~> λ[α => String] = new (Id ~> λ[α => String]) { + def apply[A](a: A): String = a.toString + } + + z.analyze(asString) should === ("xy") + } }