Explicit functor variance

[edmundnoble]

This uses casts to implement Functor.widen and Contravariant.narrow to take advantage of the natural variance of Functors with respect to subtyping. Functor.widen in Scalaz works similarly (though it uses Liskov and @uncheckedVariance in its implementation, it has the same underlying optimization).

[codecov-io]

Current coverage is 88.57%

Merging #1097 into master will decrease coverage by 0.35%

1. 2 files (not in diff) in .../main/scala/cats/std were modified. more
2. 2 files (not in diff) in ...main/scala/cats/data were modified. more
   • Misses +1
   • Hits -1
3. 3 files (not in diff) in .../src/main/scala/cats were modified. more
   • Misses +11
   • Hits +1

@@             master      #1097   diff @@
==========================================
  Files           226        226          
  Lines          2917       2931    +14   
  Methods        2867       2879    +12   
  Messages          0          0          
  Branches         48         49     +1   
==========================================
+ Hits           2594       2596     +2   
- Misses          323        335    +12   
  Partials          0          0          

Powered by Codecov. Last updated by 88cbe95...4c5db8b

[edmundnoble]

I think the only useful tests for widen and narrow are going to be fairly low-level. I'll add some for codecov.

[peterneyens]

You could probably add an sbt-doctest to solve the codecov difference ?

[julienrf]

Can you point to a discussion about the pros and cons of this approach versus using a variance annotation?

[ceedubs]

Could you please add another * so that this becomes a ScalaDoc comment?

[ceedubs]

@edmundnoble thanks! Looking good so far. I agree with @peterneyens that some doctest examples would be good and will help out the code coverage. Another thing that you could do is add some property-tests that widen is equivalent to map identity, but then I guess you have the question of whether you should add that for a few random functors, or add it to the laws.

[non]

I agree with @ceedubs specific comment syntax remarks, but other than that +1 from me. Tests and/or examples would be nice too if you felt like doing them.

[edmundnoble]

@julienrf That's proving surprisingly hard to find. I can find a good argument against using a contravariance annotation on Contravariant: https://issues.scala-lang.org/browse/SI-2509.

[edmundnoble]

I've made the comments into scaladoc comments, and added some doctest stuff and tests.

[ceedubs]

👍 thanks!

@julienrf I don't know of a discussion about this approach vs adding a variance annotation to point to. I can add some notes here though.

If you were to write an invariant InvList[A], your prepend method would look something like def ::(a: A): InvList[A] = ???. With a covariant list like the standard library's List[+A], this becomes a bit more complicated. A List[A] is a List of any supertype of A, so your method has to become something like def ::[B >: A](x: B): List[B]. This means that something that is likely an error like 1 :: List("b", "c") compiles (but returns List[Any]). There are reasons that you might want that behavior, but I think that it's also reasonable to not want that behavior.

The previous paragraph referred to putting the variance annotation on the data structure. If you were to put the variance annotation on the type class instances (like trait Functor[F[+_]]), then you would prohibit the creation of a Functor instance for any invariant structure (such as the NonEmptyList in cats). I consider that a deal-breaker for adding variance to type classes.

[non]

Looks good to me. 👍