Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace TransLift with MonadTrans #1621

Merged
merged 5 commits into from
May 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions core/src/main/scala/cats/MonadTrans.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package cats

/**
* A type class which abstracts over the ability to lift an M[A] into a
* MonadTransformer
*/
trait MonadTrans[MT[_[_], _]] {

/**
* Lift a value of type M[A] into a monad transformer MT[M, A]
*/
def liftT[M[_]: Monad, A](ma: M[A]): MT[M, A]
Copy link
Contributor

@adelbertc adelbertc Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing Edward Kmett noted in a talk he gave on monad homomorphisms is that MonadTrans provides no constraint that the resulting MT[M, A] has a Monad instance. I believe in his talk he fixes this by adding a function taking the Monad[M] constraint and providing a Monad[MT[M, ?]] constraint. In Scala it might look something like:

implicit def mtMonad[M[_]: Monad]: Monad[MT[M, ?]]

Would we want something like that here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scalaz does it. Not sure if it's useful. But it makes sense in theory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would communicate the law: For every lawful M[_]: Monad there exists a lawful MT[M, ?]: Monad

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we check such a law? Would be cool if we could rule out the naïve ListT for instance.

Copy link
Contributor

@adelbertc adelbertc Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A simple way would be to pick a fixed M, e.g. Option, get the Monad[MT[Option, ?]] from this method, and then law-check that.

A fancier way would be to use existentials or something to be able to vary M and do law-checking across all these :-)

EDIT Hmm I actually don't know what that would look like in terms of code though.

}

object MonadTrans {
def apply[MT[_[_], _]](implicit MT: MonadTrans[MT]): MonadTrans[MT] = MT
}
27 changes: 0 additions & 27 deletions core/src/main/scala/cats/TransLift.scala

This file was deleted.

23 changes: 0 additions & 23 deletions core/src/main/scala/cats/Trivial.scala

This file was deleted.

8 changes: 3 additions & 5 deletions core/src/main/scala/cats/data/EitherT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,9 @@ private[data] abstract class EitherTInstances extends EitherTInstances1 {
val F0: Traverse[F] = F
}

implicit def catsDataTransLiftForEitherT[E]: TransLift.Aux[EitherT[?[_], E, ?], Functor] =
new TransLift[EitherT[?[_], E, ?]] {
type TC[M[_]] = Functor[M]

def liftT[M[_]: Functor, A](ma: M[A]): EitherT[M, E, A] =
implicit def catsDataMonadTransForEitherT[E]: MonadTrans[EitherT[?[_], E, ?]] =
new MonadTrans[EitherT[?[_], E, ?]] {
def liftT[M[_]: Monad, A](ma: M[A]): EitherT[M, E, A] =
EitherT.liftT(ma)
}

Expand Down
8 changes: 3 additions & 5 deletions core/src/main/scala/cats/data/Kleisli.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,9 @@ private[data] sealed abstract class KleisliInstances extends KleisliInstances0 {
fa.local(f)
}

implicit def catsDataTransLiftForKleisli[A]: TransLift.AuxId[Kleisli[?[_], A, ?]] =
new TransLift[Kleisli[?[_], A, ?]] {
type TC[M[_]] = Trivial

def liftT[M[_], B](ma: M[B])(implicit ev: Trivial): Kleisli[M, A, B] = Kleisli.lift(ma)
implicit def catsDataMonadTransForKleisli[A]: MonadTrans[Kleisli[?[_], A, ?]] =
new MonadTrans[Kleisli[?[_], A, ?]] {
def liftT[M[_]: Monad, B](ma: M[B]): Kleisli[M, A, B] = Kleisli.lift(ma)
}

implicit def catsDataApplicativeErrorForKleisli[F[_], A, E](implicit AE: ApplicativeError[F, E]): ApplicativeError[Kleisli[F, A, ?], E] =
Expand Down
8 changes: 3 additions & 5 deletions core/src/main/scala/cats/data/OptionT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,9 @@ private[data] sealed trait OptionTInstances0 extends OptionTInstances1 {

private[data] sealed trait OptionTInstances1 extends OptionTInstances2 {
// do NOT change this to val! I know it looks like it should work, and really I agree, but it doesn't (for... reasons)
implicit def catsDataTransLiftForOptionT: TransLift.Aux[OptionT, Functor] =
new TransLift[OptionT] {
type TC[M[_]] = Functor[M]

def liftT[M[_]: Functor, A](ma: M[A]): OptionT[M, A] = OptionT.liftF(ma)
implicit def catsDataMonadTransForOptionT: MonadTrans[OptionT] =
new MonadTrans[OptionT] {
def liftT[M[_]: Monad, A](ma: M[A]): OptionT[M, A] = OptionT.liftF(ma)
}

implicit def catsDataMonoidKForOptionT[F[_]](implicit F0: Monad[F]): MonoidK[OptionT[F, ?]] =
Expand Down
12 changes: 5 additions & 7 deletions core/src/main/scala/cats/data/StateT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ private[data] sealed trait StateTInstances extends StateTInstances1 {
implicit def catsDataMonadStateForStateT[F[_], S](implicit F0: Monad[F]): MonadState[StateT[F, S, ?], S] =
new StateTMonadState[F, S] { implicit def F = F0 }

implicit def catsDataLiftForStateT[S]: TransLift.Aux[StateT[?[_], S, ?], Applicative] =
new StateTTransLift[S] {}
implicit def catsDataMonadTransForStateT[S]: MonadTrans[StateT[?[_], S, ?]] =
new StateTMonadTrans[S] {}
}

private[data] sealed trait StateTInstances1 extends StateTInstances2 {
Expand Down Expand Up @@ -289,10 +289,8 @@ private[data] sealed trait StateTMonadState[F[_], S] extends MonadState[StateT[F
def set(s: S): StateT[F, S, Unit] = StateT(_ => F.pure((s, ())))
}

private[data] sealed trait StateTTransLift[S] extends TransLift[StateT[?[_], S, ?]] {
type TC[M[_]] = Applicative[M]

def liftT[M[_]: Applicative, A](ma: M[A]): StateT[M, S, A] = StateT(s => Applicative[M].map(ma)(s -> _))
private[data] sealed trait StateTMonadTrans[S] extends MonadTrans[StateT[?[_], S, ?]] {
def liftT[M[_]: Monad, A](ma: M[A]): StateT[M, S, A] = StateT(s => Applicative[M].map(ma)(s -> _))
}

private[data] sealed trait StateTSemigroupK[F[_], S] extends SemigroupK[StateT[F, S, ?]] {
Expand All @@ -303,7 +301,7 @@ private[data] sealed trait StateTSemigroupK[F[_], S] extends SemigroupK[StateT[F
StateT(s => G.combineK(x.run(s), y.run(s)))
}

private[data] sealed trait StateTMonadCombine[F[_], S] extends MonadCombine[StateT[F, S, ?]] with StateTMonad[F, S] with StateTSemigroupK[F, S] with StateTTransLift[S] {
private[data] sealed trait StateTMonadCombine[F[_], S] extends MonadCombine[StateT[F, S, ?]] with StateTMonad[F, S] with StateTSemigroupK[F, S] with StateTMonadTrans[S] {
implicit def F: MonadCombine[F]
override def G: MonadCombine[F] = F

Expand Down
8 changes: 3 additions & 5 deletions core/src/main/scala/cats/data/WriterT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,9 @@ private[data] sealed abstract class WriterTInstances extends WriterTInstances0 {
fab.bimap(f, g)
}

implicit def catsDataTransLiftForWriterT[W](implicit W: Monoid[W]): TransLift.Aux[WriterT[?[_], W, ?], Functor] =
new TransLift[WriterT[?[_], W, ?]] {
type TC[M[_]] = Functor[M]

def liftT[M[_]: Functor, A](ma: M[A]): WriterT[M, W, A] =
implicit def catsDataMonadTransForWriterT[W](implicit W: Monoid[W]): MonadTrans[WriterT[?[_], W, ?]] =
new MonadTrans[WriterT[?[_], W, ?]] {
def liftT[M[_]: Monad, A](ma: M[A]): WriterT[M, W, A] =
WriterT(Functor[M].map(ma)((W.empty, _)))
}

Expand Down
4 changes: 2 additions & 2 deletions core/src/main/scala/cats/syntax/all.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ trait AllSyntax
with InvariantSyntax
with IorSyntax
with ListSyntax
with MonadSyntax
with MonadCombineSyntax
with MonadErrorSyntax
with MonadFilterSyntax
with MonadSyntax
with MonadTransSyntax
with MonoidSyntax
with OptionSyntax
with OrderSyntax
Expand All @@ -39,7 +40,6 @@ trait AllSyntax
with ShowSyntax
with SplitSyntax
with StrongSyntax
with TransLiftSyntax
with TraverseFilterSyntax
with TraverseSyntax
with TupleSyntax
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/scala/cats/syntax/monadTrans.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package cats
package syntax

trait MonadTransSyntax {
implicit final def catsSyntaxMonadTrans[F[_], A](fa: F[A]): MonadTransOps[F, A] = new MonadTransOps(fa)
}

final class MonadTransOps[F[_], A](val fa: F[A]) extends AnyVal {
def liftT[MT[_[_], _]](implicit F: Monad[F], MT: MonadTrans[MT]): MT[F, A] =
Copy link
Contributor

@kailuowang kailuowang Apr 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a doctest here? It will provide coverage for both here and the syntax line above. Then you should get green light from codecov

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a test in SyntaxTests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pointed out because codecov reported test miss here. Now I see that SyntaxTest is only a compilation check. I think I prefer a doctest in the type class which actually run the code (thus coverage) and provide a concrete example there. But we can certainly address that in a different PR.

MT.liftT(fa)
}
2 changes: 1 addition & 1 deletion core/src/main/scala/cats/syntax/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ package object syntax {
object show extends Show.ToShowOps
object split extends SplitSyntax
object strong extends StrongSyntax
object transLift extends TransLiftSyntax
object monadTrans extends MonadTransSyntax
object traverse extends TraverseSyntax
object traverseFilter extends TraverseFilterSyntax
object tuple extends TupleSyntax
Expand Down
35 changes: 0 additions & 35 deletions core/src/main/scala/cats/syntax/transLift.scala

This file was deleted.

9 changes: 3 additions & 6 deletions free/src/main/scala/cats/free/FreeT.scala
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,9 @@ private[free] sealed trait FreeTInstances1 extends FreeTInstances2 {
implicit def M: Applicative[M] = M0
}

implicit def catsFreeTransLiftForFreeT[S[_]]: TransLift.Aux[FreeT[S, ?[_], ?], Functor] =
new TransLift[FreeT[S, ?[_], ?]] {

type TC[M[_]] = Functor[M]

override def liftT[M[_]: Functor, A](ma: M[A]): FreeT[S, M, A] =
implicit def catsFreeMonadTransForFreeT[S[_]]: MonadTrans[FreeT[S, ?[_], ?]] =
new MonadTrans[FreeT[S, ?[_], ?]] {
override def liftT[M[_]: Monad, A](ma: M[A]): FreeT[S, M, A] =
FreeT.liftT(ma)
}
}
Expand Down
9 changes: 4 additions & 5 deletions free/src/test/scala/cats/free/FreeTTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ class FreeTTests extends CatsSuite {
checkAll("MonadState[FreeT[State[Int, ?],State[Int, ?], ?], Int]", SerializableTests.serializable(MonadState[FreeTState, Int]))
}

{
checkAll("MonadTrans[FreeT[Option, ?[_], ?]]", MonadTransTests[FreeT[Option, ?[_], ?]].monadTrans[Option, Int, Int])
}

test("FlatMap stack safety tested with 50k flatMaps") {
val expected = Applicative[FreeTOption].pure(())
val result =
Expand Down Expand Up @@ -96,11 +100,6 @@ class FreeTTests extends CatsSuite {
val b = a.hoist(FunctionK.id)
}

test("transLift for FreeT requires only Functor") {
implicit val transLiftInstance = FreeT.catsFreeTransLiftForFreeT[JustFunctor]
val d: FreeT[JustFunctor, JustFunctor, Int] = transLiftInstance.liftT[JustFunctor, Int](JustFunctor(1))
}

test("compile to universal id equivalent to original instance") {
forAll { a: FreeTOption[Int] =>
val b = a.compile(FunctionK.id)
Expand Down
17 changes: 17 additions & 0 deletions laws/src/main/scala/cats/laws/MonadTransLaws.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package cats
package laws

trait MonadTransLaws[MT[_[_], _]] {
implicit def MT: MonadTrans[MT]

def identity[G[_], A](a: A)(implicit G: Monad[G], MTM: Monad[MT[G, ?]]): IsEq[MT[G, A]] =
MT.liftT(G.pure(a)) <-> MTM.pure(a)

def composition[G[_], A, B](ga: G[A], f: A => G[B])(implicit G: Monad[G], MTM: Monad[MT[G, ?]]): IsEq[MT[G, B]] =
MT.liftT(G.flatMap(ga)(f)) <-> MTM.flatMap(MT.liftT(ga))(a => MT.liftT(f(a)))
}

object MonadTransLaws {
def apply[MT[_[_], _]](implicit ev: MonadTrans[MT]): MonadTransLaws[MT] =
new MonadTransLaws[MT] { def MT: MonadTrans[MT] = ev }
}
36 changes: 36 additions & 0 deletions laws/src/main/scala/cats/laws/discipline/MonadTransTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package cats
package laws
package discipline

import org.scalacheck.{Arbitrary, Cogen, Prop}
import org.typelevel.discipline.Laws
import Prop._

trait MonadTransTests[MT[_[_], _]] extends Laws {
def laws: MonadTransLaws[MT]

def monadTrans[G[_]: Monad, A: Arbitrary: Eq, B: Eq](implicit
MonadMTG: Monad[MT[G, ?]],
ArbGA: Arbitrary[G[A]],
ArbGB: Arbitrary[G[B]],
CogenA: Cogen[A],
EqGA: Eq[G[A]],
EqGB: Eq[G[B]],
EqMTGA: Eq[MT[G, A]],
EqMTGB: Eq[MT[G, B]]
): RuleSet = {
new DefaultRuleSet(
name = "monadTrans",
parent = None,
"monadTrans identity" -> forAll(laws.identity[G, A] _),
"monadTrans composition" -> forAll(laws.composition[G, A, B] _)
)
}
}

object MonadTransTests {
def apply[MT[_[_], _]: MonadTrans]: MonadTransTests[MT] =
new MonadTransTests[MT] {
def laws: MonadTransLaws[MT] = MonadTransLaws[MT]
}
}
2 changes: 2 additions & 0 deletions tests/src/test/scala/cats/tests/EitherTTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ class EitherTTests extends CatsSuite {
Functor[EitherT[ListWrapper, String, ?]]
Applicative[EitherT[ListWrapper, String, ?]]
Monad[EitherT[ListWrapper, String, ?]]
MonadTrans[EitherT[?[_], String, ?]]

checkAll("EitherT[ListWrapper, String, Int]", MonadErrorTests[EitherT[ListWrapper, String, ?], String].monadError[Int, Int, Int])
checkAll("MonadError[EitherT[List, ?, ?]]", SerializableTests.serializable(MonadError[EitherT[ListWrapper, String, ?], String]))
checkAll("MonadTrans[EitherT[?[_], String, ?]]", MonadTransTests[EitherT[?[_], String, ?]].monadTrans[ListWrapper, Int, Int])
}

{
Expand Down
3 changes: 3 additions & 0 deletions tests/src/test/scala/cats/tests/KleisliTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ class KleisliTests extends CatsSuite {

checkAll("Kleisli[Option, ?, Int]", ContravariantTests[Kleisli[Option, ?, Int]].contravariant[Int, Int, Int])
checkAll("Contravariant[Kleisli[Option, ?, Int]]", SerializableTests.serializable(Contravariant[Kleisli[Option, ?, Int]]))
checkAll("MonadTrans[Kleisli[?[_], Int, ?]]", MonadTransTests[Kleisli[?[_], Int, ?]].monadTrans[Option, Int, Int])

test("local composes functions") {
forAll { (f: Int => Option[String], g: Int => Int, i: Int) =>
Expand Down Expand Up @@ -226,5 +227,7 @@ class KleisliTests extends CatsSuite {
ApplicativeError[Kleisli[cats.data.Validated[Unit, ?], Int, ?], Unit]
ApplicativeError[Kleisli[Option, Int, ?], Unit]
MonadError[Kleisli[Option, Int, ?], Unit]

MonadTrans[Kleisli[?[_], Int, ?]]
}
}
3 changes: 3 additions & 0 deletions tests/src/test/scala/cats/tests/OptionTTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class OptionTTests extends CatsSuite {

checkAll("OptionT[ListWrapper, Int]", MonoidKTests[OptionT[ListWrapper, ?]].monoidK[Int])
checkAll("MonoidK[OptionT[ListWrapper, ?]]", SerializableTests.serializable(MonoidK[OptionT[ListWrapper, ?]]))
checkAll("MonadTrans[OptionT]", MonadTransTests[OptionT].monadTrans[ListWrapper, Int, Int])

FlatMap[OptionT[ListWrapper, ?]]
Applicative[OptionT[ListWrapper, ?]]
Expand Down Expand Up @@ -307,6 +308,8 @@ class OptionTTests extends CatsSuite {
implicit val T = ListWrapper.traverse
implicit val M = ListWrapper.monad
Functor[OptionT[ListWrapper, ?]]

MonadTrans[OptionT]
}

}
3 changes: 3 additions & 0 deletions tests/src/test/scala/cats/tests/StateTTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,15 @@ class StateTTests extends CatsSuite {

checkAll("StateT[ListWrapper, Int, Int]", MonadTests[StateT[ListWrapper, Int, ?]].monad[Int, Int, Int])
checkAll("Monad[StateT[ListWrapper, Int, ?]]", SerializableTests.serializable(Monad[StateT[ListWrapper, Int, ?]]))
checkAll("MonadTrans[EitherT[?[_], String, ?]]", MonadTransTests[EitherT[?[_], String, ?]].monadTrans[ListWrapper, Int, Int])

Monad[StateT[ListWrapper, Int, ?]]
FlatMap[StateT[ListWrapper, Int, ?]]
Applicative[StateT[ListWrapper, Int, ?]]
Apply[StateT[ListWrapper, Int, ?]]
Functor[StateT[ListWrapper, Int, ?]]

MonadTrans[StateT[?[_], Int, ?]]
}

{
Expand Down
Loading