Merge pull request #801 from ceedubs/freeapp-effect-order

Fix order of effects in FreeApplicative.foldMap

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]`.

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")
+  }
 }