Changes to compile with Scala 2.13.0-M4 #411

lrytz · 2018-05-15T20:53:19Z

No description provided.

lrytz · 2018-05-15T20:57:09Z

sbt ++2.13.0-M4 jvm/test passes.

This doesn't cross-compile with 2.11/2.12 and 2.13 though. If you want to keep cross-compiling a single branch, we can use version-dependent source directories. We can minimize the amount of code in those directories.

lrytz · 2018-05-19T06:15:44Z

Yay, this is green now! @ashawley raises concerns about binary compatibility over at scala/scala-xml#222. Have to check in detail and see if we can keep doing 1.14.x, or need to move to 1.15.

Co-authored-by: julienrf

This is a no-op for 2.13, as Traversable is a (deprecated) alias for Iterable. For 2.10-2.12, this ensures that the signatures don't change due to cross-building with 2.13.

lrytz · 2018-05-23T14:52:34Z

I went through all the changes again with focus on binary and source compatibility. It looks to me like we can continue to release 0.14.x builds for 2.10-2.12 after merging this PR, they should be backwards compatible.

I think this PR is ready now. Review by @rickynils :-)

raboof

(not qualified to review most of the changes, but had a quick look and a couple of things popped out)

raboof · 2018-05-25T07:39:34Z

.travis.yml

@@ -46,3 +49,5 @@ matrix:
  exclude:
    - scala: 2.10.7
      env: PLATFORM=js  SBT_PARALLEL=true  WORKERS=1 DEPLOY=true SCALAJS_VERSION=1.0.0-M3
+    - scala: 2.13.0-M4
+      env: PLATFORM=js  SBT_PARALLEL=true  WORKERS=1 DEPLOY=true SCALAJS_VERSION=1.0.0-M3


This exclude doesn't make sense like this anymore since the change from scala 2.13.0-M3 to 2.13.0-M4, right?

Not sure I understand.. This exclude skips building with Scala.js 1.0.0-M3 on Scala 2.13.0-M4, because there's no such Scala.js release yet.

A got it 👍

raboof · 2018-05-25T07:39:39Z

build.sbt

@@ -78,7 +91,11 @@ lazy val sharedSettings = MimaSettings.settings ++ scalaVersionSettings ++ Seq(
  // don't use fatal warnings in tests
  scalacOptions in Test ~= (_ filterNot (_ == "-Xfatal-warnings")),

-  mimaPreviousArtifacts := Set("org.scalacheck" %% "scalacheck" % "1.14.0"),
+  mimaPreviousArtifacts := {
+    // TODO: re-enable MiMa for 2.13.0-M4 once there is a release out


Maybe add a ticket to keep track of this?

Added a comment #410 (comment)

raboof · 2018-05-25T07:40:44Z

src/main/scala/org/scalacheck/Prop.scala

@@ -765,8 +766,8 @@ object Prop {
    /*
     * Returns the first failed result in Left or success in Right.
     */
-    def getFirstFailure(xs: Stream[T]): Either[(T,Result),(T,Result)] = {
-      assert(!xs.isEmpty, "Stream cannot be empty")
+    def getFirstFailure(xs: LazyList[T]): Either[(T,Result),(T,Result)] = {


Is this binary compatible?

Yes, because LazyList is an alias for Stream when building on 2.10-2.12 https://github.com/rickynils/scalacheck/blob/543b4056bffdb684faec49e8afb09aee4c24eb2f/src/main/scala-2.13-/org/scalacheck/ScalaVersionSpecific.scala#L19-L20

ashawley · 2018-05-30T22:43:44Z

src/main/scala/org/scalacheck/Shrink.scala


 sealed abstract class Shrink[T] extends Serializable {
-  def shrink(x: T): Stream[T]
+  def shrink(x: T): LazyList[T]


So the signature for Shrink changes here, so existing 1.14.0 tests (or libraries) that defined shrinkers would no longer compile.

The signature is actually the same on 2.10-2.12, because LazyList is an alias for Stream there: https://github.com/rickynils/scalacheck/blob/543b4056bffdb684faec49e8afb09aee4c24eb2f/src/main/scala-2.13-/org/scalacheck/ScalaVersionSpecific.scala#L19-L20

Here's an example shrinker:

object CustomShrink { implicit val dumbIntShrinker: Shrink[Int] = Shrink { case _ => Stream.empty } }

Publishing this branch as 1.14.0-543b405, the example compiles on 2.10, 2.11, and 2.12, but for 2.13.0-M4:

[error] src/main/scala/CustomShrink.scala:5:22: polymorphic expression cannot be instantiated to expected type; [error] found : [A]scala.collection.immutable.Stream[A] [error] required: LazyList[Int] [error] case _ => Stream.empty

Thank you for pointing this out.

Do poeple actually write shrinkers "ahead of time", at the same time when writing Arbitrary instances, and put them into the code base?

We could make the LazyList aliases public on 2.10-2.12, then poeople could use these. But I think this gets even more confusing. Already having the LazyList alias in the signatures on 2.10-2.12 is a bit problematic.

We could keep using Stream, but that would lead to deprecation warnings on 2.13.

Yeah, writing one's own Shrink is documented.

Aren't warnings being emitted for Traversable?

There are a number of uses of Traversable in the API, but I think they should not lead to deprecation warnings for the users. In 2.13, Traversable is a (deprecated) alias to Iterable.

So let's say scalacheck defines a method

scala> def foo: Traversable[Int] = List(1,2,3) ^ warning: type Traversable in package scala is deprecated (since 2.13.0): Use Iterable instead of Traversable foo: Traversable[Int]

When compiling scalacheck on 2.13, this will give a deprecation warning. But for users it won't:

scala> foo res0: Traversable[Int] = List(1, 2, 3) scala> foo.map(_ + 1) res1: Iterable[Int] = List(2, 3, 4) scala> foo: Iterable[Int] res2: Iterable[Int] = List(1, 2, 3)

Ah, ok. I see the distinction. Thanks for explaining.

For now, preserve Stream in Shrink, and then migrate to LazyList in 1.15.0?

lrytz · 2018-06-06T20:15:18Z

Pushed a change by @ashawley to go back to using Stream when defining Shrinkers. We can move to LazyList in a scalacheck 1.15

With `n == 0` the test effectively calls `Gen.frequency()`, which throws `IllegalArgumentException: no items with positive weights`.

ashawley · 2018-06-07T17:13:40Z

I'd say this is good to publish for 2.13.0-M4.

Since there are long-term implications with 2.13 that require further consideration going forward (like, "What should go in 1.15.0?"), maybe hold off on the merge?

rickynils · 2018-06-10T22:37:04Z

@ashawley I'v rebased and merged this to the 1.14.0_sonatype branch and published the build. I'm not sure I see why we should hold off the merge to master, though?

ashawley · 2018-06-11T03:00:16Z

This PR currently represents the conservative set of changes for making a 1.14.0 release for 2.13. Publishing was important for getting a ScalaCheck release out for the 2.13.0-M4 milestone . That's all that's been really reviewed, though.

What wasn't probably reviewed, was the first half of these commits of this PR containing the original re-implementation of ScalaCheck's use of collections for 2.13. Those modifications have compatibility breaking changes, but they would seem like the better base with which to move forward on.

lrytz · 2018-06-11T06:35:50Z

In the current form this PR doesn't have breaking changes, those parts of the earlier commits are reverted in later commits. I can squash things if that's desired.

Changes such as moving to use LazyList instead of Stream in shrinkers on 2.13 can be done in a 1.15 release.

jozic · 2018-07-18T08:09:15Z

build.sbt

+      case 10 => Seq("-Xfatal-warnings", "-Xlint")
+      case 11 => Seq("-Xfatal-warnings", "-Xlint", "-Ywarn-infer-any", "-Ywarn-unused-import")
+      case 12 => "-Xfatal-warnings" +: modern
+      case 13 => modern


does it mean 2.13 enables -Xfatal-warnings by default?

No :-) this disables fatal warnings on 2.13 because there are some deprecation warnings due to cross-building (usage of Stream)

got it, one alternative way could be to add silencer for that particular place instead of disabling it in general, but I guess it's not a big deal, as it's enabled for the rest of versions

mcanlas · 2018-08-29T19:17:33Z

Are there any blockers for this PR?

Scala 2.13.0-M5 is out now.

ashawley · 2018-08-30T12:30:02Z

M5 is out, but I'm not sure it's minted, yet.

For the record, this was merged to the 1.14.0_sonatype branch in #410, but just not master, yet. Since development isn't active, this is effectively a longer-running branch/PR for 2.13.

lrytz · 2018-10-03T10:08:18Z

It would be great to move 2.13 cross-building forward. As mentioned in a recent announcement, one goal of the postponed RC1 is to make sure we have time to get the OSS ecosystem up and running. Scalacheck is obviously very important as it's one of the roots.

I'll update this PR to M5.

ashawley · 2018-10-03T12:14:45Z

Seth got ScalaCheck most of the way there for 2.13.0-M5, see #427.

rickynils · 2019-02-25T09:39:53Z

I have merged 1.14.0_sonatype to master, so this PR is now effectively merged to master too. Closing this.

lrytz force-pushed the m4 branch from 5b7a677 to 39203ee Compare May 15, 2018 20:54

This was referenced May 15, 2018

publish for Scala 2.13.0-M4? #410

Closed

[WIP] Add support for Scala 2.13-M4 #391

Closed

lrytz force-pushed the m4 branch 5 times, most recently from d999db4 to d7e7661 Compare May 18, 2018 21:30

lrytz mentioned this pull request May 19, 2018

CI failing (scala-native) #412

Closed

lrytz added 6 commits May 23, 2018 15:00

Changes to compile with Scala 2.13.0-M4

6611977

Co-authored-by: julienrf

Cross-build from a single branch

b64e21f

Changes to make MiMa happy

4b253fb

Fix typelevel#412

fd6aa84

Don't MiMa on 2.13

5ba8ddf

Go back to using Traversable instead of Iterable

1c79131

This is a no-op for 2.13, as Traversable is a (deprecated) alias for Iterable. For 2.10-2.12, this ensures that the signatures don't change due to cross-building with 2.13.

lrytz force-pushed the m4 branch from e0d69f3 to 1c79131 Compare May 23, 2018 14:11

Remove Factory type alias, simplify version-dependent Buildable

543b405

lrytz force-pushed the m4 branch from b31cd89 to 543b405 Compare May 23, 2018 14:49

lrytz changed the title ~~[WIP] Changes to compile with Scala 2.13.0-M4~~ Changes to compile with Scala 2.13.0-M4 May 24, 2018

raboof reviewed May 25, 2018

View reviewed changes

ashawley reviewed May 30, 2018

View reviewed changes

xerial mentioned this pull request Jun 1, 2018

Scala 2.13.0-M4 wvlet/airframe#175

Merged

7 tasks

lrytz mentioned this pull request Jun 4, 2018

Port “The Architecture of Scala Collections” to the new architecture scala/docs.scala-lang#1078

Merged

ashawley and others added 2 commits June 6, 2018 10:51

Revert LazyList back to Stream for Shrink

f7d6cf8

Small user guide update

12c8a66

guersam mentioned this pull request Jun 7, 2018

Bump scalacheck version to 1.14.0 scalatest/scalatest#1369

Closed

Fix "Gen.frequency 3" test

4848ee8

With `n == 0` the test effectively calls `Gen.frequency()`, which throws `IllegalArgumentException: no items with positive weights`.

lrytz force-pushed the m4 branch from 6e1afc5 to 4848ee8 Compare June 7, 2018 14:45

kailuowang mentioned this pull request Jun 19, 2018

Publish for 2.13.0-M4 typelevel/cats#2267

Closed

jozic reviewed Jul 18, 2018

View reviewed changes

rickynils closed this Feb 25, 2019

ashawley mentioned this pull request Feb 13, 2020

Deprecation warnings because of Stream #626

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changes to compile with Scala 2.13.0-M4 #411

Changes to compile with Scala 2.13.0-M4 #411

lrytz commented May 15, 2018

lrytz commented May 15, 2018

lrytz commented May 19, 2018

lrytz commented May 23, 2018 •

edited

Loading

raboof left a comment

raboof May 25, 2018

lrytz May 25, 2018 •

edited

Loading

raboof May 25, 2018

raboof May 25, 2018

lrytz May 25, 2018

raboof May 25, 2018

lrytz May 25, 2018 •

edited

Loading

raboof May 25, 2018

ashawley May 30, 2018

lrytz May 31, 2018

ashawley May 31, 2018

lrytz May 31, 2018

ashawley May 31, 2018

ashawley May 31, 2018 •

edited

Loading

lrytz May 31, 2018

ashawley May 31, 2018

lrytz commented Jun 6, 2018

ashawley commented Jun 7, 2018

rickynils commented Jun 10, 2018

ashawley commented Jun 11, 2018

lrytz commented Jun 11, 2018

jozic Jul 18, 2018

lrytz Jul 18, 2018

jozic Jul 18, 2018 •

edited

Loading

mcanlas commented Aug 29, 2018

ashawley commented Aug 30, 2018

lrytz commented Oct 3, 2018

ashawley commented Oct 3, 2018

rickynils commented Feb 25, 2019

Changes to compile with Scala 2.13.0-M4 #411

Changes to compile with Scala 2.13.0-M4 #411

Conversation

lrytz commented May 15, 2018

lrytz commented May 15, 2018

lrytz commented May 19, 2018

lrytz commented May 23, 2018 • edited Loading

raboof left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz May 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashawley May 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz commented Jun 6, 2018

ashawley commented Jun 7, 2018

rickynils commented Jun 10, 2018

ashawley commented Jun 11, 2018

lrytz commented Jun 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jozic Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

mcanlas commented Aug 29, 2018

ashawley commented Aug 30, 2018

lrytz commented Oct 3, 2018

ashawley commented Oct 3, 2018

rickynils commented Feb 25, 2019

lrytz commented May 23, 2018 •

edited

Loading

lrytz May 25, 2018 •

edited

Loading

lrytz May 25, 2018 •

edited

Loading

ashawley May 31, 2018 •

edited

Loading

jozic Jul 18, 2018 •

edited

Loading