-
Notifications
You must be signed in to change notification settings - Fork 278
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 SemanticDB support - cont. #1508
Conversation
…icdb Mpeterson/semanticdb
This reverts commit 110442d.
This reverts commit 39b10b2.
This reverts commit cb07f15.
This reverts commit 993aa30.
This reverts commit 166c1f0.
This reverts commit 8fcbbc9.
This reverts commit 1e4df56.
Also bump versions of its dependencies: * org.ow2.asm:asm: 6.1.1 -> 9.0 * net.sf.jopt-simple:jopt-simple: 4.6 -> 5.0.4
I hope we can merge this soon. It's been a long-missing capability. |
@crt-31 sorry for the delay with a review. I'm currently on my PTO, I'll do a proper review after two weeks if this can wait. I quickly went over changes - implementation looks good. I want to do some testing and a bit of naming nit-picking when I'll be back for a proper review. |
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.
Thanks a lot @crt-31, please address nits
- fixed semanticdb example - added prefix '_' to semanticdb output path - renamed certain vars to not prepend with "scala_" - changed depsprovider var name to be plural instead of singular
@liucijus, are you able to rerun the CI tests for this PR? It seems to have hung for some reason. |
Unfortunately, no, but let's ignore it as it's flaky |
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.
Thanks @crt-31!
Description
Adds Semanticdb output capability
This PR extends the PR drafts #1467 and #1329, and as such builds on the great work so far by @olafurpg and @ricochet1k, and @aishfenton. In particular, this PR moves the semanticddb handling into a phase instead of the scalacworker, as well as some other improvements.
Features
scala_toolchain.enable_semanticdb
. Default is False for performance reasons (generating semanticdb adds considerable overhead to a build).Motivation
Semanticdb is needed by many tools and IDEs (i.e. Metals, Scalafix, etc). This will hopefully support integration of more tools with rules_scala and help accelerate its usage in the community.