Skip to content

Compile with Scala 3 #166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2022
Merged

Compile with Scala 3 #166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
fail-fast: false
matrix:
java: [8, 11, 17]
scala: [2.13.8]
scala: [2.13.8, 3.1.2-RC1]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
Expand Down
19 changes: 15 additions & 4 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
ThisBuild / scalaVersion := "2.13.8"
ThisBuild / scalaVersion := "3.1.2-RC1"
ThisBuild / crossScalaVersions := Seq((ThisBuild / scalaVersion).value, "2.13.8")

lazy val root = project.in(file("."))
.aggregate(collectionContrib.jvm, collectionContrib.js)
Expand All @@ -15,10 +16,20 @@ lazy val collectionContrib = crossProject(JVMPlatform, JSPlatform)
.settings(ScalaModulePlugin.scalaModuleSettings)
.settings(
name := "scala-collection-contrib",
versionPolicyIntention := Compatibility.BinaryCompatible,
versionPolicyIntention := Compatibility.None,
scalaModuleAutomaticModuleName := Some("scala.collection.contrib"),
Compile / compile / scalacOptions ++= Seq("-opt-warnings", "-language:higherKinds", "-deprecation", "-feature", "-Xfatal-warnings", "-Wconf:origin=scala.collection.IterableOps.toIterable:s"),
Compile / doc / scalacOptions ++= Seq("-implicits", "-groups", "-nowarn"),
Compile / compile / scalacOptions ++= {
CrossVersion.partialVersion(scalaVersion.value) match {
case Some((2, _)) => Seq("-opt-warnings", "-Werror", "-Wconf:origin=scala.collection.IterableOps.toIterable:s")
case Some((3, _)) => Seq("-Xfatal-warnings", "-Yscala-release:3.0", "-Wconf:cat=deprecation:s")
}
},
Compile / doc / scalacOptions ++= {
CrossVersion.partialVersion(scalaVersion.value) match {
case Some((2, _)) => Seq("-implicits", "-groups", "-nowarn")
case Some((3, _)) => Seq.empty
}
},
testOptions += Tests.Argument(TestFrameworks.JUnit, "-q", "-v", "-s", "-a"),
Test / parallelExecution := false, // why?
libraryDependencies ++= Seq(
Expand Down
6 changes: 3 additions & 3 deletions src/main/scala/scala/collection/decorators/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ package object decorators {
implicit def iteratorDecorator[A](it: Iterator[A]): IteratorDecorator[A] =
new IteratorDecorator[A](it)

implicit def IterableDecorator[C](coll: C)(implicit it: IsIterable[C]): IterableDecorator[C, it.type] =
implicit def iterableDecorator[C](coll: C)(implicit it: IsIterable[C]): IterableDecorator[C, it.type] =
new IterableDecorator(coll)(it)

implicit def SeqDecorator[C](coll: C)(implicit seq: IsSeq[C]): SeqDecorator[C, seq.type] =
implicit def seqDecorator[C](coll: C)(implicit seq: IsSeq[C]): SeqDecorator[C, seq.type] =
new SeqDecorator(coll)(seq)

implicit def MapDecorator[C](coll: C)(implicit map: IsMap[C]): MapDecorator[C, map.type] =
implicit def mapDecorator[C](coll: C)(implicit map: IsMap[C]): MapDecorator[C, map.type] =
Comment on lines +11 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing these names is required for it to compile with Scala 3 (the name of implicit def can't clash with the existing class). I understand this breaks the compatibility so I'm leaving the decision here to the maintainers.

Options:

  1. Change the names, break the compatibility, don't bump the major version
  2. Change the names, break the compatibility, bump the major version
  3. Change the names only for scala 3 (create a dedicated file for scala 3)

Copy link
Collaborator

@julienrf julienrf Jan 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @povder, thank you for your contribution!

Option 1 is not practicable. We have to bump the major version number if we break the binary compatibility.

I am not sure about option 3: I believe it should be safe because the Scala 3 artifacts have no previous release, and users can not depend on both Scala 2 and Scala 3 artifacts in the same build. I wonder if this can create issues somehow, still… Also, I am a bit worried by the introduced complexity on our side.

Last thougth: it would be a good opportunity to move to 1.x.y versions instead of the current 0.x.y.

Thougths?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your input @julienrf. My preferred solution is to change the names for both Scala 2 and 3, keep the single file and bump version to 1.0.0.

Copy link
Member

@SethTisue SethTisue Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't take it as given that this repo should ever go to 1.0.0. It seems plausible to me that it could stay in 0.x versions forever, with no bincompat guarantees at all.

It depends on whether you see the purpose of this library as making user-contributed code available for use in production, or just to make it available for people to try out so it can be further improved. And I'm not sure we've ever really decided that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so maybe another solution would be to go with a version 0.3.0, but I wonder what would be the problem of using 1.0.0 instead of 0.3.0? The only issue I see with 0.3.0 is that it won’t be possible to distinguish between a future release that breaks source compatibility and a future release that breaks binary compatibility (they would both have the number 0.4.0).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking more of the social aspect than the technical aspect. Version numbers send signals to human beings as well as to tooling, and staying on 0.x indefinitely helps communicate that this repo is just a pile of experimental code that may or may not have been seriously tested or vetted by anybody.

Copy link
Contributor Author

@povder povder Jan 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SethTisue I think people may still expect releases of this lib to be binary compatible on minor version bumps even if it hasn't ever been publicly communicated because versionPolicyIntention := Compatibility.BinaryCompatible is set.

Edit: after re-reading your comment I understand that your point was only about 1.0.0.not about not having a binary compatibility policy.

new MapDecorator(coll)(map)

implicit def bitSetDecorator[C <: BitSet with BitSetOps[C]](bs: C): BitSetDecorator[C] =
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/scala/collection/immutable/MultiDict.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ object MultiDict extends MapFactory[MultiDict] {

def from[K, V](source: IterableOnce[(K, V)]): MultiDict[K, V] =
source match {
case mm: MultiDict[K, V] => mm
case mm: MultiDict[K, V] @unchecked => mm
case _ => (newBuilder[K, V] ++= source).result()
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/scala/collection/immutable/MultiSet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ object MultiSet extends IterableFactory[MultiSet] {

def from[A](source: IterableOnce[A]): MultiSet[A] =
source match {
case ms: MultiSet[A] => ms
case ms: MultiSet[A] @unchecked => ms
case _ => (newBuilder[A] ++= source).result()
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/scala/collection/immutable/SortedMultiDict.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ object SortedMultiDict extends SortedMapFactory[SortedMultiDict] {

def from[K: Ordering, V](it: IterableOnce[(K, V)]): SortedMultiDict[K, V] =
it match {
case smm: SortedMultiDict[K, V] => smm
case smm: SortedMultiDict[K, V] @unchecked => smm
case _ => (newBuilder[K, V] ++= it).result()
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/scala/collection/immutable/SortedMultiSet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ object SortedMultiSet extends SortedIterableFactory[SortedMultiSet] {

def from[A: Ordering](source: IterableOnce[A]): SortedMultiSet[A] =
source match {
case sms: SortedMultiSet[A] => sms
case sms: SortedMultiSet[A] @unchecked => sms
case _ => (newBuilder[A] ++= source).result()
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/scala/collection/mutable/MultiSet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ trait MultiSet[A]
override protected def newSpecificBuilder: mutable.Builder[A, MultiSet[A]] = iterableFactory.newBuilder
override def empty: MultiSet[A] = iterableFactory.empty

override def knownSize = super[Growable].knownSize
override def knownSize: Int = super[Growable].knownSize
}

class MultiSetImpl[A] private[mutable] (val elems: Map[A, Int]) extends MultiSet[A] {
Expand Down