-
-
Notifications
You must be signed in to change notification settings - Fork 354
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
SemanticDB data generation for Java modules #2227
Conversation
I now have some working implementation, but it has still issues. The output path of the semanticDB files contains unexpected segments resembling the full root path to the source files.
|
🤔 honestly I'm not really sure what's happening. I just tested with mill-scip to see if it also did that:
And stuff seems to be where I'd expect it without the weird segments. |
Ah, but for you it seem to also contain more that just the packages, probably everything below |
22d8a05
to
3d227b8
Compare
3d227b8
to
a1d2f74
Compare
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.
Very cool @lefou. Testing it out locally with Metals even though I just reviewed it I was still trying with java 17 and wondering why it wasn't working lol.
The biggest usecase for me would be mixed-java-scala targets.
I think I need to steal your workaround from mill-scip to get Java 17 to work. Does it work with Java 11 for you? |
For pure Java modules yes! |
For me, it already works for mixed project. I'm still writing the tests, which needs some extra care because of the invalid |
For mixed compilation, we still don't support forked Java compiler, which is an issue on Java 17 and above. |
Out of curiosity, are mixed-java-scala projects with Java 17 and SemanticDB data working with Bloop? |
Interesting, the |
I opened a new PR After its review and merge, I'll rebase this PR on top of it |
…2231) We need this feature to support SemanticDB generation (#2227) via a Java compiler plugin in mixed Scala/Java projects. We already introduced non-local Java compilers in #1988. This change now also allows the use of a non-local Java compiler in mixed Java/Scala modules, if any `-J` option is given. As compilers are cached, I added the `hashCode` of the relevant runtime `javacOptions` to the cache key. I took the opportunity to remove non-relevant options from the existing Java compiler cache, to reduce the amount of cached compiler instances. Pull request: #2231
I now get some semanticDB files, but the path still contains to many sub directories, representing the full path of the source location.
This magically makes the `-sourceroot` setting work
e14ec0b
to
c2e17ca
Compare
This PR should now produce proper SemanticDB data with all supported Java runtimes and for all combinations of Java and Scala sources. |
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.
Amazing, just tested it out and was happily able to be in a Java file and trigger a find references and have it work. Great work @lefou!
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.
Updated comments
This is a follow-up to the work that I've done in scalameta#4295. At that time I only applied the changes to Scala Targets since Semanticdb was only produced there when using Mill as your build server. Now that com-lihaoyi/mill#2227 is merged Java Targets and mixed targets will now also produce semanticdb. We now add the extra check for Mill and its version when we check for semanticdb java support. This will ensure that users won't get the error in the Doctor about lacking support.
This is a follow-up to the work that I've done in scalameta#4295. At that time I only applied the changes to Scala Targets since Semanticdb was only produced there when using Mill as your build server. Now that com-lihaoyi/mill#2227 is merged Java Targets and mixed targets will now also produce semanticdb. We now add the extra check for Mill and its version when we check for semanticdb java support. This will ensure that users won't get the error in the Doctor about lacking support.
…4816) This is a follow-up to the work that I've done in #4295. At that time I only applied the changes to Scala Targets since Semanticdb was only produced there when using Mill as your build server. Now that com-lihaoyi/mill#2227 is merged Java Targets and mixed targets will now also produce semanticdb. We now add the extra check for Mill and its version when we check for semanticdb java support. This will ensure that users won't get the error in the Doctor about lacking support.
A copy mechanism was introduced in com-lihaoyi#2227 because of a bug in semanticdb-java But we don't need to use it in ScalaModule as well if the Scala semanticdb works correctly.
This adds support for SemanticDB data generation for Java modules and Mixed Java-/Scala-Modules.
Requires the following features:
Old PR comment: ...
For the sake for experimenting, I added a test project under
scratch-hello-mixed
.