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

Assess -Xsource:3 using community build #769

Closed
SethTisue opened this issue Apr 26, 2021 · 21 comments
Closed

Assess -Xsource:3 using community build #769

SethTisue opened this issue Apr 26, 2021 · 21 comments
Assignees

Comments

@SethTisue
Copy link
Member

This came up with @smarter at https://gitter.im/typelevel/general?at=6086d87e81866c680c48df4f

I've recently added a bunch of Scala 3 syntax under that flag to allow us to deprecate and potentially repurpose some old syntax without harming cross-compatibility with Scala 2, but this hinges on people actually using that flag so it'd be good to know if there's any issues with it

We should at least try it and see what happens.

@SethTisue SethTisue self-assigned this Apr 26, 2021
@SethTisue

This comment has been minimized.

@SethTisue
Copy link
Member Author

SethTisue commented Apr 30, 2021

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2703/

42 green repos, but also lots of failures, most notably scalatest,kind-projector,utest,shapeless,scalatest-3-0; transitively, those prevent most of the build from running

sampling of failure causes:

  • scalatest: "method without a parameter list overrides a method with a single empty one"
  • kind-projector: "Either[Int, _] takes no type parameters, expected: 1", I suppose because it's using the old wildcard syntax
  • utest: "symbol literal is unsupported; use Symbol("hello") instead"
  • shapeless: "Malformed literal or standard type 'a"

fixes for these could easily be submitted upstream, and then we could re-run and get more results

this is one of those times where we don't primarily want a community build that rewires all the dependencies; we would primarily want to build each repo separately

it would not be super hard for me (or somebody) to write a script that glues all the proj/*.conf files together in an alternate why where each repo is built in its own space, with check-missing: false so that all dependencies would be retrieved from Maven Central et al. at least, I don't think I would run into a bunch of weird trouble doing that

on the other hand, I don't have any further time for this either this week or next, but I could probably return to it after that

@smarter
Copy link
Member

smarter commented Apr 30, 2021

fixes for these could easily be submitted upstream

I'll see if I can look into it. I think the compiler could also stop emitting errors on symbol sytnax since it's still supported in Scala 3 under a language feature. But for kind-projector at least, the error likely comes from a test exercising the ? syntax since it's still supported even if it's deprecated, and upstream might not want to get rid of it right now.

on the other hand, I don't have any further time for this either this week or next, but I could probably return to it after that

That would be great! No pressure though :).

smarter added a commit to smarter/scalatest that referenced this issue Apr 30, 2021
This commit lets scalatest compile with the Scala 2.13 flag
`-Xsource:3` (but does not enable the flag in the build), this came up
in scala/scala-dev#769.
The changes needed were:
- Add missing parens when overriding `Iterator#next()`
- Remove usages of symbol literal syntax
- Remove usages of procedure syntax
- Fix code that relied on any2stringadd
smarter added a commit to smarter/scala that referenced this issue Apr 30, 2021
Scala 3 still supports symbol literals even if they require a language
import now (cf scala/scala3#11588), so don't
emit an error if we find one under -Xsource:3 as that could
unnecessarily impede cross-compilation as discovered in
scala/scala-dev#769.
smarter added a commit to smarter/kind-projector that referenced this issue Apr 30, 2021
Scala 2.13.6 and 2.12.14 will interpret `?` as a wildcard when using the
`-Xsource:3` flag (cf scala/scala#9560)). This
means that the old kind-projector syntax will no longer work, so it
seems like a good time to remove it. This will also allow us to compile
more of the community-build with `-Xsource:3` enabled (cf
scala/scala-dev#769).

Sincet this is a breaking change, we also bump the version to
0.12.0-SNAPSHOT.
smarter added a commit to smarter/scalatest that referenced this issue May 1, 2021
This commit lets scalatest compile with the Scala 2.13 flag
`-Xsource:3` (but does not enable the flag in the build), this came up
in scala/scala-dev#769.
The changes needed were:
- Add missing parens when overriding `Iterator#next()`
- Remove usages of symbol literal syntax
- Remove usages of procedure syntax
- Fix code that relied on any2stringadd
smarter added a commit to smarter/scalatest that referenced this issue May 3, 2021
This commit lets scalatest compile with the Scala 2.13 flag
`-Xsource:3` (but does not enable the flag in the build), this came up
in scala/scala-dev#769.
The changes needed were:
- Add missing parens when overriding `Iterator#next()`
- Remove usages of symbol literal syntax
- Remove usages of procedure syntax
- Fix code that relied on any2stringadd
@smarter
Copy link
Member

smarter commented May 23, 2021

@SethTisue do you think you could try running this again now that all the known issues are fixed?

@SethTisue
Copy link
Member Author

@smarter sure, after scala/community-build#1418 lands

@SethTisue
Copy link
Member Author

@smarter
Copy link
Member

smarter commented May 27, 2021

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2787/artifact/logs/kind-projector-build.log

[kind-projector] [error] /home/jenkins/workspace/scala-2.13.x-jdk11-integrate-community-build/target-0.9.17/project-builds/kind-projector-b0a85b5f7bb05e14140d78d7b4b2515b9a3ab274/src/test/scala/test.scala:17:7: Either[Int, _] takes no type parameters, expected: 1
[kind-projector] [error]   bar[Either[Int, ?]]
[kind-projector] [error]       ^

Looks like kind-projector isn't up-to-date in the community build since that doesn't match the current sources: https://github.com/typelevel/kind-projector/blob/main/src/test/scala/test.scala#L17

@smarter
Copy link
Member

smarter commented Jun 15, 2021

Some observations:

  • Most failures are due to existing warnings turning into errors
  • specs2 is failing because it's still using the old ? kind-projector syntax, I'll try to make a PR to fix that.
  • For some reason, the * - test syntax from utest doesn't work anymore, but it's deprecated anyway.
  • quicklens fails with seemingly weird errors:
[error] /home/smarter/opt/quicklens/quicklens/src/main/scala-2.13+/com.softwaremill.quicklens/package.scala:125:38: method apply in object LensHelper cannot be accessed as a member of object com.softwaremill.quicklens.package.LensHelper from package object quicklens in package quicklens
[error] error after rewriting to `package`.this.LensHelper.<apply: error>
[error] possible cause: maybe a wrong Dynamic method signature?
[error]   def modifyLens[T]: LensHelper[T] = LensHelper[T]()
[error]                                      ^
[error] /home/smarter/opt/quicklens/quicklens/src/main/scala-2.13+/com.softwaremill.quicklens/package.scala:127:46: method apply in object MultiLensHelper cannot be accessed as a member of object com.softwaremill.quicklens.package.MultiLensHelper from package object quicklens in package quicklens
[error] error after rewriting to `package`.this.MultiLensHelper.<apply: error>
[error] possible cause: maybe a wrong Dynamic method signature?
[error]   def modifyAllLens[T]: MultiLensHelper[T] = MultiLensHelper[T]()
[error]                                              ^
[error] two errors found

Which are actually caused by the private on the definition of LensHelper:

case class LensHelper[T] private () {

which now also implies that the apply method is private. This can be fixed by explicitly declaring these methods as public:

diff --git quicklens/src/main/scala-2.13+/com.softwaremill.quicklens/package.scala quicklens/src/main/scala-2.13+/com.softwaremill.quicklens/package.scala
index 9675d9d..ff647d3 100644
--- quicklens/src/main/scala-2.13+/com.softwaremill.quicklens/package.scala
+++ quicklens/src/main/scala-2.13+/com.softwaremill.quicklens/package.scala
@@ -127,14 +127,19 @@ package object quicklens {
   def modifyAllLens[T]: MultiLensHelper[T] = MultiLensHelper[T]()

   case class LensHelper[T] private () {
-
     def apply[U](path: T => U): PathLazyModify[T, U] = macro QuicklensMacros.modifyLazy_impl[T, U]
   }
+  object LensHelper {
+    def apply[T](): LensHelper[T] = new LensHelper[T]
+  }

   case class MultiLensHelper[T] private () {

     def apply[U](path1: T => U, paths: (T => U)*): PathLazyModify[T, U] = macro QuicklensMacros.modifyLazyAll_impl[T, U]
   }
+  object MultiLensHelper {
+    def apply[T](): MultiLensHelper[T] = new MultiLensHelper[T]
+  }

   case class PathLazyModify[T, U](doModify: (T, U => U) => T) {

So overall this is looking good (except the specs2 and utest failures prevent a lot of projects from compiling).

@smarter
Copy link
Member

smarter commented Jun 15, 2021

etorreborre/specs2#953

smarter added a commit to smarter/utest that referenced this issue Jun 15, 2021
This syntax doesn't compile with `-Xsource:3` and thus prevents us from
running utest in the Scala 2 community build with this flag
enabled (scala/scala-dev#769).
@smarter
Copy link
Member

smarter commented Jun 15, 2021

com-lihaoyi/utest#253

SethTisue pushed a commit to scalacommunitybuild/utest that referenced this issue Jun 15, 2021
This syntax doesn't compile with `-Xsource:3` and thus prevents us from
running utest in the Scala 2 community build with this flag
enabled (scala/scala-dev#769).
@SethTisue
Copy link
Member Author

SethTisue commented Jun 15, 2021

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2881/

@SethTisue
Copy link
Member Author

new run with fixes for specs2, fastparse, fastparse-scalameta, scalameta:

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/2887/

@SethTisue
Copy link
Member Author

SethTisue commented Jun 16, 2021

run 2887 shows: BLOCKING DOWNSTREAM: scalatest-3-0 (62), spray-json (53), wartremover (53), json4s (47), scodec-bits (45), fansi (22), akka-stream (21), acyclic (19), scalatags (11), http4s-parboiled2 (9), spire (7), scalamock (2), hasher (1), lift-json (1), decline (1), mainargs (1), tut (1)

for scalatest-3-0 we could just use a JAR from Maven Central instead

spray-json, wartremover, scodec-bits look to me like they probably just need trivial fixes

json4s situation isn't clear to me at a glance

@smarter
Copy link
Member

smarter commented Jun 16, 2021

[json4s] [error] def ~>*[B >: A, X2 >: X](f: => Rule[S, S, B => B, X2]) = for (a <- rule; fs <- f*) yield fs.foldLeft[B](a) { (b, f) => f(b) }

That f* is a postfix application of a * operator, which conflicts with the new varargs syntax, looks like this was recently fixed upstream: json4s/json4s@b91c3ff

@SethTisue
Copy link
Member Author

@smarter after scala/community-build#1456 is done, doing this sort of experiment will become easier

@SethTisue
Copy link
Member Author

after scala/community-build#1456 is done, doing this sort of experiment will become easier

that's done now, so I intend to return to this soon-ish

SethTisue added a commit to SethTisue/community-build that referenced this issue Sep 13, 2021
SethTisue added a commit to SethTisue/community-build that referenced this issue Sep 13, 2021
SethTisue added a commit to SethTisue/community-build that referenced this issue Sep 13, 2021
SethTisue added a commit to scala/community-build that referenced this issue Sep 13, 2021
@SethTisue
Copy link
Member Author

SethTisue commented Sep 13, 2021

there is a new full run here: https://scala-ci.typesafe.com/view/scala-2.13.x/job/scala-2.13.x-jdk11-integrate-community-build/3190/artifact/report.html (scroll down for links to per-repo failure logs)

181 repos are green, so that's good

these are the failures where -Xsource:3 appears, offhand, to be functioning as intended:

  • sttp, scodec, scalastyle, scala-collection-laws, play-ws, hasher, grizzled, finagle, breeze:
    • method without a parameter list overrides a method with a single empty one
  • spire, scodec-bits, scalamock, scalafx, parboiled, machinist, http4s-parboiled2:
    • unary prefix operator definition with empty parameter list is unsupported
  • specs2, scribe, scalatags, scalafmt, playframework, mainargs, ammonite, akka-management:
    • any2stringadd
  • zinc, twitter-util, scoverage, scala-java-time, macwire, kafka, jwt-scala, acyclic:
    • procedure syntax
  • tut, spray-json, lagom:
    • Methods without a parameter list and by-name params can not be converted to functions as m _
  • treehugger, lift-json:
    • view bounds
  • slick:
    • shadowing a nested class of a parent is unsupported

we would need to dig deeper into the following failures to understand them:

  • akka-http, alpakka-kafka, http4s, lila-ws, mima, quicklens: error after rewriting [...] possible cause: maybe a wrong Dynamic method signature?
  • data-class, fansi, pprint, sbt-io: (some kind of * vs ? vs _ stuff)
  • scalachess: type mismatch (* related?)
  • pascal: PolyVals.this.Const[A, _] takes no type parameters, expected: 1
  • scala-supertagged: Type-checking succeeded unexpectedly
  • scalariform: test failure (perhaps just a too-fragile test?)

@smarter
Copy link
Member

smarter commented Sep 13, 2021

type mismatch (* related?)

Likely the same as #769 (comment), we should deprecate postfix * by default.

@SethTisue
Copy link
Member Author

The easily-nerd-sniped side of my personality wants to investigate the failures, but I think I've concluded that wouldn't be an ideal use of my time.

Overall, the results of this experiment were good. I invite anyone disquieted by any of the failures to investigate and, if appropriate, open specific individual tickets on them in scala/bug.

lolgab pushed a commit to com-lihaoyi/utest that referenced this issue Oct 6, 2021
This syntax doesn't compile with `-Xsource:3` and thus prevents us from
running utest in the Scala 2 community build with this flag
enabled (scala/scala-dev#769).
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

No branches or pull requests

2 participants