Add SortedMap and SortedSet instances/Move Set and Map instan…

…ces to Alleycats (#1972)

* Add SortedMap instances

* Add law tests

* Remove optimized combineAll

* Add SortedSet instances

* Try to fix some tests

* Rename

* wip move map and set instance to alleycats

* Add Order constraint for arbitrary

* Add hash implementation

* Fix tut docs

* Address feedback

* Add order consistency test

* Add law for foldable

* Remove failing tests from Alleycats

* Add requirement that Foldable instances should be ordered

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

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
-
 }

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
+    }
+}

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)
     }
 }
 

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
 

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")){

alleycats-tests/src/test/scala/alleycats/tests/MapSuite.scala

@@ -0,0 +1,8 @@
+package alleycats.tests
+
+import cats.laws.discipline.SerializableTests
+import cats.Traverse
+
+class MapSuite extends AlleycatsSuite {
+  checkAll("Traverse[Map[Int, ?]]", SerializableTests.serializable(Traverse[Map[Int, ?]]))
+}

alleycats-tests/src/test/scala/alleycats/tests/SetSuite.scala

@@ -0,0 +1,16 @@
+package alleycats.tests
+
+import alleycats.laws.discipline._
+import cats.Foldable
+import cats.kernel.laws.discipline.SerializableTests
+
+import alleycats.std.all._
+
+class SetSuite extends AlleycatsSuite {
+  checkAll("FlatMapRec[Set]", FlatMapRecTests[Set].tailRecM[Int])
+
+  checkAll("Foldable[Set]", SerializableTests.serializable(Foldable[Set]))
+}
+
+
+

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.

core/src/main/scala/cats/instances/all.scala

@@ -23,6 +23,8 @@ trait AllInstances
   with    PartialOrderingInstances
   with    QueueInstances
   with    SetInstances
+  with    SortedMapInstances
+  with    SortedSetInstances
   with    StreamInstances
   with    StringInstances
   with    SymbolInstances

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
 }

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

@@ -30,6 +30,8 @@ package object instances {
   object queue           extends QueueInstances
   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

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]] {