Implementation of LombokReferenceSearcher and LombokMethodReferenceSearcher (fix for #685)

[npiguet]

This is still a WIP at this point, but is intended to fix the following issue:

"Analyze Data Flow" does not work with the lombok plugin #685

[npiguet]

There's some encouraging progress. The plugin can now find field usages inside the body of generated methods, follow the references to the generated methods parameters, and then find usages of the generated method itself.

Note that in the current state, LombokFieldFindUsagesHandlerFactory is disabled in plugin.xml but that's is only meant as a debugging help, to avoid messing with the results I get from the other search components.

[mplushnikov]

Thank you Nicolas! It would be really great improvement! I already tried to get ReferenceSearcher to work, but has no success and have implemented FindUsageHandler instead.

[npiguet]

It's getting better I think all the references are being found, but some nodes still don't look very descriptive when displayed in the "find usages" view, which is partially shared between "find usages" and "Dataflow to/from here".

[mplushnikov]

You are doing a really hard work! Really great!

I tried to figure out, why I have implemented LombokLightMethodBuilder#rebuildMethodFromString() method, this is very old code and maybe not nessairy any more, or used by some actions in Intellij, where I doesn't have automatically tests for it.

Maybe you can try to remove it and simplify your ReferenceSearch to work only with LightElements?

I only have find out, that LombokLightMethodBuilder#copy() should be implemented to support Intellij inline refactoring action (Ctrl + Alt + N) for lombok getters for example.
This can be replaced with:

  public PsiElement copy() {
    return new LightMethod(getManager(), this, getContainingClass());
  }

But this results in incorrect code, there 'this.' is not correctly applyed.
I think this could be generall problem of current implementation for CodeBlock generation. They are generated during AugmentProcessing and saved in LightMethod-Instances. They could still references some PsiElements which are changed or get invalidated some time later by IntelliJ...I think you saw something similar too?

[mplushnikov]

We can also try to ask Anna Kozlova (@akozlova) to help to get it done well. She already mention in https://youtrack.jetbrains.com/issue/IDEA-179790, that implementation of ReferenceSearch would be a good idea.

[npiguet]

The thing about LightElements, is that they don't seem designed to handle more than declarations. So classes, methods, fields, etc... can be done, but statements, expressions, code blocks, etc... can't be done.

Using LightElement is good enough to implement "find usages" or "search for references" that just replaces searching for the field by searching for the methods. But "Dataflow to/from here" (SliceUtil, SliceForwardUtil) doesn't understand the references returned when we do that. When SliceUtil looks for variable references, it does a bunch of checks to see if it's in an assignment, if its on the RHS or LHS of the = operator, etc... This stuff does not translate well when we replace a field reference by a method reference.

So in the end, I think we should keep both the FindUsages handler and the ReferenceSearch/MethodReferenceSearch. That should cover both use cases.

[npiguet]

It looks like in term of the display of the results, I can't do much better. each line of the results view is split in two:

1. A syntax highlighted view of the line at which the reference was found, with the reference itself highlighted in bold.
2. A light grey "in (...)" that tells you in which method or class that line exists.

It turns out that both of these elements are generated by IntelliJ, and we can't really tweak our implementation to get it to display right.

1. The line is taken from the Document instance that is associated with the PsiFile containing the PsiElement. In our case, that file is a DummyHolder which is flagged as non-physical, non-system-event-enabled. That fact means that IntelliJ doesn't even try to find a matching line anywhere, and at best, just prints PsiElement.getText().
2. I couldn't find the exact code for that "in (...)" bit, but it seems to just navigate the ancestors of the PsiElement until it finds either a PsiMethod or a PsiClass. Since the references are in method bodies, they are all children of the PsiMethodImpl version of the method, which doesn't have a proper parent. As a result, we only get "in method()" instead of "in ParentClass.method()".

In any case, the references themselves are returned correctly, but the way they are displayed is not great, and in the current state of things, it doesn't look like we can improve it further.

I still need to make the ReferenceSearcher and MethodReferenceSearcher a bit more thorough so they can find references properly for @Singular fields and @With classes.

[npiguet]

Ok, at this point it looks like everything is working as expected. I get the same results when I run "Dataflow to here" on a class with lombok annotations as I get when I run it on the same class with all annotations de-lomboked.

The display of the results is a bit different due to the point mentioned earlier, but they are references to the same expressions.

On top of this small differences, there are a few caveats to which I couldn't find viable solutions:

1. Find usages now shows some extra entries (up to 2) that are labelled just with "expression". That's the result of the IDE looking for reference to the field (which uses LombokReferenceSearcher) and getting results that point to generated code. Accessor and wither method references are found as usual.
2. Setter methods on Builder classes are now delomboked to "MyClassBuilder.this.field = field" instead of "this.field = field". It's semantically the same when delomboked, but the prefix is required for IntelliJ to properly resolve field references in Builder methods.
3. There are some weird areas that are highlighted at the top of the file whenever the caret is placed on top of a class field. This seems to be because the IDE tries to highlight the field usages returned by LombokReferenceSearcher, but since they are in generated code, it gets confused and generates a garbage highlight.

Please let me know if any of these side-effects are show stoppers.

Maybe we should should indeed ask for help from @akozlova to resolve those issues, do you know what is the best way to contact her?

[npiguet]

Here is the sample project that I have used to test all this stuff.
sample.zip

[abimarank]

Care to remove System.out statements?