Issue 3564 - Replace StringBuffer with StringBuilder to run against new eclipse ibuilds

[robstryker]

This is a naive PR by someone unfamiliar with the project.

I noticed there were only 11 references to StringBuffer in the entire project and it was likely that those references were only there because of eclipse API. I simply did an in-place sed and opened the PR to see what breaks.

[robstryker]

This requires updating to the newest eclipse deps to even see if it works.

[Rawi01]

Simply replacing every StringBuffer with StringBuilder will cause issues with older Eclipse versions. The direct print reference in EclipseHandlerUtil can be fixed by using ASTNode::toString. To update the patched methods in the Eclipse agent, we need to modify them to support both the new and old signatures. Hopefully, we can make these updates without resorting to copying and pasting the patches.

[robstryker]

I went a little further with trying to separate out old and new method calls... but since I'm clearly a bit unfamiliar with the project, it's rough.

ScriptBuilder.replaceMethodCall() would need to be updated to allow replacing both the old and new version of the method. I couldn't figure out how to chain this so I just stopped here for a bit.

But the most difficult part imo will be EclipseHandlerUtil.java L1335...

                        if (expressions != null) for (Expression ex : expressions) {
-                               StringBuffer sb = new StringBuffer();
+                               StringBuilder sb = new StringBuilder();
                                ex.print(0, sb);
                                raws.add(sb.toString());

The issue here is that Expression is the official AST class and so regardless of overriding linkages at the replaceMethodCall, the main entry point here still requires you to choose either a builder or a buffer and it mus tmatch the currently running environment before the linkages even take hold.

Anyway, without knowing more about how to build and test this repo, I can't really do much more, but hopefully I've gotten the process moving a bit.

[rspilker]

There is no reason at all to replace these. For non hotspots, the gain is too low, and the modern hotspot compiler understands and eliminates locks that never escape the running thread.

[rgrunber]

@rspilker this is meant to fix #3564 .

[robstryker]

@rspilker Without fixes like this, lombok will be unable to work with new eclipse i-builds. Eclipse had API changes and lombok now fails with stack traces as follows:

[Error - 08:33:53] Dec 5, 2023, 8:33:53 AM Lombok annotation handler class lombok.eclipse.handlers.HandleConstructor$HandleRequiredArgsConstructor failed
'java.lang.StringBuffer org.eclipse.jdt.internal.compiler.ast.Expression.print(int, java.lang.StringBuffer)'
java.lang.NoSuchMethodError: 'java.lang.StringBuffer org.eclipse.jdt.internal.compiler.ast.Expression.print(int, java.lang.StringBuffer)'
	at lombok.eclipse.handlers.EclipseHandlerUtil.createAnnotation(EclipseHandlerUtil.java:1335)

[robstryker]

Sorry, I should clarify. Eclipse didn't have "API changes" but rather changed the method signatures of their internal classes to use StringBuilder instead of StringBuffer. Because of this, Lombok fails when trying to call the print functionality.

[rspilker]

Oh, that does change things

[robstryker]

I think this patch is suitable for testing.

I will admit that I have no idea what I'm doing in the patchForTests part, though. The rest I think I've figured out, but still haven't tested it other than ensuring it compiles.

[robstryker]

@rspilker Any chance you can approve the workflows to build?

[robstryker]

@Rawi01 any progress?

[Rawi01]

@robstryker Fixed the remaining stuff and run tests against the latest integration version. It should work now. Can you add yourself to the AUTHORS file?

[rgrunber]

@robstryker is away as of today (I think until start of January). If that's the only remaining issue, feel free to go ahead and modify the AUTHORS file on his behalf based on the name/email in his author header. (ie. Rob Stryker [email protected] )

[rspilker]

@rgrunber he has to add his name himself and do the commit for legal reasons

[rgrunber]

Thanks Rob! Looks to be ready now.

[robstryker]

Hope everyone had a good end of year break!

This issue looks clean to be merged, no? @Rawi01 @rspilker

[fbricon]

@Rawi01 @rspilker any chance we can this one merged in? It'd be nice to have at least a snapshot or an edge build available from the lombok project, that we can ship in vscode-java

[rzwitserloot]

This issue looks clean to be merged, no? @Rawi01 @rspilker

Indeed it does and so it has been. Thank you for the contribution! It'll be in the next release.

We'll push out an edge release within 24h, @fbricon.

[fbricon]

@rzwitserloot thanks!