From b9b8144f464848791371822b45b191b684f7e4cd Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Sat, 27 Apr 2019 22:01:25 -0400 Subject: [PATCH 1/8] added a new foldRight lazy law, move forAll and exist laws --- .../main/scala/cats/laws/FoldableLaws.scala | 31 +++++++++++++++++++ .../cats/laws/UnorderedFoldableLaws.scala | 18 ----------- .../cats/laws/discipline/FoldableTests.scala | 1 + 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/laws/src/main/scala/cats/laws/FoldableLaws.scala b/laws/src/main/scala/cats/laws/FoldableLaws.scala index 64cb6e83ca..9cd2bcf127 100644 --- a/laws/src/main/scala/cats/laws/FoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/FoldableLaws.scala @@ -17,6 +17,17 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { b |+| f(a) } + + def foldRightLazy[A](fa: F[A]): Boolean = { + var i = 0 + F.foldRight(fa, Eval.now("empty")) { (_, _) => + i += 1 + Eval.now("not empty") + }.value + i == (if (F.isEmpty(fa)) 0 else 1) + } + + def rightFoldConsistentWithFoldMap[A, B]( fa: F[A], f: A => B @@ -111,6 +122,26 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { def orderedConsistency[A: Eq](x: F[A], y: F[A])(implicit ev: Eq[F[A]]): IsEq[List[A]] = if (x === y) (F.toList(x) <-> F.toList(y)) else List.empty[A] <-> List.empty[A] + + + def existsLazy[A](fa: F[A]): Boolean = { + var i = 0 + F.exists(fa) { _ => + i = i + 1 + true + } + i == (if (F.isEmpty(fa)) 0 else 1) + } + + def forallLazy[A](fa: F[A]): Boolean = { + var i = 0 + F.forall(fa) { _ => + i = i + 1 + false + } + i == (if (F.isEmpty(fa)) 0 else 1) + } + } object FoldableLaws { diff --git a/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala b/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala index 7df29a0c3e..410a09efc1 100644 --- a/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala @@ -21,24 +21,6 @@ trait UnorderedFoldableLaws[F[_]] { (F.isEmpty(fa) || F.exists(fa)(p)) } else true // can't test much in this case - def existsLazy[A](fa: F[A]): Boolean = { - var i = 0 - F.exists(fa) { _ => - i = i + 1 - true - } - i == (if (F.isEmpty(fa)) 0 else 1) - } - - def forallLazy[A](fa: F[A]): Boolean = { - var i = 0 - F.forall(fa) { _ => - i = i + 1 - false - } - i == (if (F.isEmpty(fa)) 0 else 1) - } - /** * If `F[A]` is empty, forall must return true. */ diff --git a/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala b/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala index 65d10b2081..c6376695ed 100644 --- a/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala @@ -27,6 +27,7 @@ trait FoldableTests[F[_]] extends UnorderedFoldableTests[F] { parent = Some(unorderedFoldable[A, B]), "foldLeft consistent with foldMap" -> forAll(laws.leftFoldConsistentWithFoldMap[A, B] _), "foldRight consistent with foldMap" -> forAll(laws.rightFoldConsistentWithFoldMap[A, B] _), + "foldRight is lazy" -> forAll(laws.foldRightLazy[A] _), "ordered constistency" -> forAll(laws.orderedConsistency[A] _), "exists consistent with find" -> forAll(laws.existsConsistentWithFind[A] _), "exists is lazy" -> forAll(laws.existsLazy[A] _), From 70d513f743048a77d3faea222724bac56f6d6cde Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Sat, 27 Apr 2019 22:23:06 -0400 Subject: [PATCH 2/8] reformat --- laws/src/main/scala/cats/laws/FoldableLaws.scala | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/laws/src/main/scala/cats/laws/FoldableLaws.scala b/laws/src/main/scala/cats/laws/FoldableLaws.scala index 9cd2bcf127..94dd75615b 100644 --- a/laws/src/main/scala/cats/laws/FoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/FoldableLaws.scala @@ -17,17 +17,16 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { b |+| f(a) } - def foldRightLazy[A](fa: F[A]): Boolean = { var i = 0 F.foldRight(fa, Eval.now("empty")) { (_, _) => - i += 1 - Eval.now("not empty") - }.value + i += 1 + Eval.now("not empty") + } + .value i == (if (F.isEmpty(fa)) 0 else 1) } - def rightFoldConsistentWithFoldMap[A, B]( fa: F[A], f: A => B @@ -123,7 +122,6 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { if (x === y) (F.toList(x) <-> F.toList(y)) else List.empty[A] <-> List.empty[A] - def existsLazy[A](fa: F[A]): Boolean = { var i = 0 F.exists(fa) { _ => From d6b1584fb8e75fe14447e035424e857e4cb24f2e Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Sun, 28 Apr 2019 07:40:38 -0400 Subject: [PATCH 3/8] fix foldRight on NonEmptyChain --- core/src/main/scala/cats/data/NonEmptyChain.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/data/NonEmptyChain.scala b/core/src/main/scala/cats/data/NonEmptyChain.scala index c1ae0faf8b..035ae7e097 100644 --- a/core/src/main/scala/cats/data/NonEmptyChain.scala +++ b/core/src/main/scala/cats/data/NonEmptyChain.scala @@ -452,7 +452,7 @@ sealed abstract private[data] class NonEmptyChainInstances extends NonEmptyChain fa.foldLeft(b)(f) override def foldRight[A, B](fa: NonEmptyChain[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = - fa.foldRight(lb)(f) + Foldable[Chain].foldRight(fa.toChain, lb)(f) override def foldMap[A, B](fa: NonEmptyChain[A])(f: A => B)(implicit B: Monoid[B]): B = B.combineAll(fa.toChain.iterator.map(f)) From 58111262f95efaa42556632e15f5d81adf6896bc Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Sun, 28 Apr 2019 08:01:17 -0400 Subject: [PATCH 4/8] reformat --- .../main/scala/cats/laws/FoldableLaws.scala | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/laws/src/main/scala/cats/laws/FoldableLaws.scala b/laws/src/main/scala/cats/laws/FoldableLaws.scala index 94dd75615b..d2da8b567a 100644 --- a/laws/src/main/scala/cats/laws/FoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/FoldableLaws.scala @@ -8,6 +8,16 @@ import scala.collection.mutable trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { implicit def F: Foldable[F] + def foldRightLazy[A](fa: F[A]): Boolean = { + var i = 0 + F.foldRight(fa, Eval.now("empty")) { (_, _) => + i += 1 + Eval.now("not empty") + } + .value + i == (if (F.isEmpty(fa)) 0 else 1) + } + def leftFoldConsistentWithFoldMap[A, B]( fa: F[A], f: A => B @@ -17,16 +27,6 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { b |+| f(a) } - def foldRightLazy[A](fa: F[A]): Boolean = { - var i = 0 - F.foldRight(fa, Eval.now("empty")) { (_, _) => - i += 1 - Eval.now("not empty") - } - .value - i == (if (F.isEmpty(fa)) 0 else 1) - } - def rightFoldConsistentWithFoldMap[A, B]( fa: F[A], f: A => B From 1e9fcd7a491174d3fadf2863cd37fd2256efc745 Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Sun, 28 Apr 2019 08:54:22 -0400 Subject: [PATCH 5/8] reformat --- laws/src/main/scala/cats/laws/FoldableLaws.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/laws/src/main/scala/cats/laws/FoldableLaws.scala b/laws/src/main/scala/cats/laws/FoldableLaws.scala index d2da8b567a..302d8c45b6 100644 --- a/laws/src/main/scala/cats/laws/FoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/FoldableLaws.scala @@ -11,9 +11,9 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { def foldRightLazy[A](fa: F[A]): Boolean = { var i = 0 F.foldRight(fa, Eval.now("empty")) { (_, _) => - i += 1 - Eval.now("not empty") - } + i += 1 + Eval.now("not empty") + } .value i == (if (F.isEmpty(fa)) 0 else 1) } From f75c18a541dcea6dd4cbb9ad785da31c4424fa50 Mon Sep 17 00:00:00 2001 From: "Kai(luo) Wang" Date: Sun, 28 Apr 2019 14:16:10 -0400 Subject: [PATCH 6/8] Update FoldableLaws.scala --- laws/src/main/scala/cats/laws/FoldableLaws.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/laws/src/main/scala/cats/laws/FoldableLaws.scala b/laws/src/main/scala/cats/laws/FoldableLaws.scala index 302d8c45b6..f23c83615e 100644 --- a/laws/src/main/scala/cats/laws/FoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/FoldableLaws.scala @@ -125,7 +125,7 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { def existsLazy[A](fa: F[A]): Boolean = { var i = 0 F.exists(fa) { _ => - i = i + 1 + i += 1 true } i == (if (F.isEmpty(fa)) 0 else 1) @@ -134,7 +134,7 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { def forallLazy[A](fa: F[A]): Boolean = { var i = 0 F.forall(fa) { _ => - i = i + 1 + i += 1 false } i == (if (F.isEmpty(fa)) 0 else 1) From cc8f633335995402d5ed46e71f3e4a3c5174ee0e Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Sat, 4 May 2019 20:32:44 -0400 Subject: [PATCH 7/8] incorporated Ross and Oscar suggestion --- .../main/scala/cats/UnorderedFoldable.scala | 28 +++++++++++-------- .../main/scala/cats/laws/FoldableLaws.scala | 19 ------------- .../cats/laws/UnorderedFoldableLaws.scala | 18 ++++++++++++ .../cats/laws/discipline/FoldableTests.scala | 2 -- .../discipline/UnorderedFoldableTests.scala | 4 ++- .../cats/tests/UnorderedFoldableSuite.scala | 5 +++- 6 files changed, 41 insertions(+), 35 deletions(-) diff --git a/core/src/main/scala/cats/UnorderedFoldable.scala b/core/src/main/scala/cats/UnorderedFoldable.scala index a6a88d215a..4336908d1f 100644 --- a/core/src/main/scala/cats/UnorderedFoldable.scala +++ b/core/src/main/scala/cats/UnorderedFoldable.scala @@ -29,7 +29,7 @@ import cats.instances.long._ * If there are no elements, the result is `false`. */ def exists[A](fa: F[A])(p: A => Boolean): Boolean = - unorderedFoldMap(fa)(a => Eval.later(p(a)))(UnorderedFoldable.commutativeMonoidEval(UnorderedFoldable.orMonoid)).value + unorderedFoldMap(fa)(a => Eval.later(p(a)))(UnorderedFoldable.orEvalMonoid).value /** * Check whether all elements satisfy the predicate. @@ -37,7 +37,7 @@ import cats.instances.long._ * If there are no elements, the result is `true`. */ def forall[A](fa: F[A])(p: A => Boolean): Boolean = - unorderedFoldMap(fa)(a => Eval.later(p(a)))(UnorderedFoldable.commutativeMonoidEval(UnorderedFoldable.andMonoid)).value + unorderedFoldMap(fa)(a => Eval.later(p(a)))(UnorderedFoldable.andEvalMonoid).value /** * The size of this UnorderedFoldable. @@ -51,19 +51,23 @@ import cats.instances.long._ } object UnorderedFoldable { - private val orMonoid: CommutativeMonoid[Boolean] = new CommutativeMonoid[Boolean] { - val empty: Boolean = false + private val orEvalMonoid: CommutativeMonoid[Eval[Boolean]] = new CommutativeMonoid[Eval[Boolean]] { + val empty: Eval[Boolean] = Eval.False - def combine(x: Boolean, y: Boolean): Boolean = x || y + def combine(lx: Eval[Boolean], ly: Eval[Boolean]): Eval[Boolean] = + lx.flatMap { + case true => Eval.True + case false => ly + } } - private val andMonoid: CommutativeMonoid[Boolean] = new CommutativeMonoid[Boolean] { - val empty: Boolean = true + private val andEvalMonoid: CommutativeMonoid[Eval[Boolean]] = new CommutativeMonoid[Eval[Boolean]] { + val empty: Eval[Boolean] = Eval.True - def combine(x: Boolean, y: Boolean): Boolean = x && y + def combine(lx: Eval[Boolean], ly: Eval[Boolean]): Eval[Boolean] = + lx.flatMap { + case true => ly + case false => Eval.False + } } - - private def commutativeMonoidEval[A: CommutativeMonoid]: CommutativeMonoid[Eval[A]] = - new EvalMonoid[A] with CommutativeMonoid[Eval[A]] { val algebra = Monoid[A] } - } diff --git a/laws/src/main/scala/cats/laws/FoldableLaws.scala b/laws/src/main/scala/cats/laws/FoldableLaws.scala index f23c83615e..b959ec4be0 100644 --- a/laws/src/main/scala/cats/laws/FoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/FoldableLaws.scala @@ -121,25 +121,6 @@ trait FoldableLaws[F[_]] extends UnorderedFoldableLaws[F] { def orderedConsistency[A: Eq](x: F[A], y: F[A])(implicit ev: Eq[F[A]]): IsEq[List[A]] = if (x === y) (F.toList(x) <-> F.toList(y)) else List.empty[A] <-> List.empty[A] - - def existsLazy[A](fa: F[A]): Boolean = { - var i = 0 - F.exists(fa) { _ => - i += 1 - true - } - i == (if (F.isEmpty(fa)) 0 else 1) - } - - def forallLazy[A](fa: F[A]): Boolean = { - var i = 0 - F.forall(fa) { _ => - i += 1 - false - } - i == (if (F.isEmpty(fa)) 0 else 1) - } - } object FoldableLaws { diff --git a/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala b/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala index 410a09efc1..45b765c7ee 100644 --- a/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/UnorderedFoldableLaws.scala @@ -21,6 +21,24 @@ trait UnorderedFoldableLaws[F[_]] { (F.isEmpty(fa) || F.exists(fa)(p)) } else true // can't test much in this case + def existsLazy[A](fa: F[A]): Boolean = { + var i = 0 + F.exists(fa) { _ => + i += 1 + true + } + i == (if (F.isEmpty(fa)) 0 else 1) + } + + def forallLazy[A](fa: F[A]): Boolean = { + var i = 0 + F.forall(fa) { _ => + i += 1 + false + } + i == (if (F.isEmpty(fa)) 0 else 1) + } + /** * If `F[A]` is empty, forall must return true. */ diff --git a/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala b/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala index c6376695ed..2ef720cef2 100644 --- a/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala @@ -30,8 +30,6 @@ trait FoldableTests[F[_]] extends UnorderedFoldableTests[F] { "foldRight is lazy" -> forAll(laws.foldRightLazy[A] _), "ordered constistency" -> forAll(laws.orderedConsistency[A] _), "exists consistent with find" -> forAll(laws.existsConsistentWithFind[A] _), - "exists is lazy" -> forAll(laws.existsLazy[A] _), - "forall is lazy" -> forAll(laws.forallLazy[A] _), "foldM identity" -> forAll(laws.foldMIdentity[A, B] _), "reduceLeftOption consistent with reduceLeftToOption" -> forAll(laws.reduceLeftOptionConsistentWithReduceLeftToOption[A] _), diff --git a/laws/src/main/scala/cats/laws/discipline/UnorderedFoldableTests.scala b/laws/src/main/scala/cats/laws/discipline/UnorderedFoldableTests.scala index 321e84a5f0..2ec35220de 100644 --- a/laws/src/main/scala/cats/laws/discipline/UnorderedFoldableTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/UnorderedFoldableTests.scala @@ -25,7 +25,9 @@ trait UnorderedFoldableTests[F[_]] extends Laws { "unorderedFold consistent with unorderedFoldMap" -> forAll(laws.unorderedFoldConsistentWithUnorderedFoldMap[A] _), "forall consistent with exists" -> forAll(laws.forallConsistentWithExists[A] _), "forall true if empty" -> forAll(laws.forallEmpty[A] _), - "nonEmpty reference" -> forAll(laws.nonEmptyRef[A] _) + "nonEmpty reference" -> forAll(laws.nonEmptyRef[A] _), + "exists is lazy" -> forAll(laws.existsLazy[A] _), + "forall is lazy" -> forAll(laws.forallLazy[A] _) ) } diff --git a/tests/src/test/scala/cats/tests/UnorderedFoldableSuite.scala b/tests/src/test/scala/cats/tests/UnorderedFoldableSuite.scala index 3970fdc441..e6e85b6282 100644 --- a/tests/src/test/scala/cats/tests/UnorderedFoldableSuite.scala +++ b/tests/src/test/scala/cats/tests/UnorderedFoldableSuite.scala @@ -4,8 +4,10 @@ package tests import org.scalacheck.Arbitrary import cats.instances.all._ import cats.kernel.CommutativeMonoid +import cats.laws.discipline.UnorderedFoldableTests -sealed abstract class UnorderedFoldableSuite[F[_]](name: String)(implicit ArbFString: Arbitrary[F[String]]) +sealed abstract class UnorderedFoldableSuite[F[_]](name: String)(implicit ArbFString: Arbitrary[F[String]], + ArbFInt: Arbitrary[F[Int]]) extends CatsSuite { def iterator[T](fa: F[T]): Iterator[T] @@ -42,6 +44,7 @@ sealed abstract class UnorderedFoldableSuite[F[_]](name: String)(implicit ArbFSt fa.count(Function.const(true)) should ===(fa.size) } } + checkAll("F[Int]", UnorderedFoldableTests[F](instance).unorderedFoldable[Int, Int]) } final class UnorderedFoldableSetSuite extends UnorderedFoldableSuite[Set]("set") { From fbc7cf83169102c4648feaec9ed7e0da8fb6901c Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Sat, 4 May 2019 21:16:43 -0400 Subject: [PATCH 8/8] override default implementation in map and set --- core/src/main/scala/cats/instances/map.scala | 4 ++++ core/src/main/scala/cats/instances/set.scala | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/core/src/main/scala/cats/instances/map.scala b/core/src/main/scala/cats/instances/map.scala index e5669de7c6..ff7dc0c264 100644 --- a/core/src/main/scala/cats/instances/map.scala +++ b/core/src/main/scala/cats/instances/map.scala @@ -84,6 +84,10 @@ trait MapInstances extends cats.kernel.instances.MapInstances { override def unorderedFold[A](fa: Map[K, A])(implicit A: CommutativeMonoid[A]): A = A.combineAll(fa.values) + override def forall[A](fa: Map[K, A])(p: A => Boolean): Boolean = fa.forall(pair => p(pair._2)) + + override def exists[A](fa: Map[K, A])(p: A => Boolean): Boolean = fa.exists(pair => p(pair._2)) + } // scalastyle:on method.length diff --git a/core/src/main/scala/cats/instances/set.scala b/core/src/main/scala/cats/instances/set.scala index ba62b96b63..6abb316fce 100644 --- a/core/src/main/scala/cats/instances/set.scala +++ b/core/src/main/scala/cats/instances/set.scala @@ -32,7 +32,11 @@ trait SetInstances extends cats.kernel.instances.SetInstances { override def forall[A](fa: Set[A])(p: A => Boolean): Boolean = fa.forall(p) + override def exists[A](fa: Set[A])(p: A => Boolean): Boolean = + fa.exists(p) + override def isEmpty[A](fa: Set[A]): Boolean = fa.isEmpty + } implicit def catsStdShowForSet[A: Show]: Show[Set[A]] = new Show[Set[A]] {