Merge pull request #495 from lrytz/toConserve

lrytz · web-flow · commit d55a12f6143d · 2021-11-08T14:40:49.000+01:00
preserve immutable collections in to(Target) conversions
diff --git a/build.sbt b/build.sbt
@@ -72,44 +72,53 @@ lazy val compat = new MultiScalaCrossProject(
             sharedSourceDir / "scala-2.11_2.12"
         }
       },
+      Test / unmanagedSourceDirectories += {
+        val sharedSourceDir = (ThisBuild / baseDirectory).value / "compat/src/test"
+        CrossVersion.partialVersion(scalaVersion.value) match {
+          case Some((3, _) | (2, 13)) =>
+            sharedSourceDir / "scala-2.13"
+          case _ =>
+            sharedSourceDir / "scala-2.11_2.12"
+        }
+      },
       versionPolicyIntention := Compatibility.BinaryCompatible,
-    )
-    .jvmSettings(
-      Test / unmanagedSourceDirectories += (ThisBuild / baseDirectory).value / "compat/src/test/scala-jvm",
-      junit,
       mimaBinaryIssueFilters ++= {
         import com.typesafe.tools.mima.core._
         import com.typesafe.tools.mima.core.ProblemFilters._
         Seq(
           exclude[ReversedMissingMethodProblem]("scala.collection.compat.PackageShared.*"), // it's package-private
+          exclude[Problem]("scala.collection.compat.*PreservingBuilder*")
         )
       },
     )
+    .jvmSettings(
+      Test / unmanagedSourceDirectories += (ThisBuild / baseDirectory).value / "compat/src/test/scala-jvm",
+      junit,
+    )
     .disablePlugins(ScalafixPlugin),
   _.jsSettings(
-      scalacOptions ++= {
-        val x = (LocalRootProject / baseDirectory).value.toURI.toString
-        val y = "https://raw.githubusercontent.com/scala/scala-collection-compat/" + sys.process
-          .Process("git rev-parse HEAD")
-          .lineStream_!
-          .head
-        val opt = CrossVersion.partialVersion(scalaVersion.value) match {
-          case Some((3, _)) => "-scalajs-mapSourceURI"
-          case _            => "-P:scalajs:mapSourceURI"
-        }
-        Seq(s"$opt:$x->$y/")
-      },
-      Test / fork := false // Scala.js cannot run forked tests
-    )
-    .jsEnablePlugins(ScalaJSJUnitPlugin),
+    scalacOptions ++= {
+      val x = (LocalRootProject / baseDirectory).value.toURI.toString
+      val y = "https://raw.githubusercontent.com/scala/scala-collection-compat/" + sys.process
+        .Process("git rev-parse HEAD")
+        .lineStream_!
+        .head
+      val opt = CrossVersion.partialVersion(scalaVersion.value) match {
+        case Some((3, _)) => "-scalajs-mapSourceURI"
+        case _            => "-P:scalajs:mapSourceURI"
+      }
+      Seq(s"$opt:$x->$y/")
+    },
+    Test / fork := false // Scala.js cannot run forked tests
+  ).jsEnablePlugins(ScalaJSJUnitPlugin),
   _.nativeSettings(
-      nativeLinkStubs := true,
-      addCompilerPlugin(
-        "org.scala-native" % "junit-plugin" % nativeVersion cross CrossVersion.full
-      ),
-      libraryDependencies += "org.scala-native" %%% "junit-runtime" % nativeVersion,
-      Test / fork := false // Scala Native cannot run forked tests
-    )
+    nativeLinkStubs := true,
+    addCompilerPlugin(
+      "org.scala-native" % "junit-plugin" % nativeVersion cross CrossVersion.full
+    ),
+    libraryDependencies += "org.scala-native" %%% "junit-runtime" % nativeVersion,
+    Test / fork := false // Scala Native cannot run forked tests
+  )
 )
 
 val compat211 = compat(Seq(JSPlatform, JVMPlatform, NativePlatform), scala211)
@@ -171,7 +180,7 @@ lazy val scalafixRules = project
   .settings(
     scalaModuleAutomaticModuleName := None,
     versionPolicyIntention := Compatibility.None,
-    versionCheck := {},  // I don't understand why this fails otherwise?! oh well
+    versionCheck := {}, // I don't understand why this fails otherwise?! oh well
     name := "scala-collection-migrations",
     scalaVersion := scalafixScala212,
     libraryDependencies += "ch.epfl.scala" %% "scalafix-core" % scalafixVersion
@@ -289,14 +298,14 @@ lazy val scalafixTests = project
   .dependsOn(scalafixInput, scalafixRules)
   .enablePlugins(BuildInfoPlugin, ScalafixTestkitPlugin)
 
-val ciScalaVersion     = sys.env.get("CI_SCALA_VERSION").flatMap(Version.parse)
-val isScalaJs          = sys.env.get("CI_PLATFORM") == Some("js")
-val isScalaNative      = sys.env.get("CI_PLATFORM") == Some("native")
-val isScalafix         = sys.env.get("CI_MODE") == Some("testScalafix")
-val isScalafmt         = sys.env.get("CI_MODE") == Some("testScalafmt")
-val isBinaryCompat     = sys.env.get("CI_MODE") == Some("testBinaryCompat")
-val isHeaderCheck      = sys.env.get("CI_MODE") == Some("headerCheck")
-val jdkVersion         = sys.env.get("CI_JDK").map(_.toInt)
+val ciScalaVersion = sys.env.get("CI_SCALA_VERSION").flatMap(Version.parse)
+val isScalaJs      = sys.env.get("CI_PLATFORM") == Some("js")
+val isScalaNative  = sys.env.get("CI_PLATFORM") == Some("native")
+val isScalafix     = sys.env.get("CI_MODE") == Some("testScalafix")
+val isScalafmt     = sys.env.get("CI_MODE") == Some("testScalafmt")
+val isBinaryCompat = sys.env.get("CI_MODE") == Some("testBinaryCompat")
+val isHeaderCheck  = sys.env.get("CI_MODE") == Some("headerCheck")
+val jdkVersion     = sys.env.get("CI_JDK").map(_.toInt)
 
 // required by sbt-scala-module
 inThisBuild {
diff --git a/compat/src/main/scala-2.11_2.12/scala/collection/compat/CompatImpl.scala b/compat/src/main/scala-2.11_2.12/scala/collection/compat/CompatImpl.scala
@@ -14,37 +14,35 @@ package scala.collection.compat
 
 import scala.reflect.ClassTag
 import scala.collection.generic.CanBuildFrom
+import scala.{collection => c}
 import scala.collection.{immutable => i, mutable => m}
 
 /* builder optimized for a single ++= call, which returns identity on result if possible
  * and defers to the underlying builder if not.
  */
-private final class IdentityPreservingBuilder[A, CC[X] <: TraversableOnce[X]](
-    that: m.Builder[A, CC[A]])(implicit ct: ClassTag[CC[A]])
-    extends m.Builder[A, CC[A]] {
+private abstract class PreservingBuilder[A, C <: TraversableOnce[A]] extends m.Builder[A, C] {
+  val that: m.Builder[A, C]
+  val ct: ClassTag[C]
 
   //invariant: ruined => (collection == null)
-  var collection: CC[A] = null.asInstanceOf[CC[A]]
-  var ruined            = false
+  var collection: C = null.asInstanceOf[C]
+  var ruined        = false
 
   private[this] def ruin(): Unit = {
     if (collection != null) that ++= collection
-    collection = null.asInstanceOf[CC[A]]
+    collection = null.asInstanceOf[C]
     ruined = true
   }
 
-  override def ++=(elems: TraversableOnce[A]): this.type =
-    elems match {
-      case ct(ca) if collection == null && !ruined => {
-        collection = ca
-        this
-      }
-      case _ => {
+  override def ++=(elems: TraversableOnce[A]): this.type = {
+    (if (collection == null && !ruined) ct.unapply(elems) else None) match {
+      case Some(c) => collection = c
+      case _ =>
         ruin()
         that ++= elems
-        this
-      }
     }
+    this
+  }
 
   def +=(elem: A): this.type = {
     ruin()
@@ -53,14 +51,31 @@ private final class IdentityPreservingBuilder[A, CC[X] <: TraversableOnce[X]](
   }
 
   def clear(): Unit = {
-    collection = null.asInstanceOf[CC[A]]
-    if (ruined) that.clear()
-    ruined = false
+    collection = null.asInstanceOf[C]
+    if (ruined) {
+      that.clear()
+      ruined = false
+    }
   }
 
-  def result(): CC[A] = if (collection == null) that.result() else collection
+  def result(): C = if (collection == null) that.result() else collection
 }
 
+private final class IdentityPreservingBuilder[A, CC[X] <: TraversableOnce[X]](
+    val that: m.Builder[A, CC[A]])(implicit val ct: ClassTag[CC[A]])
+    extends PreservingBuilder[A, CC[A]]
+
+private final class IdentityPreservingBitSetBuilder[C <: c.BitSet](val that: m.Builder[Int, C])(
+    implicit val ct: ClassTag[C])
+    extends PreservingBuilder[Int, C]
+
+private final class IdentityPreservingMapBuilder[
+    K,
+    V,
+    CC[X, Y] <: c.Map[X, Y] with c.MapLike[X, Y, CC[X, Y]]](val that: m.Builder[(K, V), CC[K, V]])(
+    implicit val ct: ClassTag[CC[K, V]])
+    extends PreservingBuilder[(K, V), CC[K, V]]
+
 private[compat] object CompatImpl {
   def simpleCBF[A, C](f: => m.Builder[A, C]): CanBuildFrom[Any, A, C] =
     new CanBuildFrom[Any, A, C] {
diff --git a/compat/src/main/scala-2.11_2.12/scala/collection/compat/PackageShared.scala b/compat/src/main/scala-2.11_2.12/scala/collection/compat/PackageShared.scala
@@ -12,10 +12,10 @@
 
 package scala.collection.compat
 
+import scala.annotation.nowarn
 import scala.collection.generic._
 import scala.reflect.ClassTag
 import scala.collection.{
-  BitSet,
   GenTraversable,
   IterableLike,
   IterableView,
@@ -61,39 +61,129 @@ private[compat] trait PackageShared {
        must be non-strict
      */
     def builder: m.Builder[A, CC[A]] = fact match {
-      case c.Seq | i.Seq => new IdentityPreservingBuilder[A, i.Seq](i.Seq.newBuilder[A])
+      case c.Seq | i.Seq =>
+        new IdentityPreservingBuilder[A, i.Seq](i.Seq.newBuilder[A])
+
       case c.LinearSeq | i.LinearSeq =>
         new IdentityPreservingBuilder[A, i.LinearSeq](i.LinearSeq.newBuilder[A])
+      case i.Queue =>
+        new IdentityPreservingBuilder[A, i.Queue](i.Queue.newBuilder[A])
+      case i.Stream =>
+        new IdentityPreservingBuilder[A, i.Stream](i.Stream.newBuilder[A])
+      case i.Stack =>
+        new IdentityPreservingBuilder[A, i.Stack](i.Stack.newBuilder[A]): @nowarn("cat=deprecation")
+      case i.List =>
+        new IdentityPreservingBuilder[A, i.List](i.List.newBuilder[A])
+
+      case c.IndexedSeq | i.IndexedSeq =>
+        new IdentityPreservingBuilder[A, i.IndexedSeq](i.IndexedSeq.newBuilder[A])
+      case i.Vector =>
+        new IdentityPreservingBuilder[A, i.Vector](i.Vector.newBuilder[A])
+
+      case c.Set | i.Set =>
+        new IdentityPreservingBuilder[A, i.Set](i.Set.newBuilder[A])
+
+      case i.HashSet =>
+        new IdentityPreservingBuilder[A, i.HashSet](i.HashSet.newBuilder[A])
+      case i.ListSet =>
+        new IdentityPreservingBuilder[A, i.ListSet](i.ListSet.newBuilder[A])
+
+      case c.Iterable | i.Iterable =>
+        new IdentityPreservingBuilder[A, i.Iterable](i.Iterable.newBuilder[A])
+
+      case c.Traversable | i.Traversable =>
+        new IdentityPreservingBuilder[A, i.Traversable](i.Traversable.newBuilder[A])
+
       case _ => fact.newBuilder[A]
     }
     simpleCBF(builder)
   }
 
   implicit def sortedSetCompanionToCBF[A: Ordering,
                                        CC[X] <: c.SortedSet[X] with c.SortedSetLike[X, CC[X]]](
-      fact: SortedSetFactory[CC]): CanBuildFrom[Any, A, CC[A]] =
-    simpleCBF(fact.newBuilder[A])
+      fact: SortedSetFactory[CC]): CanBuildFrom[Any, A, CC[A]] = {
+    def builder: m.Builder[A, CC[A]] = {
+      val b = fact match {
+        case c.SortedSet | i.SortedSet =>
+          new IdentityPreservingBuilder[A, i.SortedSet](i.SortedSet.newBuilder[A])
+        case i.TreeSet =>
+          new IdentityPreservingBuilder[A, i.TreeSet](i.TreeSet.newBuilder[A])
+        case _ =>
+          fact.newBuilder[A]
+      }
+      // Cast needed because GADT inference doesn't unify CC (didn't dig down why). Example:
+      //   def t: CC[A] = fact match { case i.SortedSet => null: i.SortedSet[A] }
+      b.asInstanceOf[m.Builder[A, CC[A]]]
+    }
+    simpleCBF(builder)
+  }
 
   implicit def arrayCompanionToCBF[A: ClassTag](fact: Array.type): CanBuildFrom[Any, A, Array[A]] =
     simpleCBF(Array.newBuilder[A])
 
-  implicit def mapFactoryToCBF[K, V, CC[A, B] <: Map[A, B] with MapLike[A, B, CC[A, B]]](
-      fact: MapFactory[CC]): CanBuildFrom[Any, (K, V), CC[K, V]] =
-    simpleCBF(fact.newBuilder[K, V])
+  // bounds should be `c.` but binary compatibility
+  implicit def mapFactoryToCBF[K,
+                               V,
+                               CC[A, B] <: /*c.*/ Map[A, B] with /*c.*/ MapLike[A, B, CC[A, B]]](
+      fact: MapFactory[CC]): CanBuildFrom[Any, (K, V), CC[K, V]] = {
+    def builder: m.Builder[(K, V), CC[K, V]] = {
+      val b = fact match {
+        case c.Map | i.Map =>
+          new IdentityPreservingMapBuilder[K, V, i.Map](i.Map.newBuilder[K, V])
+        case i.HashMap =>
+          new IdentityPreservingMapBuilder[K, V, i.HashMap](i.HashMap.newBuilder[K, V])
+        case i.ListMap =>
+          new IdentityPreservingMapBuilder[K, V, i.ListMap](i.ListMap.newBuilder[K, V])
+        case _ =>
+          fact.newBuilder[K, V]
+      }
+      // Cast needed because GADT inference doesn't unify CC (didn't dig down why). Example:
+      //   def t: CC[K, V] = fact match { case i.Map => null: i.Map[K, V] }
+      b.asInstanceOf[m.Builder[(K, V), CC[K, V]]]
+    }
+    simpleCBF(builder)
+  }
 
   implicit def sortedMapFactoryToCBF[
       K: Ordering,
       V,
       CC[A, B] <: c.SortedMap[A, B] with c.SortedMapLike[A, B, CC[A, B]]](
-      fact: SortedMapFactory[CC]): CanBuildFrom[Any, (K, V), CC[K, V]] =
-    simpleCBF(fact.newBuilder[K, V])
+      fact: SortedMapFactory[CC]): CanBuildFrom[Any, (K, V), CC[K, V]] = {
+    def builder: m.Builder[(K, V), CC[K, V]] = {
+      val b = fact match {
+        case c.SortedMap | i.SortedMap =>
+          new IdentityPreservingMapBuilder[K, V, i.SortedMap](i.SortedMap.newBuilder[K, V])
+        case i.TreeMap =>
+          new IdentityPreservingMapBuilder[K, V, i.TreeMap](i.TreeMap.newBuilder[K, V])
+        case _ =>
+          fact.newBuilder[K, V]
+      }
+      b.asInstanceOf[m.Builder[(K, V), CC[K, V]]]
+    }
+    simpleCBF(builder)
+  }
 
-  implicit def bitSetFactoryToCBF(fact: BitSetFactory[BitSet]): CanBuildFrom[Any, Int, BitSet] =
-    simpleCBF(fact.newBuilder)
+  implicit def bitSetFactoryToCBF(
+      fact: BitSetFactory[c.BitSet]): CanBuildFrom[Any, Int, c.BitSet] = {
+    def builder: m.Builder[Int, c.BitSet] = fact match {
+      case c.BitSet =>
+        new IdentityPreservingBitSetBuilder[i.BitSet](i.BitSet.newBuilder)
+      case _ =>
+        fact.newBuilder
+    }
+    simpleCBF(builder)
+  }
 
   implicit def immutableBitSetFactoryToCBF(
-      fact: BitSetFactory[i.BitSet]): CanBuildFrom[Any, Int, ImmutableBitSetCC[Int]] =
-    simpleCBF(fact.newBuilder)
+      fact: BitSetFactory[i.BitSet]): CanBuildFrom[Any, Int, ImmutableBitSetCC[Int]] = {
+    def builder: m.Builder[Int, i.BitSet] = fact match {
+      case i.BitSet =>
+        new IdentityPreservingBitSetBuilder[i.BitSet](i.BitSet.newBuilder)
+      case _ =>
+        fact.newBuilder
+    }
+    simpleCBF(builder)
+  }
 
   implicit def mutableBitSetFactoryToCBF(
       fact: BitSetFactory[m.BitSet]): CanBuildFrom[Any, Int, MutableBitSetCC[Int]] =
diff --git a/compat/src/test/scala-2.11_2.12/test/scala/collection/ToConserveTest.scala b/compat/src/test/scala-2.11_2.12/test/scala/collection/ToConserveTest.scala
