Add MonadError instance for EitherT that recovers from F[_] errors. (#…

…1644)

* Added map and recover F functions

* Added tests for new EitherT functions

* Removed tests with Future in EitherTTests

* Added addtional MonadError instance for EitherT for recovering from F Errors

* Added transformF and deleted mapF

* Changed implicit parameter names in recoverF and recoverFWith

* Added tests for EitherT collectRight and applyAlt

* Added MonadError tests for EitherT instance that recovers from F

* Removed MonadError tests with Future, replaced it with Option

* Added serializable test

* Changed the right type from Int to String in MonadError test for EitherT

* added option -Xlog-implicits to build.sbt

* Removed transformF from EitherT

* Removed unused imports

* Redefined Eq instances in EitherTTests

* Redo of the collectRight fix in EitherTTests

* Removed unnecesary syntax import in EitherT tests

* Deleted recoverF and recoverFWith functions from EitherT

* Added comment doc for catsDataMonadErrorFForEitherT

* Separated EitherT test for F recovery to another block

* Fixed comment in EitherT tests

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

@@ -483,9 +483,27 @@ private[data] abstract class EitherTInstances1 extends EitherTInstances2 {
     new EitherTBitraverse[F] {
       val F0: Traverse[F] = F
     }
+
+  /**  Monad error instance for recovering errors in F instead of
+    *  the underlying Either.
+    *
+    * {{{
+    * scala> import cats.data.EitherT
+    * scala> import cats.MonadError
+    * scala> import cats.instances.option._
+    * scala> val noInt: Option[Either[String, Int]] = None
+    * scala> val et = EitherT[Option, String, Int](noInt)
+    * scala> val me = MonadError[EitherT[Option, String, ?], Unit]
+    * scala> me.recover(et) { case () => 1 }
+    * res0: cats.data.EitherT[Option,String,Int] = EitherT(Some(Right(1)))
+    * }}}
+    */
+  implicit def catsDataMonadErrorFForEitherT[F[_], E, L](implicit FE0: MonadError[F, E]): MonadError[EitherT[F, L, ?], E] =
+    new EitherTMonadErrorF[F, E, L] { implicit val F = FE0 }
 }
 
 private[data] abstract class EitherTInstances2 extends EitherTInstances3 {
+
   implicit def catsDataMonadErrorForEitherT[F[_], L](implicit F0: Monad[F]): MonadError[EitherT[F, L, ?], L] =
     new EitherTMonadError[F, L] {
       implicit val F = F0
@@ -547,6 +565,15 @@ private[data] trait EitherTMonad[F[_], L] extends Monad[EitherT[F, L, ?]] with E
     }))
 }
 
+private[data] trait EitherTMonadErrorF[F[_], E, L] extends MonadError[EitherT[F, L, ?], E] with EitherTMonad[F, L] {
+  implicit val F: MonadError[F, E]
+
+  def handleErrorWith[A](fea: EitherT[F, L, A])(f: E => EitherT[F, L, A]): EitherT[F, L, A] =
+    EitherT(F.handleErrorWith(fea.value)(f(_).value))
+
+  def raiseError[A](e: E): EitherT[F, L, A] = EitherT(F.raiseError(e))
+}
+
 private[data] trait EitherTMonadError[F[_], L] extends MonadError[EitherT[F, L, ?], L] with EitherTMonad[F, L] {
   def handleErrorWith[A](fea: EitherT[F, L, A])(f: L => EitherT[F, L, A]): EitherT[F, L, A] =
     EitherT(F.flatMap(fea.value) {

tests/src/test/scala/cats/tests/EitherTTests.scala

@@ -6,7 +6,8 @@ import cats.functor.Bifunctor
 import cats.functor._
 import cats.laws.discipline._
 import cats.laws.discipline.arbitrary._
-import cats.kernel.laws.{OrderLaws, GroupLaws}
+import cats.kernel.laws.{GroupLaws, OrderLaws}
+
 
 class EitherTTests extends CatsSuite {
   implicit val iso = CartesianTests.Isomorphisms.invariant[EitherT[ListWrapper, String, ?]](EitherT.catsDataFunctorForEitherT(ListWrapper.functor))
@@ -46,6 +47,7 @@ class EitherTTests extends CatsSuite {
 
   {
     //if a Monad is defined
+
     implicit val F = ListWrapper.monad
     implicit val eq0 = EitherT.catsDataEqForEitherT[ListWrapper, String, Either[String, Int]]
     implicit val eq1 = EitherT.catsDataEqForEitherT[EitherT[ListWrapper, String, ?], String, Int](eq0)
@@ -54,13 +56,30 @@ class EitherTTests extends CatsSuite {
     Applicative[EitherT[ListWrapper, String, ?]]
     Monad[EitherT[ListWrapper, String, ?]]
     MonadTrans[EitherT[?[_], String, ?]]
-
+    // Tests for catsDataMonadErrorForEitherT, for recovery on errors on Either.
     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]))
+    // Tests for MonadTrans instance.
     checkAll("EitherT[ListWrapper, String, Int]]", MonadTransTests[EitherT[?[_], String, ?]].monadTrans[ListWrapper, Int, Int])
     checkAll("MonadTrans[EitherT[?[_], String, ?]]", SerializableTests.serializable(MonadTrans[EitherT[?[_], String, ?]]))
   }
 
+  {
+    // if a MonadError is defined
+    // Tests for catsDataMonadErrorFForEitherT instance, for recovery on errors of F.
+
+    implicit val eq1 = EitherT.catsDataEqForEitherT[Option, String, Either[Unit, String]]
+    implicit val eq2 = EitherT.catsDataEqForEitherT[EitherT[Option, String, ?], Unit, String](eq1)
+    implicit val me = EitherT.catsDataMonadErrorFForEitherT[Option, Unit, String](catsStdInstancesForOption)
+
+    Functor[EitherT[Option, String, ?]]
+    Applicative[EitherT[Option, String, ?]]
+    Monad[EitherT[Option, String, ?]]
+
+    checkAll("EitherT[Option, String, String]", MonadErrorTests[EitherT[Option, String, ?], Unit].monadError[String, String, String])
+    checkAll("MonadError[EitherT[Option, ?, ?]]", SerializableTests.serializable(MonadError[EitherT[Option, String, ?], Unit]))
+  }
+
   {
     //if a Monad is defined
     implicit val F = ListWrapper.monad
@@ -220,11 +239,6 @@ class EitherTTests extends CatsSuite {
     eithert.recoverWith { case "noteithert" => EitherT.pure[Id, String](5) } should === (eithert)
   }
 
-  test("recoverWith ignores the right side") {
-    val eithert = EitherT.pure[Id, String](10)
-    eithert.recoverWith { case "eithert" => EitherT.pure[Id, String](5) } should === (eithert)
-  }
-
   test("transform consistent with value.map") {
     forAll { (eithert: EitherT[List, String, Int], f: Either[String, Int] => Either[Long, Double]) =>
       eithert.transform(f) should === (EitherT(eithert.value.map(f)))
@@ -340,6 +354,18 @@ class EitherTTests extends CatsSuite {
     }
   }
 
+  test("collectRight with Option consistent with flattening a to[Option]") {
+    forAll { (et: EitherT[Option, String, Int]) =>
+      et.collectRight should === (et.to[Option].flatten)
+    }
+  }
+
+  test("applyAlt with Id consistent with EitherT map") {
+    forAll { (et: EitherT[Id, String, Int], f: Int => String) =>
+      et.applyAlt(EitherT.pure(f)) should === (et.map(f))
+    }
+  }
+
   test("merge with Id consistent with Either merge") {
     forAll { (x: EitherT[Id, Int, Int]) =>
       x.merge should === (x.value.merge)