Missing Contravariant instance for OptionT

[barambani]

It should close #2545

While checking the Invariant I noticed that there is an issue with the priority of ContravariantMonoidal and Traverse in Const so I fixed it and added tests. Can be reproduced with

scala> import cats._, cats.implicits._, cats.data._
import cats._
import cats.implicits._
import cats.data._

scala> Invariant[Const[String, ?]]
<console>:21: error: ambiguous implicit values:
 both method catsDataContravariantMonoidalForConst in class ConstInstances of type [D](implicit evidence$3: cats.Monoid[D])cats.ContravariantMonoidal[[β$0$]cats.data.Const[D,β$0$]]
 and method catsDataTraverseForConst in class ConstInstances of type [C]=> cats.Traverse[[β$2$]cats.data.Const[C,β$2$]]
 match expected type cats.Invariant[[β$0$]cats.data.Const[String,β$0$]]
       Invariant[Const[String, ?]]

[codecov-io]

Codecov Report

Merging #2548 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2548      +/-   ##
==========================================
+ Coverage   95.17%   95.19%   +0.01%     
==========================================
  Files         359      359              
  Lines        6553     6570      +17     
  Branches      278      289      +11     
==========================================
+ Hits         6237     6254      +17     
  Misses        316      316

Impacted Files	Coverage Δ
core/src/main/scala/cats/data/WriterT.scala	91.8% <100%> (+0.27%)	⬆️
core/src/main/scala/cats/data/OptionT.scala	96.22% <100%> (+0.3%)	⬆️
core/src/main/scala/cats/data/Const.scala	100% <100%> (ø)	⬆️
core/src/main/scala/cats/instances/vector.scala	100% <0%> (ø)	⬆️
core/src/main/scala/cats/data/IndexedStateT.scala	89.69% <0%> (+0.21%)	⬆️

---

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 15f4e59...325d0ec. Read the comment docs.

[barambani]

I believe the problem with scala 2.13 on the CI was related to this scala/bug#11068
I had to swap the positions of the Invariant and the Contravariant instances. According to the comments is not clear if that's a regression or an expected behaviour. Anyway I kind of liked the idea that I could lay down the hierarchy to be able to select the Invariant when the only instance needed was just that. In this implementation the Contravariant instance will be summoned for both instead.

[kailuowang]

thanks so much!

[barambani]

Sorry it was a bit noisy. 👍

[ceedubs]

Thanks @barambani! I've left one comment that I think would be an improved check that we have all of the proper instances.

[ceedubs]

Ideally we'd have a check here for a type that has an Invariant instance but not a Contravariant instance to make sure that we cover this use-case. I think that Semigroup would be an example of such a type.

[ceedubs]

I know that I mentioned Semigroup as an example of an Invariant before, but now that I see this I realize that it's awkward (and inefficient) to test. I think that it would simplify things if you use ListWrapper like we are using it elsewhere to grab an instance with only an Invariant instance. Sorry for leading you in the wrong direction on this one.

[barambani]

I will make the change. Not at all, thanks for helping me.

[ceedubs]

👍 thanks a bunch @barambani!

[barambani]

thank you, 👍