From 82e91eeadcad994eb5325481f5bd09231ff93b71 Mon Sep 17 00:00:00 2001
From: "P. Oscar Boykin" <boykin@pobox.com>
Date: Fri, 31 Jan 2020 12:34:49 -1000
Subject: [PATCH 1/5] Add Semigroup and Monoid combinators reverse and
 intercalate

---
 .../cats/kernel/laws/SemigroupLaws.scala      | 45 +++++++++++++++++++
 .../laws/discipline/SemigroupTests.scala      | 12 ++++-
 kernel/src/main/scala/cats/kernel/Band.scala  |  6 ++-
 .../cats/kernel/BoundedSemilattice.scala      |  7 ++-
 .../scala/cats/kernel/CommutativeMonoid.scala |  4 +-
 .../cats/kernel/CommutativeSemigroup.scala    |  4 +-
 .../src/main/scala/cats/kernel/Monoid.scala   |  9 +++-
 kernel/src/main/scala/cats/kernel/Order.scala |  4 ++
 .../main/scala/cats/kernel/Semigroup.scala    | 23 +++++++++-
 .../cats/kernel/instances/ListInstances.scala | 13 +++++-
 .../kernel/instances/StringInstances.scala    | 14 +++++-
 11 files changed, 132 insertions(+), 9 deletions(-)

diff --git a/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala b/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala
index 52265cbb54..25920c48a9 100644
--- a/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala
+++ b/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala
@@ -17,6 +17,51 @@ trait SemigroupLaws[A] {
   def combineAllOption(xs: Vector[A]): IsEq[Option[A]] =
     S.combineAllOption(xs) <-> xs.reduceOption(S.combine)
 
+  def reverseReverses(a: A, b: A): IsEq[A] =
+    S.combine(a, b) <-> S.reverse.combine(b, a)
+
+  def reverseSemigroupAssociative(x: A, y: A, z: A): IsEq[A] = {
+    val rev = S.reverse
+    rev.combine(rev.combine(x, y), z) <-> rev.combine(x, rev.combine(y, z))
+  }
+
+  def reverseRepeat1(a: A): IsEq[A] = {
+    val rev = S.reverse
+    rev.combineN(a, 1) <-> a
+  }
+
+  def reverseRepeat2(a: A): IsEq[A] = {
+    val rev = S.reverse
+    rev.combineN(a, 2) <-> rev.combine(a, a)
+  }
+
+  def reverseCombineAllOption(xs: Vector[A]): IsEq[Option[A]] = {
+    val rev = S.reverse
+    rev.combineAllOption(xs) <-> xs.reduceOption(rev.combine)
+  }
+
+  def intercalateIntercalates(a: A, m: A, b: A): IsEq[A] =
+    S.combine(a, S.combine(m, b)) <-> S.intercalate(m).combine(a, b)
+
+  def intercalateSemigroupAssociative(m: A, x: A, y: A, z: A): IsEq[A] = {
+    val withMiddle = S.intercalate(m)
+    withMiddle.combine(withMiddle.combine(x, y), z) <-> withMiddle.combine(x, withMiddle.combine(y, z))
+  }
+
+  def intercalateRepeat1(m: A, a: A): IsEq[A] = {
+    val withMiddle = S.intercalate(m)
+    withMiddle.combineN(a, 1) <-> a
+  }
+
+  def intercalateRepeat2(m: A, a: A): IsEq[A] = {
+    val withMiddle = S.intercalate(m)
+    withMiddle.combineN(a, 2) <-> withMiddle.combine(a, a)
+  }
+
+  def intercalateCombineAllOption(m: A, xs: Vector[A]): IsEq[Option[A]] = {
+    val withMiddle = S.intercalate(m)
+    withMiddle.combineAllOption(xs) <-> xs.reduceOption(withMiddle.combine)
+  }
 }
 
 object SemigroupLaws {
diff --git a/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala b/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala
index 213b2287d3..75ab4050ef 100644
--- a/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala
+++ b/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala
@@ -18,7 +18,17 @@ trait SemigroupTests[A] extends Laws {
       "associative" -> forAll(laws.semigroupAssociative _),
       "repeat1" -> forAll(laws.repeat1 _),
       "repeat2" -> forAll(laws.repeat2 _),
-      "combineAllOption" -> forAll(laws.combineAllOption _)
+      "combineAllOption" -> forAll(laws.combineAllOption _),
+      "reverseReverses" -> forAll(laws.reverseReverses _),
+      "reverseAssociative" -> forAll(laws.reverseSemigroupAssociative _),
+      "reverseRepeat1" -> forAll(laws.reverseRepeat1 _),
+      "reverseRepeat2" -> forAll(laws.reverseRepeat2 _),
+      "reverseCombineAllOption" -> forAll(laws.reverseCombineAllOption _),
+      "intercalateIntercalates" -> forAll(laws.intercalateIntercalates _),
+      "intercalateAssociative" -> forAll(laws.intercalateSemigroupAssociative _),
+      "intercalateRepeat1" -> forAll(laws.intercalateRepeat1 _),
+      "intercalateRepeat2" -> forAll(laws.intercalateRepeat2 _),
+      "intercalateCombineAllOption" -> forAll(laws.intercalateCombineAllOption _)
     )
 }
 
diff --git a/kernel/src/main/scala/cats/kernel/Band.scala b/kernel/src/main/scala/cats/kernel/Band.scala
index 48249e9cd1..48ab98ddd0 100644
--- a/kernel/src/main/scala/cats/kernel/Band.scala
+++ b/kernel/src/main/scala/cats/kernel/Band.scala
@@ -6,7 +6,11 @@ import scala.{specialized => sp}
  * Bands are semigroups whose operation
  * (i.e. combine) is also idempotent.
  */
-trait Band[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A]
+trait Band[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A] {
+  override def combineN(a: A, n: Int): A =
+    if (n <= 0) throw new IllegalArgumentException("Repeated combining for semigroups must have n > 0")
+    else a // combine(a, a) == a
+}
 
 object Band extends SemigroupFunctions[Band] {
 
diff --git a/kernel/src/main/scala/cats/kernel/BoundedSemilattice.scala b/kernel/src/main/scala/cats/kernel/BoundedSemilattice.scala
index 400ec9f765..33211e47c1 100644
--- a/kernel/src/main/scala/cats/kernel/BoundedSemilattice.scala
+++ b/kernel/src/main/scala/cats/kernel/BoundedSemilattice.scala
@@ -2,7 +2,12 @@ package cats.kernel
 
 import scala.{specialized => sp}
 
-trait BoundedSemilattice[@sp(Int, Long, Float, Double) A] extends Any with Semilattice[A] with CommutativeMonoid[A]
+trait BoundedSemilattice[@sp(Int, Long, Float, Double) A] extends Any with Semilattice[A] with CommutativeMonoid[A] {
+  override def combineN(a: A, n: Int): A =
+    if (n < 0) throw new IllegalArgumentException("Repeated combining for monoids must have n >= 0")
+    else if (n == 0) empty
+    else a // combine(a, a) == a for a semilattice
+}
 
 object BoundedSemilattice extends SemilatticeFunctions[BoundedSemilattice] {
 
diff --git a/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala b/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala
index eaf71b0ed4..ea4c82319b 100644
--- a/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala
+++ b/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala
@@ -7,7 +7,9 @@ import scala.{specialized => sp}
  *
  * A monoid is commutative if for all x and y, x |+| y === y |+| x.
  */
-trait CommutativeMonoid[@sp(Int, Long, Float, Double) A] extends Any with Monoid[A] with CommutativeSemigroup[A]
+trait CommutativeMonoid[@sp(Int, Long, Float, Double) A] extends Any with Monoid[A] with CommutativeSemigroup[A] { self =>
+  override def reverse: CommutativeMonoid[A] = self
+}
 
 object CommutativeMonoid extends MonoidFunctions[CommutativeMonoid] {
 
diff --git a/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala b/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
index 3151c65f40..8bf6b61808 100644
--- a/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
+++ b/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
@@ -7,7 +7,9 @@ import scala.{specialized => sp}
  *
  * A semigroup is commutative if for all x and y, x |+| y === y |+| x.
  */
-trait CommutativeSemigroup[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A]
+trait CommutativeSemigroup[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A] { self =>
+  override def reverse: CommutativeSemigroup[A] = self
+}
 
 object CommutativeSemigroup extends SemigroupFunctions[CommutativeSemigroup] {
 
diff --git a/kernel/src/main/scala/cats/kernel/Monoid.scala b/kernel/src/main/scala/cats/kernel/Monoid.scala
index 07c1514883..e79cb88dc2 100644
--- a/kernel/src/main/scala/cats/kernel/Monoid.scala
+++ b/kernel/src/main/scala/cats/kernel/Monoid.scala
@@ -9,7 +9,7 @@ import compat.scalaVersionSpecific._
  * `combine(x, empty) == combine(empty, x) == x`. For example, if we have `Monoid[String]`,
  * with `combine` as string concatenation, then `empty = ""`.
  */
-trait Monoid[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A] {
+trait Monoid[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A] { self =>
 
   /**
    * Return the identity element for this monoid.
@@ -83,6 +83,13 @@ trait Monoid[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A] {
 
   override def combineAllOption(as: IterableOnce[A]): Option[A] =
     if (as.iterator.isEmpty) None else Some(combineAll(as))
+
+  override def reverse: Monoid[A] =
+    new Monoid[A] {
+      def empty = self.empty
+      def combine(a: A, b: A) = self.combine(b, a)
+      override def reverse = self
+    }
 }
 
 @suppressUnusedImportWarningForScalaVersionSpecific
diff --git a/kernel/src/main/scala/cats/kernel/Order.scala b/kernel/src/main/scala/cats/kernel/Order.scala
index 64cefa7ae4..603e1f3925 100644
--- a/kernel/src/main/scala/cats/kernel/Order.scala
+++ b/kernel/src/main/scala/cats/kernel/Order.scala
@@ -216,6 +216,10 @@ object Order extends OrderFunctions[Order] with OrderToOrderingConversion {
     new Monoid[Order[A]] with Band[Order[A]] {
       val empty: Order[A] = allEqual[A]
       def combine(x: Order[A], y: Order[A]): Order[A] = Order.whenEqual(x, y)
+      override def combineN(a: Order[A], n: Int): Order[A] =
+        if (n < 0) throw new IllegalArgumentException("Repeated combining for monoids must have n >= 0")
+        else if (n == 0) empty
+        else a // combine(a, a) == a for a band
     }
 
   def fromOrdering[A](implicit ev: Ordering[A]): Order[A] =
diff --git a/kernel/src/main/scala/cats/kernel/Semigroup.scala b/kernel/src/main/scala/cats/kernel/Semigroup.scala
index 602609b157..980c14c226 100644
--- a/kernel/src/main/scala/cats/kernel/Semigroup.scala
+++ b/kernel/src/main/scala/cats/kernel/Semigroup.scala
@@ -7,7 +7,7 @@ import compat.scalaVersionSpecific._
 /**
  * A semigroup is any set `A` with an associative operation (`combine`).
  */
-trait Semigroup[@sp(Int, Long, Float, Double) A] extends Any with Serializable {
+trait Semigroup[@sp(Int, Long, Float, Double) A] extends Any with Serializable { self =>
 
   /**
    * Associative operation which combines two values.
@@ -77,6 +77,27 @@ trait Semigroup[@sp(Int, Long, Float, Double) A] extends Any with Serializable {
    */
   def combineAllOption(as: IterableOnce[A]): Option[A] =
     as.reduceOption(combine)
+
+  /**
+   * return a semigroup that reverses the order
+   * so combine(a, b) == reverse.combine(b, a)
+   */
+  def reverse: Semigroup[A] =
+    new Semigroup[A] {
+      def combine(a: A, b: A): A = self.combine(b, a)
+      // a + a + a + ... is the same when reversed
+      override def combineN(a: A, n: Int): A = self.combineN(a, n)
+      override def reverse = self
+    }
+
+  /**
+   * Between each pair of elements insert middle
+   */
+  def intercalate(middle: A): Semigroup[A] =
+    new Semigroup[A] {
+      def combine(a: A, b: A): A =
+        self.combine(a, self.combine(middle, b))
+    }
 }
 
 abstract class SemigroupFunctions[S[T] <: Semigroup[T]] {
diff --git a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
index 0e1cf882ce..a19a142c36 100644
--- a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
+++ b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
@@ -83,7 +83,7 @@ class ListEq[A](implicit ev: Eq[A]) extends Eq[List[A]] {
   }
 }
 
-class ListMonoid[A] extends Monoid[List[A]] {
+class ListMonoid[A] extends Monoid[List[A]] { self =>
   def empty: List[A] = Nil
   def combine(x: List[A], y: List[A]): List[A] = x ::: y
 
@@ -92,4 +92,15 @@ class ListMonoid[A] extends Monoid[List[A]] {
 
   override def combineAll(xs: IterableOnce[List[A]]): List[A] =
     StaticMethods.combineAllIterable(List.newBuilder[A], xs)
+
+  override def reverse: Monoid[List[A]] =
+    new Monoid[List[A]] {
+      def empty: List[A] = Nil
+      def combine(x: List[A], y: List[A]) = y ::: x
+
+      override def combineAll(xs: IterableOnce[List[A]]): List[A] =
+        xs.foldLeft(empty) { (acc, item) => item ::: acc }
+
+      override def reverse = self
+    }
 }
diff --git a/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala b/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
index a566c214ef..996bb22de6 100644
--- a/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
+++ b/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
@@ -23,7 +23,7 @@ class StringOrder extends Order[String] with Hash[String] with StringLowerBounde
   override val partialOrder: PartialOrder[String] = self
 }
 
-class StringMonoid extends Monoid[String] {
+class StringMonoid extends Monoid[String] { self =>
   def empty: String = ""
   def combine(x: String, y: String): String = x + y
 
@@ -32,4 +32,16 @@ class StringMonoid extends Monoid[String] {
     xs.iterator.foreach(sb.append)
     sb.toString
   }
+
+  override def reverse: Monoid[String] =
+    new Monoid[String] {
+      def empty = self.empty
+      def combine(x: String, y: String) = y + x
+      override def combineAll(xs: IterableOnce[String]): String = {
+        val revStrings = xs.foldLeft(List.empty[String]) { (acc, s) => s :: acc }
+        self.combineAll(revStrings)
+      }
+
+      override def reverse = self
+    }
 }

From d76a534d8f63d3490b2f34b50223065bf0583271 Mon Sep 17 00:00:00 2001
From: "P. Oscar Boykin" <boykin@pobox.com>
Date: Fri, 31 Jan 2020 12:38:26 -1000
Subject: [PATCH 2/5] format

---
 kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala     | 3 ++-
 .../src/main/scala/cats/kernel/instances/ListInstances.scala  | 4 +++-
 .../main/scala/cats/kernel/instances/StringInstances.scala    | 4 +++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala b/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala
index ea4c82319b..b03362eda8 100644
--- a/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala
+++ b/kernel/src/main/scala/cats/kernel/CommutativeMonoid.scala
@@ -7,7 +7,8 @@ import scala.{specialized => sp}
  *
  * A monoid is commutative if for all x and y, x |+| y === y |+| x.
  */
-trait CommutativeMonoid[@sp(Int, Long, Float, Double) A] extends Any with Monoid[A] with CommutativeSemigroup[A] { self =>
+trait CommutativeMonoid[@sp(Int, Long, Float, Double) A] extends Any with Monoid[A] with CommutativeSemigroup[A] {
+  self =>
   override def reverse: CommutativeMonoid[A] = self
 }
 
diff --git a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
index a19a142c36..5595999b58 100644
--- a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
+++ b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
@@ -99,7 +99,9 @@ class ListMonoid[A] extends Monoid[List[A]] { self =>
       def combine(x: List[A], y: List[A]) = y ::: x
 
       override def combineAll(xs: IterableOnce[List[A]]): List[A] =
-        xs.foldLeft(empty) { (acc, item) => item ::: acc }
+        xs.foldLeft(empty) { (acc, item) =>
+          item ::: acc
+        }
 
       override def reverse = self
     }
diff --git a/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala b/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
index 996bb22de6..815231f8ee 100644
--- a/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
+++ b/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
@@ -38,7 +38,9 @@ class StringMonoid extends Monoid[String] { self =>
       def empty = self.empty
       def combine(x: String, y: String) = y + x
       override def combineAll(xs: IterableOnce[String]): String = {
-        val revStrings = xs.foldLeft(List.empty[String]) { (acc, s) => s :: acc }
+        val revStrings = xs.foldLeft(List.empty[String]) { (acc, s) =>
+          s :: acc
+        }
         self.combineAll(revStrings)
       }
 

From 1c0e300b2df087adf1c3a1be061e19a99f0fd1ff Mon Sep 17 00:00:00 2001
From: "P. Oscar Boykin" <boykin@pobox.com>
Date: Fri, 31 Jan 2020 12:50:04 -1000
Subject: [PATCH 3/5] fix combineN on Monoid.reverse and Duration tests

---
 .../shared/src/test/scala/cats/kernel/laws/LawTests.scala     | 4 ++--
 kernel/src/main/scala/cats/kernel/Monoid.scala                | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala b/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala
index efecdb7071..e82f51c6d3 100644
--- a/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala
+++ b/kernel-laws/shared/src/test/scala/cats/kernel/laws/LawTests.scala
@@ -34,7 +34,7 @@ object KernelCheck {
   implicit val arbitraryDuration: Arbitrary[Duration] = {
     // max range is +/- 292 years, but we give ourselves some extra headroom
     // to ensure that we can add these things up. they crash on overflow.
-    val n = (292L * 365) / 50
+    val n = (292L * 365) / 500
     Arbitrary(
       Gen.oneOf(
         Gen.choose(-n, n).map(Duration(_, DAYS)),
@@ -51,7 +51,7 @@ object KernelCheck {
   implicit val arbitraryFiniteDuration: Arbitrary[FiniteDuration] = {
     // max range is +/- 292 years, but we give ourselves some extra headroom
     // to ensure that we can add these things up. they crash on overflow.
-    val n = (292L * 365) / 50
+    val n = (292L * 365) / 500
     Arbitrary(
       Gen.oneOf(
         Gen.choose(-n, n).map(FiniteDuration(_, DAYS)),
diff --git a/kernel/src/main/scala/cats/kernel/Monoid.scala b/kernel/src/main/scala/cats/kernel/Monoid.scala
index e79cb88dc2..5e86dd0cd8 100644
--- a/kernel/src/main/scala/cats/kernel/Monoid.scala
+++ b/kernel/src/main/scala/cats/kernel/Monoid.scala
@@ -88,6 +88,8 @@ trait Monoid[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A] { se
     new Monoid[A] {
       def empty = self.empty
       def combine(a: A, b: A) = self.combine(b, a)
+      // a + a + a + ... is the same when reversed
+      override def combineN(a: A, n: Int): A = self.combineN(a, n)
       override def reverse = self
     }
 }

From c4e8104798898489f8ff73b4879c399ce0ecd2c4 Mon Sep 17 00:00:00 2001
From: "P. Oscar Boykin" <boykin@pobox.com>
Date: Sat, 1 Feb 2020 10:07:53 -1000
Subject: [PATCH 4/5] respond to review comments

---
 core/src/main/scala/cats/Foldable.scala              |  5 ++++-
 core/src/main/scala/cats/Reducible.scala             |  6 +-----
 .../main/scala/cats/kernel/laws/SemigroupLaws.scala  | 10 ----------
 .../cats/kernel/laws/discipline/SemigroupTests.scala |  2 --
 .../scala/cats/kernel/CommutativeSemigroup.scala     | 12 ++++++++++++
 kernel/src/main/scala/cats/kernel/Semigroup.scala    |  1 +
 .../scala/cats/kernel/instances/ListInstances.scala  |  2 +-
 .../cats/kernel/instances/StringInstances.scala      |  2 +-
 8 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/core/src/main/scala/cats/Foldable.scala b/core/src/main/scala/cats/Foldable.scala
index 55e8f5ca58..668614a0bd 100644
--- a/core/src/main/scala/cats/Foldable.scala
+++ b/core/src/main/scala/cats/Foldable.scala
@@ -757,7 +757,10 @@ import Foldable.sentinel
    * }}}
    */
   def intercalate[A](fa: F[A], a: A)(implicit A: Monoid[A]): A =
-    A.combineAll(intersperseList(toList(fa), a))
+    combineAllOption(fa)(A.intercalate(a)) match {
+      case None    => A.empty
+      case Some(a) => a
+    }
 
   protected def intersperseList[A](xs: List[A], x: A): List[A] = {
     val bld = List.newBuilder[A]
diff --git a/core/src/main/scala/cats/Reducible.scala b/core/src/main/scala/cats/Reducible.scala
index ac37ceffc7..a57ed051d4 100644
--- a/core/src/main/scala/cats/Reducible.scala
+++ b/core/src/main/scala/cats/Reducible.scala
@@ -238,11 +238,7 @@ import simulacrum.{noop, typeclass}
    * }}}
    */
   def nonEmptyIntercalate[A](fa: F[A], a: A)(implicit A: Semigroup[A]): A =
-    toNonEmptyList(fa) match {
-      case NonEmptyList(hd, Nil) => hd
-      case NonEmptyList(hd, tl) =>
-        Reducible[NonEmptyList].reduce(NonEmptyList(hd, a :: intersperseList(tl, a)))
-    }
+    reduce(fa)(A.intercalate(a))
 
   /**
    * Partition this Reducible by a separating function `A => Either[B, C]`
diff --git a/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala b/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala
index 25920c48a9..92b6a082e0 100644
--- a/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala
+++ b/kernel-laws/shared/src/main/scala/cats/kernel/laws/SemigroupLaws.scala
@@ -20,11 +20,6 @@ trait SemigroupLaws[A] {
   def reverseReverses(a: A, b: A): IsEq[A] =
     S.combine(a, b) <-> S.reverse.combine(b, a)
 
-  def reverseSemigroupAssociative(x: A, y: A, z: A): IsEq[A] = {
-    val rev = S.reverse
-    rev.combine(rev.combine(x, y), z) <-> rev.combine(x, rev.combine(y, z))
-  }
-
   def reverseRepeat1(a: A): IsEq[A] = {
     val rev = S.reverse
     rev.combineN(a, 1) <-> a
@@ -43,11 +38,6 @@ trait SemigroupLaws[A] {
   def intercalateIntercalates(a: A, m: A, b: A): IsEq[A] =
     S.combine(a, S.combine(m, b)) <-> S.intercalate(m).combine(a, b)
 
-  def intercalateSemigroupAssociative(m: A, x: A, y: A, z: A): IsEq[A] = {
-    val withMiddle = S.intercalate(m)
-    withMiddle.combine(withMiddle.combine(x, y), z) <-> withMiddle.combine(x, withMiddle.combine(y, z))
-  }
-
   def intercalateRepeat1(m: A, a: A): IsEq[A] = {
     val withMiddle = S.intercalate(m)
     withMiddle.combineN(a, 1) <-> a
diff --git a/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala b/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala
index 75ab4050ef..94ef8b79cf 100644
--- a/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala
+++ b/kernel-laws/shared/src/main/scala/cats/kernel/laws/discipline/SemigroupTests.scala
@@ -20,12 +20,10 @@ trait SemigroupTests[A] extends Laws {
       "repeat2" -> forAll(laws.repeat2 _),
       "combineAllOption" -> forAll(laws.combineAllOption _),
       "reverseReverses" -> forAll(laws.reverseReverses _),
-      "reverseAssociative" -> forAll(laws.reverseSemigroupAssociative _),
       "reverseRepeat1" -> forAll(laws.reverseRepeat1 _),
       "reverseRepeat2" -> forAll(laws.reverseRepeat2 _),
       "reverseCombineAllOption" -> forAll(laws.reverseCombineAllOption _),
       "intercalateIntercalates" -> forAll(laws.intercalateIntercalates _),
-      "intercalateAssociative" -> forAll(laws.intercalateSemigroupAssociative _),
       "intercalateRepeat1" -> forAll(laws.intercalateRepeat1 _),
       "intercalateRepeat2" -> forAll(laws.intercalateRepeat2 _),
       "intercalateCombineAllOption" -> forAll(laws.intercalateCombineAllOption _)
diff --git a/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala b/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
index 8bf6b61808..9cdb22c6a2 100644
--- a/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
+++ b/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
@@ -9,6 +9,18 @@ import scala.{specialized => sp}
  */
 trait CommutativeSemigroup[@sp(Int, Long, Float, Double) A] extends Any with Semigroup[A] { self =>
   override def reverse: CommutativeSemigroup[A] = self
+  override def intercalate(middle: A): CommutativeSemigroup[A] =
+    new CommutativeSemigroup[A] {
+      def combine(a: A, b: A): A =
+        self.combine(a, self.combine(middle, b))
+
+      override def combineN(a: A, n: Int): A =
+        if (n <= 1) self.combineN(a, n)
+        else {
+          // a + m + a ... = combineN(a, n) + combineN(m, n - 1)
+          self.combine(self.combineN(a, n), self.combineN(middle, n - 1))
+        }
+    }
 }
 
 object CommutativeSemigroup extends SemigroupFunctions[CommutativeSemigroup] {
diff --git a/kernel/src/main/scala/cats/kernel/Semigroup.scala b/kernel/src/main/scala/cats/kernel/Semigroup.scala
index 980c14c226..2628eb21c2 100644
--- a/kernel/src/main/scala/cats/kernel/Semigroup.scala
+++ b/kernel/src/main/scala/cats/kernel/Semigroup.scala
@@ -92,6 +92,7 @@ trait Semigroup[@sp(Int, Long, Float, Double) A] extends Any with Serializable {
 
   /**
    * Between each pair of elements insert middle
+   * This name matches the term used in Foldable and Reducible and a similar Haskell function.
    */
   def intercalate(middle: A): Semigroup[A] =
     new Semigroup[A] {
diff --git a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
index 5595999b58..a23e846469 100644
--- a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
+++ b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala
@@ -99,7 +99,7 @@ class ListMonoid[A] extends Monoid[List[A]] { self =>
       def combine(x: List[A], y: List[A]) = y ::: x
 
       override def combineAll(xs: IterableOnce[List[A]]): List[A] =
-        xs.foldLeft(empty) { (acc, item) =>
+        xs.iterator.foldLeft(empty) { (acc, item) =>
           item ::: acc
         }
 
diff --git a/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala b/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
index 815231f8ee..fdd5a31913 100644
--- a/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
+++ b/kernel/src/main/scala/cats/kernel/instances/StringInstances.scala
@@ -38,7 +38,7 @@ class StringMonoid extends Monoid[String] { self =>
       def empty = self.empty
       def combine(x: String, y: String) = y + x
       override def combineAll(xs: IterableOnce[String]): String = {
-        val revStrings = xs.foldLeft(List.empty[String]) { (acc, s) =>
+        val revStrings = xs.iterator.foldLeft(List.empty[String]) { (acc, s) =>
           s :: acc
         }
         self.combineAll(revStrings)

From 86a50882141721f5f70626e1724799b3a933b614 Mon Sep 17 00:00:00 2001
From: "P. Oscar Boykin" <boykin@pobox.com>
Date: Sat, 1 Feb 2020 11:01:51 -1000
Subject: [PATCH 5/5] remove optimization of commutative semigroup intercalate
 combineN which fails for almost commutative BigDecimal

---
 .../src/main/scala/cats/kernel/CommutativeSemigroup.scala  | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala b/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
index 9cdb22c6a2..ed2a036c30 100644
--- a/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
+++ b/kernel/src/main/scala/cats/kernel/CommutativeSemigroup.scala
@@ -13,13 +13,6 @@ trait CommutativeSemigroup[@sp(Int, Long, Float, Double) A] extends Any with Sem
     new CommutativeSemigroup[A] {
       def combine(a: A, b: A): A =
         self.combine(a, self.combine(middle, b))
-
-      override def combineN(a: A, n: Int): A =
-        if (n <= 1) self.combineN(a, n)
-        else {
-          // a + m + a ... = combineN(a, n) + combineN(m, n - 1)
-          self.combine(self.combineN(a, n), self.combineN(middle, n - 1))
-        }
     }
 }