2611 - MonadError syntax for OptionT uses incorrect instance (#2615)

* Fixed priority, added test * Added mima exceptions and tests * Format and comma
typelevel · Nov 19, 2018 · 4331f64 · 4331f64
1 parent d7e9e99
commit 4331f64
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 7 deletions.
diff --git a/binCompatTest/src/main/scala/catsBC/MimaExceptions.scala b/binCompatTest/src/main/scala/catsBC/MimaExceptions.scala
@@ -19,10 +19,11 @@ object MimaExceptions {
       cats.data.IRWST.catsDataStrongForIRWST[List, Int, Int, Int],
       cats.data.OptionT.catsDataMonadErrorMonadForOptionT[List],
       FunctionK.lift(headOption),
+      cats.data.OptionT.catsDataMonadErrorForOptionT[Either[String, ?], String],
+      cats.data.OptionT[Either[String, ?], Int](Right(Some(17))).ensure("error")(_ => true),
       "blah".leftNec[Int],
       List(Some(4), None).nested,
       cats.data.EitherT.left[Int](Option("err")),
       true.iterateUntilM(Option(_))(identity _)
   )
-
 }
diff --git a/build.sbt b/build.sbt
@@ -280,7 +280,8 @@ def mimaSettings(moduleName: String) =
         exclude[DirectMissingMethodProblem]("cats.data.KleisliInstances1.catsDataCommutativeArrowForKleisli"),
         exclude[DirectMissingMethodProblem]("cats.data.KleisliInstances4.catsDataCommutativeFlatMapForKleisli"),
         exclude[DirectMissingMethodProblem]("cats.data.IRWSTInstances1.catsDataStrongForIRWST"),
-        exclude[DirectMissingMethodProblem]("cats.data.OptionTInstances1.catsDataMonadErrorMonadForOptionT")
+        exclude[DirectMissingMethodProblem]("cats.data.OptionTInstances1.catsDataMonadErrorMonadForOptionT"),
+        exclude[DirectMissingMethodProblem]("cats.data.OptionTInstances1.catsDataMonadErrorForOptionT"),
       ) ++
         //These things are Ops classes that shouldn't have the `value` exposed. These should have never been public because they don't
         //provide any value. Making them private because of issues like #2514 and #2613.
@@ -646,7 +647,7 @@ lazy val binCompatTest = project
   .disablePlugins(CoursierPlugin)
   .settings(noPublishSettings)
   .settings(
-    addCompilerPlugin("org.spire-math" %% "kind-projector" % "0.9.7"),
+    addCompilerPlugin("org.spire-math" %% "kind-projector" % "0.9.8"),
     libraryDependencies ++= List(
       {
         if (priorTo2_13(scalaVersion.value))

diff --git a/core/src/main/scala/cats/data/OptionT.scala b/core/src/main/scala/cats/data/OptionT.scala
@@ -270,8 +270,16 @@ sealed abstract private[data] class OptionTInstances extends OptionTInstances0 {
 }
 
 sealed abstract private[data] class OptionTInstances0 extends OptionTInstances1 {
-  implicit def catsDataMonadErrorMonadForOptionT[F[_]](implicit F0: Monad[F]): MonadError[OptionT[F, ?], Unit] =
-    new OptionTMonadErrorMonad[F] { implicit val F = F0 }
+
+  // the Dummy type is to make this one more specific than catsDataMonadErrorMonadForOptionT on 2.13.x
+  // see https://github.com/typelevel/cats/pull/2335#issuecomment-408249775
+  implicit def catsDataMonadErrorForOptionT[F[_], E](
+    implicit F0: MonadError[F, E]
+  ): MonadError[OptionT[F, ?], E] { type Dummy } =
+    new OptionTMonadError[F, E] {
+      type Dummy
+      implicit val F = F0
+    }
 
   implicit def catsDataContravariantMonoidalForOptionT[F[_]](
     implicit F0: ContravariantMonoidal[F]
@@ -303,8 +311,8 @@ sealed abstract private[data] class OptionTInstances1 extends OptionTInstances2
   implicit def catsDataEqForOptionT[F[_], A](implicit F0: Eq[F[Option[A]]]): Eq[OptionT[F, A]] =
     new OptionTEq[F, A] { implicit val F = F0 }
 
-  implicit def catsDataMonadErrorForOptionT[F[_], E](implicit F0: MonadError[F, E]): MonadError[OptionT[F, ?], E] =
-    new OptionTMonadError[F, E] { implicit val F = F0 }
+  implicit def catsDataMonadErrorMonadForOptionT[F[_]](implicit F0: Monad[F]): MonadError[OptionT[F, ?], Unit] =
+    new OptionTMonadErrorMonad[F] { implicit val F = F0 }
 }
 
 sealed abstract private[data] class OptionTInstances2 extends OptionTInstances3 {

diff --git a/tests/src/test/scala/cats/tests/OptionTSuite.scala b/tests/src/test/scala/cats/tests/OptionTSuite.scala
@@ -184,6 +184,12 @@ class OptionTSuite extends CatsSuite {
     checkAll("MonadError[OptionT[List, ?]]", SerializableTests.serializable(MonadError[OptionT[ListWrapper, ?], Unit]))
   }
 
+  test("MonadError[OptionT[F, ?], E] instance has higher priority than MonadError[OptionT[F, ?], Unit]") {
+
+    def shouldCompile[F[_]: MonadError[?[_], E], E](x: OptionT[F, Int], e: E): OptionT[F, Int] =
+      x.ensure(e)(_ => true)
+  }
+
   test("fold and cata consistent") {
     forAll { (o: OptionT[List, Int], s: String, f: Int => String) =>
       o.fold(s)(f) should ===(o.cata(s, f))