From 4331f64867a73592e3a2b12d691c1a3bac2b7313 Mon Sep 17 00:00:00 2001 From: Filippo Mariotti Date: Mon, 19 Nov 2018 15:59:23 +0000 Subject: [PATCH] 2611 - MonadError syntax for OptionT uses incorrect instance (#2615) * Fixed priority, added test * Added mima exceptions and tests * Format and comma --- .../src/main/scala/catsBC/MimaExceptions.scala | 3 ++- build.sbt | 5 +++-- core/src/main/scala/cats/data/OptionT.scala | 16 ++++++++++++---- .../src/test/scala/cats/tests/OptionTSuite.scala | 6 ++++++ 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/binCompatTest/src/main/scala/catsBC/MimaExceptions.scala b/binCompatTest/src/main/scala/catsBC/MimaExceptions.scala index 3777168f18..3d955ebe50 100644 --- 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 index c06ab12cd0..14851f8486 100644 --- 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 index 7b96cec97e..c66ff4ad0a 100644 --- 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 index 9b9e25160d..6edb9ef5d2 100644 --- 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))