Consider remove dependency on scalacheck from cats-laws

[kailuowang]

Consider abstract out the dependency on scalacheck from cats-laws with some intermediate type class. Then we can have a new module cats-testkit-scalacheck, which is more justfiable to have different versioning which could be more tied to Scalacheck breaking releases. In fact this approach may allow the community to push the dependency on scalacheck to where the tests run, rather than where the law is defined.

Update: I created a proof of concept for this design in my mind, It compiles to show the type should work First the usage API on the laws site

class MyRuleSet[P] extends RuleSetDSL[P] {
  //define some laws
  def mylaw[A]= (a:A) => IsEq(a, a)   
  def mylaw2[A, B]= (a:A, b: B) => IsEq(a, a)

  // define the set of laws (potentially inherit from parents
  def ruleSet[A, B](implicit c1 : Check1[A, A],
                             c2: Check2[A, B, A]): Seq[(String, P)] =
    Seq(law("dbad", mylaw[A]),
        law("bgdf", mylaw2[A, B]))
}

In this usage when user added a new law, the compiler will prompt on what CheckX instance is needed.

Here is the law test site that is dependent on scalacheck

class MyRunner extends RunnerBase {
  import cats.kernel.instances.string._
  checkAll(new MyRuleSet[Prop].props[String, String])
}

The Check is a type class that abstract out the operation that converts a defined law A1 => IsEq[A2] to a testable unified type P

trait Checkable1[A1, A2, P] {
  def apply(f: A1 => IsEq[A2]): P
}
trait Checkable2[A1, A2, A3, P] {
  def apply(f: (A1, A2) => IsEq[A3]): P
}

We are going have to create multiple arity version of this type class

The simplified usage on the law site was enabled by the following DSL trait

trait RuleSetDSL[P] {

  type Check1[A1, A2] = Checkable1[A1, A2, P]
  type Check2[A1, A2, A3] = Checkable2[A1, A2, A3, P]

  implicit def law[A1, A2](s : String, f: A1 => IsEq[A2])(implicit c: Check1[A1, A2]): (String, P) =
    (s, c(f))

  implicit def law[A1, A2, A3](s : String, f: (A1, A2) => IsEq[A3])(implicit c: Check2[A1, A2, A3]): (String, P) =
    (s, c(f))
}

apparently we need more arity versions.

On the test site we are going to provide the Checkable Instance for scalacheck derived from Arbitrary instances.

import org.scalacheck.util.Pretty

class RunnerBase {

  //should be trivial to implement to following method with scalatest or claimant
  def checkAll(p: Seq[(String, Prop)]): Unit = ???

  import cats.kernel.Eq

  //copied from cats-laws
  implicit def catsLawsIsEqToProp[A](isEq: IsEq[A])(implicit ev: Eq[A], pp: A => Pretty): Prop =
    isEq match {
      case IsEq(x, y) =>
        if (ev.eqv(x, y)) Prop.proved
        else
          Prop.falsified :| {
            val exp = Pretty.pretty[A](y, Pretty.Params(0))
            val act = Pretty.pretty[A](x, Pretty.Params(0))
            s"Expected: $exp\n" + s"Received: $act"
          }
    }

  implicit def tp1[A1: Arbitrary, A2](implicit ev: Eq[A2], pp: A2 => Pretty): Checkable1[A1, A2, Prop] = { f =>
    forAll((a: A1) => f(a))
  }

  implicit def tp2[A1: Arbitrary, A2: Arbitrary, A3](implicit ev: Eq[A3], pp: A3 => Pretty): Checkable2[A1, A2, A3, Prop] = { f =>
    forAll((a:A1, b: A2) => f(a,b))
  }

}

The law definition usage site is quite different with a different set of evidences, but they are probably easier to understand, whatever you law's type is, you just add a corresponding Check[A, B, C] evidence, without the need to understand all the scalacheck types. What do you guys think?

[travisbrown]

I'm a weak :-1: on this, at least off the top of my head, since ScalaCheck has been around a lot longer than Cats, breaks bincompat very rarely and only for good reasons, and has trustworthy / present maintainers. Also my guess is that plumbing Arbitrary and Shrink instances through this abstraction everywhere will be kind of messy—if someone shows that it can be done nicely I'd be happy to change my mind.

[mpilquist]

I'm in favor of this so I can use ScalaTest's generators/property testing. I'd imagine other folks might want to use scalaprops or one of the other newer property testing libraries.

On Fri, Apr 19, 2019 at 12:19 PM Travis Brown notifications@github.com wrote:

I'm a weak 👎 on this, at least off the top of my head, since ScalaCheck has been around a lot longer than Cats, breaks bincompat very rarely and only for good reasons, and has trustworthy / present maintainers. Also my guess is that plumbing Arbitrary and Shrink instances through this abstraction everywhere will be kind of messy—if someone shows that it can be done nicely I'd be happy to change my mind.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/typelevel/cats/issues/2800#issuecomment-484946940, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA42PT6UXPPT437W5POOU3PRHWJPANCNFSM4HHE3WHQ .

[tpolecat]

Also in favor, if it doesn't complicate things too much. ScalaCheck isn't really state of the art anymore and it would be nice to open things up if we can.

[kailuowang]

I added a proof of concept design, any thoughts?

[travisbrown]

@kailuowang Looks reasonable to me (you'll need to add Shrink).

[kailuowang]

cc @larsrh, any thought? From my preliminary poc, it seems possible to decouple law testing definition from scalacheck? I introduced an intermediate type class Checkable (strictly speaking a series Checkables with a version for each arity). Although I think it might force too many changes to the community as all laws tests have to be modified to use Checkable evidence instead of Arbitrarys and Cogens

[larsrh]

@kailuowang I have no opinion on this.

[tpolecat]

We do law-checking and I would be fine with mechanical changes. Honestly it's so complicated to get everything set up I can't imagine many people are doing it.