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

Projects

None yet

1 participant

@lombokissues
Collaborator

Migrated from Google Code (issue 393)

@lombokissues
Collaborator

๐Ÿ‘ค 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
Collaborator

๐Ÿ‘ค lahoda ย  ๐Ÿ•— Jul 10, 2012 at 14:22 UTC

๐Ÿ”— nb-215078.diff View file

@lombokissues
Collaborator

๐Ÿ‘ค 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
Collaborator

๐Ÿ‘ค 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
Collaborator

End of migration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment