typelevel / typelevel/cats

Consider remove dependency on scalacheck from cats-laws

Open
#2,800 8 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Scala
Stars
5.5k
Forks
1.2k
Avg merge
2d 10h
Merged PRs (30d)
5

Description

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?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the cats-laws dependency and the proposed Checkable/RuleSetDSL design; the issue names no files or tests. Done would be an agreed implementation that removes ScalaCheck from cats-laws and provides the proposed cats-testkit-scalacheck integration.

Written by the indexing model from the issue text.

Assessment

Tech stack
scala
Domain
testing
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.