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

Get annotation processing going with Eclipse Compiler for Java #8913

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

arouel
Copy link
Contributor

@arouel arouel commented Jun 26, 2024

Problem

I'm writing a HTTP feature for Helidon (related to #8897) and want to work with Eclipse IDE, but the Helidon annotation processors throw an exception because the Eclipse Compiler for Java (ECJ) behaves slightly different compared to javac.

Solution

Some changes were necessary to get the annotation processing going:

  • Due to the implementation of IdeFilerImpl:236 there is no handling for javax.tools.StandardLocation.SOURCE_PATH implemented and therefore the search for module-info.java fails with a thrown IllegalArgumentException. Ignoring this exception when compiling with the ECJ is fine.
  • There is also a case where the ECJ cannot resolve a type name and the Element parameter is null. Returning an empty Optional enables the caller to fall back.
  • Due to the implementation of IdeInputFileObject:50 the deletion of a FileObject throws an IllegalStateException. Ignoring any exception when compiling with the ECJ should be fine.

I have successfully tested the changes suggested here in a current Eclipse IDE (Version: 2024-06 (4.32.0)) on my local computer.

Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Jun 26, 2024
@arouel arouel changed the title Get annotation processing running in Eclipse IDE Get annotation processing going with Eclipse Compiler for Java Jun 26, 2024
@tomas-langer
Copy link
Member

If you want to go forward with this PR, kindly sign the OCA, as otherwise we cannot accept your contribution.

@arouel
Copy link
Contributor Author

arouel commented Jun 27, 2024

@tomas-langer I added one more commit to handle the last case I had issues with when using Eclipse IDE. Can you have a look at it as well? I signed the OCA already, I guess it needs a bit to be approved.

Copy link

Thank you for signing the OCA.

@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Jul 1, 2024
@@ -76,7 +76,11 @@ public void lines(List<String> newLines) {
public void write() {
if (modified) {
if (originalResource != null) {
originalResource.delete();
try {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that it will only work with clean compile and will not be able to do incremental build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume deleting a resource only fails when using ECJ and javac wouldn't fail to delete the fail, right?

@@ -161,6 +161,10 @@ public static Optional<TypeName> createTypeName(TypeElement element, TypeMirror
* @return the associated type name instance
*/
public static Optional<TypeName> createTypeName(Element type) {
if (type == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually violates Helidon design (we only support non-null parameters - always).
We can merge it, I will create a follow up to fix this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall I add a comment here that this was added only to get ECJ running?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a comment as to why this check was added.

Copy link
Member

@tomas-langer tomas-langer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I cannot merge this:

  1. The ConfigMetadataHandler needs copyright update (I could do this on my own)
  2. The change you did for writing the stack trace is causing a spotbugs failure. So either this needs to be added as an exception (requires etc/spotbugs/exclude.xml file in the module + update of pom file (example can be found in config/config module. or it must be reverted to original

@arouel
Copy link
Contributor Author

arouel commented Oct 4, 2024

@tomas-langer I updated the PR, can you have a look again?

@arouel
Copy link
Contributor Author

arouel commented Oct 14, 2024

@tomas-langer can you have a look at the PR again? I think I addressed all your points. It would be great to gave it to get Eclipse IDE running.

@arouel
Copy link
Contributor Author

arouel commented Oct 16, 2024

I fixed the Checkstyle error just now. I don't know what we do with the failing tests/native-image-mp-1-macos job. Could it be flappy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants