Skip to content

Commit

Permalink
Add consistency laws for .combineAll and .combineAllOption.
Browse files Browse the repository at this point in the history
These tests will catch sitautions where the optimized methods are
broken (i.e. become inconsistent with the behavior of .combine).
These tests caught an issue with Monoid[Map[K, V]].combineAll, which
was broken.

Fixes typelevel#1346.
  • Loading branch information
erik-stripe committed Sep 1, 2016
1 parent 449e0b0 commit ba1e07e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
14 changes: 11 additions & 3 deletions kernel-laws/src/main/scala/cats/kernel/laws/GroupLaws.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cats.kernel.laws
package cats.kernel
package laws

import cats.kernel._
import cats.kernel.instances.option._

import org.typelevel.discipline.Laws

Expand All @@ -27,7 +29,10 @@ trait GroupLaws[A] extends Laws {
Rules.serializable(A),
Rules.associativity(A.combine),
Rules.repeat1("combineN")(A.combineN),
Rules.repeat2("combineN", "|+|")(A.combineN)(A.combine)
Rules.repeat2("combineN", "|+|")(A.combineN)(A.combine),
"combineAllOption" -> forAll { (xs: Vector[A]) =>
A.combineAllOption(xs) ?== xs.reduceOption(A.combine)
}
)

def band(implicit A: Band[A]): GroupProperties = new GroupProperties(
Expand Down Expand Up @@ -55,7 +60,10 @@ trait GroupLaws[A] extends Laws {
Rules.rightIdentity(A.empty)(A.combine),
Rules.repeat0("combineN", "id", A.empty)(A.combineN),
Rules.collect0("combineAll", "id", A.empty)(A.combineAll),
Rules.isId("isEmpty", A.empty)(A.isEmpty)
Rules.isId("isEmpty", A.empty)(A.isEmpty),
"combineAll" -> forAll { (xs: Vector[A]) =>
A.combineAll(xs) ?== (A.empty +: xs).reduce(A.combine)
}
)

def commutativeMonoid(implicit A: CommutativeMonoid[A]): GroupProperties = new GroupProperties(
Expand Down
2 changes: 1 addition & 1 deletion kernel/src/main/scala/cats/kernel/instances/map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class MapMonoid[K, V](implicit V: Semigroup[V]) extends Monoid[Map[K, V]] {
val it = m.iterator
while (it.hasNext) {
val (k, v) = it.next
m.updated(k, Semigroup.maybeCombine(m.get(k), v))
acc(k) = Semigroup.maybeCombine(acc.get(k), v)
}
}
StaticMethods.wrapMutableMap(acc)
Expand Down

0 comments on commit ba1e07e

Please sign in to comment.