Add guidance on how to migrate scala.Seq #1326

eed3si9n · 2019-05-10T20:03:07Z

No description provided.

_overviews/core/collections-migration-213.md

dwijnand · 2019-05-10T20:57:52Z

Not being able to pass an array I don't think is "a breaking change in the API semantics". It's just source-incompatible.

I actually think continuing to use scala.Seq is what should be used for libraries. 2.12 code will continue to use scala.collection.Seq and 2.13 code will use scala.collection.immutable.Seq. That might mean some arrays need to .toSeq before : _*.

Co-Authored-By: Dale Wijnand <344610+dwijnand@users.noreply.github.com>

eed3si9n · 2019-05-10T21:52:42Z

Not being able to pass an array I don't think is "a breaking change in the API semantics". It's just source-incompatible.

The meaning of your API changed, so I think it's a breaking change. For most libraries it might be an acceptable change, but for others it might not work. We don't know.

The vararg you have no option, but you do have an option to use CSeq for your own API.

NthPortal · 2019-05-11T22:29:17Z

_overviews/core/collections-migration-213.md

+import scala.collection.Seq
+~~~
+
+In the future when your API is able to break the source compatibility, it might also make sense to migrate towards the `scala.collection.immutable.Seq` ("ISeq") for both Scala 2.12 and Scala 2.13.


it doesn't seem like you refer to "ISeq" or "CSeq" anywhere, so the parentheticals can probably be removed

Added commits that ended up adding references to ISeq and CSeq.

lrytz · 2019-05-14T09:53:40Z

_overviews/core/collections-migration-213.md

+- if you cross build with Scala 2.12 and want to maintain the API semantics for 2.13 version of your library, or
+- if your library users frequently uses mutable collections such as `Array`
+
+you can import `scala.collection.Seq` ("CSeq") explicitly in your code.


It might be worth mentioning that this is a potentially source-breaking change. If a user in 2.12 has:

// `Seq` is the default `scala.Seq` val a: Seq[Food] = orderFood(myOrder)

Then that will no longer compile in 2.13

Also, it might be a good strategy for APIs to only move to immutable.Seq in the result type, but keep accepting collection.Seq arguments, if the method implementation can work fine with mutable collections.

I've added this in 2b4de2d.

szeiger · 2019-05-14T14:02:11Z

_overviews/core/collections-migration-213.md

+
+In Scala 2.13 `scala.Seq[+A]` is an alias for `scala.collection.immutable.Seq[A]`, instead of `scala.collection.Seq[A]`. This change requires some planning depending on how your code is going to be used.
+
+If you're making an application, and simply migrating a Scala 2.12 code base to 2.13, it might be ok to keep using `scala.Seq` in your code.


This sounds like "keep using scala.Seq" should be the safe and simple option. I'd argue it's the opposite (unless your code happens to just work out of the box). In a complex codebase the simplest way forward is to change all references from scala.Seq to scala.collection.Seq and possibly insert toSeq calls where necessary. This is easy to do and will cross-compile on 2.12 without any changes in semantics.

Switching your own codebase from scala.collection.Seq to scala.immutable.Seq is the more advanced refactoring. In many cases it may have been the intention all along, so it makes sense to do it. It also avoids unnecessary copying when varargs are involved, but it's much harder to get right consistently in a large codebase.

Interesting. I'll mention recommendation to use scala.collection.Seq first then.

I've added this in 2b4de2d.

szeiger · 2019-05-14T14:03:47Z

_overviews/core/collections-migration-213.md

+If you're making a library intended to be used by other programmers, then using `scala.Seq` or varargs is going to be a breaking change in the API semantics. For example, if there was a function `def orderFood(order: Seq[Order]): Seq[Food]`, previously the library user would have been able to pass in an array of `Order`, but it won't work for 2.13.
+
+- if you cross build with Scala 2.12 and want to maintain the API semantics for 2.13 version of your library, or
+- if your library users frequently uses mutable collections such as `Array`


If code has been optimized to the point that you build an array instead of a regular collection, you're probably okay calling unsafeWrapArray, so it will still work with immutable varargs.

Arrays might come up simply as Java API that you provide as a subset of API.

dwijnand · 2019-05-21T06:14:40Z

_overviews/core/collections-migration-213.md

+
+### Masking scala.Seq
+
+To use the compiler to bad the use of plain `Seq`, you can declare your own `Seq` to mask `scala.Seq`.


dwijnand · 2019-05-21T06:15:07Z

_overviews/core/collections-migration-213.md

+- if you cross build with Scala 2.12 and want to maintain the API semantics for 2.13 version of your library, or
+- if your library users frequently uses mutable collections such as `Array`
+
+you can import collection Seq explicitly in your code.


"import collection Seq" looks weird and kind of ambiguous.

Add guidance on how to migrate scala.Seq

2d8f8e5

eed3si9n requested review from szeiger, SethTisue and dwijnand May 10, 2019 20:03

dwijnand reviewed May 10, 2019

View reviewed changes

_overviews/core/collections-migration-213.md Outdated Show resolved Hide resolved

_overviews/core/collections-migration-213.md Outdated Show resolved Hide resolved

_overviews/core/collections-migration-213.md Outdated Show resolved Hide resolved

eed3si9n and others added 3 commits May 10, 2019 17:28

Update _overviews/core/collections-migration-213.md

8178166

Co-Authored-By: Dale Wijnand <344610+dwijnand@users.noreply.github.com>

Update _overviews/core/collections-migration-213.md

54b38f7

Co-Authored-By: Dale Wijnand <344610+dwijnand@users.noreply.github.com>

Update _overviews/core/collections-migration-213.md

6c56623

Co-Authored-By: Dale Wijnand <344610+dwijnand@users.noreply.github.com>

NthPortal reviewed May 11, 2019

View reviewed changes

lrytz reviewed May 14, 2019

View reviewed changes

szeiger reviewed May 14, 2019

View reviewed changes

eed3si9n added 2 commits May 20, 2019 20:19

reflect review comments

2b4de2d

scala.IndexedSeq too

cc9b455

SethTisue merged commit 8d2f0ed into scala:master May 21, 2019

eed3si9n deleted the wip/seq branch May 21, 2019 01:38

dwijnand reviewed May 21, 2019

View reviewed changes


		In Scala 2.13 `scala.Seq[+A]` is an alias for `scala.collection.immutable.Seq[A]`, instead of `scala.collection.Seq[A]`. This change requires some planning depending on how your code is going to be used.

		If you're making an application, and simply migrating a Scala 2.12 code base to 2.13, it might be ok to keep using `scala.Seq` in your code.


		### Masking scala.Seq

		To use the compiler to bad the use of plain `Seq`, you can declare your own `Seq` to mask `scala.Seq`.

Add guidance on how to migrate scala.Seq #1326

Add guidance on how to migrate scala.Seq #1326

Uh oh!

Conversation

eed3si9n commented May 10, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dwijnand commented May 10, 2019

Uh oh!

eed3si9n commented May 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lrytz May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lrytz May 14, 2019 •

edited

Loading