From 096f4b3ef7c3fad1c323a39c8f7830140baa9f32 Mon Sep 17 00:00:00 2001 From: Tim Golding Date: Fri, 31 May 2024 18:28:26 +0100 Subject: [PATCH 1/2] Add distinctBy to NonEmptyCollection and all impls --- .../scala-2.13+/cats/data/NonEmptyLazyList.scala | 13 ++++++++----- core/src/main/scala/cats/data/NonEmptyChain.scala | 5 ++++- .../main/scala/cats/data/NonEmptyCollection.scala | 1 + core/src/main/scala/cats/data/NonEmptyList.scala | 11 +++++++---- core/src/main/scala/cats/data/NonEmptySeq.scala | 13 ++++++++----- core/src/main/scala/cats/data/NonEmptyVector.scala | 13 ++++++++----- .../cats/tests/NonEmptyLazyListSuite.scala | 7 +++++++ .../test/scala/cats/tests/NonEmptyChainSuite.scala | 6 ++++++ .../test/scala/cats/tests/NonEmptyListSuite.scala | 6 ++++++ .../test/scala/cats/tests/NonEmptySeqSuite.scala | 12 ++++++++++++ .../test/scala/cats/tests/NonEmptyVectorSuite.scala | 6 ++++++ 11 files changed, 73 insertions(+), 20 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala b/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala index 846b960ef1..08ee02dc40 100644 --- a/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala +++ b/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala @@ -345,14 +345,17 @@ class NonEmptyLazyListOps[A](private val value: NonEmptyLazyList[A]) /** * Remove duplicates. Duplicates are checked using `Order[_]` instance. */ - def distinct[AA >: A](implicit O: Order[AA]): NonEmptyLazyList[AA] = { - implicit val ord: Ordering[AA] = O.toOrdering + override def distinct[AA >: A](implicit O: Order[AA]): NonEmptyLazyList[AA] = distinctBy(identity[AA]) + + override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyLazyList[AA] = { + implicit val ord: Ordering[B] = O.toOrdering val buf = LazyList.newBuilder[AA] - toLazyList.foldLeft(TreeSet.empty[AA]) { (elementsSoFar, a) => - if (elementsSoFar(a)) elementsSoFar + toLazyList.foldLeft(TreeSet.empty[B]) { (elementsSoFar, a) => + val b = f(a) + if (elementsSoFar(b)) elementsSoFar else { - buf += a; elementsSoFar + a + buf += a; elementsSoFar + b } } diff --git a/core/src/main/scala/cats/data/NonEmptyChain.scala b/core/src/main/scala/cats/data/NonEmptyChain.scala index 3742a92268..094d0efdd0 100644 --- a/core/src/main/scala/cats/data/NonEmptyChain.scala +++ b/core/src/main/scala/cats/data/NonEmptyChain.scala @@ -603,7 +603,10 @@ class NonEmptyChainOps[A](private val value: NonEmptyChain[A]) * Remove duplicates. Duplicates are checked using `Order[_]` instance. */ final def distinct[AA >: A](implicit O: Order[AA]): NonEmptyChain[AA] = - create(toChain.distinct[AA]) + distinctBy(identity[AA]) + + final def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyChain[AA] = + create(toChain.distinctBy[B](f)) final def sortBy[B](f: A => B)(implicit B: Order[B]): NonEmptyChain[A] = create(toChain.sortBy(f)) final def sorted[AA >: A](implicit AA: Order[AA]): NonEmptyChain[AA] = create(toChain.sorted[AA]) diff --git a/core/src/main/scala/cats/data/NonEmptyCollection.scala b/core/src/main/scala/cats/data/NonEmptyCollection.scala index 72c158a84e..b71660ef3c 100644 --- a/core/src/main/scala/cats/data/NonEmptyCollection.scala +++ b/core/src/main/scala/cats/data/NonEmptyCollection.scala @@ -52,6 +52,7 @@ private[cats] trait NonEmptyCollection[+A, U[+_], NE[+_]] extends Any { def zipWithIndex: NE[(A, Int)] def distinct[AA >: A](implicit O: Order[AA]): NE[AA] + def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NE[AA] def sortBy[B](f: A => B)(implicit B: Order[B]): NE[A] def sorted[AA >: A](implicit AA: Order[AA]): NE[AA] def groupByNem[B](f: A => B)(implicit B: Order[B]): NonEmptyMap[B, NE[A]] diff --git a/core/src/main/scala/cats/data/NonEmptyList.scala b/core/src/main/scala/cats/data/NonEmptyList.scala index c2e86caa60..498ecb7ded 100644 --- a/core/src/main/scala/cats/data/NonEmptyList.scala +++ b/core/src/main/scala/cats/data/NonEmptyList.scala @@ -340,14 +340,17 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) extends NonEmptyCollec /** * Remove duplicates. Duplicates are checked using `Order[_]` instance. */ - def distinct[AA >: A](implicit O: Order[AA]): NonEmptyList[AA] = { - implicit val ord: Ordering[AA] = O.toOrdering + override def distinct[AA >: A](implicit O: Order[AA]): NonEmptyList[AA] = distinctBy(identity[AA]) + + override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyList[AA] = { + implicit val ord: Ordering[B] = O.toOrdering val buf = ListBuffer.empty[AA] - tail.foldLeft(TreeSet(head: AA)) { (elementsSoFar, b) => + tail.foldLeft(TreeSet(f(head): B)) { (elementsSoFar, a) => + val b = f(a) if (elementsSoFar(b)) elementsSoFar else { - buf += b; elementsSoFar + b + buf += a; elementsSoFar + b } } diff --git a/core/src/main/scala/cats/data/NonEmptySeq.scala b/core/src/main/scala/cats/data/NonEmptySeq.scala index f43ef75ac4..55632f9e34 100644 --- a/core/src/main/scala/cats/data/NonEmptySeq.scala +++ b/core/src/main/scala/cats/data/NonEmptySeq.scala @@ -236,14 +236,17 @@ final class NonEmptySeq[+A] private (val toSeq: Seq[A]) extends AnyVal with NonE /** * Remove duplicates. Duplicates are checked using `Order[_]` instance. */ - def distinct[AA >: A](implicit O: Order[AA]): NonEmptySeq[AA] = { - implicit val ord: Ordering[AA] = O.toOrdering + override def distinct[AA >: A](implicit O: Order[AA]): NonEmptySeq[AA] = distinctBy(identity[AA]) + + override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptySeq[AA] = { + implicit val ord: Ordering[B] = O.toOrdering val buf = Seq.newBuilder[AA] - tail.foldLeft(TreeSet(head: AA)) { (elementsSoFar, a) => - if (elementsSoFar(a)) elementsSoFar + tail.foldLeft(TreeSet(f(head): B)) { (elementsSoFar, a) => + val b = f(a) + if (elementsSoFar(b)) elementsSoFar else { - buf += a; elementsSoFar + a + buf += a; elementsSoFar + b } } diff --git a/core/src/main/scala/cats/data/NonEmptyVector.scala b/core/src/main/scala/cats/data/NonEmptyVector.scala index 583dd63612..d671f536fc 100644 --- a/core/src/main/scala/cats/data/NonEmptyVector.scala +++ b/core/src/main/scala/cats/data/NonEmptyVector.scala @@ -246,14 +246,17 @@ final class NonEmptyVector[+A] private (val toVector: Vector[A]) /** * Remove duplicates. Duplicates are checked using `Order[_]` instance. */ - def distinct[AA >: A](implicit O: Order[AA]): NonEmptyVector[AA] = { - implicit val ord: Ordering[AA] = O.toOrdering + override def distinct[AA >: A](implicit O: Order[AA]): NonEmptyVector[AA] = distinctBy(identity[AA]) + + override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyVector[AA] = { + implicit val ord: Ordering[B] = O.toOrdering val buf = Vector.newBuilder[AA] - tail.foldLeft(TreeSet(head: AA)) { (elementsSoFar, a) => - if (elementsSoFar(a)) elementsSoFar + tail.foldLeft(TreeSet(f(head): B)) { (elementsSoFar, a) => + val b = f(a) + if (elementsSoFar(b)) elementsSoFar else { - buf += a; elementsSoFar + a + buf += a; elementsSoFar + b } } diff --git a/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala b/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala index a4cd1352e5..2ecd733d39 100644 --- a/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala +++ b/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala @@ -49,6 +49,7 @@ class NonEmptyLazyListSuite extends NonEmptyCollectionSuite[LazyList, NonEmptyLa checkAll(s"NonEmptyLazyList[Int]", HashTests[NonEmptyLazyList[Int]].hash) checkAll(s"Hash[NonEmptyLazyList[Int]]", SerializableTests.serializable(Hash[NonEmptyLazyList[Int]])) + checkAll("NonEmptyLazyList[Int]", NonEmptyAlternativeTests[NonEmptyLazyList].nonEmptyAlternative[Int, Int, Int]) checkAll("NonEmptyLazyList[Int]", NonEmptyAlternativeTests[NonEmptyLazyList].nonEmptyAlternative[Int, Int, Int]) checkAll("NonEmptyAlternative[NonEmptyLazyList]", SerializableTests.serializable(NonEmptyAlternative[NonEmptyLazyList]) @@ -175,6 +176,12 @@ class NonEmptyLazyListSuite extends NonEmptyCollectionSuite[LazyList, NonEmptyLa } } + test("NonEmptyLazyList#distinctBy is consistent with List#distinctBy") { + forAll { (ci: NonEmptyLazyList[Int], f: Int => String) => + assert(ci.distinctBy(f).toList === (ci.toList.distinctBy(f))) + } + } + test("NonEmptyLazyList#distinct is consistent with List#distinct") { forAll { (ci: NonEmptyLazyList[Int]) => assert(ci.distinct.toList === (ci.toList.distinct)) diff --git a/tests/shared/src/test/scala/cats/tests/NonEmptyChainSuite.scala b/tests/shared/src/test/scala/cats/tests/NonEmptyChainSuite.scala index d90eb488ce..5fd0fadf74 100644 --- a/tests/shared/src/test/scala/cats/tests/NonEmptyChainSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/NonEmptyChainSuite.scala @@ -203,6 +203,12 @@ class NonEmptyChainSuite extends NonEmptyCollectionSuite[Chain, NonEmptyChain, N } } + test("NonEmptyChain#distinctBy is consistent with List#distinctBy") { + forAll { (ci: NonEmptyChain[Int], f: Int => String) => + assert(ci.distinctBy(f).toList === (ci.toList.distinctBy(f))) + } + } + test("NonEmptyChain#distinct is consistent with List#distinct") { forAll { (ci: NonEmptyChain[Int]) => assert(ci.distinct.toList === (ci.toList.distinct)) diff --git a/tests/shared/src/test/scala/cats/tests/NonEmptyListSuite.scala b/tests/shared/src/test/scala/cats/tests/NonEmptyListSuite.scala index 2cda3b290e..a72995abf8 100644 --- a/tests/shared/src/test/scala/cats/tests/NonEmptyListSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/NonEmptyListSuite.scala @@ -296,6 +296,12 @@ class NonEmptyListSuite extends NonEmptyCollectionSuite[List, NonEmptyList, NonE } } + test("NonEmptyList#distinctBy is consistent with List#distinctBy") { + forAll { (nel: NonEmptyList[Int], f: Int => String) => + assert(nel.distinctBy(f).toList === (nel.toList.distinctBy(f))) + } + } + test("NonEmptyList#reverse is consistent with List#reverse") { forAll { (nel: NonEmptyList[Int]) => assert(nel.reverse.toList === (nel.toList.reverse)) diff --git a/tests/shared/src/test/scala/cats/tests/NonEmptySeqSuite.scala b/tests/shared/src/test/scala/cats/tests/NonEmptySeqSuite.scala index 8c59cc1cf6..36899298bc 100644 --- a/tests/shared/src/test/scala/cats/tests/NonEmptySeqSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/NonEmptySeqSuite.scala @@ -90,4 +90,16 @@ class NonEmptySeqSuite extends NonEmptyCollectionSuite[Seq, NonEmptySeq, NonEmpt assert(a.zip(b).toSeq === (a.toSeq.zip(b.toSeq))) } } + + test("NonEmptySeq#distinct is consistent with List#distinct") { + forAll { (nes: NonEmptySeq[Int]) => + assert(nes.distinct.toList === (nes.toList.distinct)) + } + } + + test("NonEmptySeq#distinctBy is consistent with List#distinctBy") { + forAll { (nes: NonEmptySeq[Int], f: Int => String) => + assert(nes.distinctBy(f).toList === (nes.toList.distinctBy(f))) + } + } } diff --git a/tests/shared/src/test/scala/cats/tests/NonEmptyVectorSuite.scala b/tests/shared/src/test/scala/cats/tests/NonEmptyVectorSuite.scala index 13b2ae8276..cc38da31b8 100644 --- a/tests/shared/src/test/scala/cats/tests/NonEmptyVectorSuite.scala +++ b/tests/shared/src/test/scala/cats/tests/NonEmptyVectorSuite.scala @@ -359,6 +359,12 @@ class NonEmptyVectorSuite extends NonEmptyCollectionSuite[Vector, NonEmptyVector assert(compileErrors("val bad: NonEmptyVector[Int] = NonEmptyVector(Vector.empty[Int])").nonEmpty) } + test("NonEmptyVector#distinctBy is consistent with Vector#distinctBy") { + forAll { (nonEmptyVector: NonEmptyVector[Int], f: Int => String) => + assert(nonEmptyVector.distinctBy(f).toVector === (nonEmptyVector.toVector.distinctBy(f))) + } + } + test("NonEmptyVector#distinct is consistent with Vector#distinct") { forAll { (nonEmptyVector: NonEmptyVector[Int]) => assert(nonEmptyVector.distinct.toVector === (nonEmptyVector.toVector.distinct)) From a39d75fca080a30510aae8e989ff19b0d4649daf Mon Sep 17 00:00:00 2001 From: Tim Golding Date: Wed, 5 Jun 2024 11:32:57 +0100 Subject: [PATCH 2/2] Simplify type signature of distinctBy --- core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala | 4 ++-- core/src/main/scala/cats/data/NonEmptyChain.scala | 2 +- core/src/main/scala/cats/data/NonEmptyCollection.scala | 2 +- core/src/main/scala/cats/data/NonEmptyList.scala | 4 ++-- core/src/main/scala/cats/data/NonEmptySeq.scala | 4 ++-- core/src/main/scala/cats/data/NonEmptyVector.scala | 4 ++-- .../test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala | 1 - 7 files changed, 10 insertions(+), 11 deletions(-) diff --git a/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala b/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala index 08ee02dc40..6faf08c5b6 100644 --- a/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala +++ b/core/src/main/scala-2.13+/cats/data/NonEmptyLazyList.scala @@ -347,10 +347,10 @@ class NonEmptyLazyListOps[A](private val value: NonEmptyLazyList[A]) */ override def distinct[AA >: A](implicit O: Order[AA]): NonEmptyLazyList[AA] = distinctBy(identity[AA]) - override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyLazyList[AA] = { + override def distinctBy[B](f: A => B)(implicit O: Order[B]): NonEmptyLazyList[A] = { implicit val ord: Ordering[B] = O.toOrdering - val buf = LazyList.newBuilder[AA] + val buf = LazyList.newBuilder[A] toLazyList.foldLeft(TreeSet.empty[B]) { (elementsSoFar, a) => val b = f(a) if (elementsSoFar(b)) elementsSoFar diff --git a/core/src/main/scala/cats/data/NonEmptyChain.scala b/core/src/main/scala/cats/data/NonEmptyChain.scala index 094d0efdd0..3ab876bc43 100644 --- a/core/src/main/scala/cats/data/NonEmptyChain.scala +++ b/core/src/main/scala/cats/data/NonEmptyChain.scala @@ -605,7 +605,7 @@ class NonEmptyChainOps[A](private val value: NonEmptyChain[A]) final def distinct[AA >: A](implicit O: Order[AA]): NonEmptyChain[AA] = distinctBy(identity[AA]) - final def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyChain[AA] = + final def distinctBy[B](f: A => B)(implicit O: Order[B]): NonEmptyChain[A] = create(toChain.distinctBy[B](f)) final def sortBy[B](f: A => B)(implicit B: Order[B]): NonEmptyChain[A] = create(toChain.sortBy(f)) diff --git a/core/src/main/scala/cats/data/NonEmptyCollection.scala b/core/src/main/scala/cats/data/NonEmptyCollection.scala index b71660ef3c..6debb0e055 100644 --- a/core/src/main/scala/cats/data/NonEmptyCollection.scala +++ b/core/src/main/scala/cats/data/NonEmptyCollection.scala @@ -52,7 +52,7 @@ private[cats] trait NonEmptyCollection[+A, U[+_], NE[+_]] extends Any { def zipWithIndex: NE[(A, Int)] def distinct[AA >: A](implicit O: Order[AA]): NE[AA] - def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NE[AA] + def distinctBy[B](f: A => B)(implicit O: Order[B]): NE[A] def sortBy[B](f: A => B)(implicit B: Order[B]): NE[A] def sorted[AA >: A](implicit AA: Order[AA]): NE[AA] def groupByNem[B](f: A => B)(implicit B: Order[B]): NonEmptyMap[B, NE[A]] diff --git a/core/src/main/scala/cats/data/NonEmptyList.scala b/core/src/main/scala/cats/data/NonEmptyList.scala index 498ecb7ded..b86ac76ec9 100644 --- a/core/src/main/scala/cats/data/NonEmptyList.scala +++ b/core/src/main/scala/cats/data/NonEmptyList.scala @@ -342,10 +342,10 @@ final case class NonEmptyList[+A](head: A, tail: List[A]) extends NonEmptyCollec */ override def distinct[AA >: A](implicit O: Order[AA]): NonEmptyList[AA] = distinctBy(identity[AA]) - override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyList[AA] = { + override def distinctBy[B](f: A => B)(implicit O: Order[B]): NonEmptyList[A] = { implicit val ord: Ordering[B] = O.toOrdering - val buf = ListBuffer.empty[AA] + val buf = ListBuffer.empty[A] tail.foldLeft(TreeSet(f(head): B)) { (elementsSoFar, a) => val b = f(a) if (elementsSoFar(b)) elementsSoFar diff --git a/core/src/main/scala/cats/data/NonEmptySeq.scala b/core/src/main/scala/cats/data/NonEmptySeq.scala index 55632f9e34..65d76440fb 100644 --- a/core/src/main/scala/cats/data/NonEmptySeq.scala +++ b/core/src/main/scala/cats/data/NonEmptySeq.scala @@ -238,10 +238,10 @@ final class NonEmptySeq[+A] private (val toSeq: Seq[A]) extends AnyVal with NonE */ override def distinct[AA >: A](implicit O: Order[AA]): NonEmptySeq[AA] = distinctBy(identity[AA]) - override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptySeq[AA] = { + override def distinctBy[B](f: A => B)(implicit O: Order[B]): NonEmptySeq[A] = { implicit val ord: Ordering[B] = O.toOrdering - val buf = Seq.newBuilder[AA] + val buf = Seq.newBuilder[A] tail.foldLeft(TreeSet(f(head): B)) { (elementsSoFar, a) => val b = f(a) if (elementsSoFar(b)) elementsSoFar diff --git a/core/src/main/scala/cats/data/NonEmptyVector.scala b/core/src/main/scala/cats/data/NonEmptyVector.scala index d671f536fc..b2cca67fe1 100644 --- a/core/src/main/scala/cats/data/NonEmptyVector.scala +++ b/core/src/main/scala/cats/data/NonEmptyVector.scala @@ -248,10 +248,10 @@ final class NonEmptyVector[+A] private (val toVector: Vector[A]) */ override def distinct[AA >: A](implicit O: Order[AA]): NonEmptyVector[AA] = distinctBy(identity[AA]) - override def distinctBy[AA >: A, B](f: A => B)(implicit O: Order[B]): NonEmptyVector[AA] = { + override def distinctBy[B](f: A => B)(implicit O: Order[B]): NonEmptyVector[A] = { implicit val ord: Ordering[B] = O.toOrdering - val buf = Vector.newBuilder[AA] + val buf = Vector.newBuilder[A] tail.foldLeft(TreeSet(f(head): B)) { (elementsSoFar, a) => val b = f(a) if (elementsSoFar(b)) elementsSoFar diff --git a/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala b/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala index 2ecd733d39..6043bb870e 100644 --- a/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala +++ b/tests/shared/src/test/scala-2.13+/cats/tests/NonEmptyLazyListSuite.scala @@ -49,7 +49,6 @@ class NonEmptyLazyListSuite extends NonEmptyCollectionSuite[LazyList, NonEmptyLa checkAll(s"NonEmptyLazyList[Int]", HashTests[NonEmptyLazyList[Int]].hash) checkAll(s"Hash[NonEmptyLazyList[Int]]", SerializableTests.serializable(Hash[NonEmptyLazyList[Int]])) - checkAll("NonEmptyLazyList[Int]", NonEmptyAlternativeTests[NonEmptyLazyList].nonEmptyAlternative[Int, Int, Int]) checkAll("NonEmptyLazyList[Int]", NonEmptyAlternativeTests[NonEmptyLazyList].nonEmptyAlternative[Int, Int, Int]) checkAll("NonEmptyAlternative[NonEmptyLazyList]", SerializableTests.serializable(NonEmptyAlternative[NonEmptyLazyList])