-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[BUG] Internal package changes in ECJ will break Lombok. #3564
Comments
The code snippet lombok/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java Lines 1334 to 1336 in d6ee06c
can just be replaced with
since |
@HannesWell - so that would be the only change required? What about all references in Actually I don't think I even understand |
Do you have instructions to build this project and also import as a eclipse project into latest Eclipse IDE release ? |
@gayanper You can try Lombok 1.18.31 |
@snjeza can you point to your branch ? |
I don't know anything about lombok and how it is using Eclipse JDT so I cannot say more, sorry for that.
Could be, I have no clue. :) The call to |
Here in Lombok, the
What this appears to be doing is saying, any time anyone tries to call |
@gayanper you can try the following
|
gives me errors, did you run any other targets before and is there specific ant version requirement ? |
@gayanper Could you show me errors? My environment:
The updated patch
fixes EclipsePatcher too. |
Ok guys just to add some context here, from my code sluething. The main driver is those first 3 lines we saw before: lombok/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java Lines 1333 to 1338 in d6ee06c
If we change this as @snjeza has done, it is extremely likely that the code will compile and run. However, it appears that the overrides that Lombok is attempting to add will not function. Lombok is attempting to introspect all calls to lombok/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java Lines 989 to 995 in d6ee06c
Lombok's lombok/src/eclipseAgent/lombok/launch/PatchFixesHider.java Lines 432 to 444 in d6ee06c
From there, it redirects to Lombok's lombok/src/eclipseAgent/lombok/eclipse/agent/PatchJavadoc.java Lines 68 to 80 in d6ee06c
Basically, Simply put, using only the 3 line fix above will let it compile and run, but will likely not allow Lombok's override behavior to take effect, unless we add similar replacements for each step along this chain. |
I believe my PR (#3565) might be a lot closer to a solution that maintains the current behavior without breaking, but I have no way to test it. @gayanper once you have a workflow to build PRs and test them, let me know. There might be some places where we might need to add some exception handling in my PR. |
@snjeza your updated patch has a few errors. First You're missing a change in Second, your Third, calls to I believe my PR over at #3565 is more complete at this time. |
Seems like it has some compilation issues |
This is the sample code that cause the jdt failure
|
|
I couldn't get none of the above changes compiled, so I did it on my own
Please try to compare the patches and may be @snjeza can try my example code with latest jdt build on vscode. |
@gayanper the problem with this patch is that it won't be making any use at all of the overrides that lombok wants to apply in In fact, I think a reflection solution will be required here. |
It works for me |
@snjeza or @robstryker please feel free to take my patch and update the PR. |
My PR over at #3565 now compiles. I have to be honest, @gayanper, your patch is fine IF AND ONLY IF lombok had absolutely no reason to try to override the print method at all. If that's the case, or if the original reason for overriding the print method no longer applies, then your patch is simpler and most likely will work as expected. However, if lombok had a legitimate reason to try to override the jdt implementation of the print method, and if that reason still applies, then my patch is most likely more suitable as we still override those functions with the lombok-custom behavior. Unfortunately I'm not sure any of us here know the history of why lombok was trying to override this behavior. @rspilker do you think you can add some color to this for us? Or decide between the patches? |
I have tested it using 4.31-I-builds//I20231206-041 |
I love that so many people here are actively engaged in solving the problem, it's always helpful when more people gather experiences with the codebase. I will try to add the missing test stuff. Does anyone know if this issue also affect eclipse 2023-12? We override the print methods only for our test cases, one for generated javadoc and one to align the output between different eclipse versions. We can not simply change them without breaking older tests but I think there could be a shorter and easier solution for our |
No, this issue doesn't affect Eclipse 2023-12 (Eclipse 4.30). |
hey @Rawi01 - just to let you know I'm pretty much done with what I can contribute further to this patch, at least without knowing how to use or test this thing ;) I've actually never used lombok and have no idea what it is :X I just saw a bug that needed fixing and tried my best, but I've reached what I think is the end of the road since I don't know how to test or trace through any scenarios, or even what those scenarios might be. I hope you can take what I've done and run with it further. |
…der calls Signed-off-by: Rob Stryker <rob@oxbeef.net>
Was this fixed in the latest Eclipse update ? I updated today and lombok is broken, with the same error above. Tried reverting, got "No repository found containing: (and a bunch of links)". Now I'm broken since all my projects use lombok.
Versions: Any advice on how to revert just one Eclipse plugin, i.e. |
The fix was for Lombok. Have you tried with https://projectlombok.org/lombok-edge.jar ? It should work against the newer versions of Eclipse. |
OK changed eclipse.ini from: Thanks so much for your help ! |
Yes, indeed. I just upgraded Spring Tools Suite to newest (Eclipse v4.22.0.202403071427), which broke lombok-1.18.30. Using the edge version in Is there any news about a due date, or could a patch be released to the Maven repo? |
This is part of the latest edge release, all feedback is welcome. |
eclipse_installation_info.txt
|
I got the same error after STS upgrade to
with lomok version 1.18.32 this is the error from error log |
Please double check that you use the latest lombok version in eclipse (the version in Maven/Gradle doesn't matter) |
Originally filed at eclipse-jdtls/eclipse.jdt.ls#2985 .
Lombok is using JDT ECJ (
org.eclipse.jdt.core.compiler.batch
) internals (eg.lombok/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
Lines 1333 to 1336 in d6ee06c
In eclipse-jdt/eclipse.jdt.core#1403 these internals were updated (basically the single argument to
print
changed fromStringBuffer
toStringBuilder
. There's a few other references toStringBuffer
in the code that should be checked also.lombok/src/eclipseAgent/lombok/eclipse/agent/PatchJavadoc.java
Lines 68 to 80 in d6ee06c
lombok/src/eclipseAgent/lombok/launch/PatchFixesHider.java
Line 435 in d6ee06c
lombok/src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java
Lines 990 to 992 in d6ee06c
The text was updated successfully, but these errors were encountered: