Add adaptError override to MonadError

[travisbrown]

We have a pretty bad binary compatibility breakage in 2.1.0-RC1: if you have cats-effect 2.0.0 and cats-core 2.1.0-RC1, you get the following (on both 2.12 and 2.13):

scala> import cats.syntax.monadError._
import cats.syntax.monadError._

scala> cats.effect.IO(1).adaptError { case e => e }
java.lang.NoSuchMethodError: cats.MonadError.adaptError$(Lcats/MonadError;Ljava/lang/Object;Lscala/PartialFunction;)Ljava/lang/Object;
  at cats.effect.IOLowPriorityInstances$IOEffect.adaptError(IO.scala:767)
  at cats.syntax.MonadErrorOps$.adaptError$extension(monadError.scala:31)
  ... 36 elided

Or:

scala> cats.MonadError[cats.effect.IO, Throwable].adaptError(cats.effect.IO(1)) { case e => e }
java.lang.NoSuchMethodError: cats.MonadError.adaptError$(Lcats/MonadError;Ljava/lang/Object;Lscala/PartialFunction;)Ljava/lang/Object;
  at cats.effect.IOLowPriorityInstances$IOEffect.adaptError(IO.scala:767)
  ... 36 elided

And sure enough, if you check the 2.1.0-RC1 jars this synthetic method is missing.

I've confirmed that adding this override reinstates the synthetic method and fixes the issue, or at least this particular manifestation of it.

I don't understand how MiMa didn't catch this, and I'm trying to figure that out now, and to make sure there aren't other changes like this in 2.1.0-RC1. I'll publish an RC2 as soon as I'm confident there aren't more changes like this.

Thanks to @bastewart for catching this issue.

[codecov-io]

Codecov Report

Merging #3162 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3162      +/-   ##
==========================================
+ Coverage   93.04%   93.04%   +<.01%     
==========================================
  Files         376      376              
  Lines        7374     7375       +1     
  Branches      209      215       +6     
==========================================
+ Hits         6861     6862       +1     
  Misses        513      513

Flag	Coverage Δ
#scala_version_212	93.37% <100%> (+0.02%)	⬆️
#scala_version_213	90.62% <100%> (-0.02%)	⬇️

Impacted Files	Coverage Δ
core/src/main/scala/cats/MonadError.scala	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55c48a8...7043b05. Read the comment docs.

[travisbrown]

Okay, I've hacked together a fix for MiMa and while this is the only breakage like this in cats-core, there are a bunch in cats-kernel. Fortunately they're much harder to run into—you have to be doing something like this:

import cats.kernel.{Monoid, Semigroup}

object myOptionInstances extends cats.kernel.instances.OptionInstances

object Test {
  import myOptionInstances._
  import cats.kernel.instances.string._
  def main(args: Array[String]): Unit = println(Monoid[Option[String]])
}

If you compile this with cats-kernel 2.0.0 and run it with 2.1.0-RC1 you'll get a NoSuchMethodError.

There are dozens of these, I think all related to prioritization fixes like #3100. I'm working on a PR now.

[johnynek]

I wonder if we need our own binary compatibility test: like we compile with the current version, then run some test load compiled against the previous.

[johnynek]

Nice work Travis.

[travisbrown]

Thanks, @johnynek. Any chance you'd want to review the other part of the bincompat fix (and maybe some of the other PRs in the milestone) so I can publish a not-broken 2.1.0-RC2?

[dwijnand]

Apologise for the false negative in MiMa...

I wonder if we need our own binary compatibility test: like we compile with the current version, then run some test load compiled against the previous.

It would be a failure of MiMa if this were the outcome (but you should absolutely make the best decision for Cats). I'm going to see if we can do something along these lines within MiMa's test suite, for the suite of changes we believe are binary compatible.

(edit: opened lightbend-labs/mima#427 to that end)