From a1705a900ffe2736c9ec689ddbbe14723f0c4f07 Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Sun, 15 Oct 2017 22:55:19 -0400 Subject: [PATCH 01/15] Add SortedMap instances --- core/src/main/scala/cats/instances/all.scala | 1 + .../main/scala/cats/instances/package.scala | 1 + .../main/scala/cats/instances/sortedMap.scala | 173 ++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100644 core/src/main/scala/cats/instances/sortedMap.scala diff --git a/core/src/main/scala/cats/instances/all.scala b/core/src/main/scala/cats/instances/all.scala index a1204d4cc2..f6d6fd4e6b 100644 --- a/core/src/main/scala/cats/instances/all.scala +++ b/core/src/main/scala/cats/instances/all.scala @@ -23,6 +23,7 @@ trait AllInstances with QueueInstances with SemigroupInstances with SetInstances + with SortedMapInstances with StreamInstances with StringInstances with SymbolInstances diff --git a/core/src/main/scala/cats/instances/package.scala b/core/src/main/scala/cats/instances/package.scala index 85dadb037c..31c6f59425 100644 --- a/core/src/main/scala/cats/instances/package.scala +++ b/core/src/main/scala/cats/instances/package.scala @@ -20,6 +20,7 @@ package object instances { object list extends ListInstances object long extends LongInstances object map extends MapInstances + object sortedMap extends SortedMapInstances object monoid extends MonoidInstances object option extends OptionInstances object order extends OrderInstances diff --git a/core/src/main/scala/cats/instances/sortedMap.scala b/core/src/main/scala/cats/instances/sortedMap.scala new file mode 100644 index 0000000000..c4da92a8de --- /dev/null +++ b/core/src/main/scala/cats/instances/sortedMap.scala @@ -0,0 +1,173 @@ +package cats.instances + +import cats.{Always, Applicative, Eval, FlatMap, Foldable, Monoid, Show, Traverse} +import cats.kernel._ +import cats.kernel.instances.StaticMethods + +import scala.annotation.tailrec +import scala.collection.immutable.SortedMap +import scala.collection.mutable + +trait SortedMapInstances extends SortedMapInstances1 { + + implicit def catsKernelStdHashForSortedMap[K: Hash: Order, V: Hash]: Hash[SortedMap[K, V]] = + new SortedMapHash[K, V] + + implicit def catsKernelStdMonoidForSortedMap[K: Order, V: Semigroup]: Monoid[SortedMap[K, V]] = + new SortedMapMonoid[K, V] + + implicit def catsStdShowForSortedMap[A: Order, B](implicit showA: Show[A], showB: Show[B]): Show[SortedMap[A, B]] = + new Show[SortedMap[A, B]] { + def show(m: SortedMap[A, B]): String = + m.iterator + .map { case (a, b) => showA.show(a) + " -> " + showB.show(b) } + .mkString("SortedMap(", ", ", ")") + } + + + // scalastyle:off method.length + implicit def catsStdInstancesForSortedMap[K: Order]: Traverse[SortedMap[K, ?]] with FlatMap[SortedMap[K, ?]] = + new Traverse[SortedMap[K, ?]] with FlatMap[SortedMap[K, ?]] { + + implicit val orderingK: Ordering[K] = Order[K].toOrdering + + def traverse[G[_], A, B](fa: SortedMap[K, A])(f: A => G[B])(implicit G: Applicative[G]): G[SortedMap[K, B]] = { + val gba: Eval[G[SortedMap[K, B]]] = Always(G.pure(SortedMap.empty(Order[K].toOrdering))) + Foldable.iterateRight(fa.iterator, gba){ (kv, lbuf) => + G.map2Eval(f(kv._2), lbuf)({ (b, buf) => buf + (kv._1 -> b)}) + }.value + } + + def flatMap[A, B](fa: SortedMap[K, A])(f: A => SortedMap[K, B]): SortedMap[K, B] = + fa.flatMap { case (k, a) => f(a).get(k).map((k, _)) } + + override def map[A, B](fa: SortedMap[K, A])(f: A => B): SortedMap[K, B] = + fa.map { case (k, a) => (k, f(a)) } + + override def map2[A, B, Z](fa: SortedMap[K, A], fb: SortedMap[K, B])(f: (A, B) => Z): SortedMap[K, Z] = + if (fb.isEmpty) SortedMap.empty(Order[K].toOrdering) // do O(1) work if fb is empty + else fa.flatMap { case (k, a) => fb.get(k).map(b => (k, f(a, b))) } + + override def map2Eval[A, B, Z](fa: SortedMap[K, A], fb: Eval[SortedMap[K, B]])(f: (A, B) => Z): Eval[SortedMap[K, Z]] = + if (fa.isEmpty) Eval.now(SortedMap.empty(Order[K].toOrdering)) // no need to evaluate fb + else fb.map(fb => map2(fa, fb)(f)) + + override def ap[A, B](ff: SortedMap[K, A => B])(fa: SortedMap[K, A]): SortedMap[K, B] = + fa.flatMap { case (k, a) => ff.get(k).map(f => (k, f(a))) }(scala.collection.breakOut) + + override def ap2[A, B, Z](f: SortedMap[K, (A, B) => Z])(fa: SortedMap[K, A], fb: SortedMap[K, B]): SortedMap[K, Z] = + f.flatMap { case (k, f) => + for { a <- fa.get(k); b <- fb.get(k) } yield (k, f(a, b)) + } + + def foldLeft[A, B](fa: SortedMap[K, A], b: B)(f: (B, A) => B): B = + fa.foldLeft(b) { case (x, (k, a)) => f(x, a)} + + def foldRight[A, B](fa: SortedMap[K, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = + Foldable.iterateRight(fa.values.iterator, lb)(f) + + def tailRecM[A, B](a: A)(f: A => SortedMap[K, Either[A, B]]): SortedMap[K, B] = { + val bldr = SortedMap.newBuilder[K, B](Order[K].toOrdering) + + @tailrec def descend(k: K, either: Either[A, B]): Unit = + either match { + case Left(a) => + f(a).get(k) match { + case Some(x) => descend(k, x) + case None => () + } + case Right(b) => + bldr += ((k, b)) + () + } + + f(a).foreach { case (k, a) => descend(k, a) } + bldr.result + } + + override def size[A](fa: SortedMap[K, A]): Long = fa.size.toLong + + override def get[A](fa: SortedMap[K, A])(idx: Long): Option[A] = + if (idx < 0L || Int.MaxValue < idx) None + else { + val n = idx.toInt + if (n >= fa.size) None + else Some(fa.valuesIterator.drop(n).next) + } + + override def isEmpty[A](fa: SortedMap[K, A]): Boolean = fa.isEmpty + + override def fold[A](fa: SortedMap[K, A])(implicit A: Monoid[A]): A = + A.combineAll(fa.values) + + override def toList[A](fa: SortedMap[K, A]): List[A] = fa.values.toList + } + +} + +trait SortedMapInstances1 { + implicit def catsKernelStdEqForSortedMap[K: Order, V: Eq]: Eq[SortedMap[K, V]] = + new SortedMapEq[K, V] +} + +class SortedMapHash[K, V](implicit V: Hash[V], O: Order[K]) extends SortedMapEq[K, V]()(V, O) with Hash[SortedMap[K, V]] { + // adapted from [[scala.util.hashing.MurmurHash3]], + // but modified standard `Any#hashCode` to `ev.hash`. + import scala.util.hashing.MurmurHash3._ + def hash(x: SortedMap[K, V]): Int = { + var a, b, n = 0 + var c = 1; + x foreach { case (k, v) => + // use the default hash on keys because that's what Scala's Map does + val h = StaticMethods.product2Hash(k.hashCode(), V.hash(v)) + a += h + b ^= h + if (h != 0) c *= h + n += 1 + } + var h = mapSeed + h = mix(h, a) + h = mix(h, b) + h = mixLast(h, c) + finalizeHash(h, n) + } +} + +class SortedMapEq[K, V](implicit V: Eq[V], O: Order[K]) extends Eq[SortedMap[K, V]] { + def eqv(x: SortedMap[K, V], y: SortedMap[K, V]): Boolean = + if (x eq y) true + else x.size == y.size && x.forall { case (k, v1) => + y.get(k) match { + case Some(v2) => V.eqv(v1, v2) + case None => false + } + } +} + +class SortedMapMonoid[K, V](implicit V: Semigroup[V], O: Order[K]) extends Monoid[SortedMap[K, V]] { + + def empty: SortedMap[K, V] = SortedMap.empty(O.toOrdering) + + def combine(xs: SortedMap[K, V], ys: SortedMap[K, V]): SortedMap[K, V] = + if (xs.size <= ys.size) { + xs.foldLeft(ys) { case (my, (k, x)) => + my.updated(k, Semigroup.maybeCombine(x, my.get(k))) + } + } else { + ys.foldLeft(xs) { case (mx, (k, y)) => + mx.updated(k, Semigroup.maybeCombine(mx.get(k), y)) + } + } + + override def combineAll(xss: TraversableOnce[SortedMap[K, V]]): SortedMap[K, V] = { + val acc = mutable.SortedMap.empty[K, V](O.toOrdering) + xss.foreach { m => + val it = m.iterator + while (it.hasNext) { + val (k, v) = it.next + acc(k) = Semigroup.maybeCombine(acc.get(k), v) + } + } + SortedMap.empty[K, V](O.toOrdering) ++ acc + } +} From 2ec0b0aa88f30321e6f5fc78496c7c1866c5e309 Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Mon, 16 Oct 2017 10:54:14 -0400 Subject: [PATCH 02/15] Add law tests --- .../scala/cats/kernel/laws/LawTests.scala | 2 -- .../scala/cats/tests/SortedMapTests.scala | 32 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 tests/src/test/scala/cats/tests/SortedMapTests.scala diff --git a/kernel-laws/src/test/scala/cats/kernel/laws/LawTests.scala b/kernel-laws/src/test/scala/cats/kernel/laws/LawTests.scala index bc5f011e09..232e82e4f0 100644 --- a/kernel-laws/src/test/scala/cats/kernel/laws/LawTests.scala +++ b/kernel-laws/src/test/scala/cats/kernel/laws/LawTests.scala @@ -153,8 +153,6 @@ class LawTests extends FunSuite with Discipline { checkAll("Monoid[Stream[Int]]", SerializableTests.serializable(Monoid[Stream[Int]])) checkAll("Monoid[List[String]]", MonoidLawTests[List[String]].monoid) checkAll("Monoid[List[String]]", SerializableTests.serializable(Monoid[List[String]])) - checkAll("Monoid[Map[String, Int]]", MonoidLawTests[Map[String, Int]].monoid) - checkAll("Monoid[Map[String, Int]]", SerializableTests.serializable(Monoid[Map[String, Int]])) checkAll("Monoid[Queue[Int]]", MonoidLawTests[Queue[Int]].monoid) checkAll("Monoid[Queue[Int]]", SerializableTests.serializable(Monoid[Queue[Int]])) diff --git a/tests/src/test/scala/cats/tests/SortedMapTests.scala b/tests/src/test/scala/cats/tests/SortedMapTests.scala new file mode 100644 index 0000000000..938161527e --- /dev/null +++ b/tests/src/test/scala/cats/tests/SortedMapTests.scala @@ -0,0 +1,32 @@ +package cats +package tests + +import cats.kernel.laws.discipline.{HashTests => HashLawTests, MonoidLawTests} +import cats.laws.discipline.{FlatMapTests, SemigroupalTests, SerializableTests, TraverseTests} + +import scala.collection.immutable.SortedMap + +class SortedMapTests extends CatsSuite { + implicit val iso = SemigroupalTests.Isomorphisms.invariant[SortedMap[Int, ?]] + + checkAll("SortedMap[Int, Int]", SemigroupalTests[SortedMap[Int, ?]].semigroupal[Int, Int, Int]) + checkAll("Semigroupal[SortedMap[Int, ?]]", SerializableTests.serializable(Semigroupal[SortedMap[Int, ?]])) + + checkAll("SortedMap[Int, Int]", FlatMapTests[SortedMap[Int, ?]].flatMap[Int, Int, Int]) + checkAll("FlatMap[SortedMap[Int, ?]]", SerializableTests.serializable(FlatMap[SortedMap[Int, ?]])) + + checkAll("SortedMap[Int, Int] with Option", TraverseTests[SortedMap[Int, ?]].traverse[Int, Int, Int, Int, Option, Option]) + checkAll("Traverse[SortedMap[Int, ?]]", SerializableTests.serializable(Traverse[SortedMap[Int, ?]])) + + test("show isn't empty and is formatted as expected") { + forAll { (map: SortedMap[Int, String]) => + map.show.nonEmpty should === (true) + map.show.startsWith("SortedMap(") should === (true) + map.show should === (implicitly[Show[SortedMap[Int, String]]].show(map)) + } + } + + checkAll("Hash[Map[Int, String]]" , HashLawTests[Map[Int, String]].hash) + checkAll("Monoid[Map[String, Int]]", MonoidLawTests[Map[String, Int]].monoid) + checkAll("Monoid[Map[String, Int]]", SerializableTests.serializable(Monoid[Map[String, Int]])) +} From 62ec8d4aa93fe80eb346310dc529cbd7397a908c Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Mon, 16 Oct 2017 12:18:02 -0400 Subject: [PATCH 03/15] Remove optimized combineAll --- .../main/scala/cats/instances/sortedMap.scala | 24 +++---------------- .../cats/laws/discipline/Arbitrary.scala | 11 +++++++++ .../scala/cats/tests/SortedMapTests.scala | 8 +++---- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/core/src/main/scala/cats/instances/sortedMap.scala b/core/src/main/scala/cats/instances/sortedMap.scala index c4da92a8de..5d17a7d8fb 100644 --- a/core/src/main/scala/cats/instances/sortedMap.scala +++ b/core/src/main/scala/cats/instances/sortedMap.scala @@ -6,14 +6,13 @@ import cats.kernel.instances.StaticMethods import scala.annotation.tailrec import scala.collection.immutable.SortedMap -import scala.collection.mutable trait SortedMapInstances extends SortedMapInstances1 { - implicit def catsKernelStdHashForSortedMap[K: Hash: Order, V: Hash]: Hash[SortedMap[K, V]] = + implicit def catsStdHashForSortedMap[K: Hash: Order, V: Hash]: Hash[SortedMap[K, V]] = new SortedMapHash[K, V] - implicit def catsKernelStdMonoidForSortedMap[K: Order, V: Semigroup]: Monoid[SortedMap[K, V]] = + implicit def catsStdMonoidForSortedMap[K: Order, V: Semigroup]: Monoid[SortedMap[K, V]] = new SortedMapMonoid[K, V] implicit def catsStdShowForSortedMap[A: Order, B](implicit showA: Show[A], showB: Show[B]): Show[SortedMap[A, B]] = @@ -44,17 +43,11 @@ trait SortedMapInstances extends SortedMapInstances1 { override def map[A, B](fa: SortedMap[K, A])(f: A => B): SortedMap[K, B] = fa.map { case (k, a) => (k, f(a)) } - override def map2[A, B, Z](fa: SortedMap[K, A], fb: SortedMap[K, B])(f: (A, B) => Z): SortedMap[K, Z] = - if (fb.isEmpty) SortedMap.empty(Order[K].toOrdering) // do O(1) work if fb is empty - else fa.flatMap { case (k, a) => fb.get(k).map(b => (k, f(a, b))) } override def map2Eval[A, B, Z](fa: SortedMap[K, A], fb: Eval[SortedMap[K, B]])(f: (A, B) => Z): Eval[SortedMap[K, Z]] = if (fa.isEmpty) Eval.now(SortedMap.empty(Order[K].toOrdering)) // no need to evaluate fb else fb.map(fb => map2(fa, fb)(f)) - override def ap[A, B](ff: SortedMap[K, A => B])(fa: SortedMap[K, A]): SortedMap[K, B] = - fa.flatMap { case (k, a) => ff.get(k).map(f => (k, f(a))) }(scala.collection.breakOut) - override def ap2[A, B, Z](f: SortedMap[K, (A, B) => Z])(fa: SortedMap[K, A], fb: SortedMap[K, B]): SortedMap[K, Z] = f.flatMap { case (k, f) => for { a <- fa.get(k); b <- fb.get(k) } yield (k, f(a, b)) @@ -110,7 +103,7 @@ trait SortedMapInstances1 { new SortedMapEq[K, V] } -class SortedMapHash[K, V](implicit V: Hash[V], O: Order[K]) extends SortedMapEq[K, V]()(V, O) with Hash[SortedMap[K, V]] { +class SortedMapHash[K, V](implicit V: Hash[V], O: Order[K], K: Hash[K]) extends SortedMapEq[K, V]()(V, O) with Hash[SortedMap[K, V]] { // adapted from [[scala.util.hashing.MurmurHash3]], // but modified standard `Any#hashCode` to `ev.hash`. import scala.util.hashing.MurmurHash3._ @@ -159,15 +152,4 @@ class SortedMapMonoid[K, V](implicit V: Semigroup[V], O: Order[K]) extends Monoi } } - override def combineAll(xss: TraversableOnce[SortedMap[K, V]]): SortedMap[K, V] = { - val acc = mutable.SortedMap.empty[K, V](O.toOrdering) - xss.foreach { m => - val it = m.iterator - while (it.hasNext) { - val (k, v) = it.next - acc(k) = Semigroup.maybeCombine(acc.get(k), v) - } - } - SortedMap.empty[K, V](O.toOrdering) ++ acc - } } diff --git a/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala b/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala index 2de9d40bdf..5c8e54a05e 100644 --- a/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala +++ b/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala @@ -3,6 +3,7 @@ package laws package discipline import scala.util.{Failure, Success, Try} +import scala.collection.immutable.SortedMap import cats.data._ import org.scalacheck.{Arbitrary, Cogen, Gen} import org.scalacheck.Arbitrary.{arbitrary => getArbitrary} @@ -159,6 +160,16 @@ object arbitrary extends ArbitraryInstances0 { def compare(x: A, y: A): Int = java.lang.Integer.compare(f(x.##), f(y.##)) })) + implicit def catsLawsArbitraryForSortedMap[K: Arbitrary, V: Arbitrary]: Arbitrary[SortedMap[K, V]] = + Arbitrary(getArbitrary[Map[K, V]].flatMap(m => implicitly[Arbitrary[Order[K]]].arbitrary.map(o => SortedMap[K, V]()(o.toOrdering) ++ m))) + + implicit def catsLawsCogenForSortedMap[K: Order: Cogen, V: Order: Cogen]: Cogen[SortedMap[K, V]] = { + implicit val orderingK = Order[K].toOrdering + implicit val orderingV = Order[V].toOrdering + + implicitly[Cogen[Map[K, V]]].contramap(_.toMap) + } + implicit def catsLawsArbitraryForOrdering[A: Arbitrary]: Arbitrary[Ordering[A]] = Arbitrary(getArbitrary[Order[A]].map(Order.catsKernelOrderingForOrder(_))) diff --git a/tests/src/test/scala/cats/tests/SortedMapTests.scala b/tests/src/test/scala/cats/tests/SortedMapTests.scala index 0f6d068c78..dfde9c6a75 100644 --- a/tests/src/test/scala/cats/tests/SortedMapTests.scala +++ b/tests/src/test/scala/cats/tests/SortedMapTests.scala @@ -3,7 +3,7 @@ package tests import cats.kernel.laws.discipline.{HashTests => HashLawTests, MonoidTests => MonoidLawTests} import cats.laws.discipline.{FlatMapTests, SemigroupalTests, SerializableTests, TraverseTests} - +import cats.laws.discipline.arbitrary._ import scala.collection.immutable.SortedMap class SortedMapTests extends CatsSuite { @@ -26,7 +26,7 @@ class SortedMapTests extends CatsSuite { } } - checkAll("Hash[Map[Int, String]]" , HashLawTests[Map[Int, String]].hash) - checkAll("Monoid[Map[String, Int]]", MonoidLawTests[Map[String, Int]].monoid) - checkAll("Monoid[Map[String, Int]]", SerializableTests.serializable(Monoid[Map[String, Int]])) + checkAll("Hash[SortedMap[Int, String]]" , HashLawTests[SortedMap[Int, String]].hash) + checkAll("Monoid[SortedMap[String, Int]]", MonoidLawTests[SortedMap[String, Int]].monoid) + checkAll("Monoid[SortedMap[String, Int]]", SerializableTests.serializable(Monoid[SortedMap[String, Int]])) } From ec82e5f4f7b905338b3c33d28d2dd5bca65bd25a Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Wed, 18 Oct 2017 19:34:06 -0400 Subject: [PATCH 04/15] Add SortedSet instances --- core/src/main/scala/cats/instances/all.scala | 1 + .../main/scala/cats/instances/package.scala | 3 +- .../main/scala/cats/instances/sortedSet.scala | 96 +++++++++++++++++++ .../cats/laws/discipline/Arbitrary.scala | 11 ++- .../scala/cats/tests/SortedSetTests.scala | 38 ++++++++ 5 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 core/src/main/scala/cats/instances/sortedSet.scala create mode 100644 tests/src/test/scala/cats/tests/SortedSetTests.scala diff --git a/core/src/main/scala/cats/instances/all.scala b/core/src/main/scala/cats/instances/all.scala index f6d6fd4e6b..51393e0047 100644 --- a/core/src/main/scala/cats/instances/all.scala +++ b/core/src/main/scala/cats/instances/all.scala @@ -24,6 +24,7 @@ trait AllInstances with SemigroupInstances with SetInstances with SortedMapInstances + with SortedSetInstances with StreamInstances with StringInstances with SymbolInstances diff --git a/core/src/main/scala/cats/instances/package.scala b/core/src/main/scala/cats/instances/package.scala index 31c6f59425..614aa7306e 100644 --- a/core/src/main/scala/cats/instances/package.scala +++ b/core/src/main/scala/cats/instances/package.scala @@ -20,7 +20,6 @@ package object instances { object list extends ListInstances object long extends LongInstances object map extends MapInstances - object sortedMap extends SortedMapInstances object monoid extends MonoidInstances object option extends OptionInstances object order extends OrderInstances @@ -31,6 +30,8 @@ package object instances { object semigroup extends SemigroupInstances object set extends SetInstances object short extends ShortInstances + object sortedMap extends SortedMapInstances + object sortedSet extends SortedSetInstances object stream extends StreamInstances object string extends StringInstances object try_ extends TryInstances diff --git a/core/src/main/scala/cats/instances/sortedSet.scala b/core/src/main/scala/cats/instances/sortedSet.scala new file mode 100644 index 0000000000..7c2158c6ba --- /dev/null +++ b/core/src/main/scala/cats/instances/sortedSet.scala @@ -0,0 +1,96 @@ +package cats +package instances + +import cats.kernel.{BoundedSemilattice, Hash, PartialOrder} +import scala.collection.immutable.SortedSet +import scala.annotation.tailrec +import cats.syntax.show._ + +trait SortedSetInstances extends SortedSetInstances1 { + + implicit val catsStdInstancesForSortedSet: Foldable[SortedSet] with SemigroupK[SortedSet] = + new Foldable[SortedSet] with SemigroupK[SortedSet] { + + def combineK[A](x: SortedSet[A], y: SortedSet[A]): SortedSet[A] = x | y + + def foldLeft[A, B](fa: SortedSet[A], b: B)(f: (B, A) => B): B = + fa.foldLeft(b)(f) + + def foldRight[A, B](fa: SortedSet[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = + Foldable.iterateRight(fa.iterator, lb)(f) + + override def get[A](fa: SortedSet[A])(idx: Long): Option[A] = { + @tailrec + def go(idx: Int, it: Iterator[A]): Option[A] = { + if (it.hasNext) { + if (idx == 0) Some(it.next) else { + it.next + go(idx - 1, it) + } + } else None + } + if (idx < Int.MaxValue && idx >= 0L) go(idx.toInt, fa.toIterator) else None + } + + override def size[A](fa: SortedSet[A]): Long = fa.size.toLong + + override def exists[A](fa: SortedSet[A])(p: A => Boolean): Boolean = + fa.exists(p) + + override def forall[A](fa: SortedSet[A])(p: A => Boolean): Boolean = + fa.forall(p) + + override def isEmpty[A](fa: SortedSet[A]): Boolean = fa.isEmpty + + override def fold[A](fa: SortedSet[A])(implicit A: Monoid[A]): A = A.combineAll(fa) + + override def toList[A](fa: SortedSet[A]): List[A] = fa.toList + + override def reduceLeftOption[A](fa: SortedSet[A])(f: (A, A) => A): Option[A] = + fa.reduceLeftOption(f) + + override def find[A](fa: SortedSet[A])(f: A => Boolean): Option[A] = fa.find(f) + } + + implicit def catsStdShowForSortedSet[A:Show]: Show[SortedSet[A]] = new Show[SortedSet[A]] { + def show(fa: SortedSet[A]): String = + fa.toIterator.map(_.show).mkString("SortedSet(", ", ", ")") + } + + implicit def catsKernelStdHashForSortedSet[A]: Hash[SortedSet[A]] = + new SortedSetHash[A] +} + +trait SortedSetInstances1 { + implicit def catsKernelStdPartialOrderForSortedSet[A]: PartialOrder[SortedSet[A]] = + new SortedSetPartialOrder[A] + + implicit def catsKernelStdSemilatticeForSortedSet[A: Order]: BoundedSemilattice[SortedSet[A]] = + new SortedSetSemilattice[A] +} + +class SortedSetPartialOrder[A] extends PartialOrder[SortedSet[A]] { + def partialCompare(x: SortedSet[A], y: SortedSet[A]): Double = + if (x eq y) 0.0 + else if (x.size < y.size) if (x.subsetOf(y)) -1.0 else Double.NaN + else if (y.size < x.size) if (y.subsetOf(x)) 1.0 else Double.NaN + else if (x == y) 0.0 + else Double.NaN + + // Does not require an Eq on elements: Scala sets must use the universal `equals`. + override def eqv(x: SortedSet[A], y: SortedSet[A]): Boolean = x == y +} + +class SortedSetHash[A] extends Hash[SortedSet[A]] { + // Does not require a Hash on elements: Scala sets must use the universal `hashCode`. + def hash(x: SortedSet[A]): Int = x.hashCode() + + // Does not require an Eq on elements: Scala sets must use the universal `equals`. + def eqv(x: SortedSet[A], y: SortedSet[A]): Boolean = x == y +} + +class SortedSetSemilattice[A: Order] extends BoundedSemilattice[SortedSet[A]] { + def empty: SortedSet[A] = SortedSet.empty(implicitly[Order[A]].toOrdering) + def combine(x: SortedSet[A], y: SortedSet[A]): SortedSet[A] = x | y +} + diff --git a/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala b/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala index 5c8e54a05e..b5b116c84c 100644 --- a/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala +++ b/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala @@ -3,7 +3,7 @@ package laws package discipline import scala.util.{Failure, Success, Try} -import scala.collection.immutable.SortedMap +import scala.collection.immutable.{SortedMap, SortedSet} import cats.data._ import org.scalacheck.{Arbitrary, Cogen, Gen} import org.scalacheck.Arbitrary.{arbitrary => getArbitrary} @@ -170,6 +170,15 @@ object arbitrary extends ArbitraryInstances0 { implicitly[Cogen[Map[K, V]]].contramap(_.toMap) } + implicit def catsLawsArbitraryForSortedSet[A: Arbitrary]: Arbitrary[SortedSet[A]] = + Arbitrary(getArbitrary[Set[A]].flatMap(s => implicitly[Arbitrary[Order[A]]].arbitrary.map(o => SortedSet[A]()(o.toOrdering) ++ s))) + + implicit def catsLawsCogenForSortedSet[A: Order: Cogen]: Cogen[SortedSet[A]] = { + implicit val orderingA = Order[A].toOrdering + + implicitly[Cogen[Set[A]]].contramap(_.toSet) + } + implicit def catsLawsArbitraryForOrdering[A: Arbitrary]: Arbitrary[Ordering[A]] = Arbitrary(getArbitrary[Order[A]].map(Order.catsKernelOrderingForOrder(_))) diff --git a/tests/src/test/scala/cats/tests/SortedSetTests.scala b/tests/src/test/scala/cats/tests/SortedSetTests.scala new file mode 100644 index 0000000000..b6ab8e1a4b --- /dev/null +++ b/tests/src/test/scala/cats/tests/SortedSetTests.scala @@ -0,0 +1,38 @@ +package cats +package tests + +import cats.kernel.{BoundedSemilattice, Semilattice} +import cats.laws.discipline.{FoldableTests, SemigroupKTests, SerializableTests} +import cats.kernel.laws.discipline.{BoundedSemilatticeTests, HashTests => HashLawTests, MonoidTests => MonoidLawTests, PartialOrderTests => PartialOrderLawTests} +import cats.laws.discipline.arbitrary._ + +import scala.collection.immutable.SortedSet + +class SortedSetTests extends CatsSuite { + checkAll("SortedSet[Int]", MonoidLawTests[SortedSet[Int]].monoid) + + checkAll("SortedSet[Int]", SemigroupKTests[SortedSet].semigroupK[Int]) + checkAll("SemigroupK[SortedSet]", SerializableTests.serializable(SemigroupK[SortedSet])) + + checkAll("SortedSet[Int]", FoldableTests[SortedSet].foldable[Int, Int]) + checkAll("PartialOrder[SortedSet[Int]]", PartialOrderLawTests[SortedSet[Int]].partialOrder) + checkAll("PartialOrder[SortedSet[Int]].reverse", PartialOrderLawTests(PartialOrder[SortedSet[Int]].reverse).partialOrder) + checkAll("PartialOrder[SortedSet[Int]].reverse.reverse", PartialOrderLawTests(PartialOrder[SortedSet[Int]].reverse.reverse).partialOrder) + + checkAll("BoundedSemilattice[SortedSet[Int]]", BoundedSemilatticeTests[SortedSet[Int]].boundedSemilattice) + checkAll("BoundedSemilattice[SortedSet[Int]]", SerializableTests.serializable(BoundedSemilattice[SortedSet[Int]])) + + checkAll("Semilattice.asMeetPartialOrder[SortedSet[Int]]", PartialOrderLawTests(Semilattice.asMeetPartialOrder[SortedSet[Int]]).partialOrder) + checkAll("Semilattice.asJoinPartialOrder[SortedSet[Int]]", PartialOrderLawTests(Semilattice.asJoinPartialOrder[SortedSet[Int]]).partialOrder) + checkAll("Hash[SortedSet[Int]]" , HashLawTests[SortedSet[Int]].hash) + + + test("show keeps separate entries for items that map to identical strings"){ + //note: this val name has to be the same to shadow the cats.instances instance + implicit val catsStdShowForInt: Show[Int] = Show.show(_ => "1") + // an implementation implemented as set.map(_.show).mkString(", ") would + // only show one entry in the result instead of 3, because SortedSet.map combines + // duplicate items in the codomain. + SortedSet(1, 2, 3).show should === ("SortedSet(1, 1, 1)") + } +} \ No newline at end of file From 1bf9b9482d4ac7eea5f2bf44cd65d1bfdde35e02 Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Wed, 18 Oct 2017 20:20:24 -0400 Subject: [PATCH 05/15] Try to fix some tests --- .../main/scala/cats/instances/sortedMap.scala | 4 +- .../main/scala/cats/instances/sortedSet.scala | 45 ++++++++++--------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/core/src/main/scala/cats/instances/sortedMap.scala b/core/src/main/scala/cats/instances/sortedMap.scala index 5d17a7d8fb..41fbb1081e 100644 --- a/core/src/main/scala/cats/instances/sortedMap.scala +++ b/core/src/main/scala/cats/instances/sortedMap.scala @@ -23,7 +23,6 @@ trait SortedMapInstances extends SortedMapInstances1 { .mkString("SortedMap(", ", ", ")") } - // scalastyle:off method.length implicit def catsStdInstancesForSortedMap[K: Order]: Traverse[SortedMap[K, ?]] with FlatMap[SortedMap[K, ?]] = new Traverse[SortedMap[K, ?]] with FlatMap[SortedMap[K, ?]] { @@ -111,8 +110,7 @@ class SortedMapHash[K, V](implicit V: Hash[V], O: Order[K], K: Hash[K]) extends var a, b, n = 0 var c = 1; x foreach { case (k, v) => - // use the default hash on keys because that's what Scala's Map does - val h = StaticMethods.product2Hash(k.hashCode(), V.hash(v)) + val h = StaticMethods.product2Hash(K.hash(k), V.hash(v)) a += h b ^= h if (h != 0) c *= h diff --git a/core/src/main/scala/cats/instances/sortedSet.scala b/core/src/main/scala/cats/instances/sortedSet.scala index 7c2158c6ba..cad6f9af04 100644 --- a/core/src/main/scala/cats/instances/sortedSet.scala +++ b/core/src/main/scala/cats/instances/sortedSet.scala @@ -1,10 +1,10 @@ package cats package instances -import cats.kernel.{BoundedSemilattice, Hash, PartialOrder} +import cats.kernel.{BoundedSemilattice, Hash, Order} import scala.collection.immutable.SortedSet import scala.annotation.tailrec -import cats.syntax.show._ +import cats.implicits._ trait SortedSetInstances extends SortedSetInstances1 { @@ -52,41 +52,46 @@ trait SortedSetInstances extends SortedSetInstances1 { override def find[A](fa: SortedSet[A])(f: A => Boolean): Option[A] = fa.find(f) } - implicit def catsStdShowForSortedSet[A:Show]: Show[SortedSet[A]] = new Show[SortedSet[A]] { + implicit def catsStdShowForSortedSet[A: Show]: Show[SortedSet[A]] = new Show[SortedSet[A]] { def show(fa: SortedSet[A]): String = fa.toIterator.map(_.show).mkString("SortedSet(", ", ", ")") } - implicit def catsKernelStdHashForSortedSet[A]: Hash[SortedSet[A]] = - new SortedSetHash[A] + implicit def catsKernelStdOrderForSortedSet[A: Order]: Order[SortedSet[A]] = + new SortedSetOrder[A] } trait SortedSetInstances1 { - implicit def catsKernelStdPartialOrderForSortedSet[A]: PartialOrder[SortedSet[A]] = - new SortedSetPartialOrder[A] + implicit def catsKernelStdHashForSortedSet[A: Order: Hash]: Hash[SortedSet[A]] = + new SortedSetHash[A] implicit def catsKernelStdSemilatticeForSortedSet[A: Order]: BoundedSemilattice[SortedSet[A]] = new SortedSetSemilattice[A] } -class SortedSetPartialOrder[A] extends PartialOrder[SortedSet[A]] { - def partialCompare(x: SortedSet[A], y: SortedSet[A]): Double = - if (x eq y) 0.0 - else if (x.size < y.size) if (x.subsetOf(y)) -1.0 else Double.NaN - else if (y.size < x.size) if (y.subsetOf(x)) 1.0 else Double.NaN - else if (x == y) 0.0 - else Double.NaN +class SortedSetOrder[A: Order] extends Order[SortedSet[A]] { + def compare(a1: SortedSet[A], a2: SortedSet[A]) = { - // Does not require an Eq on elements: Scala sets must use the universal `equals`. - override def eqv(x: SortedSet[A], y: SortedSet[A]): Boolean = x == y + Order[Int].compare(a1.size, a2.size) match { + case 0 => Order.compare(a1.toStream, a2.toStream) + case x => x + } + } + + override def eqv(s1: SortedSet[A], s2: SortedSet[A]) = { + implicit val x = Order[A].toOrdering + s1.toStream.corresponds(s2.toStream)(Order[A].eqv) + } } -class SortedSetHash[A] extends Hash[SortedSet[A]] { - // Does not require a Hash on elements: Scala sets must use the universal `hashCode`. +class SortedSetHash[A: Order: Hash] extends Hash[SortedSet[A]] { + // TODO replace def hash(x: SortedSet[A]): Int = x.hashCode() - // Does not require an Eq on elements: Scala sets must use the universal `equals`. - def eqv(x: SortedSet[A], y: SortedSet[A]): Boolean = x == y + override def eqv(s1: SortedSet[A], s2: SortedSet[A]) = { + implicit val x = Order[A].toOrdering + s1.toStream.corresponds(s2.toStream)(Order[A].eqv) + } } class SortedSetSemilattice[A: Order] extends BoundedSemilattice[SortedSet[A]] { From a5948aa3e8714d9a41e277f438b3031098dc49bb Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Tue, 24 Oct 2017 18:42:19 +0200 Subject: [PATCH 06/15] Rename --- .../main/scala/cats/instances/sortedMap.scala | 2 +- ...tedMapTests.scala => SortedMapSuite.scala} | 9 +++---- ...tedSetTests.scala => SortedSetSuite.scala} | 24 +++++++++---------- 3 files changed, 17 insertions(+), 18 deletions(-) rename tests/src/test/scala/cats/tests/{SortedMapTests.scala => SortedMapSuite.scala} (80%) rename tests/src/test/scala/cats/tests/{SortedSetTests.scala => SortedSetSuite.scala} (56%) diff --git a/core/src/main/scala/cats/instances/sortedMap.scala b/core/src/main/scala/cats/instances/sortedMap.scala index 6aa71437fe..66ddfb41d1 100644 --- a/core/src/main/scala/cats/instances/sortedMap.scala +++ b/core/src/main/scala/cats/instances/sortedMap.scala @@ -98,7 +98,7 @@ trait SortedMapInstances extends SortedMapInstances1 { } trait SortedMapInstances1 { - implicit def catsKernelStdEqForSortedMap[K: Order, V: Eq]: Eq[SortedMap[K, V]] = + implicit def catsStdEqForSortedMap[K: Order, V: Eq]: Eq[SortedMap[K, V]] = new SortedMapEq[K, V] } diff --git a/tests/src/test/scala/cats/tests/SortedMapTests.scala b/tests/src/test/scala/cats/tests/SortedMapSuite.scala similarity index 80% rename from tests/src/test/scala/cats/tests/SortedMapTests.scala rename to tests/src/test/scala/cats/tests/SortedMapSuite.scala index dfde9c6a75..118fb7afe5 100644 --- a/tests/src/test/scala/cats/tests/SortedMapTests.scala +++ b/tests/src/test/scala/cats/tests/SortedMapSuite.scala @@ -1,12 +1,13 @@ package cats package tests -import cats.kernel.laws.discipline.{HashTests => HashLawTests, MonoidTests => MonoidLawTests} +import cats.kernel.laws.discipline.{HashTests, MonoidTests} import cats.laws.discipline.{FlatMapTests, SemigroupalTests, SerializableTests, TraverseTests} import cats.laws.discipline.arbitrary._ + import scala.collection.immutable.SortedMap -class SortedMapTests extends CatsSuite { +class SortedMapSuite extends CatsSuite { implicit val iso = SemigroupalTests.Isomorphisms.invariant[SortedMap[Int, ?]] checkAll("SortedMap[Int, Int]", SemigroupalTests[SortedMap[Int, ?]].semigroupal[Int, Int, Int]) @@ -26,7 +27,7 @@ class SortedMapTests extends CatsSuite { } } - checkAll("Hash[SortedMap[Int, String]]" , HashLawTests[SortedMap[Int, String]].hash) - checkAll("Monoid[SortedMap[String, Int]]", MonoidLawTests[SortedMap[String, Int]].monoid) + checkAll("Hash[SortedMap[Int, String]]" , HashTests[SortedMap[Int, String]].hash) + checkAll("Monoid[SortedMap[String, Int]]", MonoidTests[SortedMap[String, Int]].monoid) checkAll("Monoid[SortedMap[String, Int]]", SerializableTests.serializable(Monoid[SortedMap[String, Int]])) } diff --git a/tests/src/test/scala/cats/tests/SortedSetTests.scala b/tests/src/test/scala/cats/tests/SortedSetSuite.scala similarity index 56% rename from tests/src/test/scala/cats/tests/SortedSetTests.scala rename to tests/src/test/scala/cats/tests/SortedSetSuite.scala index b6ab8e1a4b..fa2ac4cc80 100644 --- a/tests/src/test/scala/cats/tests/SortedSetTests.scala +++ b/tests/src/test/scala/cats/tests/SortedSetSuite.scala @@ -3,28 +3,26 @@ package tests import cats.kernel.{BoundedSemilattice, Semilattice} import cats.laws.discipline.{FoldableTests, SemigroupKTests, SerializableTests} -import cats.kernel.laws.discipline.{BoundedSemilatticeTests, HashTests => HashLawTests, MonoidTests => MonoidLawTests, PartialOrderTests => PartialOrderLawTests} +import cats.kernel.laws.discipline.{BoundedSemilatticeTests, HashTests, PartialOrderTests} import cats.laws.discipline.arbitrary._ import scala.collection.immutable.SortedSet -class SortedSetTests extends CatsSuite { - checkAll("SortedSet[Int]", MonoidLawTests[SortedSet[Int]].monoid) - +class SortedSetSuite extends CatsSuite { checkAll("SortedSet[Int]", SemigroupKTests[SortedSet].semigroupK[Int]) checkAll("SemigroupK[SortedSet]", SerializableTests.serializable(SemigroupK[SortedSet])) checkAll("SortedSet[Int]", FoldableTests[SortedSet].foldable[Int, Int]) - checkAll("PartialOrder[SortedSet[Int]]", PartialOrderLawTests[SortedSet[Int]].partialOrder) - checkAll("PartialOrder[SortedSet[Int]].reverse", PartialOrderLawTests(PartialOrder[SortedSet[Int]].reverse).partialOrder) - checkAll("PartialOrder[SortedSet[Int]].reverse.reverse", PartialOrderLawTests(PartialOrder[SortedSet[Int]].reverse.reverse).partialOrder) + checkAll("PartialOrder[SortedSet[Int]]", PartialOrderTests[SortedSet[Int]].partialOrder) + checkAll("PartialOrder[SortedSet[Int]].reverse", PartialOrderTests(PartialOrder[SortedSet[Int]].reverse).partialOrder) + checkAll("PartialOrder[SortedSet[Int]].reverse.reverse", PartialOrderTests(PartialOrder[SortedSet[Int]].reverse.reverse).partialOrder) - checkAll("BoundedSemilattice[SortedSet[Int]]", BoundedSemilatticeTests[SortedSet[Int]].boundedSemilattice) - checkAll("BoundedSemilattice[SortedSet[Int]]", SerializableTests.serializable(BoundedSemilattice[SortedSet[Int]])) + checkAll("BoundedSemilattice[SortedSet[String]]", BoundedSemilatticeTests[SortedSet[String]].boundedSemilattice) + checkAll("BoundedSemilattice[SortedSet[String]]", SerializableTests.serializable(BoundedSemilattice[SortedSet[String]])) - checkAll("Semilattice.asMeetPartialOrder[SortedSet[Int]]", PartialOrderLawTests(Semilattice.asMeetPartialOrder[SortedSet[Int]]).partialOrder) - checkAll("Semilattice.asJoinPartialOrder[SortedSet[Int]]", PartialOrderLawTests(Semilattice.asJoinPartialOrder[SortedSet[Int]]).partialOrder) - checkAll("Hash[SortedSet[Int]]" , HashLawTests[SortedSet[Int]].hash) + checkAll("Semilattice.asMeetPartialOrder[SortedSet[Int]]", PartialOrderTests(Semilattice.asMeetPartialOrder[SortedSet[Int]]).partialOrder) + checkAll("Semilattice.asJoinPartialOrder[SortedSet[Int]]", PartialOrderTests(Semilattice.asJoinPartialOrder[SortedSet[Int]]).partialOrder) + checkAll("Hash[SortedSet[Int]]" , HashTests[SortedSet[Int]].hash) test("show keeps separate entries for items that map to identical strings"){ @@ -35,4 +33,4 @@ class SortedSetTests extends CatsSuite { // duplicate items in the codomain. SortedSet(1, 2, 3).show should === ("SortedSet(1, 1, 1)") } -} \ No newline at end of file +} From 4e13e8e2f0adbc87ed1f22c313273abc5468973c Mon Sep 17 00:00:00 2001 From: Kailuo Wang Date: Thu, 26 Oct 2017 16:05:01 -0400 Subject: [PATCH 07/15] wip move map and set instance to alleycats --- .../src/main/scala/alleycats/std/all.scala | 2 +- .../src/main/scala/alleycats/std/map.scala | 46 +++++++++++++++++++ .../src/main/scala/alleycats/std/set.scala | 35 +++++++++++++- .../alleycats/tests/AlleycatsSuite.scala | 7 ++- .../test/scala/alleycats/tests/MapSuite.scala | 9 ++++ .../test/scala/alleycats/tests/SetSuite.scala | 21 +++++++++ .../test/scala/alleycats/tests/SetTests.scala | 14 ------ core/src/main/scala/cats/instances/map.scala | 35 +------------- core/src/main/scala/cats/instances/set.scala | 43 +---------------- 9 files changed, 118 insertions(+), 94 deletions(-) create mode 100644 alleycats-core/src/main/scala/alleycats/std/map.scala create mode 100644 alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala create mode 100644 alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala delete mode 100644 alleycats-tests/src/test/scala/alleycats/tests/SetTests.scala diff --git a/alleycats-core/src/main/scala/alleycats/std/all.scala b/alleycats-core/src/main/scala/alleycats/std/all.scala index ad3f85d0cb..e7eb304881 100644 --- a/alleycats-core/src/main/scala/alleycats/std/all.scala +++ b/alleycats-core/src/main/scala/alleycats/std/all.scala @@ -10,4 +10,4 @@ import export._ SetInstances, TryInstances, IterableInstances -) object all extends LegacySetInstances with LegacyTryInstances with LegacyIterableInstances +) object all extends LegacySetInstances with LegacyTryInstances with LegacyIterableInstances with MapInstances diff --git a/alleycats-core/src/main/scala/alleycats/std/map.scala b/alleycats-core/src/main/scala/alleycats/std/map.scala new file mode 100644 index 0000000000..42317bed29 --- /dev/null +++ b/alleycats-core/src/main/scala/alleycats/std/map.scala @@ -0,0 +1,46 @@ +package alleycats +package std + +import cats._ + +trait MapInstances { + + // toList is inconsistent. See https://github.com/typelevel/cats/issues/1831 + implicit def alleycatsStdInstancesForMap[K]: Traverse[Map[K, ?]] = + new Traverse[Map[K, ?]] { + + def traverse[G[_], A, B](fa: Map[K, A])(f: A => G[B])(implicit G: Applicative[G]): G[Map[K, B]] = { + val gba: Eval[G[Map[K, B]]] = Always(G.pure(Map.empty)) + val gbb = Foldable.iterateRight(fa, gba){ (kv, lbuf) => + G.map2Eval(f(kv._2), lbuf)({ (b, buf) => buf + (kv._1 -> b)}) + }.value + G.map(gbb)(_.toMap) + } + + override def map[A, B](fa: Map[K, A])(f: A => B): Map[K, B] = + fa.map { case (k, a) => (k, f(a)) } + + def foldLeft[A, B](fa: Map[K, A], b: B)(f: (B, A) => B): B = + fa.foldLeft(b) { case (x, (k, a)) => f(x, a)} + + def foldRight[A, B](fa: Map[K, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = + Foldable.iterateRight(fa.values, lb)(f) + + override def size[A](fa: Map[K, A]): Long = fa.size.toLong + + override def get[A](fa: Map[K, A])(idx: Long): Option[A] = + if (idx < 0L || Int.MaxValue < idx) None + else { + val n = idx.toInt + if (n >= fa.size) None + else Some(fa.valuesIterator.drop(n).next) + } + + override def isEmpty[A](fa: Map[K, A]): Boolean = fa.isEmpty + + override def fold[A](fa: Map[K, A])(implicit A: Monoid[A]): A = + A.combineAll(fa.values) + + override def toList[A](fa: Map[K, A]): List[A] = fa.values.toList + } +} diff --git a/alleycats-core/src/main/scala/alleycats/std/set.scala b/alleycats-core/src/main/scala/alleycats/std/set.scala index da851068e4..d5edf04fb4 100644 --- a/alleycats-core/src/main/scala/alleycats/std/set.scala +++ b/alleycats-core/src/main/scala/alleycats/std/set.scala @@ -1,6 +1,6 @@ package alleycats.std -import cats.{Applicative, Eval, Foldable, Monad, Traverse} +import cats.{Applicative, Eval, Foldable, Monad, Monoid, Traverse} import export._ import scala.annotation.tailrec @@ -67,12 +67,45 @@ object SetInstances { fa.foldLeft(b)(f) def foldRight[A, B](fa: Set[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = Foldable.iterateRight(fa, lb)(f) + def traverse[G[_]: Applicative, A, B](sa: Set[A])(f: A => G[B]): G[Set[B]] = { val G = Applicative[G] sa.foldLeft(G.pure(Set.empty[B])) { (buf, a) => G.map2(buf, f(a))(_ + _) } } + + override def get[A](fa: Set[A])(idx: Long): Option[A] = { + @tailrec + def go(idx: Int, it: Iterator[A]): Option[A] = { + if (it.hasNext) { + if (idx == 0) Some(it.next) else { + it.next + go(idx - 1, it) + } + } else None + } + if (idx < Int.MaxValue && idx >= 0L) go(idx.toInt, fa.toIterator) else None + } + + override def size[A](fa: Set[A]): Long = fa.size.toLong + + override def exists[A](fa: Set[A])(p: A => Boolean): Boolean = + fa.exists(p) + + override def forall[A](fa: Set[A])(p: A => Boolean): Boolean = + fa.forall(p) + + override def isEmpty[A](fa: Set[A]): Boolean = fa.isEmpty + + override def fold[A](fa: Set[A])(implicit A: Monoid[A]): A = A.combineAll(fa) + + override def toList[A](fa: Set[A]): List[A] = fa.toList + + override def reduceLeftOption[A](fa: Set[A])(f: (A, A) => A): Option[A] = + fa.reduceLeftOption(f) + + override def find[A](fa: Set[A])(f: A => Boolean): Option[A] = fa.find(f) } } diff --git a/alleycats-tests/src/test/scala/alleycats/tests/AlleycatsSuite.scala b/alleycats-tests/src/test/scala/alleycats/tests/AlleycatsSuite.scala index 9ced1461bc..c076904711 100644 --- a/alleycats-tests/src/test/scala/alleycats/tests/AlleycatsSuite.scala +++ b/alleycats-tests/src/test/scala/alleycats/tests/AlleycatsSuite.scala @@ -2,17 +2,16 @@ package alleycats package tests +import alleycats.std.MapInstances import catalysts.Platform - import cats._ import cats.instances.AllInstances import cats.syntax.{AllSyntax, EqOps} import cats.tests.StrictCatsEquality -import org.scalactic.anyvals.{PosZDouble, PosInt, PosZInt} +import org.scalactic.anyvals.{PosInt, PosZDouble, PosZInt} import org.scalatest.{FunSuite, Matchers} import org.scalatest.prop.{Configuration, GeneratorDrivenPropertyChecks} import org.typelevel.discipline.scalatest.Discipline - import org.scalacheck.{Arbitrary, Gen} import org.scalacheck.Arbitrary.arbitrary @@ -37,7 +36,7 @@ trait TestSettings extends Configuration with Matchers { * An opinionated stack of traits to improve consistency and reduce * boilerplate in Alleycats tests. Derived from Cats. */ -trait AlleycatsSuite extends FunSuite with Matchers with GeneratorDrivenPropertyChecks with Discipline with TestSettings with AllInstances with AllSyntax with TestInstances with StrictCatsEquality { +trait AlleycatsSuite extends FunSuite with Matchers with GeneratorDrivenPropertyChecks with Discipline with TestSettings with AllInstances with AllSyntax with TestInstances with StrictCatsEquality with MapInstances { implicit override val generatorDrivenConfig: PropertyCheckConfiguration = checkConfiguration diff --git a/alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala b/alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala new file mode 100644 index 0000000000..c47bafd72f --- /dev/null +++ b/alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala @@ -0,0 +1,9 @@ +package alleycats.tests + +import cats.laws.discipline.{SerializableTests, TraverseTests} +import cats.Traverse + +class MapSuite extends AlleycatsSuite { + checkAll("Map[Int, Int] with Option", TraverseTests[Map[Int, ?]].traverse[Int, Int, Int, Int, Option, Option]) + checkAll("Traverse[Map[Int, ?]]", SerializableTests.serializable(Traverse[Map[Int, ?]])) +} diff --git a/alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala b/alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala new file mode 100644 index 0000000000..76d48f8bee --- /dev/null +++ b/alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala @@ -0,0 +1,21 @@ +package alleycats.tests + +import alleycats.laws.discipline._ +import cats.Foldable +import cats.kernel.laws.discipline.SerializableTests +import cats.laws.discipline.FoldableTests + +import alleycats.std.all._ + +class SetSuite extends AlleycatsSuite { + + checkAll("FlatMapRec[Set]", FlatMapRecTests[Set].tailRecM[Int]) + + checkAll("Set[Int]", FoldableTests[Set].foldable[Int, Int]) + checkAll("Foldable[Set]", SerializableTests.serializable(Foldable[Set])) + + +} + + + diff --git a/alleycats-tests/src/test/scala/alleycats/tests/SetTests.scala b/alleycats-tests/src/test/scala/alleycats/tests/SetTests.scala deleted file mode 100644 index fd4c7382fb..0000000000 --- a/alleycats-tests/src/test/scala/alleycats/tests/SetTests.scala +++ /dev/null @@ -1,14 +0,0 @@ -package alleycats.tests - -import alleycats.laws.discipline._ - -import alleycats.std.all._ - -class SetsTests extends AlleycatsSuite { - - checkAll("FlatMapRec[Set]", FlatMapRecTests[Set].tailRecM[Int]) - -} - - - diff --git a/core/src/main/scala/cats/instances/map.scala b/core/src/main/scala/cats/instances/map.scala index db8b09f2c3..2c04776da7 100644 --- a/core/src/main/scala/cats/instances/map.scala +++ b/core/src/main/scala/cats/instances/map.scala @@ -14,16 +14,8 @@ trait MapInstances extends cats.kernel.instances.MapInstances { } // scalastyle:off method.length - implicit def catsStdInstancesForMap[K]: Traverse[Map[K, ?]] with FlatMap[Map[K, ?]] = - new Traverse[Map[K, ?]] with FlatMap[Map[K, ?]] { - - def traverse[G[_], A, B](fa: Map[K, A])(f: A => G[B])(implicit G: Applicative[G]): G[Map[K, B]] = { - val gba: Eval[G[Map[K, B]]] = Always(G.pure(Map.empty)) - val gbb = Foldable.iterateRight(fa, gba){ (kv, lbuf) => - G.map2Eval(f(kv._2), lbuf)({ (b, buf) => buf + (kv._1 -> b)}) - }.value - G.map(gbb)(_.toMap) - } + implicit def catsStdInstancesForMap[K]: FlatMap[Map[K, ?]] = + new FlatMap[Map[K, ?]] { override def map[A, B](fa: Map[K, A])(f: A => B): Map[K, B] = fa.map { case (k, a) => (k, f(a)) } @@ -47,12 +39,6 @@ trait MapInstances extends cats.kernel.instances.MapInstances { def flatMap[A, B](fa: Map[K, A])(f: (A) => Map[K, B]): Map[K, B] = fa.flatMap { case (k, a) => f(a).get(k).map((k, _)) } - def foldLeft[A, B](fa: Map[K, A], b: B)(f: (B, A) => B): B = - fa.foldLeft(b) { case (x, (k, a)) => f(x, a)} - - def foldRight[A, B](fa: Map[K, A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = - Foldable.iterateRight(fa.values, lb)(f) - def tailRecM[A, B](a: A)(f: A => Map[K, Either[A, B]]): Map[K, B] = { val bldr = Map.newBuilder[K, B] @@ -71,23 +57,6 @@ trait MapInstances extends cats.kernel.instances.MapInstances { f(a).foreach { case (k, a) => descend(k, a) } bldr.result } - - override def size[A](fa: Map[K, A]): Long = fa.size.toLong - - override def get[A](fa: Map[K, A])(idx: Long): Option[A] = - if (idx < 0L || Int.MaxValue < idx) None - else { - val n = idx.toInt - if (n >= fa.size) None - else Some(fa.valuesIterator.drop(n).next) - } - - override def isEmpty[A](fa: Map[K, A]): Boolean = fa.isEmpty - - override def fold[A](fa: Map[K, A])(implicit A: Monoid[A]): A = - A.combineAll(fa.values) - - override def toList[A](fa: Map[K, A]): List[A] = fa.values.toList } // 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 94637abb03..21a6dfd48b 100644 --- a/core/src/main/scala/cats/instances/set.scala +++ b/core/src/main/scala/cats/instances/set.scala @@ -1,56 +1,17 @@ package cats package instances -import scala.annotation.tailrec - import cats.syntax.show._ trait SetInstances extends cats.kernel.instances.SetInstances { - implicit val catsStdInstancesForSet: Foldable[Set] with MonoidK[Set] = - new Foldable[Set] with MonoidK[Set] { + implicit val catsStdInstancesForSet: MonoidK[Set] = + new MonoidK[Set] { def empty[A]: Set[A] = Set.empty[A] def combineK[A](x: Set[A], y: Set[A]): Set[A] = x | y - def foldLeft[A, B](fa: Set[A], b: B)(f: (B, A) => B): B = - fa.foldLeft(b)(f) - - def foldRight[A, B](fa: Set[A], lb: Eval[B])(f: (A, Eval[B]) => Eval[B]): Eval[B] = - Foldable.iterateRight(fa, lb)(f) - - override def get[A](fa: Set[A])(idx: Long): Option[A] = { - @tailrec - def go(idx: Int, it: Iterator[A]): Option[A] = { - if (it.hasNext) { - if (idx == 0) Some(it.next) else { - it.next - go(idx - 1, it) - } - } else None - } - if (idx < Int.MaxValue && idx >= 0L) go(idx.toInt, fa.toIterator) else None - } - - override def size[A](fa: Set[A]): Long = fa.size.toLong - - override def exists[A](fa: Set[A])(p: A => Boolean): Boolean = - fa.exists(p) - - override def forall[A](fa: Set[A])(p: A => Boolean): Boolean = - fa.forall(p) - - override def isEmpty[A](fa: Set[A]): Boolean = fa.isEmpty - - override def fold[A](fa: Set[A])(implicit A: Monoid[A]): A = A.combineAll(fa) - - override def toList[A](fa: Set[A]): List[A] = fa.toList - - override def reduceLeftOption[A](fa: Set[A])(f: (A, A) => A): Option[A] = - fa.reduceLeftOption(f) - - override def find[A](fa: Set[A])(f: A => Boolean): Option[A] = fa.find(f) } implicit def catsStdShowForSet[A:Show]: Show[Set[A]] = new Show[Set[A]] { From 93f273abb3da926e48f33d1e495572d53481e6ab Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Fri, 27 Oct 2017 13:59:04 +0200 Subject: [PATCH 08/15] Add Order constraint for arbitrary --- laws/src/main/scala/cats/laws/discipline/Arbitrary.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala b/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala index b5b116c84c..dbd6e722f8 100644 --- a/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala +++ b/laws/src/main/scala/cats/laws/discipline/Arbitrary.scala @@ -160,8 +160,8 @@ object arbitrary extends ArbitraryInstances0 { def compare(x: A, y: A): Int = java.lang.Integer.compare(f(x.##), f(y.##)) })) - implicit def catsLawsArbitraryForSortedMap[K: Arbitrary, V: Arbitrary]: Arbitrary[SortedMap[K, V]] = - Arbitrary(getArbitrary[Map[K, V]].flatMap(m => implicitly[Arbitrary[Order[K]]].arbitrary.map(o => SortedMap[K, V]()(o.toOrdering) ++ m))) + implicit def catsLawsArbitraryForSortedMap[K: Arbitrary: Order, V: Arbitrary]: Arbitrary[SortedMap[K, V]] = + Arbitrary(getArbitrary[Map[K, V]].map(s => SortedMap.empty[K, V](implicitly[Order[K]].toOrdering) ++ s)) implicit def catsLawsCogenForSortedMap[K: Order: Cogen, V: Order: Cogen]: Cogen[SortedMap[K, V]] = { implicit val orderingK = Order[K].toOrdering @@ -170,8 +170,8 @@ object arbitrary extends ArbitraryInstances0 { implicitly[Cogen[Map[K, V]]].contramap(_.toMap) } - implicit def catsLawsArbitraryForSortedSet[A: Arbitrary]: Arbitrary[SortedSet[A]] = - Arbitrary(getArbitrary[Set[A]].flatMap(s => implicitly[Arbitrary[Order[A]]].arbitrary.map(o => SortedSet[A]()(o.toOrdering) ++ s))) + implicit def catsLawsArbitraryForSortedSet[A: Arbitrary: Order]: Arbitrary[SortedSet[A]] = + Arbitrary(getArbitrary[Set[A]].map(s => SortedSet.empty[A](implicitly[Order[A]].toOrdering) ++ s)) implicit def catsLawsCogenForSortedSet[A: Order: Cogen]: Cogen[SortedSet[A]] = { implicit val orderingA = Order[A].toOrdering From 151412cb6b274c8ad6d8ebbe824e0ab7e4c2ba6c Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Fri, 27 Oct 2017 17:44:26 +0200 Subject: [PATCH 09/15] Add hash implementation --- .../main/scala/cats/instances/sortedSet.scala | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/core/src/main/scala/cats/instances/sortedSet.scala b/core/src/main/scala/cats/instances/sortedSet.scala index eb8ec97d79..851be067a0 100644 --- a/core/src/main/scala/cats/instances/sortedSet.scala +++ b/core/src/main/scala/cats/instances/sortedSet.scala @@ -85,9 +85,26 @@ class SortedSetOrder[A: Order] extends Order[SortedSet[A]] { } class SortedSetHash[A: Order: Hash] extends Hash[SortedSet[A]] { - // TODO replace - def hash(x: SortedSet[A]): Int = x.hashCode() - + import scala.util.hashing.MurmurHash3._ + + // adapted from [[scala.util.hashing.MurmurHash3]], + // but modified standard `Any#hashCode` to `ev.hash`. + def hash(xs: SortedSet[A]): Int = { + var a, b, n = 0 + var c = 1 + xs foreach { x => + val h = Hash[A].hash(x) + a += h + b ^= h + if (h != 0) c *= h + n += 1 + } + var h = setSeed + h = mix(h, a) + h = mix(h, b) + h = mixLast(h, c) + finalizeHash(h, n) + } override def eqv(s1: SortedSet[A], s2: SortedSet[A]): Boolean = { implicit val x = Order[A].toOrdering s1.toStream.corresponds(s2.toStream)(Order[A].eqv) From 40f9de5046876bd4b612d9d0f26c44eda1a6ae1c Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Sat, 28 Oct 2017 09:47:46 +0200 Subject: [PATCH 10/15] Fix tut docs --- docs/src/main/tut/typeclasses/foldable.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/src/main/tut/typeclasses/foldable.md b/docs/src/main/tut/typeclasses/foldable.md index 4e73e33f39..7c2ec546ba 100644 --- a/docs/src/main/tut/typeclasses/foldable.md +++ b/docs/src/main/tut/typeclasses/foldable.md @@ -40,10 +40,10 @@ Foldable[List].reduceLeftToOption(List[Int]())(_.toString)((s,i) => s + i) Foldable[List].reduceLeftToOption(List(1,2,3,4))(_.toString)((s,i) => s + i) Foldable[List].reduceRightToOption(List(1,2,3,4))(_.toString)((i,s) => Later(s.value + i)).value Foldable[List].reduceRightToOption(List[Int]())(_.toString)((i,s) => Later(s.value + i)).value -Foldable[Set].find(Set(1,2,3))(_ > 2) -Foldable[Set].exists(Set(1,2,3))(_ > 2) -Foldable[Set].forall(Set(1,2,3))(_ > 2) -Foldable[Set].forall(Set(1,2,3))(_ < 4) +Foldable[List].find(List(1,2,3))(_ > 2) +Foldable[List].exists(List(1,2,3))(_ > 2) +Foldable[List].forall(List(1,2,3))(_ > 2) +Foldable[List].forall(List(1,2,3))(_ < 4) Foldable[Vector].filter_(Vector(1,2,3))(_ < 3) Foldable[List].isEmpty(List(1,2)) Foldable[Option].isEmpty(None) From 36eeada59c698e789e2fc2d5df34ae538427aba3 Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Sat, 28 Oct 2017 09:59:25 +0200 Subject: [PATCH 11/15] Address feedback --- tests/src/test/scala/cats/tests/FoldableSuite.scala | 8 ++++++++ tests/src/test/scala/cats/tests/ParallelTests.scala | 3 ++- tests/src/test/scala/cats/tests/RegressionSuite.scala | 6 ++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/src/test/scala/cats/tests/FoldableSuite.scala b/tests/src/test/scala/cats/tests/FoldableSuite.scala index 2a86f4b576..487e8f32f2 100644 --- a/tests/src/test/scala/cats/tests/FoldableSuite.scala +++ b/tests/src/test/scala/cats/tests/FoldableSuite.scala @@ -218,6 +218,14 @@ class FoldableSuiteAdditional extends CatsSuite { checkFoldMStackSafety[Vector](_.toVector) } + test("Foldable[SortedSet].foldM stack safety") { + checkFoldMStackSafety[SortedSet](_.to) + } + + test("Foldable[SortedMap[String, ?]].foldM stack safety") { + checkFoldMStackSafety[SortedMap[String, ?]](xs => SortedMap.empty[String, Int] ++ xs.map(x => x.toString -> x).toMap) + } + test("Foldable[NonEmptyList].foldM stack safety") { checkFoldMStackSafety[NonEmptyList](xs => NonEmptyList.fromListUnsafe(xs.toList)) } diff --git a/tests/src/test/scala/cats/tests/ParallelTests.scala b/tests/src/test/scala/cats/tests/ParallelTests.scala index ba8999a3ce..d2c31752a3 100644 --- a/tests/src/test/scala/cats/tests/ParallelTests.scala +++ b/tests/src/test/scala/cats/tests/ParallelTests.scala @@ -9,6 +9,7 @@ import cats.laws.discipline.eq._ import cats.laws.discipline.arbitrary._ import org.scalacheck.Arbitrary import org.typelevel.discipline.scalatest.Discipline +import scala.collection.immutable.SortedSet class ParallelTests extends CatsSuite with ApplicativeErrorForEitherTest { @@ -30,7 +31,7 @@ class ParallelTests extends CatsSuite with ApplicativeErrorForEitherTest { } test("ParTraverse_ identity should be equivalent to parSequence_") { - forAll { es: List[Either[String, Int]] => + forAll { es: SortedSet[Either[String, Int]] => Parallel.parTraverse_(es)(identity) should === (Parallel.parSequence_(es)) } } diff --git a/tests/src/test/scala/cats/tests/RegressionSuite.scala b/tests/src/test/scala/cats/tests/RegressionSuite.scala index b02de43046..570b7691a8 100644 --- a/tests/src/test/scala/cats/tests/RegressionSuite.scala +++ b/tests/src/test/scala/cats/tests/RegressionSuite.scala @@ -3,6 +3,7 @@ package tests import cats.data.{Const, NonEmptyList} import scala.collection.mutable +import scala.collection.immutable.SortedMap class RegressionSuite extends CatsSuite { // toy state class @@ -101,6 +102,11 @@ class RegressionSuite extends CatsSuite { Stream(1,2,6,8).traverse(validate) should === (Either.left("6 is greater than 5")) checkAndResetCount(3) + type StringMap[A] = SortedMap[String, A] + val intMap: StringMap[Int] = SortedMap("A" -> 1, "B" -> 2, "C" -> 6, "D" -> 8) + intMap.traverse(validate) should === (Either.left("6 is greater than 5")) + checkAndResetCount(3) + NonEmptyList.of(1,2,6,8).traverse(validate) should === (Either.left("6 is greater than 5")) checkAndResetCount(3) From 4e6456a6c1b45294e76f44d16a21099eb0b164ff Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Sun, 29 Oct 2017 16:46:44 +0100 Subject: [PATCH 12/15] Add order consistency test --- .../main/scala/cats/laws/TraverseLaws.scala | 21 +++++++++++++++++++ .../cats/laws/discipline/TraverseTests.scala | 1 + 2 files changed, 22 insertions(+) diff --git a/laws/src/main/scala/cats/laws/TraverseLaws.scala b/laws/src/main/scala/cats/laws/TraverseLaws.scala index e7929fe4bf..16b83c6d0a 100644 --- a/laws/src/main/scala/cats/laws/TraverseLaws.scala +++ b/laws/src/main/scala/cats/laws/TraverseLaws.scala @@ -67,6 +67,27 @@ trait TraverseLaws[F[_]] extends FunctorLaws[F] with FoldableLaws[F] { lhs <-> rhs } + def traverseOrderConsistent[A](fa: F[A]) = { + class FirstOption[T](val o: Option[T]) + + implicit val firstOptionMonoid = new Monoid[FirstOption[A]] { + def empty = new FirstOption(None) + def combine(x: FirstOption[A], y: FirstOption[A]) = new FirstOption(x.o.orElse(y.o)) + } + + def liftId[T](a: T): Id[T] = a + def store[T](a: T): Const[FirstOption[T], T] = Const(new FirstOption(Some(a))) + + val first = F.traverse[Const[FirstOption[A], ?], A, A](fa)(store).getConst.o + val traverseFirst = F.traverse[Const[FirstOption[A], ?], A, A]( + F.traverse(fa)(liftId) + )(store).getConst.o + + first <-> traverseFirst + + + } + def mapWithIndexRef[A, B](fa: F[A], f: (A, Int) => B): IsEq[F[B]] = { val lhs = F.mapWithIndex(fa)(f) val rhs = F.traverse(fa)(a => diff --git a/laws/src/main/scala/cats/laws/discipline/TraverseTests.scala b/laws/src/main/scala/cats/laws/discipline/TraverseTests.scala index 7cb5b1be4f..0184105349 100644 --- a/laws/src/main/scala/cats/laws/discipline/TraverseTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/TraverseTests.scala @@ -44,6 +44,7 @@ trait TraverseTests[F[_]] extends FunctorTests[F] with FoldableTests[F] { "traverse sequential composition" -> forAll(laws.traverseSequentialComposition[A, B, C, X, Y] _), "traverse parallel composition" -> forAll(laws.traverseParallelComposition[A, B, X, Y] _), "traverse derive foldMap" -> forAll(laws.foldMapDerived[A, M] _), + "traverse order consistency" -> forAll(laws.traverseOrderConsistent[A] _), "traverse ref mapWithIndex" -> forAll(laws.mapWithIndexRef[A, C] _), "traverse ref traverseWithIndexM" -> forAll(laws.traverseWithIndexMRef[Option, A, C] _), "traverse ref zipWithIndex" -> forAll(laws.zipWithIndexRef[A, C] _) From 8745b55207f3579e345f4d68c295d54b865c6d2f Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Sun, 29 Oct 2017 18:00:31 +0100 Subject: [PATCH 13/15] Add law for foldable --- laws/src/main/scala/cats/laws/FoldableLaws.scala | 4 ++++ laws/src/main/scala/cats/laws/TraverseLaws.scala | 5 +---- laws/src/main/scala/cats/laws/discipline/FoldableTests.scala | 2 ++ .../src/main/scala/cats/laws/discipline/ReducibleTests.scala | 1 + 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/laws/src/main/scala/cats/laws/FoldableLaws.scala b/laws/src/main/scala/cats/laws/FoldableLaws.scala index 8adc63040b..d87ab329ab 100644 --- a/laws/src/main/scala/cats/laws/FoldableLaws.scala +++ b/laws/src/main/scala/cats/laws/FoldableLaws.scala @@ -141,6 +141,10 @@ trait FoldableLaws[F[_]] { F.dropWhile_(fa)(p) <-> F.foldLeft(fa, mutable.ListBuffer.empty[A]) { (buf, a) => if (buf.nonEmpty || !p(a)) buf += a else buf }.toList + + 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] } object FoldableLaws { diff --git a/laws/src/main/scala/cats/laws/TraverseLaws.scala b/laws/src/main/scala/cats/laws/TraverseLaws.scala index 16b83c6d0a..aa6b74a547 100644 --- a/laws/src/main/scala/cats/laws/TraverseLaws.scala +++ b/laws/src/main/scala/cats/laws/TraverseLaws.scala @@ -67,7 +67,7 @@ trait TraverseLaws[F[_]] extends FunctorLaws[F] with FoldableLaws[F] { lhs <-> rhs } - def traverseOrderConsistent[A](fa: F[A]) = { + def traverseOrderConsistent[A](fa: F[A]): IsEq[Option[A]] = { class FirstOption[T](val o: Option[T]) implicit val firstOptionMonoid = new Monoid[FirstOption[A]] { @@ -84,10 +84,7 @@ trait TraverseLaws[F[_]] extends FunctorLaws[F] with FoldableLaws[F] { )(store).getConst.o first <-> traverseFirst - - } - def mapWithIndexRef[A, B](fa: F[A], f: (A, Int) => B): IsEq[F[B]] = { val lhs = F.mapWithIndex(fa)(f) val rhs = F.traverse(fa)(a => diff --git a/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala b/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala index d7a3868bec..3c75e4417d 100644 --- a/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/FoldableTests.scala @@ -18,6 +18,7 @@ trait FoldableTests[F[_]] extends Laws { CogenA: Cogen[A], CogenB: Cogen[B], EqA: Eq[A], + EqFA: Eq[F[A]], EqB: Eq[B], EqOptionA: Eq[Option[A]] ): RuleSet = { @@ -26,6 +27,7 @@ trait FoldableTests[F[_]] extends Laws { parent = None, "foldLeft consistent with foldMap" -> forAll(laws.leftFoldConsistentWithFoldMap[A, B] _), "foldRight consistent with foldMap" -> forAll(laws.rightFoldConsistentWithFoldMap[A, B] _), + "ordered constistency" -> forAll(laws.orderedConsistency[A] _), "exists consistent with find" -> forAll(laws.existsConsistentWithFind[A] _), "forall consistent with exists" -> forAll(laws.forallConsistentWithExists[A] _), "forall true if empty" -> forAll(laws.forallEmpty[A] _), diff --git a/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala b/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala index df9edce95b..298f8fcf2f 100644 --- a/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala +++ b/laws/src/main/scala/cats/laws/discipline/ReducibleTests.scala @@ -21,6 +21,7 @@ trait ReducibleTests[F[_]] extends FoldableTests[F] { EqG: Eq[G[Unit]], EqA: Eq[A], EqB: Eq[B], + EqFA: Eq[F[A]], EqOptionA: Eq[Option[A]], MonoidA: Monoid[A], MonoidB: Monoid[B] From 37e36ea2584a487933e90f4c05024619c60ec763 Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Mon, 30 Oct 2017 09:21:09 +0100 Subject: [PATCH 14/15] Remove failing tests from Alleycats --- .../src/main/scala/alleycats/std/iterable.scala | 1 - .../scala/alleycats/tests/IterableTests.scala | 16 ++++++---------- .../test/scala/alleycats/tests/MapSuite.scala | 3 +-- .../test/scala/alleycats/tests/SetSuite.scala | 5 ----- 4 files changed, 7 insertions(+), 18 deletions(-) diff --git a/alleycats-core/src/main/scala/alleycats/std/iterable.scala b/alleycats-core/src/main/scala/alleycats/std/iterable.scala index 7ccde80e75..21908b0d14 100644 --- a/alleycats-core/src/main/scala/alleycats/std/iterable.scala +++ b/alleycats-core/src/main/scala/alleycats/std/iterable.scala @@ -23,5 +23,4 @@ object IterableInstances { // TODO: remove when cats.Foldable support export-hook trait LegacyIterableInstances { implicit def legacyIterableFoldable(implicit e: ExportOrphan[Foldable[Iterable]]): Foldable[Iterable] = e.instance - } diff --git a/alleycats-tests/src/test/scala/alleycats/tests/IterableTests.scala b/alleycats-tests/src/test/scala/alleycats/tests/IterableTests.scala index a6ad6a416b..157e637f14 100644 --- a/alleycats-tests/src/test/scala/alleycats/tests/IterableTests.scala +++ b/alleycats-tests/src/test/scala/alleycats/tests/IterableTests.scala @@ -2,20 +2,16 @@ package alleycats package tests import cats.{Eval, Foldable} -import cats.laws.discipline._ - import alleycats.std.all._ class IterableTests extends AlleycatsSuite { - checkAll("Foldable[Iterable]", FoldableTests[Iterable].foldable[Int, Int]) - - test("foldLeft sum == sum"){ - val it = Iterable(1, 2, 3) - Foldable[Iterable].foldLeft(it, 0){ - case (b, a) => a + b - } shouldEqual(it.sum) - } + test("foldLeft sum == sum"){ + val it = Iterable(1, 2, 3) + Foldable[Iterable].foldLeft(it, 0){ + case (b, a) => a + b + } shouldEqual(it.sum) + } test("foldRight early termination"){ Foldable[Iterable].foldRight(Iterable(1, 2, 3), Eval.now("KO")){ diff --git a/alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala b/alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala index c47bafd72f..7a0ff1a9f3 100644 --- a/alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala +++ b/alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala @@ -1,9 +1,8 @@ package alleycats.tests -import cats.laws.discipline.{SerializableTests, TraverseTests} +import cats.laws.discipline.SerializableTests import cats.Traverse class MapSuite extends AlleycatsSuite { - checkAll("Map[Int, Int] with Option", TraverseTests[Map[Int, ?]].traverse[Int, Int, Int, Int, Option, Option]) checkAll("Traverse[Map[Int, ?]]", SerializableTests.serializable(Traverse[Map[Int, ?]])) } diff --git a/alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala b/alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala index 76d48f8bee..6f1136d383 100644 --- a/alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala +++ b/alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala @@ -3,18 +3,13 @@ package alleycats.tests import alleycats.laws.discipline._ import cats.Foldable import cats.kernel.laws.discipline.SerializableTests -import cats.laws.discipline.FoldableTests import alleycats.std.all._ class SetSuite extends AlleycatsSuite { - checkAll("FlatMapRec[Set]", FlatMapRecTests[Set].tailRecM[Int]) - checkAll("Set[Int]", FoldableTests[Set].foldable[Int, Int]) checkAll("Foldable[Set]", SerializableTests.serializable(Foldable[Set])) - - } From 2aef3552651a1aa950bc31b76f99d4447f6b3394 Mon Sep 17 00:00:00 2001 From: Luka Jacobowitz Date: Mon, 30 Oct 2017 16:49:36 +0100 Subject: [PATCH 15/15] Add requirement that Foldable instances should be ordered --- core/src/main/scala/cats/Foldable.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/cats/Foldable.scala b/core/src/main/scala/cats/Foldable.scala index bc70080186..f8474ebdb7 100644 --- a/core/src/main/scala/cats/Foldable.scala +++ b/core/src/main/scala/cats/Foldable.scala @@ -8,12 +8,14 @@ import simulacrum.typeclass /** * Data structures that can be folded to a summary value. * - * In the case of a collection (such as `List` or `Set`), these + * In the case of a collection (such as `List` or `Vector`), these * methods will fold together (combine) the values contained in the * collection to produce a single result. Most collection types have * `foldLeft` methods, which will usually be used by the associated * `Foldable[_]` instance. * + * Instances of Foldable should be ordered collections to allow for consistent folding. + * * Foldable[F] is implemented in terms of two basic methods: * * - `foldLeft(fa, b)(f)` eagerly folds `fa` from left-to-right.