Improve MonadError rethrow syntax to be more flexible

[bpholt]

This allows calling MonadError's rethrow directly on an F[Either[E, A]], where E <: Throwable. Currently I don't think this is possible: one has to explicitly leftWiden to Throwable.

[codecov-io]

Codecov Report

Merging #2880 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master    #2880   +/-   ##
=======================================
  Coverage   94.21%   94.21%           
=======================================
  Files         368      368           
  Lines        6948     6948           
  Branches      308      307    -1     
=======================================
  Hits         6546     6546           
  Misses        402      402

Impacted Files	Coverage Δ
core/src/main/scala/cats/MonadError.scala	100% <ø> (ø)	⬆️
core/src/main/scala/cats/syntax/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 a4ef424...2ffaa24. Read the comment docs.

[LukaJCB]

Hi @bpholt thanks for creating this PR, is there a reason you're specializing it to Throwable? I think we should be able to provide this for any type that's a supertype of the E . in question :)

[kailuowang]

agree with @LukaJCB
I haven't tried myself, but maybe changing the existing rethrow syntax in https://github.com/typelevel/cats/blob/master/core/src/main/scala/cats/syntax/monadError.scala#L34-L36

final class MonadErrorRethrowOps[F[_], E, A](private val fea: F[Either[E, A]]) extends AnyVal {
  def rethrow(implicit F: MonadError[F, E]): F[A] = F.rethrow(fea)
}

to

  def rethrow(implicit F: MonadError[F, _ :> E]): F[A] = F.rethrow(fea)

will achieve what you want?
and that would be binary compatible change.

[bpholt]

@LukaJCB Widening beyond Throwable didn't occur to me. I don't have any principled objection to it if we can get it working.

@kailuowang That doesn't look like it compiles (I assume you meant _ >: E, not _ :> E):

[error] ~/cats/core/src/main/scala/cats/syntax/monadError.scala:35:68: type mismatch;
[error]  found   : MonadErrorRethrowOps.this.fea.type (with underlying type F[Either[E,A]])
[error]  required: F[Either[_$1,A]]
[error] Note: Either[E,A] <: Either[_$1,A], but type F is invariant in type _.
[error] You may wish to define _ as +_ instead. (SLS 4.5)
[error]   def rethrow(implicit F: MonadError[F, _ >: E]): F[A] = F.rethrow(fea)
[error]                                                                    ^
[error] one error found

I tried implementing this as a low-priority implicit (to avoid ambiguity with the existing one, ignoring bincompat for now) but I can't seem to get the type inference to work.

implicit final def catsSyntaxMonadErrorWidenRethrow[F[_], E, EE >: E, A](
  fea: F[Either[E, A]]
)(implicit F: MonadError[F, EE]): MonadErrorWidenRethrowOps[F, E, EE, A] =
  new MonadErrorWidenRethrowOps(fea)

If I specify at the call site the super type EE for which the MonadError exists, it compiles. If anyone has ideas on how to help the inference, I'd appreciate them.

[kailuowang]

@bpholt thanks for trying this out.
It looks like we encountered a scala compiler bug - I filed it here

As a work around I suggested that we make two changes (both binary compatible), I tested they should work.
Change 1, in MonadError
We change rethrow to allow narrower E

  def rethrow[A, EE <: E](fa: F[Either[EE, A]]): F[A] =
    flatMap(fa)(_.fold(raiseError, pure))

Change 2, in MonadError syntax we change the exising rethrow to (comment included)

def rethrow(implicit F: MonadError[F, _ >: E]): F[A] =
    F.flatMap(fea)(_.fold(F.raiseError, F.pure))  //it's a dup from the type class implemenation, due to https://github.com/scala/bug/issues/11562. Once fixed should be able to replace with `F.rethrow(fea)`

[kailuowang]

fixes #2561 cc @djspiewak

[kailuowang]

Thanks!
Update: clicked the wrong button

[LukaJCB]

Looks good, thanks! :)