Skip to content
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.

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

Fix Scala 2.13.1 integration #283

Merged
merged 1 commit into from
Oct 17, 2019
Merged

Fix Scala 2.13.1 integration #283

merged 1 commit into from
Oct 17, 2019

Conversation

nrinaudo
Copy link

Background

Scala 2.13.1 compiler tools are unfortunately not binary (or even source) compatible with 2.13.0, which means that scoverage currently cannot run on 2.13.1.

The solution this PR takes to the problem is to make scoverage compatible with 2.13.1, while dropping support for 2.13.0. The reasoning behind that is:

  • the alternative would be to do full cross-version publication - publish an artifact per x.y.z Scala version, rather than just per x.y
  • that'd be rather more work than what I'm submitting
  • what I'm submitting works (I think) now, and unblocks all the people waiting for scoverage to be published for 2.13.1. This buys maintainers time to decide whether they want to go the full cross-version way.

Additionally, the submitted PR has the advantage of being a long term fix if Scala resumes its tradition of being binary compatible between patch versions.

As things stand, releasing this would mean scoverage is compatible with:

  • Scala 2.10.x
  • Scala 2.11.x
  • Scala 2.12.x
  • Scala 2.13.1, but not Scala 2.13.0

Notes

I decided to upgrade to the latest 2.12 and 2.13 versions in a single commit, because that feels atomic to me. I'm perfectly open to the ideas that:

  • it's not atomic for maintainers and I should break that in 2 separate commits
  • upgrading to Scala 2.12.10 is not necessary to fixing 2.13.1 support and should be dropped from this PR

I also had to rename a constructor parameter in ScoverageCompiler as it was causing ambiguities under 2.13.1. I didn't do that just to be annoying.

Copy link
Member

@0xRoch 0xRoch left a comment

Choose a reason for hiding this comment

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

Very clear thank you for the contribution !

@0xRoch 0xRoch merged commit 8882338 into scoverage:master Oct 17, 2019
rtyley added a commit to guardian/twitter4s that referenced this pull request Oct 21, 2019
This project now compiles with Scala 2.13 by default, while still
cross-compiling to Scala 2.12 & 2.11.

Although Scala 2.13.1 has been released, this change sticks with 2.13.0,
as `scoverage` has not yet released an update compatible with 2.13.1:

scoverage/sbt-scoverage#295
scoverage/scalac-scoverage-plugin#283
scoverage/sbt-scoverage#299


Migration issues addressed with the update to Scala 2.13:

### Underscore (`_`) is no longer a valid identifer for `val`s

Using underscore like this is no longer allowed...

```
implicit val _ = ...
```

...we have to give the `val` a valid name (even if it's just `v`):


```
implicit val v = ...
```

See also:

* scala/bug#10384
* scala/bug#7691
* scala/scala#6218

### `Map.mapValues()` now returns a `MapView` rather than a `Map`

We usually want a `Map`, so we can just add in a `.toMap` to the end of
the expression - it's harmless in Scala 2.12, and allows the code to
compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala
2.13, but using the new recommended alternative doesn't work in Scala 2.12.

https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes

See also:

* scala/bug#10919
* scala/scala#7014

### `.to` can no longer infer what resulting collection type you want

This is all part of `CanBuildFrom` going away in Scala 2.13:

https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
rtyley added a commit to guardian/twitter4s that referenced this pull request Oct 21, 2019
This project now compiles with Scala 2.13 by default, while still
cross-compiling to Scala 2.12 & 2.11.

Although Scala 2.13.1 has been released, this change sticks with 2.13.0,
as `scoverage` has not yet released an update compatible with 2.13.1:

scoverage/sbt-scoverage#295
scoverage/scalac-scoverage-plugin#283
scoverage/sbt-scoverage#299


Migration issues addressed with the update to Scala 2.13:

### Underscore (`_`) is no longer a valid identifer for `val`s

Using underscore like this is no longer allowed...

```
implicit val _ = ...
```

...we have to give the `val` a valid name (even if it's just `v`):


```
implicit val v = ...
```

See also:

* scala/bug#10384
* scala/bug#7691
* scala/scala#6218

### `Map.mapValues()` now returns a `MapView` rather than a `Map`

We usually want a `Map`, so we can just add in a `.toMap` to the end of
the expression - it's harmless in Scala 2.12, and allows the code to
compile in Scala 2.13. `Map.mapValues()` itself is deprecated in Scala
2.13, but using the new recommended alternative doesn't work in Scala 2.12.

https://docs.scala-lang.org/overviews/core/collections-migration-213.html#what-are-the-breaking-changes

See also:

* scala/bug#10919
* scala/scala#7014

### `.to` can no longer infer what resulting collection type you want

This is all part of `CanBuildFrom` going away in Scala 2.13:

https://www.scala-lang.org/blog/2018/06/13/scala-213-collections.html#life-without-canbuildfrom
@intracer
Copy link

@D-Roch Somehow 1.4.1 with this fix did not go to maven central atfer two weeks from tagging
https://github.com/scoverage/scalac-scoverage-plugin/releases/tag/v1.4.1

@neko-kai
Copy link

neko-kai commented Nov 2, 2019

@D-Roch @gslowikowski Could you please try to republish 1.4.1 to central?

@viluon
Copy link

viluon commented Nov 6, 2019

I second @neko-kai's request, the latest published version is still 1.4.0.
scoverage/sbt-scoverage#299 is waiting for this release, and many end-users are in turn waiting for sbt-coverage with Scala 2.13.1 support.

Issue #285 has been opened for this reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants