MonadError instance for Ior (#1548)

* Added MonadError instance for Ior

* Revert "Added MonadError instance for Ior"

This reverts commit 344e232.

* Added MonadError instance for Ior without mixing Traverse

* Removed Monad instance

* Added MonadError tests to Ior

* Added MonadErrorTests for Ior

* Revert "Added MonadErrorTests for Ior"

This reverts commit 213452a.

* Separated Traverse instance from ApplicativeError instance for Validated

* Added test for ior valueOr

* Fixed some slastyle comments

* Deleted duplicated import

core/src/main/scala/cats/data/Ior.scala

@@ -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)
+          case r @ Ior.Right(_) => r
+          case Ior.Both(e, _) => f(e)
+        }
+
+      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] {
@@ -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)
   }

core/src/main/scala/cats/data/Validated.scala

@@ -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)
@@ -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
@@ -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
-}
-
-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 {

tests/src/test/scala/cats/tests/IorTests.scala

@@ -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 {
@@ -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 ?]))
@@ -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, _ + _))
+    }
+  }
+
   test("isLeft consistent with toOption") {
     forAll { (i: Int Ior String) =>
       i.isLeft should === (i.toOption.isEmpty)