-
Notifications
You must be signed in to change notification settings - Fork 401
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
Support Gradle annotation processing #2319
Conversation
- Add a new preference 'java.import.gradle.annotationProcessing.enabled' to specify whether the Gradle annotation processing will be enabled or not. - Create a init script tp get the annotation processing configuration from Gradle. And delegate the processing to JDT APT. - Move the init script related utilities from GradleProjectImporter to GradleUtils. Signed-off-by: Sheng Chen <sheche@microsoft.com>
@donat This is the PR to support annotation processing for Gradle. It uses the init script to get the annotation processing configurations and sync those configs to JDT APT. I'm open to move this into Buildship. Meanwhile, there are some limitations for the current implementation:
|
Signed-off-by: Sheng Chen <sheche@microsoft.com>
Looks like the test See: https://ci.eclipse.org/ls/job/jdt-ls-pr/3717/ |
test this please |
Signed-off-by: Sheng Chen <sheche@microsoft.com>
Change looks good to me, but maybe we can wait for Graeme to confirm against Micronaut. |
I tried micronaut with this sample project: https://github.com/micronaut-projects/micronaut-examples/tree/master/hello-world-java, and roughly compared the results with m2e-apt and command line build tool.
ResultThe compiled class files are same between gradle(with this PR) and maven(with m2e) - at least for the file names and numbers. But the compiled test output is different from build tools: There is an error observed in the log, using m2e-apt and this PR are the same
Most likely this is related with the implementation of ecj. |
Update: The problem mentioned above could be solved once the NPE issue is fixed: eclipse-jdt/eclipse.jdt.core#545 |
@jdneo could you try with this example https://guides.micronaut.io/latest/creating-your-first-micronaut-app-gradle-java.zip The referenced example is very old |
There is one small gap with the new example. The current implementation will skip fetching the apt configurations if the user's Gradle project contains My original assumption is that if there is such plugin configured in the project, that's a hint that user will do the code generation by himself. When I first opened the sample project, it did not generate the .class files because the task is available in the sample project. (I guess it's brought by After I remove the check, it then worked as expected: Now I think it's ok to remove those checks to bring out-of-box gradle annotation processor support. Since we have a setting for this, user can turn it off if he really wants to do it manually. |
Signed-off-by: Sheng Chen <sheche@microsoft.com>
test this please |
@rgrunber @fbricon @testforstephen I think we can now merge this one.(if the name of preference key looks good). For other ecj related issues, we can address them separately. Let me know if there are any concerns :) |
For me, it was good to merge. |
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.
Sorry I asked @melix to take a look from a Gradle perspective and he picked up on a point that annotation processors on the test annotation processor classpath are not handled correctly.
List<String> compilerArgs = new LinkedList<>() | ||
// Compiler args for test are ignored. | ||
// Due to JDT does not differentiate the args between main and test. | ||
collectApConfiguration(project, "compileJava", processors, compilerArgs) |
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.
There may be different processors present on compileJava
vs compileTestJava
, how does this PR address that?
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.
The reason that test annotation processors are ignored is that the APT support on JDT side cannot differentiate the processors from different source sets. My approach here is to ignore test annotation processors.
Another approach we can take is to mix all the processors from main and test together, but personally I think this is not ideal neither.
This is the case that we cannot handle gracefully now until the JDT can support that.
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.
I filed an issue to track the testAnnotationProcessor
support: #2345, which we can address separately from this pull request.
Hi all, I merged the PR, and we can address the issue about test annotation processor separately. |
Requires: redhat-developer/vscode-java#2793
fix redhat-developer/vscode-java#1660
fix redhat-developer/vscode-java#1039
Sample project for verification: https://github.com/mapstruct/mapstruct-examples/tree/main/mapstruct-on-gradle
Signed-off-by: Sheng Chen sheche@microsoft.com