Implement several Functor methods to avoid using map when not needed

[johnynek]

I reached for tupleLeft in this PR:

typelevel/cats-collections#779

But then looked and noticed that actually it and related functions haven't been overridden in many places. This PR addresses that.

[johnynek]

I couldn't find the native error. I guess is may be some issue with the native being flakey? I can't see why the changes would fail on native but not JVM.

[danicheg]

Adding some tests would be nice. Anyway, LGTM

[johnynek]

Adding some tests would be nice. Anyway, LGTM

I've added some tests. Ideally these would be added to FunctorLaws saying that those methods need to match the map based implementations. Doing that, however, would break compatibility of FunctorLaws because it requires more Arbitrary and Eq instances (a major weakness of our Laws).

Honestly, I wonder if the laws need some higher kinded change. Something like ArbitraryK[[A] =>> [Arbitrary[A] => Arbitrary[F[A]]] and similiar for Eq and Cogen and then maybe define case class TestType[A](arbA: Arbitrary[A], eqA: Eq[A], cogenA: Cogen[A]) and then the system defines ArbitraryK[TestType] and then we could define new laws without requiring changes to the function types and breaking compatibility. But obviously this is a major rewrite of how the laws work.

[johnynek]

@danicheg how does this look? I think it is ready to merge.

[satorg]

I wonder – should we get void here for completeness? As long as as is here.

[satorg]

@johnynek I totally agree that ideally these tests should be covered in the Laws. However, since it not feasible to achieve it easily, wouldn't it make sense to generalize them anyway. E.g. something like this:

trait FunctorSuiteBase { self: CatsSuite =>
  def testMapBasedImplementations[F[_]: Functor](implicit
    arbFI: Arbitrary[F[Int]], arbFIS: Arbitrary[F[(Int, String)]],
    eqFU: Eq[F[Unit]], eqFI: Eq[F[Int]], eqFS: Eq[F[String]], eqFIS: Eq[F[(Int, String)]], eqFSI: Eq[F[(String, Int)]], eqFIL: Eq[F[(Int, Long)]], eqFLI: Eq[F[(Long, Int)]]
  ): Unit =
    test("functor default methods match map-based implementations") {
      val F = Functor[F]
      forAll { (fa: F[Int], b: String, f: Int => Long) =>
        assert(F.as(fa, b) === F.map(fa)(_ => b))
        assert(F.tupleLeft(fa, b) === F.map(fa)(a => (b, a)))
        assert(F.tupleRight(fa, b) === F.map(fa)(a => (a, b)))
        assert(F.fproduct(fa)(f) === F.map(fa)(a => (a, f(a))))
        assert(F.fproductLeft(fa)(f) === F.map(fa)(a => (f(a), a)))
        assert(F.void(fa) === F.map(fa)(_ => ()))
      }
      forAll { (fab: F[(Int, String)]) =>
        assert(F.unzip(fab) === (F.map(fab)(_._1), F.map(fab)(_._2)))
      }
    }
}

and then call testMapBasedImplementations from every affected suite.

All these new tests look so identical that I couldn't help but suggest it out :)

[johnynek]

@satorg I've added unit and void to cats.Id but I'd like to hold off on the abstraction you suggested. In my experience, the wrong abstraction is worse that a bit of simple duplication like this, and I've recently been tripped up by the law code, which is published and has binary compatibility requirements, causing issues. I'd rather just leave this very simple duplication for now vs get into designing the right abstraction here.

For instance, what if we add more map-based methods we'd like to test, but they require more implicits than the ones we add to that trait? Ideally we would have a clean story to add laws to the original FunctorLaws, (such as the higher kinded Arbitrary and Eq suggestion I made).

Really we want something like

trait Higher[F[_], G[_]] {
  def lower[A](f: F[A]): F[G[A]]
}

Then we would have these laws be given Higher[Arbitrary, F], Higher[Eq, F], Higher[Cogen, F]. We would also have have something like:

class TestType {
  type T
  def arbT: Arbitrary[T]
  def eqT: Eq[T]
  def cogenT: Cogen[T]
}

class TestTypeK {
  type T[_]
  def arbT: Higher[Arbitrary, T]
  def eqT: Higher[Eq, T]
  def cogenT: Higher[Cogen, T]
}

with this, I think maybe all laws on F[_] could be parameterized on Higher[Arbitrary, F], Higher[Eq, F], Higher[Cogen, F], Arbitrary[TestType], Arbitrary[TestTypeK]. or something like this...

In any case, I think this is way beyond the scope of this PR. Can we punt the abstraction of tests and just merge this relatively simple improvement to map-based functor methods for these small common types?

[satorg]

Thanks!