-
Notifications
You must be signed in to change notification settings - Fork 5k
Add support for Scala 3 #18001
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
Add support for Scala 3 #18001
Conversation
File libraryJar = findFile(ArtifactInfo.ScalaLibraryID, scalaClasspath); | ||
File compilerJar = findFile(ArtifactInfo.ScalaCompilerID, scalaClasspath); | ||
URL[] libraryUrls = new URL[]{libraryJar.toURI().tourl("")}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should contain both scala-library and scala3-library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't seem to break anything, but sure I can add it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
@tgodzik Wow! Thank you so much!!! I thought that to gain Scala 3 support, we have to use post-1.5 Zinc (that is why all the Zinc changes in my draft)... |
We don't actually need that, there is a fallback step currently, which we will want to remove, but not necessarily now. For sure we should update Zinc as the next step. I didn't want to everything in one PR, but I will most likely follow up with that. Also, sorry I didn't notice your PR! |
Excellent! Thank you very much!
You couldn't: I published mine after you did yours ;) |
@dubinsky I see that you actually updated zinc, so maybe we could rebase your PR later on and update to latest zinc this way? I see that you also spotted some places that might require some investigation (commented with |
Sure, I am ready to help in any way. I never used Zinc before, though, so I do not really know what I am doing.
Looking forward to it! By the way, does Thanks for all your work! |
I also thought zinc 1.5 was needed. First time digging into gradle code but over the weekend I managed to swap in all the VirtualFile to uptake zinc 1.5 to add scala 3 support. Thought I was close until I ran into xsbti.* errors and then ended up butchering my working tree trying to figure it out. From looking at this and #18003 its nice to see I wasn't completely off base with my attempt. Good learning experience for me. |
The main issue wasn't actually zinc version, but wrong jars in the class loader of ScalaInstance. We can still upgrade it for sure, but it might be easier to do it in a separate pr |
Good question, I will check!
It's probably better to rework it but I wanted to keep it to a minimum, so that it is easier to review. I think We can also do a follow up. |
@jastice are you able to confirm that's enough to make Scala 3 work for Gradle with Intellij? Or is there something more we need to do? |
Also, anyone knows how I can get the PR reviewed? |
Thank you!!! |
Maybe @eskatos could help |
@tgodzik some observations when using your branch:
|
I am getting no issues in Intellij either with Gradle or Intellij building it, are you on the newest version? The support for Scala 3 is still being improved, so best to be on the latest nightly (though I am using the latest release and seems to work fine). As far as the positions go, I am not sure why it's pointing to characters, we might need a zinc upgrade for that or to change something in the plugin still. I will take a look how we are handling it in other build tools. |
This is embarrassing: I thought I was, but turns out I was not. |
When Idea is configured to build directly, positions are reported correctly and messages click through to the correct source positions. When Idea builds using Gradle, positions are reported as character numbers... |
@tgodzik Positions like
Before logging a |
There is one tangential issue, just as a heads up for anyone looking to use this with Metals. buildscript {
repositories {
mavenLocal()
mavenCentral()
}
dependencies {
classpath "ch.epfl.scala:gradle-bloop_2.12:1.4.8-106-8722ebfd"
}
}
plugins {
id "java"
id "scala"
id "io.quarkus"
}
// Bloop plugin uses old DSL -- MUST be applied like this
apply plugin: "bloop"
repositories {
mavenCentral()
mavenLocal()
}
dependencies {
// scala3-compiler_3 will pull in corresponding scala3-library_3, which will also pull in corresponding scala-library (Scala 2), 2.13.6 in this case
implementation "org.scala-lang:scala3-compiler_3:3.0.2-RC1"
}
group = "org.example"
version = "1.0.0-SNAPSHOT"
java {
sourceCompatibility = JavaVersion.VERSION_16
targetCompatibility = JavaVersion.VERSION_16
}
tasks.withType(ScalaCompile) {
scalaCompileOptions.additionalParameters = [
"-feature",
"-explain",
"-Ysafe-init",
"-Xsemanticdb"
]
}
compileJava {
options.encoding = 'UTF-8'
options.compilerArgs << '-parameters'
}
compileTestJava {
options.encoding = 'UTF-8'
} Now the issue is, that this won't work with Metals. It gets detected as a Scala 2.13.6 project, and Scala 3 syntax will error in VS Code 🙁 You can check the "scala": {
"organization": "org.scala-lang",
"name": "scala-compiler",
"version": "2.13.6",
"options": [
"-Xsemanticdb",
"-Ysafe-init",
"-deprecation",
"-explain",
"-feature",
"-unchecked"
], And I fixed this by replacing But I think this might need a fix in the Gradle plugin. Anyways, hope this is helpful for anyone that tries to test this in Metals and gets confused/runs into the same thing! 🙂 |
@dubinsky good catch, I will look into that! @GavinRay97 ach yes, I was actually checking that out and haven't yet worked on a fix in the Bloop plugin. I will work on it a bit later. |
For now - adding this to your Gradle build after you apply the plugin should work...
|
It works! Thanks a lot for the help @ljacomet ! Should I do something more with the PR? |
@tgodzik Thanks a lot for the work. If you are willing, consider adding an entry to I also launched a longer test suite to check there are no surprises with your changes. |
Will do in a sec! |
Issue: gradle#16527 Previously, we would need to compile the Scala compiler bridge in order to use it with a particular Scala version. Now, it's possible to download the jar, which is released together with a particular Scala version. From other changes, a new compiler is needed together with Scala library, however it still reuses the Scala 2.13 library underneath. Sample project this works with: ```kotlin plugins { scala } repositories { mavenCentral() mavenLocal() } dependencies { implementation("org.scala-lang:scala3-library_3:3.0.1") } ``` Signed-off-by: Tomasz Godzik <tomek.godzik@gmail.com>
Signed-off-by: Tomasz Godzik <tomek.godzik@gmail.com>
…loaders for Scala 2 Signed-off-by: Tomasz Godzik <tomek.godzik@gmail.com>
Signed-off-by: tgodzik <tgodzik@virtuslab.com>
Signed-off-by: tgodzik <tgodzik@virtuslab.com>
@ljacomet I added the additional point to release notes and the tests seemed to all pass 🎉 |
OK, I've already triggered a build for you. |
Pre-tested commit build failed. |
@bot-gradle test and merge |
OK, I've already triggered a build for you. |
You should be able to test this by running...
Thanks a lot @tgodzik and co for this PR |
Previously, we would need to compile the Scala compiler bridge in order to use it with a particular Scala version. Now, it's possible to download the jar, which is released together with a particular Scala version.
From other changes, a new compiler is needed together with Scala library, however it still reuses the Scala 2.13 library underneath.
Sample project this works with:
plugins { scala } repositories { mavenCentral() mavenLocal() } dependencies { implementation("org.scala-lang:scala3-library_3:3.0.1") }
Fixes #16527
Context
Scala 3 has been released in May and a lot of its' potential users use Gradle, which is de facto standard in the JVM world. This is why
Contributor Checklist
<subproject>/src/integTest
) to verify changes from a user perspective<subproject>/src/test
) to verify logic./gradlew sanityCheck
./gradlew <changed-subproject>:quickTest
Gradle Core Team Checklist