Move arbitrary instance of StateT to laws (#1584)

* move arbitray instance of StateT to laws

* fix FreeTTests

* added arbitrary for Reader

* added a test for the Reader Functor so that the arbitrary instance of Reader is covered

* minor

core/src/main/scala/cats/data/package.scala

@@ -14,6 +14,7 @@ package object data {
   val ReaderT = Kleisli
 
   type Reader[A, B] = ReaderT[Id, A, B]
+
   object Reader {
     def apply[A, B](f: A => B): Reader[A, B] = ReaderT[Id, A, B](f)
   }

free/src/test/scala/cats/free/FreeTTests.scala

@@ -7,7 +7,7 @@ import cats.data._
 import cats.laws.discipline._
 import cats.tests.CatsSuite
 import cats.instances.option._
-
+import cats.laws.discipline.arbitrary.catsLawArbitraryForState
 import org.scalacheck.{Arbitrary, Gen, Cogen}
 
 class FreeTTests extends CatsSuite {
@@ -209,7 +209,7 @@ trait FreeTTestsInstances {
 
   implicit def intStateEq[A: Eq]: Eq[IntState[A]] = stateEq[Int, A]
 
-  implicit def intStateArb[A: Arbitrary]: Arbitrary[IntState[A]] = stateArbitrary[Int, A]
+  implicit def intStateArb[A: Arbitrary]: Arbitrary[IntState[A]] = catsLawArbitraryForState[Int, A]
 
   implicit def freeTOptionEq[A](implicit A: Eq[A], OM: Monad[Option]): Eq[FreeTOption[A]] = new Eq[FreeTOption[A]] {
     def eqv(a: FreeTOption[A], b: FreeTOption[A]) = Eq[Option[A]].eqv(a.runM(identity), b.runM(identity))

laws/src/main/scala/cats/laws/discipline/Arbitrary.scala

@@ -76,8 +76,6 @@ object arbitrary extends ArbitraryInstances0 {
       B.perturb(seed, _),
       (a, b) => A.perturb(B.perturb(seed, b), a)))
 
-  implicit def catsLawsArbitraryForKleisli[F[_], A, B](implicit F: Arbitrary[F[B]]): Arbitrary[Kleisli[F, A, B]] =
-    Arbitrary(F.arbitrary.map(fb => Kleisli[F, A, B](_ => fb)))
 
   implicit def catsLawsArbitraryForCokleisli[F[_], A, B](implicit B: Arbitrary[B]): Arbitrary[Cokleisli[F, A, B]] =
     Arbitrary(B.arbitrary.map(b => Cokleisli[F, A, B](_ => b)))
@@ -157,12 +155,26 @@ object arbitrary extends ArbitraryInstances0 {
 
   implicit def catsLawsArbitraryForNested[F[_], G[_], A](implicit FG: Arbitrary[F[G[A]]]): Arbitrary[Nested[F, G, A]] =
     Arbitrary(FG.arbitrary.map(Nested(_)))
+
+  implicit def catsLawArbitraryForState[S: Arbitrary: Cogen, A: Arbitrary]: Arbitrary[State[S, A]] =
+    catsLawArbitraryForStateT[Eval, S, A]
+
+  implicit def catsLawArbitraryForReader[A: Arbitrary, B: Arbitrary]: Arbitrary[Reader[A, B]] =
+    catsLawsArbitraryForKleisli[Id, A, B]
 }
 
 private[discipline] sealed trait ArbitraryInstances0 {
+
+  implicit def catsLawArbitraryForStateT[F[_]: Applicative, S, A](implicit F: Arbitrary[S => F[(S, A)]]): Arbitrary[StateT[F, S, A]] =
+    Arbitrary(F.arbitrary.map(f => StateT(f)))
+
   implicit def catsLawsArbitraryForWriterT[F[_], L, V](implicit F: Arbitrary[F[(L, V)]]): Arbitrary[WriterT[F, L, V]] =
     Arbitrary(F.arbitrary.map(WriterT(_)))
 
   implicit def catsLawsCogenForWriterT[F[_], L, V](implicit F: Cogen[F[(L, V)]]): Cogen[WriterT[F, L, V]] =
     F.contramap(_.run)
+
+  implicit def catsLawsArbitraryForKleisli[F[_], A, B](implicit F: Arbitrary[F[B]]): Arbitrary[Kleisli[F, A, B]] =
+    Arbitrary(F.arbitrary.map(fb => Kleisli[F, A, B](_ => fb)))
+
 }

tests/src/test/scala/cats/tests/KleisliTests.scala

@@ -15,6 +15,9 @@ class KleisliTests extends CatsSuite {
   implicit def kleisliEq[F[_], A, B](implicit A: Arbitrary[A], FB: Eq[F[B]]): Eq[Kleisli[F, A, B]] =
     Eq.by[Kleisli[F, A, B], A => F[B]](_.run)
 
+  implicit def readerEq[A, B](implicit A: Arbitrary[A], FB: Eq[Id[B]]): Eq[Reader[A, B]] =
+    kleisliEq
+
   implicit val eitherTEq = EitherT.catsDataEqForEitherT[Kleisli[Option, Int, ?], Unit, Int]
 
   implicit val iso = CartesianTests.Isomorphisms.invariant[Kleisli[Option, Int, ?]]
@@ -81,7 +84,7 @@ class KleisliTests extends CatsSuite {
     checkAll("Kleisli[Option, Int, Int]", FunctorTests[Kleisli[Option, Int, ?]].functor[Int, Int, Int])
     checkAll("Functor[Kleisli[Option, Int, ?]]", SerializableTests.serializable(Functor[Kleisli[Option, Int, ?]]))
   }
-
+  
   {
     implicit val catsDataMonoidForKleisli = Kleisli.catsDataMonoidForKleisli[Option, Int, String]
     checkAll("Kleisli[Option, Int, String]", GroupLaws[Kleisli[Option, Int, String]].monoid)
@@ -106,6 +109,8 @@ class KleisliTests extends CatsSuite {
     checkAll("SemigroupK[λ[α => Kleisli[Option, α, α]]]", SerializableTests.serializable(catsDataSemigroupKForKleisli))
   }
 
+  checkAll("Reader[Int, Int]", FunctorTests[Reader[Int, ?]].functor[Int, Int, Int])
+
   checkAll("Kleisli[Option, ?, Int]", ContravariantTests[Kleisli[Option, ?, Int]].contravariant[Int, Int, Int])
   checkAll("Contravariant[Kleisli[Option, ?, Int]]", SerializableTests.serializable(Contravariant[Kleisli[Option, ?, Int]]))
 

tests/src/test/scala/cats/tests/StateTTests.scala

@@ -6,7 +6,7 @@ import cats.kernel.instances.tuple._
 import cats.laws.discipline._
 import cats.laws.discipline.eq._
 import cats.laws.discipline.arbitrary._
-import org.scalacheck.{Arbitrary, Cogen}
+import org.scalacheck.Arbitrary
 
 class StateTTests extends CatsSuite {
   import StateTTests._
@@ -294,15 +294,10 @@ object StateTTests extends StateTTestsInstances {
   implicit def stateEq[S:Eq:Arbitrary, A:Eq]: Eq[State[S, A]] =
     stateTEq[Eval, S, A]
 
-  implicit def stateArbitrary[S: Arbitrary: Cogen, A: Arbitrary]: Arbitrary[State[S, A]] =
-    stateTArbitrary[Eval, S, A]
-
   val add1: State[Int, Int] = State(n => (n + 1, n))
 }
 
 sealed trait StateTTestsInstances {
-  implicit def stateTArbitrary[F[_]: Applicative, S, A](implicit F: Arbitrary[S => F[(S, A)]]): Arbitrary[StateT[F, S, A]] =
-    Arbitrary(F.arbitrary.map(f => StateT(f)))
 
   implicit def stateTEq[F[_], S, A](implicit S: Arbitrary[S], FSA: Eq[F[(S, A)]], F: FlatMap[F]): Eq[StateT[F, S, A]] =
     Eq.by[StateT[F, S, A], S => F[(S, A)]](state =>