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

Reducing gap between Lombok-javac model and the original source code while in the NetBeans editor #466

Open
lombokissues opened this issue Jul 14, 2015 · 5 comments
Labels
accepted The issue/enhancement is valid, sensible, and explained in sufficient detail

Comments

@lombokissues
Copy link

Migrated from Google Code (issue 393)

@lombokissues
Copy link
Author

👤 lahoda   🕗 Jul 10, 2012 at 14:22 UTC

Filling this because of NetBeans bug:
http://netbeans.org/bugzilla/show_bug.cgi?id=215078

The problem is that a lot of IDE features are misbehaving w.r.t content of methods marked with @ Synchronized (and similarly @ SneakyThrows and @ Cleanup (on local variables, of course)). The example in the bug talks about unused imports, which basically ignores content of the @ Synchronized methods, but the problem is not limited to this one feature or to that annotation.

The immediate cause is that when a synthetic tree node wraps existing tree nodes, the IDE considers all children of such "synthetic" tree node to be synthetic (holds for all standard javac trees, to the best of my knowledge). The high level problem for the @ Synchronized is that the model (standard javac trees) is very different from the actual code in the source code.

I think that in general the javac trees and the source code should be as similar as possible, at least as long as the (NetBeans) Java language support is expected to handle the Lombok source code. Adding a synthetic tree into a list usually does not cause big problems (several problems related to this were workarounded in NetBeans and there were related changes in Lombok as well, IIRC), but wrapping trees that correspond to part of an actual source code with a synthetic tree does cause problems. The only solution I see in NetBeans would be for each feature to second-guess what the lombok-adjusted trees mean in the source code, which is defeating the purpose of reusing Java language support for Lombok sources, IMO.

I think that ideally the javac model when running inside the NetBeans editor should be made more similar to what is written in the source code, to make it easier to consume by the NetBeans Java language support.

I am attaching a patch that shows what I mean for:
-@ Synchronized: as this only changes semantics, nothing needs to be done when in the NB editor, and the model will match the source code exactly.
-@ Cleanup: similar to @ Synchronized, except that NB might produce warning about not closed resource, but that might be resolved in NB if needed
-@ SneakyThrows: this one was hard and the solution is quite hacky. The solution I sketched in the patch is to add a synthetic "throws $$undefined" clause, and prevent the error that is produced for the unresolvable identifier (the hack with the Log.recorded).

The patch does not contain any tests - I will see if I can find time to create some.

As a side note, looking at the extensions methods, I am not sure if that can be supported in the NetBeans IDE. But the current version seems to produce uncompilable trees for javac, so I cannot say much about that.

@lombokissues
Copy link
Author

👤 lahoda   🕗 Jul 10, 2012 at 14:22 UTC

🔗 nb-215078.diff View file

@lombokissues
Copy link
Author

👤 reinierz   🕗 Jul 10, 2012 at 22:55 UTC

Looking at the diff, why go through the effort of making $$undefined and then hacking the error out of the reporter? Wouldn't it be easier to take each of the sneakythrown types and just throw them on the 'throws' typeline JUST for the javac run that isn't background compilation but is an IDE?

Also, the $$undefined option would seem to me like it means NO exceptions, of ANY kind, will flag a compiler error. If I write @ SneakyThrows(IOException.class) and my method throws SQLException, then the line that throws this SQLException should be flagged with a 'catch or declare in throws clause' compiler error, and I don't think that will happen with this hack.

By adding each sneakily thrown item verbatim (as in, a direct clone of the AST Node used in the @ SneakyThrows annotation, with 'java.lang.Throwable' for the default option of including none in the annotation), there's no need to hack the error log.

I'm guessing calling code will erroneously error with 'you need to handle [sneakily thrown exception here]' in the IDE, which is why you have to resort to using $$undefined. In that case, maybe this is the 'best effort' solution.

Other than the @ SneakyThrows solution this looks great, so I committed the fixes for cleanup and synchronized:

6bf2b92

as we'll be releasing an 0.11.4 very soon to solve a memory leak issue on eclipse this will be out soon.

@lombokissues lombokissues added the accepted The issue/enhancement is valid, sensible, and explained in sufficient detail label Jul 14, 2015
@lombokissues
Copy link
Author

👤 lahoda   🕗 Jul 13, 2012 at 11:37 UTC

Thanks a lot for adding the support for @ Synchronized and @ Cleanup.

I forgot about @ SneakyThrows(exception). I was considering simply amending the throws clause with Throwable (or alike), but it has exactly the problem you mentioned. I was considering a few more options as well, but unfortunately the hack in the patch is the best I was able to think up.

@lombokissues
Copy link
Author

End of migration

lianhaijun pushed a commit to lianhaijun/lombok that referenced this issue May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted The issue/enhancement is valid, sensible, and explained in sufficient detail
Projects
None yet
Development

No branches or pull requests

1 participant