MonadError instance for Kleisli

[durban]

Fixes #1542.

[codecov-io]

Codecov Report

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

@@            Coverage Diff             @@
##           master    #1543      +/-   ##
==========================================
+ Coverage   92.48%   92.48%   +<.01%     
==========================================
  Files         246      246              
  Lines        3885     3886       +1     
  Branches      133      132       -1     
==========================================
+ Hits         3593     3594       +1     
  Misses        292      292

Impacted Files	Coverage Δ
core/src/main/scala/cats/data/Kleisli.scala	92.59% <100%> (+0.09%)	✅

---

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 ecdd27f...cf7d5c1. Read the comment docs.

[kailuowang]

MonadError is more specific than ApplicativeError and thus should be placed in higher priority, no?

[durban]

I'm not aware of such a rule (and if there is one, it is already broken by Applicative and MonadReader for Kleisli). Is this documented somewhere? Can it cause any problems having these with a lower priority?

[kailuowang]

@durban, you are right, I believe MonadReader is in the wrong priority according to this rule (mind to fix this on the way as well?). I don't think this rule is documented anywhere here, it's not a cats specific rule, it's more a best practice to my knowledge. My understanding is that this pratice has two benefits:

1. having an implicit sequence is better not having one.
2. if you assign the most specific one with the highest priority, it takes less implicit search, so it's slightly more compilation efficient. And sometimes the most specific (and thus powerful) instance is also more performant.
   I might've missed other benefits.

[peterneyens]

Like this we have the same code twice:

• here in KleisliFlatMap
• and in catsDataMonadReaderForKleisli

Maybe we could create a KleisliMonad which can be extended by a KleisliMonadReader and a KleisliMonadError (similar as how it is done in StateT) ?

[durban]

@kailuowang @peterneyens I've amended my commit to address your comments. (I've also rebased to the current master.)

[kailuowang]

LGTM, thanks very much!

[peterneyens]

👍 Thanks for the changes!