new 2.12-removal-enabled features for NonEmpty (#11933)

* replace pour with a new, total, uncurried apply to create NonEmpty's * use the new NonEmpty apply in place of pour * non-empty cons, snoc, head, tail * add map and flatMap for NonEmpty iterables * remove scala-collection-compat from scalautils * tests for map, flatMap, cons * no changelog CHANGELOG_BEGIN CHANGELOG_END * missing 'extends AnyVal' * colliding map and flatMap for Maps * Revert "colliding map and flatMap for Maps" * more specific Map and Set return types * type tests for map operations * add 'to' conversions
digital-asset · Dec 7, 2021 · c4d82f7 · c4d82f7
1 parent 9e5bea1
commit c4d82f7
Show file tree

Hide file tree

Showing 7 changed files with 130 additions and 55 deletions.
diff --git a/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/QueryBenchmark.scala b/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/QueryBenchmark.scala
@@ -52,7 +52,7 @@ trait QueryBenchmark extends ContractDaoBenchmark {
     val result = instanceUUIDLogCtx(implicit lc =>
       dao
         .transact(
-          ContractDao.selectContracts(NonEmpty.pour(Party(party)) into Set, tpid, fr"1 = 1")
+          ContractDao.selectContracts(NonEmpty(Set, Party(party)), tpid, fr"1 = 1")
         )
         .unsafeRunSync()
     )

diff --git a/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/QueryPayloadBenchmark.scala b/ledger-service/http-json/src/bench/scala/com/daml/http/dbbackend/QueryPayloadBenchmark.scala
@@ -76,7 +76,7 @@ trait QueryPayloadBenchmark extends ContractDaoBenchmark {
     val result = instanceUUIDLogCtx(implicit lc =>
       dao
         .transact(
-          ContractDao.selectContracts(NonEmpty.pour(Party(party)) into Set, tpid, whereClause)
+          ContractDao.selectContracts(NonEmpty(Set, Party(party)), tpid, whereClause)
         )
         .unsafeRunSync()
     )

diff --git a/ledger-service/http-json/src/test/scala/com/digitalasset/http/DomainSpec.scala b/ledger-service/http-json/src/test/scala/com/digitalasset/http/DomainSpec.scala
@@ -20,13 +20,13 @@ final class DomainSpec extends AnyFreeSpec with Matchers {
     "parties deduplicates between actAs/submitter and readAs" in {
       val payload =
         JwtWritePayload(ledgerId, appId, submitter = NonEmptyList(alice), readAs = List(alice, bob))
-      payload.parties should ===(NonEmpty.pour(alice, bob) into Set)
+      payload.parties should ===(NonEmpty(Set, alice, bob))
     }
   }
   "JwtPayload" - {
     "parties deduplicates between actAs and readAs" in {
       val payload = JwtPayload(ledgerId, appId, actAs = List(alice), readAs = List(alice, bob))
-      payload.map(_.parties) should ===(Some(NonEmpty.pour(alice, bob) into Set))
+      payload.map(_.parties) should ===(Some(NonEmpty(Set, alice, bob)))
     }
     "returns None if readAs and actAs are empty" in {
       val payload = JwtPayload(ledgerId, appId, actAs = List(), readAs = List())

diff --git a/libs-scala/scala-utils/BUILD.bazel b/libs-scala/scala-utils/BUILD.bazel
@@ -25,7 +25,6 @@ da_scala_library(
         silencer_plugin,
     ],
     scala_deps = [
-        "@maven//:org_scala_lang_modules_scala_collection_compat",
         "@maven//:org_scalaz_scalaz_core",
     ],
     scalacopts = scalacopts,

diff --git a/libs-scala/scala-utils/src/main/scala/com/daml/scalautil/nonempty/NonEmpty.scala b/libs-scala/scala-utils/src/main/scala/com/daml/scalautil/nonempty/NonEmpty.scala
@@ -1,24 +1,23 @@
 // Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-package com.daml.scalautil.nonempty
+package com.daml.scalautil
+package nonempty
 
-import scala.collection.compat._
-import scala.collection.{immutable => imm}, imm.Map, imm.Set
+import scala.collection.{Factory, IterableOnce, immutable => imm}, imm.Iterable, imm.Map, imm.Set
 import scalaz.Id.Id
 import scalaz.{Foldable, Foldable1, OneAnd, Semigroup, Traverse}
 import scalaz.Leibniz, Leibniz.===
 import scalaz.Liskov, Liskov.<~<
 import scalaz.syntax.std.option._
 
-import com.daml.scalautil.FoldableContravariant
+import Statement.discard
 import NonEmptyCollCompat._
 
 /** The visible interface of [[NonEmpty]]; use that value to access
   * these members.
   */
 sealed abstract class NonEmptyColl {
-  import NonEmptyColl.Pouring
 
   /** Use its alias [[com.daml.scalautil.nonempty.NonEmpty]]. */
   type NonEmpty[+A]
@@ -43,17 +42,17 @@ sealed abstract class NonEmptyColl {
     */
   def equiv[F[_], A]: NonEmpty[F[A]] === NonEmptyF[F, A]
 
-  /** Check whether `self` is non-empty; if so, return it as the non-empty subtype. */
-  @deprecated("misleading apply, use `case NonEmpty(xs)` instead", since = "1.18.0")
-  final def apply[Self](self: Self with imm.Iterable[_]): Option[NonEmpty[Self]] = unapply(self)
-
   /** {{{
-    *  NonEmpty.pour(1, 2, 3) into List : NonEmpty[List[Int]] // with (1, 2, 3) as elements
+    *  NonEmpty(List, 1, 2, 3) : NonEmpty[List[Int]] // with (1, 2, 3) as elements
     * }}}
-    *
-    * The weird argument order is to support Scala 2.12.
     */
-  final def pour[A](x: A, xs: A*): Pouring[A] = new Pouring(x, xs: _*)
+  final def apply[Fct, A, C <: imm.Iterable[A]](into: Fct, hd: A, tl: A*)(implicit
+      fct: Fct => Factory[A, C]
+  ): NonEmpty[C] = {
+    val bb = into.newBuilder
+    discard { (bb += hd) ++= tl }
+    unsafeNarrow(bb.result())
+  }
 
   /** In pattern matching, think of [[NonEmpty]] as a sub-case-class of every
     * [[imm.Iterable]]; matching `case NonEmpty(ne)` ''adds'' the non-empty type
@@ -85,71 +84,68 @@ object NonEmptyColl extends NonEmptyCollInstances {
     private[nonempty] override def unsafeNarrow[Self <: imm.Iterable[Any]](self: Self) = self
   }
 
-  final class Pouring[A](hd: A, tl: A*) {
-    import NonEmpty.{unsafeNarrow => un}
-    // XXX SC this can be done more efficiently by not supporting 2.12
-    @SuppressWarnings(Array("org.wartremover.warts.NonUnitStatements"))
-    def into[C <: imm.Iterable[A]](into: Factory[A, C]): NonEmpty[C] = un {
-      val bb = into.newBuilder
-      bb += hd
-      bb ++= tl
-      bb.result()
-    }
-  }
-
   implicit final class ReshapeOps[F[_], A](private val nfa: NonEmpty[F[A]]) extends AnyVal {
     def toF: NonEmptyF[F, A] = NonEmpty.equiv[F, A](nfa)
   }
 
-  // many of these Map and Set operations can return more specific map and set types;
-  // however, the way to do that is incompatible between Scala 2.12 and 2.13.
-  // So we won't do it at least until 2.12 support is removed
-
   /** Operations that can ''return'' new maps.  There is no reason to include any other
     * kind of operation here, because they are covered by `#widen`.
     */
-  implicit final class MapOps[K, V](private val self: NonEmpty[Map[K, V]]) extends AnyVal {
-    private type ESelf = Map[K, V]
+  implicit final class `Map Ops`[K, V, CC[X, +Y] <: imm.Map[X, Y] with imm.MapOps[X, Y, CC, _]](
+      private val self: NonEmpty[imm.MapOps[K, V, CC, _]]
+  ) extends AnyVal {
+    private type ESelf = imm.MapOps[K, V, CC, _]
     import NonEmpty.{unsafeNarrow => un}
     // You can't have + because of the dumb string-converting thing in stdlib
-    def updated(key: K, value: V): NonEmpty[Map[K, V]] = un((self: ESelf).updated(key, value))
-    def ++(xs: Iterable[(K, V)]): NonEmpty[Map[K, V]] = un((self: ESelf) ++ xs)
+    def updated(key: K, value: V): NonEmpty[CC[K, V]] = un((self: ESelf).updated(key, value))
+    def ++(xs: IterableOnce[(K, V)]): NonEmpty[CC[K, V]] = un((self: ESelf) ++ xs)
     def keySet: NonEmpty[Set[K]] = un((self: ESelf).keySet)
-    def transform[W](f: (K, V) => W): NonEmpty[Map[K, W]] = un((self: ESelf) transform f)
+    def transform[W](f: (K, V) => W): NonEmpty[CC[K, W]] = un((self: ESelf) transform f)
   }
 
   /** Operations that can ''return'' new sets.  There is no reason to include any other
     * kind of operation here, because they are covered by `#widen`.
     */
-  implicit final class SetOps[A](private val self: NonEmpty[Set[A]]) extends AnyVal {
-    private type ESelf = Set[A]
+  implicit final class `Set Ops`[A, CC[_], C <: imm.SetOps[A, CC, C] with Iterable[A]](
+      private val self: NonEmpty[imm.SetOps[A, CC, C]]
+  ) extends AnyVal {
+    private type ESelf = imm.SetOps[A, CC, C]
     import NonEmpty.{unsafeNarrow => un}
     // You can't have + because of the dumb string-converting thing in stdlib
-    def incl(elem: A): NonEmpty[Set[A]] = un((self: ESelf) + elem)
-    def ++(that: Iterable[A]): NonEmpty[Set[A]] = un((self: ESelf) ++ that)
+    def incl(elem: A): NonEmpty[C] = un((self: ESelf) + elem)
+    def ++(that: IterableOnce[A]): NonEmpty[C] = un((self: ESelf) ++ that)
   }
 
   implicit final class NEPreservingOps[A, C](
       private val self: NonEmpty[IterableOps[A, imm.Iterable, C with imm.Iterable[A]]]
-  ) {
+  ) extends AnyVal {
     import NonEmpty.{unsafeNarrow => un}
     private type ESelf = IterableOps[A, imm.Iterable, C with imm.Iterable[A]]
     def toList: NonEmpty[List[A]] = un((self: ESelf).toList)
     def toVector: NonEmpty[Vector[A]] = un((self: ESelf).toVector)
-    def toSeq: NonEmpty[imm.Seq[A]] = un((self: ESelf) match {
-      case is: imm.Seq[A] => is
-      case other => other.to(imm.Seq)
-    }) // can just use .toSeq in scala 2.13
+    def toSeq: NonEmpty[imm.Seq[A]] = un((self: ESelf).toSeq)
     def toSet: NonEmpty[Set[A]] = un((self: ESelf).toSet)
     def toMap[K, V](implicit isPair: A <:< (K, V)): NonEmpty[Map[K, V]] = un((self: ESelf).toMap)
-    // ideas for extension: safe head/tail (not valuable unless also using
-    // wartremover to disable partial Seq ops)
+    def to[C1 <: imm.Iterable[A]](factory: Factory[A, C1]) = un((self: ESelf) to factory)
+    // (not so valuable unless also using wartremover to disable partial Seq ops)
+    @`inline` def head1: A = self.head
+    @`inline` def tail1: C = self.tail
   }
 
-  // Why not `map`?  Because it's a little tricky to do portably.  I suggest
-  // importing the appropriate Scalaz instances and using `.toF.map` if you need
-  // it; we can add collection-like `map` later if it seems to be really
-  // important.
+  implicit final class NEPreservingSeqOps[A, CC[X] <: imm.Seq[X], C](
+      private val self: NonEmpty[SeqOps[A, CC, C with imm.Seq[A]]]
+  ) extends AnyVal {
+    import NonEmpty.{unsafeNarrow => un}
+    private type ESelf = SeqOps[A, CC, C with imm.Iterable[A]]
+    // the +: :+ set here is so you don't needlessly "lose" your NE-ness
+    def +:[B >: A](elem: B): NonEmpty[CC[B]] = un(elem +: (self: ESelf))
+    def :+[B >: A](elem: B): NonEmpty[CC[B]] = un((self: ESelf) :+ elem)
+    // the +-: :-+ set here is to mirror the patterns, as the ones in
+    // `NE Seq Ops` are unreachable in normal usage, since implicit conversions
+    // do not compose
+    @`inline` def +-:[B >: A](elem: B): NonEmpty[CC[B]] = elem +: self
+    @`inline` def :-+[B >: A](elem: B): NonEmpty[CC[B]] = self :+ elem
+  }
 
   implicit final class `Seq Ops`[A, CC[_], C](
       private val self: NonEmpty[SeqOps[A, CC, C with CC[A]]]
@@ -160,6 +156,19 @@ object NonEmptyColl extends NonEmptyCollInstances {
     }
   }
 
+  implicit final class NEPseudofunctorOps[A, CC[X] <: imm.Iterable[X], C](
+      private val self: NonEmpty[IterableOps[A, CC, C with imm.Iterable[A]]]
+  ) extends AnyVal {
+    import NonEmpty.{unsafeNarrow => un}
+    private type ESelf = IterableOps[A, CC, C with imm.Iterable[A]]
+
+    def map[B](f: A => B): NonEmpty[CC[B]] = un((self: ESelf) map f)
+    def flatMap[B](f: A => NonEmpty[IterableOnce[B]]): NonEmpty[CC[B]] = {
+      type K[F[_]] = (F[ESelf], A => F[IterableOnce[B]]) => F[CC[B]]
+      NonEmpty.subst[K](_ flatMap _)(self, f)
+    }
+  }
+
   implicit def traverse[F[_]](implicit F: Traverse[F]): Traverse[NonEmptyF[F, *]] =
     NonEmpty.substF(F)
 }

diff --git a/libs-scala/scala-utils/src/main/scala/com/daml/scalautil/nonempty/NonEmptyReturningOps.scala b/libs-scala/scala-utils/src/main/scala/com/daml/scalautil/nonempty/NonEmptyReturningOps.scala
@@ -18,8 +18,15 @@ object NonEmptyReturningOps {
   ) {
     def groupBy1[K](f: A => K): Map[K, NonEmpty[C]] =
       NonEmpty.subst[Lambda[f[_] => Map[K, f[C]]]](self groupBy f)
+  }
+
+  implicit final class `NE Seq Ops`[A, CC[X] <: imm.Seq[X], C](
+      private val self: SeqOps[A, CC, C with imm.Seq[A]]
+  ) {
+    import NonEmpty.{unsafeNarrow => un}
 
-    // ideas for extension: +-: and :-+ operators
+    def +-:(elem: A): NonEmpty[CC[A]] = un(elem +: self)
+    def :-+(elem: A): NonEmpty[CC[A]] = un(self :+ elem)
   }
 
   implicit final class `NE Set Ops`[A](private val self: Set[A]) extends AnyVal {

diff --git a/libs-scala/scala-utils/src/test/scala/com/daml/scalautil/nonempty/NonEmptySpec.scala b/libs-scala/scala-utils/src/test/scala/com/daml/scalautil/nonempty/NonEmptySpec.scala
@@ -97,6 +97,30 @@ class NonEmptySpec extends AnyWordSpec with Matchers {
     }
   }
 
+  "updated" should {
+    val m = NonEmpty(imm.HashMap, 1 -> 2)
+
+    "preserve the map type" in {
+      (m.updated(1, 2): NonEmpty[imm.HashMap[Int, Int]]) should ===(m)
+    }
+
+    "preserve a wider map type" in {
+      val nhm = (m: NonEmpty[Map[Int, Int]]).updated(1, 2)
+      illTyped(
+        "nhm: NonEmpty[imm.HashMap[Int, Int]]",
+        "(?s)type mismatch.*?found.*?\\.Map.*?required.*?HashMap.*",
+      )
+      (nhm: NonEmpty[Map[Int, Int]]) should ===(m)
+    }
+  }
+
+  "to" should {
+    "accept weird type shapes" in {
+      val sm = NonEmpty(Map, 1 -> 2).to(imm.HashMap)
+      (sm: NonEmpty[imm.HashMap[Int, Int]]) shouldBe an[imm.HashMap[_, _]]
+    }
+  }
+
   "+-:" should {
     val NonEmpty(s) = Vector(1, 2)
 
@@ -105,12 +129,48 @@ class NonEmptySpec extends AnyWordSpec with Matchers {
       ((h, t): (Int, Vector[Int])) should ===((1, Vector(2)))
     }
 
+    "restructure when used as a method" in {
+      val h +-: t = s
+      import NonEmptyReturningOps._
+      (h +-: t: NonEmpty[Vector[Int]]) should ===(s)
+    }
+
     "have ±: alias" in {
       val h ±: t = s
       ((h, t): (Int, Vector[Int])) should ===((1, Vector(2)))
     }
   }
 
+  "map" should {
+    "'work' on sets, so to speak" in {
+      val r = NonEmpty(Set, 1, 2) map (_ + 2)
+      (r: NonEmpty[Set[Int]]) should ===(NonEmpty(Set, 3, 4))
+    }
+
+    "turn Maps into non-Maps" in {
+      val m: NonEmpty[Map[Int, Int]] = NonEmpty(Map, 1 -> 2, 3 -> 4)
+      val r = m map (_._2)
+      ((r: NonEmpty[imm.Iterable[Int]]): imm.Iterable[Int]) should contain theSameElementsAs Seq(
+        2,
+        4,
+      )
+    }
+  }
+
+  "flatMap" should {
+    "'work' on sets, so to speak" in {
+      val r = NonEmpty(Set, 1, 2) flatMap (n => NonEmpty(List, n + 3, n + 5))
+      (r: NonEmpty[Set[Int]]) should ===(NonEmpty(Set, 1 + 3, 1 + 5, 2 + 3, 2 + 5))
+    }
+
+    "reject possibly-empty function returns" in {
+      illTyped(
+        "(_: NonEmpty[List[Int]]) flatMap (x => List(x))",
+        "(?s)type mismatch.*?found.*?List.*?required.*?NonEmpty.*",
+      )
+    }
+  }
+
   // why we don't allow `scala.collection` types
   "scala.collection.Seq" must {
     "accept that its non-emptiness is ephemeral" in {