From eba9a59b3b36cf6e41799a997f32fc8a1d71fd32 Mon Sep 17 00:00:00 2001 From: Filippo Mariotti Date: Thu, 3 May 2018 15:24:55 +0100 Subject: [PATCH] Extend Functor implementation in the Traverse instance of EitherT, OptionT and Tuple2K (#2191) * Applied suggestion about extending Functor implementation from the traverse for monad transformer * Addressed binary compatibility * Reused existing instance in other cases following the suggestions from the code review * Trying to improve coverage * Extended SemigroupK from Alternative in IndexedReaderWriterStateT * Reverted contravariant change because of compat. --- core/src/main/scala/cats/data/EitherK.scala | 8 ++-- core/src/main/scala/cats/data/EitherT.scala | 19 ++++++--- .../cats/data/IndexedReaderWriterStateT.scala | 41 +++++++++---------- core/src/main/scala/cats/data/IorT.scala | 2 +- core/src/main/scala/cats/data/Kleisli.scala | 2 +- core/src/main/scala/cats/data/Nested.scala | 4 +- core/src/main/scala/cats/data/OptionT.scala | 2 +- core/src/main/scala/cats/data/Tuple2K.scala | 31 ++++++++------ core/src/main/scala/cats/data/WriterT.scala | 4 +- .../test/scala/cats/tests/NestedSuite.scala | 4 +- .../test/scala/cats/tests/Tuple2KSuite.scala | 2 + .../test/scala/cats/tests/WriterTSuite.scala | 2 - 12 files changed, 67 insertions(+), 54 deletions(-) diff --git a/core/src/main/scala/cats/data/EitherK.scala b/core/src/main/scala/cats/data/EitherK.scala index e62fa7d509..6872e87604 100644 --- a/core/src/main/scala/cats/data/EitherK.scala +++ b/core/src/main/scala/cats/data/EitherK.scala @@ -140,7 +140,7 @@ private[data] sealed abstract class EitherKInstances2 extends EitherKInstances3 private[data] sealed abstract class EitherKInstances1 extends EitherKInstances2 { implicit def catsDataCoflatMapForEitherK[F[_], G[_]](implicit F0: CoflatMap[F], G0: CoflatMap[G]): CoflatMap[EitherK[F, G, ?]] = - new EitherKCoflatMap[F, G] { + new EitherKCoflatMap[F, G] with EitherKFunctor[F, G] { implicit def F: CoflatMap[F] = F0 implicit def G: CoflatMap[G] = G0 @@ -149,7 +149,7 @@ private[data] sealed abstract class EitherKInstances1 extends EitherKInstances2 private[data] sealed abstract class EitherKInstances0 extends EitherKInstances1 { implicit def catsDataTraverseForEitherK[F[_], G[_]](implicit F0: Traverse[F], G0: Traverse[G]): Traverse[EitherK[F, G, ?]] = - new EitherKTraverse[F, G] { + new EitherKTraverse[F, G] with EitherKFunctor[F, G] { implicit def F: Traverse[F] = F0 implicit def G: Traverse[G] = G0 @@ -159,7 +159,7 @@ private[data] sealed abstract class EitherKInstances0 extends EitherKInstances1 private[data] sealed abstract class EitherKInstances extends EitherKInstances0 { implicit def catsDataComonadForEitherK[F[_], G[_]](implicit F0: Comonad[F], G0: Comonad[G]): Comonad[EitherK[F, G, ?]] = - new EitherKComonad[F, G] { + new EitherKComonad[F, G] with EitherKFunctor[F, G] { implicit def F: Comonad[F] = F0 implicit def G: Comonad[G] = G0 @@ -171,7 +171,7 @@ private[data] trait EitherKFunctor[F[_], G[_]] extends Functor[EitherK[F, G, ?]] implicit def G: Functor[G] - def map[A, B](a: EitherK[F, G, A])(f: A => B): EitherK[F, G, B] = + override def map[A, B](a: EitherK[F, G, A])(f: A => B): EitherK[F, G, B] = a map f } diff --git a/core/src/main/scala/cats/data/EitherT.scala b/core/src/main/scala/cats/data/EitherT.scala index 516034d7d0..748c1b029b 100644 --- a/core/src/main/scala/cats/data/EitherT.scala +++ b/core/src/main/scala/cats/data/EitherT.scala @@ -469,13 +469,14 @@ private[data] abstract class EitherTInstances extends EitherTInstances1 { Contravariant[Show].contramap(sh)(_.value) implicit def catsDataBifunctorForEitherT[F[_]](implicit F: Functor[F]): Bifunctor[EitherT[F, ?, ?]] = - new Bifunctor[EitherT[F, ?, ?]] { - override def bimap[A, B, C, D](fab: EitherT[F, A, B])(f: A => C, g: B => D): EitherT[F, C, D] = fab.bimap(f, g) + new EitherTBifunctor[F] { + val F0: Functor[F] = F } - implicit def catsDataTraverseForEitherT[F[_], L](implicit F: Traverse[F]): Traverse[EitherT[F, L, ?]] = - new EitherTTraverse[F, L] { - val F0: Traverse[F] = F + implicit def catsDataTraverseForEitherT[F[_], L](implicit FF: Traverse[F]): Traverse[EitherT[F, L, ?]] = + new EitherTTraverse[F, L] with EitherTFunctor[F, L] { + val F0: Traverse[F] = FF + val F: Functor[F] = FF } implicit def catsMonoidForEitherT[F[_], L, A](implicit F: Monoid[F[Either[L, A]]]): Monoid[EitherT[F, L, A]] = @@ -499,7 +500,7 @@ private[data] abstract class EitherTInstances1 extends EitherTInstances2 { } implicit def catsDataBitraverseForEitherT[F[_]](implicit F: Traverse[F]): Bitraverse[EitherT[F, ?, ?]] = - new EitherTBitraverse[F] { + new EitherTBitraverse[F] with EitherTBifunctor[F] { val F0: Traverse[F] = F } @@ -647,6 +648,12 @@ private[data] sealed trait EitherTBitraverse[F[_]] extends Bitraverse[EitherT[F, fab.bitraverse(f, g) } +private[data] sealed trait EitherTBifunctor[F[_]] extends Bifunctor[EitherT[F, ?, ?]] { + implicit def F0: Functor[F] + + override def bimap[A, B, C, D](fab: EitherT[F, A, B])(f: A => C, g: B => D): EitherT[F, C, D] = fab.bimap(f, g) +} + private[data] sealed trait EitherTEq[F[_], L, A] extends Eq[EitherT[F, L, A]] { implicit def F0: Eq[F[Either[L, A]]] diff --git a/core/src/main/scala/cats/data/IndexedReaderWriterStateT.scala b/core/src/main/scala/cats/data/IndexedReaderWriterStateT.scala index 2514bc8aa6..375657a80b 100644 --- a/core/src/main/scala/cats/data/IndexedReaderWriterStateT.scala +++ b/core/src/main/scala/cats/data/IndexedReaderWriterStateT.scala @@ -563,7 +563,24 @@ private[data] sealed abstract class RWSTMonad[F[_], E, L, S] extends IRWSTFuncto } } -private[data] sealed abstract class IRWSTSemigroupK[F[_], E, L, SA, SB] extends SemigroupK[IndexedReaderWriterStateT[F, E, L, SA, SB, ?]] { +private[data] sealed abstract class IRWSTSemigroupK[F[_], E, L, SA, SB] extends IRWSTSemigroupK1[F, E, L, SA, SB] + +private[data] sealed abstract class RWSTAlternative[F[_], E, L, S] extends IRWSTFunctor[F, E, L, S, S] with RWSTAlternative1[F, E, L, S] + +private[data] sealed abstract class RWSTMonadError[F[_], E, L, S, R] + extends RWSTMonad[F, E, L, S] with MonadError[ReaderWriterStateT[F, E, L, S, ?], R] { + + implicit def F: MonadError[F, R] + + def raiseError[A](r: R): ReaderWriterStateT[F, E, L, S, A] = ReaderWriterStateT.liftF(F.raiseError(r)) + + def handleErrorWith[A](fa: ReaderWriterStateT[F, E, L, S, A])(f: R => ReaderWriterStateT[F, E, L, S, A]): ReaderWriterStateT[F, E, L, S, A] = + ReaderWriterStateT { (e, s) => + F.handleErrorWith(fa.run(e, s))(r => f(r).run(e, s)) + } +} + +private trait IRWSTSemigroupK1[F[_], E, L, SA, SB] extends SemigroupK[IndexedReaderWriterStateT[F, E, L, SA, SB, ?]] { implicit def F: Monad[F] implicit def G: SemigroupK[F] @@ -574,19 +591,12 @@ private[data] sealed abstract class IRWSTSemigroupK[F[_], E, L, SA, SB] extends } } -private[data] sealed abstract class RWSTAlternative[F[_], E, L, S] - extends IRWSTFunctor[F, E, L, S, S] with Alternative[ReaderWriterStateT[F, E, L, S, ?]] { +private trait RWSTAlternative1[F[_], E, L, S] extends IRWSTSemigroupK1[F, E, L, S, S] with Alternative[ReaderWriterStateT[F, E, L, S, ?]] { implicit def F: Monad[F] def G: Alternative[F] implicit def L: Monoid[L] - def combineK[A](x: ReaderWriterStateT[F, E, L, S, A], - y: ReaderWriterStateT[F, E, L, S, A]): ReaderWriterStateT[F, E, L, S, A] = - ReaderWriterStateT { (e, sa) => - G.combineK(x.run(e, sa), y.run(e, sa)) - } - def empty[A]: ReaderWriterStateT[F, E, L, S, A] = ReaderWriterStateT.liftF(G.empty[A]) def pure[A](a: A): ReaderWriterStateT[F, E, L, S, A] = ReaderWriterStateT.pure[F, E, L, S, A](a) @@ -595,16 +605,3 @@ private[data] sealed abstract class RWSTAlternative[F[_], E, L, S] ff.flatMap(f => fa.map(f)(F))(F, L) } - -private[data] sealed abstract class RWSTMonadError[F[_], E, L, S, R] - extends RWSTMonad[F, E, L, S] with MonadError[ReaderWriterStateT[F, E, L, S, ?], R] { - - implicit def F: MonadError[F, R] - - def raiseError[A](r: R): ReaderWriterStateT[F, E, L, S, A] = ReaderWriterStateT.liftF(F.raiseError(r)) - - def handleErrorWith[A](fa: ReaderWriterStateT[F, E, L, S, A])(f: R => ReaderWriterStateT[F, E, L, S, A]): ReaderWriterStateT[F, E, L, S, A] = - ReaderWriterStateT { (e, s) => - F.handleErrorWith(fa.run(e, s))(r => f(r).run(e, s)) - } -} diff --git a/core/src/main/scala/cats/data/IorT.scala b/core/src/main/scala/cats/data/IorT.scala index d9f4292ea1..0a73124619 100644 --- a/core/src/main/scala/cats/data/IorT.scala +++ b/core/src/main/scala/cats/data/IorT.scala @@ -410,7 +410,7 @@ private[data] abstract class IorTInstances extends IorTInstances1 { } implicit def catsDataTraverseForIorT[F[_], A](implicit F: Traverse[F]): Traverse[IorT[F, A, ?]] = - new IorTTraverse[F, A] { val F0: Traverse[F] = F } + new IorTTraverse[F, A] with IorTFunctor[F, A] { val F0: Traverse[F] = F } implicit def catsDataMonoidForIorT[F[_], A, B](implicit F: Monoid[F[Ior[A, B]]]): Monoid[IorT[F, A, B]] = new IorTMonoid[F, A, B] { val F0: Monoid[F[Ior[A, B]]] = F } diff --git a/core/src/main/scala/cats/data/Kleisli.scala b/core/src/main/scala/cats/data/Kleisli.scala index fcdb3e8d3e..c0f9f264c5 100644 --- a/core/src/main/scala/cats/data/Kleisli.scala +++ b/core/src/main/scala/cats/data/Kleisli.scala @@ -266,7 +266,7 @@ private[data] sealed abstract class KleisliInstances6 extends KleisliInstances7 private[data] sealed abstract class KleisliInstances7 extends KleisliInstances8 { implicit def catsDataDistributiveForKleisli[F[_], R](implicit F0: Distributive[F]): Distributive[Kleisli[F, R, ?]] = - new KleisliDistributive[F, R] { implicit def F: Distributive[F] = F0 } + new KleisliDistributive[F, R] with KleisliFunctor[F, R] { implicit def F: Distributive[F] = F0 } } private[data] sealed abstract class KleisliInstances8 { diff --git a/core/src/main/scala/cats/data/Nested.scala b/core/src/main/scala/cats/data/Nested.scala index 2dd937e288..e37ae2f667 100644 --- a/core/src/main/scala/cats/data/Nested.scala +++ b/core/src/main/scala/cats/data/Nested.scala @@ -45,7 +45,7 @@ private[data] sealed abstract class NestedInstances extends NestedInstances0 { } implicit def catsDataContravariantMonoidalForApplicativeForNested[F[_]: Applicative, G[_]: ContravariantMonoidal]: ContravariantMonoidal[Nested[F, G, ?]] = - new NestedContravariantMonoidal[F, G] { + new NestedContravariantMonoidal[F, G] with NestedContravariant[F, G] { val FG: ContravariantMonoidal[λ[α => F[G[α]]]] = Applicative[F].composeContravariantMonoidal[G] } } @@ -284,7 +284,7 @@ private[data] trait NestedNonEmptyTraverse[F[_], G[_]] extends NonEmptyTraverse[ private[data] trait NestedContravariant[F[_], G[_]] extends Contravariant[Nested[F, G, ?]] { def FG: Contravariant[λ[α => F[G[α]]]] - def contramap[A, B](fga: Nested[F, G, A])(f: B => A): Nested[F, G, B] = + override def contramap[A, B](fga: Nested[F, G, A])(f: B => A): Nested[F, G, B] = Nested(FG.contramap(fga.value)(f)) } diff --git a/core/src/main/scala/cats/data/OptionT.scala b/core/src/main/scala/cats/data/OptionT.scala index 6b587d0dce..74fb633fb5 100644 --- a/core/src/main/scala/cats/data/OptionT.scala +++ b/core/src/main/scala/cats/data/OptionT.scala @@ -258,7 +258,7 @@ private[data] sealed abstract class OptionTInstances1 extends OptionTInstances2 private[data] sealed abstract class OptionTInstances2 extends OptionTInstances3 { implicit def catsDataTraverseForOptionT[F[_]](implicit F0: Traverse[F]): Traverse[OptionT[F, ?]] = - new OptionTTraverse[F] { implicit val F = F0 } + new OptionTTraverse[F] with OptionTFunctor[F] { implicit val F = F0 } } private[data] sealed abstract class OptionTInstances3 { diff --git a/core/src/main/scala/cats/data/Tuple2K.scala b/core/src/main/scala/cats/data/Tuple2K.scala index efa0242c7a..cfce28ccca 100644 --- a/core/src/main/scala/cats/data/Tuple2K.scala +++ b/core/src/main/scala/cats/data/Tuple2K.scala @@ -30,17 +30,18 @@ private[data] sealed abstract class Tuple2KInstances extends Tuple2KInstances0 { def G: Show[G[A]] = GF } implicit def catsDataContravariantMonoidalForTuple2k[F[_], G[_]](implicit FD: ContravariantMonoidal[F], GD: ContravariantMonoidal[G]): ContravariantMonoidal[λ[α => Tuple2K[F, G, α]]] = - new Tuple2KContravariantMonoidal[F, G] { + new Tuple2KContravariantMonoidal[F, G] with Tuple2KContravariant[F, G] { def F: ContravariantMonoidal[F] = FD def G: ContravariantMonoidal[G] = GD } } private[data] sealed abstract class Tuple2KInstances0 extends Tuple2KInstances1 { - implicit def catsDataTraverseForTuple2K[F[_], G[_]](implicit FF: Traverse[F], GF: Traverse[G]): Traverse[λ[α => Tuple2K[F, G, α]]] = new Tuple2KTraverse[F, G] { - def F: Traverse[F] = FF - def G: Traverse[G] = GF - } + implicit def catsDataTraverseForTuple2K[F[_], G[_]](implicit FF: Traverse[F], GF: Traverse[G]): Traverse[λ[α => Tuple2K[F, G, α]]] = + new Tuple2KTraverse[F, G] with Tuple2KFunctor[F, G] { + def F: Traverse[F] = FF + def G: Traverse[G] = GF + } implicit def catsDataContravariantForTuple2K[F[_], G[_]](implicit FC: Contravariant[F], GC: Contravariant[G]): Contravariant[λ[α => Tuple2K[F, G, α]]] = new Tuple2KContravariant[F, G] { def F: Contravariant[F] = FC def G: Contravariant[G] = GC @@ -109,10 +110,11 @@ private[data] sealed abstract class Tuple2KInstances6 extends Tuple2KInstances7 } private[data] sealed abstract class Tuple2KInstances7 extends Tuple2KInstances8 { - implicit def catsDataDistributiveForTuple2K[F[_], G[_]](implicit FF: Distributive[F], GG: Distributive[G]): Distributive[λ[α => Tuple2K[F, G, α]]] = new Tuple2KDistributive[F, G] { - def F: Distributive[F] = FF - def G: Distributive[G] = GG - } + implicit def catsDataDistributiveForTuple2K[F[_], G[_]](implicit FF: Distributive[F], GG: Distributive[G]): Distributive[λ[α => Tuple2K[F, G, α]]] = + new Tuple2KDistributive[F, G] with Tuple2KFunctor[F, G] { + def F: Distributive[F] = FF + def G: Distributive[G] = GG + } } private[data] sealed abstract class Tuple2KInstances8 { @@ -132,14 +134,19 @@ private[data] sealed trait Tuple2KFunctor[F[_], G[_]] extends Functor[λ[α => T private[data] sealed trait Tuple2KDistributive[F[_], G[_]] extends Distributive[λ[α => Tuple2K[F, G, α]]] { def F: Distributive[F] def G: Distributive[G] - override def distribute[H[_]: Functor, A, B](ha: H[A])(f: A => Tuple2K[F, G, B]): Tuple2K[F, G, H[B]] = Tuple2K(F.distribute(ha){a => f(a).first}, G.distribute(ha){a => f(a).second}) - override def map[A, B](fa: Tuple2K[F, G, A])(f: A => B): Tuple2K[F, G, B] = Tuple2K(F.map(fa.first)(f), G.map(fa.second)(f)) + + override def distribute[H[_]: Functor, A, B](ha: H[A])(f: A => Tuple2K[F, G, B]): Tuple2K[F, G, H[B]] = + Tuple2K(F.distribute(ha){a => f(a).first}, G.distribute(ha){a => f(a).second}) + + override def map[A, B](fa: Tuple2K[F, G, A])(f: A => B): Tuple2K[F, G, B] = + Tuple2K(F.map(fa.first)(f), G.map(fa.second)(f)) } private[data] sealed trait Tuple2KContravariant[F[_], G[_]] extends Contravariant[λ[α => Tuple2K[F, G, α]]] { def F: Contravariant[F] def G: Contravariant[G] - def contramap[A, B](fa: Tuple2K[F, G, A])(f: B => A): Tuple2K[F, G, B] = Tuple2K(F.contramap(fa.first)(f), G.contramap(fa.second)(f)) + override def contramap[A, B](fa: Tuple2K[F, G, A])(f: B => A): Tuple2K[F, G, B] = + Tuple2K(F.contramap(fa.first)(f), G.contramap(fa.second)(f)) } private[data] sealed trait Tuple2KContravariantMonoidal[F[_], G[_]] extends ContravariantMonoidal[λ[α => Tuple2K[F, G, α]]] { diff --git a/core/src/main/scala/cats/data/WriterT.scala b/core/src/main/scala/cats/data/WriterT.scala index 1f09eb8723..221c572808 100644 --- a/core/src/main/scala/cats/data/WriterT.scala +++ b/core/src/main/scala/cats/data/WriterT.scala @@ -297,7 +297,7 @@ private[data] sealed trait WriterTFunctor[F[_], L] extends Functor[WriterT[F, L, private[data] sealed trait WriterTContravariant[F[_], L] extends Contravariant[WriterT[F, L, ?]] { implicit def F0: Contravariant[F] - def contramap[A, B](fa: WriterT[F, L, A])(f: B => A): WriterT[F, L, B] = + override def contramap[A, B](fa: WriterT[F, L, A])(f: B => A): WriterT[F, L, B] = fa.contramap(f) } @@ -420,7 +420,7 @@ private[data] sealed trait WriterTContravariantMonoidal[F[_], L] extends Contrav override def unit: WriterT[F, L, Unit] = WriterT(F0.trivial[(L, Unit)]) override def contramap[A, B](fa: WriterT[F, L, A])(f: B => A): WriterT[F, L, B] = - WriterT(F0.contramap(fa.run)((d: (L, B)) => (d._1, f(d._2)))) + fa.contramap(f) override def product[A, B](fa: WriterT[F, L, A], fb: WriterT[F, L, B]): WriterT[F, L, (A, B)] = WriterT( diff --git a/tests/src/test/scala/cats/tests/NestedSuite.scala b/tests/src/test/scala/cats/tests/NestedSuite.scala index 3b52227594..fff27f9904 100644 --- a/tests/src/test/scala/cats/tests/NestedSuite.scala +++ b/tests/src/test/scala/cats/tests/NestedSuite.scala @@ -89,8 +89,10 @@ class NestedSuite extends CatsSuite { { // CommutativeApply composition + implicit val optionApply = Apply[Option] + implicit val validatedApply = Apply[Validated[Int, ?]] checkAll("Nested[Option, Validated[Int, ?], ?]", CommutativeApplyTests[Nested[Option, Validated[Int, ?], ?]].commutativeApply[Int, Int, Int]) - checkAll("CommutativeApply[Nested[List, ListWrapper, ?]]", SerializableTests.serializable(CommutativeApply[Nested[Option, Validated[Int, ?], ?]])) + checkAll("CommutativeApply[Nested[Option, Validated[Int, ?], ?], ?]]", SerializableTests.serializable(CommutativeApply[Nested[Option, Validated[Int, ?], ?]])) } { diff --git a/tests/src/test/scala/cats/tests/Tuple2KSuite.scala b/tests/src/test/scala/cats/tests/Tuple2KSuite.scala index a2fc73a6a7..91802a4ff8 100644 --- a/tests/src/test/scala/cats/tests/Tuple2KSuite.scala +++ b/tests/src/test/scala/cats/tests/Tuple2KSuite.scala @@ -47,6 +47,8 @@ class Tuple2KSuite extends CatsSuite { } { + implicit val optionApply = Apply[Option] + implicit val validatedApply = Apply[Validated[Int, ?]] checkAll("Tuple2K[Option, Validated[Int, ?], ?]", CommutativeApplyTests[Tuple2K[Option, Validated[Int, ?], ?]].commutativeApply[Int, Int, Int]) checkAll("Apply[Tuple2K[Option, Validated[Int, ?], ?]]", SerializableTests.serializable(CommutativeApply[Tuple2K[Option, Validated[Int, ?], ?]])) } diff --git a/tests/src/test/scala/cats/tests/WriterTSuite.scala b/tests/src/test/scala/cats/tests/WriterTSuite.scala index 19b98868ab..274afd9798 100644 --- a/tests/src/test/scala/cats/tests/WriterTSuite.scala +++ b/tests/src/test/scala/cats/tests/WriterTSuite.scala @@ -404,8 +404,6 @@ class WriterTSuite extends CatsSuite { { // F has a Comonad and L has a Monoid - implicit val L: Monoid[ListWrapper[Int]] = ListWrapper.monoid[Int] - Comonad[(String, ?)] Comonad[WriterT[(String, ?), ListWrapper[Int], ?]] checkAll("WriterT[(String, ?), ListWrapper[Int], ?]", ComonadTests[WriterT[(String, ?), ListWrapper[Int], ?]].comonad[Int, Int, Int])