Adding WriterT Monoid

[yilinwei]

Adding WriterT Monoid as referenced in #1012.

It's also come to my attention that it's one of the subtasks in #620.

[yilinwei]

After using in our application code further I'm seeing similar behaviour that @ceedubs is seeing for MonadError in #689.

In particular I seem to have some trouble resolving the Monoid which should exist for Id and Xor as the F[_]. The kinds with a 'normal' F[_] shape (apart from Id) such as Option and List seem to work well.

  //explicit call works, implicitly fails, resolve fails
  writerTMonoid[String Xor ?, String, Unit]
  implicitly[Monoid[WriterT[String Xor ?, String, Unit]]]
  resolveMonoid[WriterT[String Xor ?, String, ?]]

  //implicitly and resolve both work
  implicitly[Monoid[WriterT[List, String, Unit]]]
  resolveMonoid[WriterT[List, String, ?]]

  //implicitly works, and resolve fails
  implicitly[Monoid[WriterT[Id, String, Unit]]]
  resolveMonoid[WriterT[Id, String, ?]]

  //implicitly and resolve both work
  implicitly[Monoid[WriterT[Option, String, Unit]]]
  resolveMonoid[WriterT[Option, String, ?]]

[yilinwei]

I've checked in scalaz and can see the same issue with their WriterT implementation with \/ and Id. Also, the semigroup |+| works with Id as F[_] but not Xor which I guess is vaguely expected since implicitly works for Id but not Xor.

[adelbertc]

Is this SI-2712?

[yilinwei]

I compiled against @milessabin proposed fix for SI-2712 and it didn't work worked for |+| for the Xor - however the resolveMonoid method still doesn't seem to work correctly even after the compilation change.

A simplified repro,

trait TC[A]
trait Foo[A, B]
trait Bar[F[_], A]

object Check {
  implicit val a: TC[Unit] = ???
  implicit def fooTC[A, B](implicit a: TC[A]): TC[Foo[A, B]] = ???
  implicit def barTC[F[_], A](implicit fa: TC[F[A]]): TC[Bar[F, A]] = ???

  //works
  implicitly[TC[Foo[Unit, Unit]]]
  //doesn't work
  implicitly[TC[Bar[({type L[A] = Foo[Unit, A]})#L, Unit]]]
  //works
  barTC[({type L[A] = Foo[Unit, A]})#L, Unit]
}

EDIT: I don't think I enabled the flag for the unification so checking again!

[ceedubs]

@yilinwei thanks a lot for getting the ball rolling on this and identifying implicit resolution issues!

I've submitted a PR to your branch that helps with the case of Id. If you merge it, then the updates should appear on this PR.

I'm not sure what to do about the Xor case. We might just have to punt on it for now.

[codecov-io]

Current coverage is 81.99%

Merging #1014 into master will increase coverage by +<.01%

1. 6 files (not in diff) in ...main/scala/cats/free were deleted. more
2. 6 files (not in diff) in ...main/scala/cats/free were created. more
3. 2 files (not in diff) in ...n/scala/cats/functor were modified. more
4. 2 files (not in diff) in ...main/scala/cats/data were modified. more
5. 1 files (not in diff) in .../src/main/scala/cats were modified. more

@@             master      #1014   diff @@
==========================================
  Files           215        215          
  Lines          2704       2710     +6   
  Methods        2639       2647     +8   
  Messages          0          0          
  Branches         60         58     -2   
==========================================
+ Hits           2217       2222     +5   
- Misses          487        488     +1   
  Partials          0          0          

Powered by Codecov. Last updated by 8355a19...30cf9df

[yilinwei]

I think we can leave the Xor use case for now - I'm not really familiar with codecov.io - what does the check mean and is there anything I should be doing?

[ceedubs]

👍 lgtm. I don't entirely understand what's going on with the code coverage report. I think something weird is happening because of the separation of the free module that has been merged to master. I wouldn't worry about it too much.

[adelbertc]

👍