From 63d186bc141cfb285f5238b773b807c235238ed3 Mon Sep 17 00:00:00 2001 From: aaron levin Date: Sun, 24 Apr 2016 19:00:48 +0200 Subject: [PATCH 1/5] Add SemigroupK instance for Xor + tests Xor's SemigroupK instance follows that of XorT. --- core/src/main/scala/cats/data/Xor.scala | 11 +++++++++++ tests/src/test/scala/cats/tests/XorTests.scala | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/core/src/main/scala/cats/data/Xor.scala b/core/src/main/scala/cats/data/Xor.scala index 470fa448d4..c43a0d9cdf 100644 --- a/core/src/main/scala/cats/data/Xor.scala +++ b/core/src/main/scala/cats/data/Xor.scala @@ -168,6 +168,17 @@ private[data] sealed abstract class XorInstances extends XorInstances1 { def combine(x: A Xor B, y: A Xor B): A Xor B = x combine y } + implicit def xorSemigroupK[L](implicit ev: Semigroup[L]): SemigroupK[Xor[L,?]] = + new SemigroupK[Xor[L,?]] { + def combineK[A](x: Xor[L,A], y: Xor[L,A]): Xor[L,A] = x match { + case Xor.Left(lx) => y match { + case Xor.Left(ly) => Xor.Left(ev.combine(lx,ly)) + case r @ Xor.Right(_) => r + } + case _ => x + } + } + implicit def xorBifunctor: Bitraverse[Xor] = new Bitraverse[Xor] { def bitraverse[G[_], A, B, C, D](fab: Xor[A, B])(f: A => G[C], g: B => G[D])(implicit G: Applicative[G]): G[Xor[C, D]] = diff --git a/tests/src/test/scala/cats/tests/XorTests.scala b/tests/src/test/scala/cats/tests/XorTests.scala index 58d0a90de2..39ca90a79d 100644 --- a/tests/src/test/scala/cats/tests/XorTests.scala +++ b/tests/src/test/scala/cats/tests/XorTests.scala @@ -3,6 +3,7 @@ package tests import cats.data.{NonEmptyList, Xor, XorT} import cats.data.Xor._ +import cats.laws.discipline.{SemigroupKTests} import cats.laws.discipline.arbitrary._ import cats.laws.discipline.{BitraverseTests, TraverseTests, MonadErrorTests, SerializableTests, CartesianTests} import cats.laws.discipline.eq.tuple3Eq @@ -46,6 +47,12 @@ class XorTests extends CatsSuite { checkAll("Eq[ListWrapper[String] Xor ListWrapper[Int]]", SerializableTests.serializable(Eq[ListWrapper[String] Xor ListWrapper[Int]])) } + { + implicit val L = ListWrapper.semigroup[String] + checkAll("Xor[ListWrapper[String], ?]", SemigroupKTests[Xor[ListWrapper[String], ?]].semigroupK[Int]) + checkAll("SemigroupK[Xor[ListWrapper[String], ?]]", SerializableTests.serializable(SemigroupK[Xor[ListWrapper[String], ?]])) + } + implicit val arbitraryXor: Arbitrary[Xor[Int, String]] = Arbitrary { for { left <- arbitrary[Boolean] From e66d23894ed022716ea8db2300541c44feb66a4b Mon Sep 17 00:00:00 2001 From: aaron levin Date: Tue, 26 Apr 2016 09:25:00 +0200 Subject: [PATCH 2/5] XorT: remove MonoidK inst. + new SemigroupK inst. We remove the MonoidK instance as it depended on an not-well defined SemigroupK instance. The new SemigroupK instance places no constraints on `L`, and has a "first Right or Last Left" behaviour for combine. This has a closer semantic meaning to Xor(T)'s "success or failure" interpretation. It didn't make sense previously that the empty value for XortT's MonoidK instance represented failure (`XortT.left(F.pure(L.empty))(F)`). Why should every empty value signal failure? --- core/src/main/scala/cats/data/XorT.scala | 34 ++++--------------- .../src/test/scala/cats/tests/XorTTests.scala | 2 -- 2 files changed, 7 insertions(+), 29 deletions(-) diff --git a/core/src/main/scala/cats/data/XorT.scala b/core/src/main/scala/cats/data/XorT.scala index 4e0331f24a..97b4cae357 100644 --- a/core/src/main/scala/cats/data/XorT.scala +++ b/core/src/main/scala/cats/data/XorT.scala @@ -215,16 +215,6 @@ private[data] abstract class XorTInstances1 extends XorTInstances2 { } */ - /* TODO delete this when MonadCombine instance is re-enabled */ - implicit def xorTMonoidK[F[_], L](implicit F: Monad[F], L: Monoid[L]): MonoidK[XorT[F, L, ?]] = { - implicit val F0 = F - implicit val L0 = L - new MonoidK[XorT[F, L, ?]] with XorTSemigroupK[F, L] { - implicit val F = F0; implicit val L = L0 - def empty[A]: XorT[F, L, A] = XorT.left(F.pure(L.empty))(F) - } - } - implicit def xorTFoldable[F[_], L](implicit F: Foldable[F]): Foldable[XorT[F, L, ?]] = new XorTFoldable[F, L] { val F0: Foldable[F] = F @@ -242,10 +232,13 @@ private[data] abstract class XorTInstances2 extends XorTInstances3 { new XorTMonadError[F, L] { implicit val F = F0 } } - implicit def xorTSemigroupK[F[_], L](implicit F: Monad[F], L: Semigroup[L]): SemigroupK[XorT[F, L, ?]] = { - implicit val F0 = F - implicit val L0 = L - new XorTSemigroupK[F, L] { implicit val F = F0; implicit val L = L0 } + implicit def xorTSemigroupK[F[_], L](implicit F: Monad[F]): SemigroupK[XorT[F, L, ?]] = + new SemigroupK[XorT[F,L,?]] { + def combineK[A](x: XorT[F,L,A], y: XorT[F, L, A]): XorT[F, L, A] = + XorT(F.flatMap(x.value) { + case l @ Xor.Left(_) => y.value + case r @ Xor.Right(_) => F.pure(r) + }) } implicit def xorTEq[F[_], L, R](implicit F: Eq[F[L Xor R]]): Eq[XorT[F, L, R]] = @@ -288,19 +281,6 @@ private[data] trait XorTMonadError[F[_], L] extends MonadError[XorT[F, L, ?], L] fla.recoverWith(pf) } -private[data] trait XorTSemigroupK[F[_], L] extends SemigroupK[XorT[F, L, ?]] { - implicit val F: Monad[F] - implicit val L: Semigroup[L] - def combineK[A](x: XorT[F, L, A], y: XorT[F, L, A]): XorT[F, L, A] = - XorT(F.flatMap(x.value) { - case Xor.Left(l1) => F.map(y.value) { - case Xor.Left(l2) => Xor.Left(L.combine(l1, l2)) - case r @ Xor.Right(_) => r - } - case r @ Xor.Right(_) => F.pure[L Xor A](r) - }) -} - private[data] trait XorTMonadFilter[F[_], L] extends MonadFilter[XorT[F, L, ?]] with XorTMonadError[F, L] { implicit val F: Monad[F] implicit val L: Monoid[L] diff --git a/tests/src/test/scala/cats/tests/XorTTests.scala b/tests/src/test/scala/cats/tests/XorTTests.scala index 7ea4e9b343..44b1cb2bf5 100644 --- a/tests/src/test/scala/cats/tests/XorTTests.scala +++ b/tests/src/test/scala/cats/tests/XorTTests.scala @@ -14,8 +14,6 @@ class XorTTests extends CatsSuite { implicit val iso = CartesianTests.Isomorphisms.invariant[XorT[List, String, ?]] checkAll("XorT[List, String, Int]", MonadErrorTests[XorT[List, String, ?], String].monadError[Int, Int, Int]) checkAll("MonadError[XorT[List, ?, ?]]", SerializableTests.serializable(MonadError[XorT[List, String, ?], String])) - checkAll("XorT[List, String, Int]", MonoidKTests[XorT[List, String, ?]].monoidK[Int]) - checkAll("MonoidK[XorT[List, String, ?]]", SerializableTests.serializable(MonoidK[XorT[List, String, ?]])) checkAll("XorT[List, ?, ?]", BifunctorTests[XorT[List, ?, ?]].bifunctor[Int, Int, Int, String, String, String]) checkAll("Bifunctor[XorT[List, ?, ?]]", SerializableTests.serializable(Bifunctor[XorT[List, ?, ?]])) checkAll("XorT[List, Int, ?]", TraverseTests[XorT[List, Int, ?]].traverse[Int, Int, Int, Int, Option, Option]) From 0bae0caa9472248eef18689b25ee89daf593058d Mon Sep 17 00:00:00 2001 From: aaron levin Date: Tue, 26 Apr 2016 09:28:40 +0200 Subject: [PATCH 3/5] Xor: SemigroupK w/ first-right,last-left semantics Update Xor's SemigroupK instance to be aligned with XorT's, with first-right, last-left semnatics. --- core/src/main/scala/cats/data/Xor.scala | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/core/src/main/scala/cats/data/Xor.scala b/core/src/main/scala/cats/data/Xor.scala index c43a0d9cdf..16c64ce214 100644 --- a/core/src/main/scala/cats/data/Xor.scala +++ b/core/src/main/scala/cats/data/Xor.scala @@ -168,14 +168,11 @@ private[data] sealed abstract class XorInstances extends XorInstances1 { def combine(x: A Xor B, y: A Xor B): A Xor B = x combine y } - implicit def xorSemigroupK[L](implicit ev: Semigroup[L]): SemigroupK[Xor[L,?]] = + implicit def xorSemigroupK[L]: SemigroupK[Xor[L,?]] = new SemigroupK[Xor[L,?]] { def combineK[A](x: Xor[L,A], y: Xor[L,A]): Xor[L,A] = x match { - case Xor.Left(lx) => y match { - case Xor.Left(ly) => Xor.Left(ev.combine(lx,ly)) - case r @ Xor.Right(_) => r - } - case _ => x + case Xor.Left(_) => y + case Xor.Right(_) => x } } From 86363437cc150c02fbb2289f233c3f79c46b5a2f Mon Sep 17 00:00:00 2001 From: aaron levin Date: Tue, 26 Apr 2016 13:24:05 +0200 Subject: [PATCH 4/5] Remove implct Semigroup from Xor{T} SemigroupK tests --- tests/src/test/scala/cats/tests/XorTTests.scala | 1 - tests/src/test/scala/cats/tests/XorTests.scala | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/src/test/scala/cats/tests/XorTTests.scala b/tests/src/test/scala/cats/tests/XorTTests.scala index 44b1cb2bf5..f8a736e7a6 100644 --- a/tests/src/test/scala/cats/tests/XorTTests.scala +++ b/tests/src/test/scala/cats/tests/XorTTests.scala @@ -46,7 +46,6 @@ class XorTTests extends CatsSuite { } { - implicit val L = ListWrapper.semigroup[String] checkAll("XorT[Option, ListWrapper[String], ?]", SemigroupKTests[XorT[Option, ListWrapper[String], ?]].semigroupK[Int]) checkAll("SemigroupK[XorT[Option, ListWrapper[String], ?]]", SerializableTests.serializable(SemigroupK[XorT[Option, ListWrapper[String], ?]])) } diff --git a/tests/src/test/scala/cats/tests/XorTests.scala b/tests/src/test/scala/cats/tests/XorTests.scala index 39ca90a79d..12899f257a 100644 --- a/tests/src/test/scala/cats/tests/XorTests.scala +++ b/tests/src/test/scala/cats/tests/XorTests.scala @@ -48,7 +48,6 @@ class XorTests extends CatsSuite { } { - implicit val L = ListWrapper.semigroup[String] checkAll("Xor[ListWrapper[String], ?]", SemigroupKTests[Xor[ListWrapper[String], ?]].semigroupK[Int]) checkAll("SemigroupK[Xor[ListWrapper[String], ?]]", SerializableTests.serializable(SemigroupK[Xor[ListWrapper[String], ?]])) } From db83772ef4a5df9e6a96893192ffa95085441fbb Mon Sep 17 00:00:00 2001 From: aaron levin Date: Tue, 26 Apr 2016 13:27:32 +0200 Subject: [PATCH 5/5] Move Xor{T} SemigroupK tests into broader scope --- tests/src/test/scala/cats/tests/XorTTests.scala | 7 ++----- tests/src/test/scala/cats/tests/XorTests.scala | 8 +++----- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/src/test/scala/cats/tests/XorTTests.scala b/tests/src/test/scala/cats/tests/XorTTests.scala index f8a736e7a6..4e5ed83151 100644 --- a/tests/src/test/scala/cats/tests/XorTTests.scala +++ b/tests/src/test/scala/cats/tests/XorTTests.scala @@ -20,6 +20,8 @@ class XorTTests extends CatsSuite { checkAll("Traverse[XorT[List, Int, ?]]", SerializableTests.serializable(Traverse[XorT[List, Int, ?]])) checkAll("XorT[List, String, Int]", OrderLaws[XorT[List, String, Int]].order) checkAll("Order[XorT[List, String, Int]]", SerializableTests.serializable(Order[XorT[List, String, Int]])) + checkAll("XorT[Option, ListWrapper[String], ?]", SemigroupKTests[XorT[Option, ListWrapper[String], ?]].semigroupK[Int]) + checkAll("SemigroupK[XorT[Option, ListWrapper[String], ?]]", SerializableTests.serializable(SemigroupK[XorT[Option, ListWrapper[String], ?]])) { implicit val F = ListWrapper.foldable @@ -45,11 +47,6 @@ class XorTTests extends CatsSuite { checkAll("Eq[XorT[ListWrapper, String, Int]]", SerializableTests.serializable(Eq[XorT[ListWrapper, String, Int]])) } - { - checkAll("XorT[Option, ListWrapper[String], ?]", SemigroupKTests[XorT[Option, ListWrapper[String], ?]].semigroupK[Int]) - checkAll("SemigroupK[XorT[Option, ListWrapper[String], ?]]", SerializableTests.serializable(SemigroupK[XorT[Option, ListWrapper[String], ?]])) - } - // make sure that the Monad and Traverse instances don't result in ambiguous // Functor instances Functor[XorT[List, Int, ?]] diff --git a/tests/src/test/scala/cats/tests/XorTests.scala b/tests/src/test/scala/cats/tests/XorTests.scala index 12899f257a..d05830233c 100644 --- a/tests/src/test/scala/cats/tests/XorTests.scala +++ b/tests/src/test/scala/cats/tests/XorTests.scala @@ -33,6 +33,9 @@ class XorTests extends CatsSuite { checkAll("Xor[Int, String]", OrderLaws[String Xor Int].order) + checkAll("Xor[ListWrapper[String], ?]", SemigroupKTests[Xor[ListWrapper[String], ?]].semigroupK[Int]) + checkAll("SemigroupK[Xor[ListWrapper[String], ?]]", SerializableTests.serializable(SemigroupK[Xor[ListWrapper[String], ?]])) + { implicit val S = ListWrapper.partialOrder[String] implicit val I = ListWrapper.partialOrder[Int] @@ -47,11 +50,6 @@ class XorTests extends CatsSuite { checkAll("Eq[ListWrapper[String] Xor ListWrapper[Int]]", SerializableTests.serializable(Eq[ListWrapper[String] Xor ListWrapper[Int]])) } - { - checkAll("Xor[ListWrapper[String], ?]", SemigroupKTests[Xor[ListWrapper[String], ?]].semigroupK[Int]) - checkAll("SemigroupK[Xor[ListWrapper[String], ?]]", SerializableTests.serializable(SemigroupK[Xor[ListWrapper[String], ?]])) - } - implicit val arbitraryXor: Arbitrary[Xor[Int, String]] = Arbitrary { for { left <- arbitrary[Boolean]