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

MonadError instance for Ior #1548

Merged
merged 13 commits into from
Apr 9, 2017
59 changes: 40 additions & 19 deletions core/src/main/scala/cats/data/Ior.scala
Original file line number Diff line number Diff line change
Expand Up @@ -156,26 +156,42 @@ private[data] sealed abstract class IorInstances extends IorInstances0 {
def combine(x: Ior[A, B], y: Ior[A, B]) = x.combine(y)
}

implicit def catsDataMonadForIor[A: Semigroup]: Monad[A Ior ?] = new Monad[A Ior ?] {
def pure[B](b: B): A Ior B = Ior.right(b)
def flatMap[B, C](fa: A Ior B)(f: B => A Ior C): A Ior C = fa.flatMap(f)
def tailRecM[B, C](b: B)(fn: B => Ior[A, Either[B, C]]): A Ior C = {
@tailrec
def loop(v: Ior[A, Either[B, C]]): A Ior C = v match {
case Ior.Left(a) => Ior.left(a)
case Ior.Right(Right(c)) => Ior.right(c)
case Ior.Both(a, Right(c)) => Ior.both(a, c)
case Ior.Right(Left(b)) => loop(fn(b))
case Ior.Both(a, Left(b)) =>
fn(b) match {
case Ior.Left(aa) => Ior.left(Semigroup[A].combine(a, aa))
case Ior.Both(aa, x) => loop(Ior.both(Semigroup[A].combine(a, aa), x))
case Ior.Right(x) => loop(Ior.both(a, x))
}
implicit def catsDataMonadErrorForIor[A: Semigroup]: MonadError[Ior[A, ?], A] =
new MonadError[Ior[A, ?], A] {

def raiseError[B](e: A): Ior[A, B] = Ior.left(e)

def handleErrorWith[B](fa: Ior[A, B])(f: (A) => Ior[A, B]): Ior[A, B] =
fa match {
case Ior.Left(e) => f(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is not tested

case r @ Ior.Right(_) => r
case Ior.Both(e, _) => f(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is not tested

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kailuowang Thanks for pointing them out. I will get it fixed this week.

}

def flatMap[B, C](fa: Ior[A, B])(f: B => Ior[A, C]): Ior[A, C] = fa.flatMap(f)

def tailRecM[B, C](b: B)(fn: B => Ior[A, Either[B, C]]): A Ior C = {
@tailrec
def loop(v: Ior[A, Either[B, C]]): A Ior C = v match {
case Ior.Left(a) => Ior.left(a)
case Ior.Right(Right(c)) => Ior.right(c)
case Ior.Both(a, Right(c)) => Ior.both(a, c)
case Ior.Right(Left(b)) => loop(fn(b))
case Ior.Both(a, Left(b)) =>
fn(b) match {
case Ior.Left(aa) => Ior.left(Semigroup[A].combine(a, aa))
case Ior.Both(aa, x) => loop(Ior.both(Semigroup[A].combine(a, aa), x))
case Ior.Right(x) => loop(Ior.both(a, x))
}
}
loop(fn(b))
}
loop(fn(b))

override def pure[B](x: B): Ior[A, B] = Ior.right(x)

override def map[B, C](fa: A Ior B)(f: B => C): A Ior C =
fa.map(f)
}
}

implicit def catsDataBifunctorForIor: Bifunctor[Ior] =
new Bifunctor[Ior] {
Expand All @@ -185,13 +201,18 @@ private[data] sealed abstract class IorInstances extends IorInstances0 {

private[data] sealed abstract class IorInstances0 {

implicit def catsDataTraverseFunctorForIor[A]: Traverse[A Ior ?] with Functor[A Ior ?] = new Traverse[A Ior ?] with Functor[A Ior ?] {
implicit def catsDataTraverseFunctorForIor[A]: Traverse[A Ior ?] = new Traverse[A Ior ?] {
def traverse[F[_]: Applicative, B, C](fa: A Ior B)(f: B => F[C]): F[A Ior C] =
fa.traverse(f)
def foldLeft[B, C](fa: A Ior B, b: C)(f: (C, B) => C): C =
fa.foldLeft(b)(f)
def foldRight[B, C](fa: A Ior B, lc: Eval[C])(f: (B, Eval[C]) => Eval[C]): Eval[C] =
fa.foldRight(lc)(f)

override def forall[B](fa: Ior[A, B])(p: (B) => Boolean): Boolean = fa.forall(p)

override def exists[B](fa: Ior[A, B])(p: (B) => Boolean): Boolean = fa.exists(p)

override def map[B, C](fa: A Ior B)(f: B => C): A Ior C =
fa.map(f)
}
Expand Down
74 changes: 41 additions & 33 deletions core/src/main/scala/cats/data/Validated.scala
Original file line number Diff line number Diff line change
Expand Up @@ -299,17 +299,8 @@ private[data] sealed abstract class ValidatedInstances extends ValidatedInstance
fab.leftMap(f)
}

// scalastyle:off method.length
implicit def catsDataInstancesForValidated[E](implicit E: Semigroup[E]): Traverse[Validated[E, ?]] with ApplicativeError[Validated[E, ?], E] =
new Traverse[Validated[E, ?]] with ApplicativeError[Validated[E, ?], E] {
def traverse[F[_]: Applicative, A, B](fa: Validated[E, A])(f: A => F[B]): F[Validated[E, B]] =
fa.traverse(f)

def foldLeft[A, B](fa: Validated[E, A], b: B)(f: (B, A) => B): B =
fa.foldLeft(b)(f)

def foldRight[A, B](fa: Validated[E, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
fa.foldRight(lb)(f)
implicit def catsDataApplicativeErrorForValidated[E](implicit E: Semigroup[E]): ApplicativeError[Validated[E, ?], E] =
new ApplicativeError[Validated[E, ?], E] {

def pure[A](a: A): Validated[E, A] =
Validated.valid(a)
Expand All @@ -329,6 +320,44 @@ private[data] sealed abstract class ValidatedInstances extends ValidatedInstance
case v @ Validated.Valid(_) => v
}
def raiseError[A](e: E): Validated[E, A] = Validated.Invalid(e)
}
}

private[data] sealed abstract class ValidatedInstances1 extends ValidatedInstances2 {

implicit def catsDataSemigroupForValidated[A, B](implicit A: Semigroup[A], B: Semigroup[B]): Semigroup[Validated[A, B]] =
new Semigroup[Validated[A, B]] {
def combine(x: Validated[A, B], y: Validated[A, B]): Validated[A, B] = x combine y
}

implicit def catsDataPartialOrderForValidated[A: PartialOrder, B: PartialOrder]: PartialOrder[Validated[A, B]] =
new PartialOrder[Validated[A, B]] {
def partialCompare(x: Validated[A, B], y: Validated[A, B]): Double = x partialCompare y
override def eqv(x: Validated[A, B], y: Validated[A, B]): Boolean = x === y
}
}

private[data] sealed abstract class ValidatedInstances2 {
implicit def catsDataEqForValidated[A: Eq, B: Eq]: Eq[Validated[A, B]] =
new Eq[Validated[A, B]] {
def eqv(x: Validated[A, B], y: Validated[A, B]): Boolean = x === y
}

// scalastyle:off method.length
implicit def catsDataTraverseFunctorForValidated[E]: Traverse[Validated[E, ?]] =
new Traverse[Validated[E, ?]] {

override def traverse[G[_] : Applicative, A, B](fa: Validated[E, A])(f: (A) => G[B]): G[Validated[E, B]] =
fa.traverse(f)

override def foldLeft[A, B](fa: Validated[E, A], b: B)(f: (B, A) => B): B =
fa.foldLeft(b)(f)

override def foldRight[A, B](fa: Validated[E, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] =
fa.foldRight(lb)(f)

override def map[A, B](fa: Validated[E, A])(f: (A) => B): Validated[E, B] =
fa.map(f)

override def reduceLeftToOption[A, B](fa: Validated[E, A])(f: A => B)(g: (B, A) => B): Option[B] =
fa.map(f).toOption
Expand Down Expand Up @@ -362,28 +391,7 @@ private[data] sealed abstract class ValidatedInstances extends ValidatedInstance

override def isEmpty[A](fa: Validated[E, A]): Boolean = fa.isInvalid
}
// scalastyle:on method.length
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want to keep this // scalastyle:off method.length comment.

At the same time I think we can remove these comments around the ApplicativeError instance (catsDataApplicativeErrorForValidated).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peterneyens I'm on it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peterneyens done with requested changes.

}

private[data] sealed abstract class ValidatedInstances1 extends ValidatedInstances2 {

implicit def catsDataSemigroupForValidated[A, B](implicit A: Semigroup[A], B: Semigroup[B]): Semigroup[Validated[A, B]] =
new Semigroup[Validated[A, B]] {
def combine(x: Validated[A, B], y: Validated[A, B]): Validated[A, B] = x combine y
}

implicit def catsDataPartialOrderForValidated[A: PartialOrder, B: PartialOrder]: PartialOrder[Validated[A, B]] =
new PartialOrder[Validated[A, B]] {
def partialCompare(x: Validated[A, B], y: Validated[A, B]): Double = x partialCompare y
override def eqv(x: Validated[A, B], y: Validated[A, B]): Boolean = x === y
}
}

private[data] sealed abstract class ValidatedInstances2 {
implicit def catsDataEqForValidated[A: Eq, B: Eq]: Eq[Validated[A, B]] =
new Eq[Validated[A, B]] {
def eqv(x: Validated[A, B], y: Validated[A, B]): Boolean = x === y
}
// scalastyle:off method.length
}

private[data] trait ValidatedFunctions {
Expand Down
16 changes: 12 additions & 4 deletions tests/src/test/scala/cats/tests/IorTests.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package cats
package tests

import cats.data.{Ior, NonEmptyList}
import cats.kernel.laws.GroupLaws
import cats.laws.discipline.{BifunctorTests, CartesianTests, MonadErrorTests, SerializableTests, TraverseTests}
import cats.data.{Ior, NonEmptyList, EitherT}
import cats.laws.discipline.arbitrary._
import cats.laws.discipline.{BifunctorTests, CartesianTests, MonadTests, SerializableTests, TraverseTests}
import org.scalacheck.Arbitrary._

class IorTests extends CatsSuite {
Expand All @@ -14,8 +14,10 @@ class IorTests extends CatsSuite {
checkAll("Ior[String, Int]", CartesianTests[Ior[String, ?]].cartesian[Int, Int, Int])
checkAll("Cartesian[String Ior ?]]", SerializableTests.serializable(Cartesian[String Ior ?]))

checkAll("Ior[String, Int]", MonadTests[String Ior ?].monad[Int, Int, Int])
checkAll("Monad[String Ior ?]]", SerializableTests.serializable(Monad[String Ior ?]))
implicit val eq0 = EitherT.catsDataEqForEitherT[Ior[String, ?], String, Int]

checkAll("Ior[String, Int]", MonadErrorTests[String Ior ?, String].monadError[Int, Int, Int])
checkAll("MonadError[String Ior ?]", SerializableTests.serializable(MonadError[String Ior ?, String]))

checkAll("Ior[String, Int] with Option", TraverseTests[String Ior ?].traverse[Int, Int, Int, Int, Option, Option])
checkAll("Traverse[String Ior ?]", SerializableTests.serializable(Traverse[String Ior ?]))
Expand Down Expand Up @@ -63,6 +65,12 @@ class IorTests extends CatsSuite {
}
}

test("valueOr consistent with leftMap") {
forAll { (i: Int Ior String, f: Int => String) =>
i.valueOr(f) should === (i.leftMap(f).fold(identity, identity, _ + _))
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding coverage.

test("isLeft consistent with toOption") {
forAll { (i: Int Ior String) =>
i.isLeft should === (i.toOption.isEmpty)
Expand Down