From 2af29f207826079e75fa3c7506f1b828aa81af4d Mon Sep 17 00:00:00 2001 From: Cody Allen Date: Sun, 8 Nov 2015 08:53:52 -0500 Subject: [PATCH 1/2] Make Free/FreeApplicative constructors private There are a couple of reasons this might be a good idea. Currently it's possible to observe a difference when calling `.map(identity)` on a `Free` instance: ```scala scala> import cats.free._; import cats.implicits._ import cats.free._ import cats.implicits._ scala> def f(x: Free[Option, Int]) = x match { | case Free.Pure(i) => i | case _ => 0 | } f: (x: cats.free.Free[Option,Int])Int scala> val x: Free[Option, Int] = Free.pure(3) x: cats.free.Free[Option,Int] = Pure(3) scala> f(x) res0: Int = 3 scala> f(x.map(identity)) res1: Int = 0 ``` Making these private also frees us up to change the internal representation in the future without breaking user code. --- free/src/main/scala/cats/free/Free.scala | 8 +++--- .../scala/cats/free/FreeApplicative.scala | 25 ++++++------------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/free/src/main/scala/cats/free/Free.scala b/free/src/main/scala/cats/free/Free.scala index ea18cb4e03..3580344af2 100644 --- a/free/src/main/scala/cats/free/Free.scala +++ b/free/src/main/scala/cats/free/Free.scala @@ -10,13 +10,13 @@ object Free { /** * Return from the computation with the given value. */ - final case class Pure[S[_], A](a: A) extends Free[S, A] + private final case class Pure[S[_], A](a: A) extends Free[S, A] /** Suspend the computation with the given suspension. */ - final case class Suspend[S[_], A](a: S[A]) extends Free[S, A] + private final case class Suspend[S[_], A](a: S[A]) extends Free[S, A] /** Call a subroutine and continue with the given function. */ - final case class Gosub[S[_], B, C](c: Free[S, C], f: C => Free[S, B]) extends Free[S, B] + private final case class Gosub[S[_], B, C](c: Free[S, C], f: C => Free[S, B]) extends Free[S, B] /** * Suspend a value within a functor lifting it to a Free. @@ -95,7 +95,7 @@ sealed abstract class Free[S[_], A] extends Product with Serializable { loop(this) } - def run(implicit S: Comonad[S]): A = go(S.extract) + final def run(implicit S: Comonad[S]): A = go(S.extract) /** * Run to completion, using a function that maps the resumption diff --git a/free/src/main/scala/cats/free/FreeApplicative.scala b/free/src/main/scala/cats/free/FreeApplicative.scala index 5766ec867e..979709c1d6 100644 --- a/free/src/main/scala/cats/free/FreeApplicative.scala +++ b/free/src/main/scala/cats/free/FreeApplicative.scala @@ -14,14 +14,14 @@ sealed abstract class FreeApplicative[F[_], A] extends Product with Serializable b match { case Pure(f) => this.map(f) - case x@Ap() => - apply(x.pivot)(self.ap(x.fn.map(fx => a => p => fx(p)(a)))) + case Ap(pivot, fn) => + apply(pivot)(self.ap(fn.map(fx => a => p => fx(p)(a)))) } final def map[B](f: A => B): FA[F, B] = this match { case Pure(a) => Pure(f(a)) - case x@Ap() => apply(x.pivot)(x.fn.map(f compose _)) + case Ap(pivot, fn) => apply(pivot)(fn.map(f compose _)) } /** Interprets/Runs the sequence of operations using the semantics of Applicative G @@ -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 x@Ap() => G.ap(f(x.pivot))(x.fn.foldMap(f)) + case Ap(pivot, fn) => G.ap(f(pivot))(fn.foldMap(f)) } /** Interpret/run the operations using the semantics of `Applicative[F]`. @@ -48,7 +48,7 @@ sealed abstract class FreeApplicative[F[_], A] extends Product with Serializable } /** Interpret this algebra into a Monoid */ - def analyze[M:Monoid](f: F ~> λ[α => M]): M = + final def analyze[M:Monoid](f: F ~> λ[α => M]): M = foldMap[Const[M, ?]](new (F ~> Const[M, ?]) { def apply[X](x: F[X]): Const[M,X] = Const(f(x)) }).getConst @@ -65,23 +65,14 @@ sealed abstract class FreeApplicative[F[_], A] extends Product with Serializable object FreeApplicative { type FA[F[_], A] = FreeApplicative[F, A] - final case class Pure[F[_], A](a: A) extends FA[F, A] + private final case class Pure[F[_], A](a: A) extends FA[F, A] - abstract case class Ap[F[_], A]() extends FA[F, A] { - type Pivot - val pivot: F[Pivot] - val fn: FA[F, Pivot => A] - } + private final case class Ap[F[_], P, A](pivot: F[P], fn: FA[F, P => A]) extends FA[F, A] final def pure[F[_], A](a: A): FA[F, A] = Pure(a) - final def ap[F[_], P, A](fp: F[P])(f: FA[F, P => A]): FA[F, A] = - new Ap[F, A] { - type Pivot = P - val pivot: F[Pivot] = fp - val fn: FA[F, Pivot => A] = f - } + final def ap[F[_], P, A](fp: F[P])(f: FA[F, P => A]): FA[F, A] = Ap(fp, f) final def lift[F[_], A](fa: F[A]): FA[F, A] = ap(fa)(Pure(a => a)) From bc06262073fb37a5962dbfbab569b6c2afd80745 Mon Sep 17 00:00:00 2001 From: Cody Allen Date: Mon, 9 Nov 2015 07:23:49 -0500 Subject: [PATCH 2/2] Fix compile error in Trampoline.scala Also make TrampolineFunctions package-private, since it's just a workaround for a scalac bug and isn't really intended to be used directly. --- free/src/main/scala/cats/free/Trampoline.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/free/src/main/scala/cats/free/Trampoline.scala b/free/src/main/scala/cats/free/Trampoline.scala index 04917d19ef..e5f68c06b1 100644 --- a/free/src/main/scala/cats/free/Trampoline.scala +++ b/free/src/main/scala/cats/free/Trampoline.scala @@ -5,9 +5,9 @@ import cats.std.function.function0Instance // To workaround SI-7139 `object Trampoline` needs to be defined inside the package object // together with the type alias. -abstract class TrampolineFunctions { +private[free] abstract class TrampolineFunctions { def done[A](a: A): Trampoline[A] = - Free.Pure[Function0,A](a) + Free.pure[Function0, A](a) def suspend[A](a: => Trampoline[A]): Trampoline[A] = Free.suspend(a)