From d1e1277f96551a8a02e70ed81cad62c670ade9b0 Mon Sep 17 00:00:00 2001 From: Travis Brown Date: Tue, 19 Nov 2019 16:24:59 -0600 Subject: [PATCH 1/4] Revert "Issue 2891 - Ambiguous Vector instances (#3100)" This reverts commit e76699849727bd3bdf4a64b905f1df1446adf86b. --- .../scala/cats/kernel/instances/ListInstances.scala | 10 ++++------ .../scala/cats/kernel/instances/QueueInstances.scala | 10 ++++------ .../scala/cats/kernel/instances/VectorInstances.scala | 11 ++++------- tests/src/test/scala/cats/tests/ListSuite.scala | 11 ----------- tests/src/test/scala/cats/tests/QueueSuite.scala | 11 ----------- tests/src/test/scala/cats/tests/VectorSuite.scala | 11 ----------- 6 files changed, 12 insertions(+), 52 deletions(-) diff --git a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala index 40b705ef70..0e1cf882ce 100644 --- a/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/ListInstances.scala @@ -13,16 +13,14 @@ trait ListInstances extends ListInstances1 { } private[instances] trait ListInstances1 extends ListInstances2 { - implicit def catsKernelStdHashForList[A: Hash]: Hash[List[A]] = - new ListHash[A] -} - -private[instances] trait ListInstances2 extends ListInstances3 { implicit def catsKernelStdPartialOrderForList[A: PartialOrder]: PartialOrder[List[A]] = new ListPartialOrder[A] + + implicit def catsKernelStdHashForList[A: Hash]: Hash[List[A]] = + new ListHash[A] } -private[instances] trait ListInstances3 { +private[instances] trait ListInstances2 { implicit def catsKernelStdEqForList[A: Eq]: Eq[List[A]] = new ListEq[A] } diff --git a/kernel/src/main/scala/cats/kernel/instances/QueueInstances.scala b/kernel/src/main/scala/cats/kernel/instances/QueueInstances.scala index 913fb8cf60..76a8ec7e18 100644 --- a/kernel/src/main/scala/cats/kernel/instances/QueueInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/QueueInstances.scala @@ -13,16 +13,14 @@ trait QueueInstances extends QueueInstances1 { } private[instances] trait QueueInstances1 extends QueueInstances2 { - implicit def catsKernelStdHashForQueue[A: Hash]: Hash[Queue[A]] = - new QueueHash[A] -} - -private[instances] trait QueueInstances2 extends QueueInstances3 { implicit def catsKernelStdPartialOrderForQueue[A: PartialOrder]: PartialOrder[Queue[A]] = new QueuePartialOrder[A] + + implicit def catsKernelStdHashForQueue[A: Hash]: Hash[Queue[A]] = + new QueueHash[A] } -private[instances] trait QueueInstances3 { +private[instances] trait QueueInstances2 { implicit def catsKernelStdEqForQueue[A: Eq]: Eq[Queue[A]] = new QueueEq[A] } diff --git a/kernel/src/main/scala/cats/kernel/instances/VectorInstances.scala b/kernel/src/main/scala/cats/kernel/instances/VectorInstances.scala index 5acc4a3488..75193a7e64 100644 --- a/kernel/src/main/scala/cats/kernel/instances/VectorInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/VectorInstances.scala @@ -6,22 +6,19 @@ import compat.scalaVersionSpecific._ trait VectorInstances extends VectorInstances1 { implicit def catsKernelStdOrderForVector[A: Order]: Order[Vector[A]] = new VectorOrder[A] - implicit def catsKernelStdMonoidForVector[A]: Monoid[Vector[A]] = new VectorMonoid[A] } private[instances] trait VectorInstances1 extends VectorInstances2 { - implicit def catsKernelStdHashForVector[A: Hash]: Hash[Vector[A]] = - new VectorHash[A] -} - -private[instances] trait VectorInstances2 extends VectorInstances3 { implicit def catsKernelStdPartialOrderForVector[A: PartialOrder]: PartialOrder[Vector[A]] = new VectorPartialOrder[A] + + implicit def catsKernelStdHashForVector[A: Hash]: Hash[Vector[A]] = + new VectorHash[A] } -private[instances] trait VectorInstances3 { +private[instances] trait VectorInstances2 { implicit def catsKernelStdEqForVector[A: Eq]: Eq[Vector[A]] = new VectorEq[A] } diff --git a/tests/src/test/scala/cats/tests/ListSuite.scala b/tests/src/test/scala/cats/tests/ListSuite.scala index 9befc591b2..9675fc6c97 100644 --- a/tests/src/test/scala/cats/tests/ListSuite.scala +++ b/tests/src/test/scala/cats/tests/ListSuite.scala @@ -64,17 +64,6 @@ class ListSuite extends CatsSuite { l.show should ===(l.toString) } } - - test("the instance for `Eq[List[A]]` is not ambiguous when A has a Hash and a PartialOrder") { - - import cats.kernel.{Hash, PartialOrder} - - trait A - implicit def po: PartialOrder[A] = ??? - implicit def ho: Hash[A] = ??? - - lazy val _ = implicitly[Eq[List[A]]] - } } final class ListInstancesSuite extends AnyFunSuiteLike { diff --git a/tests/src/test/scala/cats/tests/QueueSuite.scala b/tests/src/test/scala/cats/tests/QueueSuite.scala index ca693536e4..255adc0f6f 100644 --- a/tests/src/test/scala/cats/tests/QueueSuite.scala +++ b/tests/src/test/scala/cats/tests/QueueSuite.scala @@ -35,15 +35,4 @@ class QueueSuite extends CatsSuite { Queue(1, 2, 3).show should ===("Queue(1, 2, 3)") Queue.empty[Int].show should ===("Queue()") } - - test("the instance for `Eq[Queue[A]]` is not ambiguous when A has a Hash and a PartialOrder") { - - import cats.kernel.{Hash, PartialOrder} - - trait A - implicit def po: PartialOrder[A] = ??? - implicit def ho: Hash[A] = ??? - - lazy val _ = implicitly[Eq[Queue[A]]] - } } diff --git a/tests/src/test/scala/cats/tests/VectorSuite.scala b/tests/src/test/scala/cats/tests/VectorSuite.scala index 51289748d7..baa0295bb8 100644 --- a/tests/src/test/scala/cats/tests/VectorSuite.scala +++ b/tests/src/test/scala/cats/tests/VectorSuite.scala @@ -59,17 +59,6 @@ class VectorSuite extends CatsSuite { test("toNev on empty vector returns None") { assert(Vector.empty[Int].toNev == None) } - - test("the instance for `Eq[Vector[A]]` is not ambiguous when A has a Hash and a PartialOrder") { - - import cats.kernel.{Hash, PartialOrder} - - trait A - implicit def po: PartialOrder[A] = ??? - implicit def ho: Hash[A] = ??? - - lazy val _ = implicitly[Eq[Vector[A]]] - } } final class VectorInstancesSuite extends AnyFunSuiteLike { From efd88a0b924053aa02f77125f20716144d06fae7 Mon Sep 17 00:00:00 2001 From: Travis Brown Date: Tue, 19 Nov 2019 16:30:33 -0600 Subject: [PATCH 2/4] Revert "Changed precedence of tupled instances for Hash and PartialOrder (#3105)" This reverts commit 6e7ff8f74b9bb44b63d61754ef26df47ba96d01a. --- PULL_REQUEST_TEMPLATE.md | 2 +- project/KernelBoiler.scala | 12 +++++----- tests/src/test/scala/cats/tests/EqSuite.scala | 22 ------------------- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index 172e3122b5..053efff255 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -1,5 +1,5 @@ Thank you for contributing to Cats! -This is a kind reminder to run `sbt +prePR` and commit the changed files, if any, before submitting. +This is a kind reminder to run `sbt prePR` and commit the changed files, if any, before submitting. diff --git a/project/KernelBoiler.scala b/project/KernelBoiler.scala index e0412152eb..105e12dba3 100644 --- a/project/KernelBoiler.scala +++ b/project/KernelBoiler.scala @@ -201,6 +201,12 @@ object KernelBoiler { .mkString(s"$tupleNHeader(", ", ", ")")}.hashCode() def eqv(x: ${`(A..N)`}, y: ${`(A..N)`}): Boolean = ${binMethod("eqv").mkString(" && ")} } + + implicit def catsKernelStdPartialOrderForTuple${arity}[${`A..N`}](implicit ${constraints("PartialOrder")}): PartialOrder[${`(A..N)`}] = + new PartialOrder[${`(A..N)`}] { + def partialCompare(x: ${`(A..N)`}, y: ${`(A..N)`}): Double = + ${binMethod("partialCompare").mkString("Array(", ", ", ")")}.find(_ != 0.0).getOrElse(0.0) + } """ } ), @@ -211,12 +217,6 @@ object KernelBoiler { import tv._ def content = block""" - implicit def catsKernelStdPartialOrderForTuple${arity}[${`A..N`}](implicit ${constraints("PartialOrder")}): PartialOrder[${`(A..N)`}] = - new PartialOrder[${`(A..N)`}] { - def partialCompare(x: ${`(A..N)`}, y: ${`(A..N)`}): Double = - ${binMethod("partialCompare").mkString("Array(", ", ", ")")}.find(_ != 0.0).getOrElse(0.0) - } - implicit def catsKernelStdBandForTuple${arity}[${`A..N`}](implicit ${constraints("Band")}): Band[${`(A..N)`}] = new Band[${`(A..N)`}] { def combine(x: ${`(A..N)`}, y: ${`(A..N)`}): ${`(A..N)`} = ${binTuple("combine")} diff --git a/tests/src/test/scala/cats/tests/EqSuite.scala b/tests/src/test/scala/cats/tests/EqSuite.scala index 1b483010d6..6898df5828 100644 --- a/tests/src/test/scala/cats/tests/EqSuite.scala +++ b/tests/src/test/scala/cats/tests/EqSuite.scala @@ -6,8 +6,6 @@ import cats.laws.discipline.{ContravariantMonoidalTests, MiniInt} import cats.laws.discipline.arbitrary._ import cats.laws.discipline.eq._ -import scala.collection.immutable.Queue - class EqSuite extends CatsSuite { Invariant[Eq] Contravariant[Eq] @@ -17,24 +15,4 @@ class EqSuite extends CatsSuite { checkAll("Eq", ContravariantMonoidalTests[Eq].contravariantMonoidal[MiniInt, Boolean, Boolean]) checkAll("ContravariantMonoidal[Eq]", SerializableTests.serializable(ContravariantMonoidal[Eq])) - test("The Eq instance for tuples of A is not ambiguous when A has a Hash and a PartialOrder") { - - import cats.kernel.{Hash, PartialOrder} - - trait A - implicit def po: PartialOrder[A] = ??? - implicit def ho: Hash[A] = ??? - - lazy val a2 = implicitly[Eq[(A, A)]] - lazy val b2 = implicitly[Eq[(List[A], List[A])]] - lazy val c2 = implicitly[Eq[(Set[A], Set[A])]] - lazy val d2 = implicitly[Eq[(Vector[A], Vector[A])]] - lazy val e2 = implicitly[Eq[(Queue[A], Queue[A])]] - - lazy val a3 = implicitly[Eq[(A, A, A)]] - lazy val b3 = implicitly[Eq[(List[A], List[A], List[A])]] - lazy val c3 = implicitly[Eq[(Set[A], Set[A], Set[A])]] - lazy val d3 = implicitly[Eq[(Vector[A], Vector[A], Vector[A])]] - lazy val e3 = implicitly[Eq[(Queue[A], Queue[A], Queue[A])]] - } } From d2a4b4e4c9d4b7f963ea6a445848d5b07b5810ea Mon Sep 17 00:00:00 2001 From: Travis Brown Date: Tue, 19 Nov 2019 16:31:34 -0600 Subject: [PATCH 3/4] Move method back to avoid bincompat breakage --- .../main/scala/cats/kernel/instances/OptionInstances.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/src/main/scala/cats/kernel/instances/OptionInstances.scala b/kernel/src/main/scala/cats/kernel/instances/OptionInstances.scala index b644c3ac67..dcee07216e 100644 --- a/kernel/src/main/scala/cats/kernel/instances/OptionInstances.scala +++ b/kernel/src/main/scala/cats/kernel/instances/OptionInstances.scala @@ -6,13 +6,13 @@ trait OptionInstances extends OptionInstances0 { new OptionOrder[A] implicit def catsKernelStdCommutativeMonoidForOption[A: CommutativeSemigroup]: CommutativeMonoid[Option[A]] = new OptionCommutativeMonoid[A] + implicit def catsKernelStdMonoidForOption[A: Semigroup]: Monoid[Option[A]] = + new OptionMonoid[A] } private[instances] trait OptionInstances0 extends OptionInstances1 { implicit def catsKernelStdPartialOrderForOption[A: PartialOrder]: PartialOrder[Option[A]] = new OptionPartialOrder[A] - implicit def catsKernelStdMonoidForOption[A: Semigroup]: Monoid[Option[A]] = - new OptionMonoid[A] } private[instances] trait OptionInstances1 extends OptionInstances2 { From d6e940b7d7c24d2368b67e5b9e2bbbff685a4f95 Mon Sep 17 00:00:00 2001 From: Travis Brown Date: Tue, 19 Nov 2019 16:32:49 -0600 Subject: [PATCH 4/4] Reinstate doc fix from #3105 --- PULL_REQUEST_TEMPLATE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md index 053efff255..d11bc1346c 100644 --- a/PULL_REQUEST_TEMPLATE.md +++ b/PULL_REQUEST_TEMPLATE.md @@ -1,5 +1,5 @@ Thank you for contributing to Cats! -This is a kind reminder to run `sbt prePR` and commit the changed files, if any, before submitting. +This is a kind reminder to run `sbt +prePR` and commit the changed files, if any, before submitting.