SC: search for references of Groovy property as part of path expression finds 1 exact and 1 potential match for same location

[eric-milles]

Split from #935

[I]n our real project I find now that for very similar cases like this (i.e.: search for references of properties in a Groovy class) I get either single potential matches (like in this case) or, most of the times, 2 matches for each reference, 1 exact and 1 potential. This is an example, where I search for references of the order property an a class named OrderPlacingOutcome (which is just a simple Groovy bean like the above one, just implementing Serializable as the only difference):

[eric-milles]

or, most of the times, 2 matches for each reference, 1 exact and 1 potential

I'll have to look more carefully at this. But it is possible that the AST contains 2 nodes that infer to the field/property. Static compilation often rewrites method call and property expressions in unexpected ways.

[eric-milles]

Here is a bit of the balancing act I referred to earlier. One of the matches comes from FieldReferenceSearchRequestor, where this bit selects the node:

        } else if (node instanceof ConstantExpression) {
            if (fieldName.equals(((ConstantExpression) node).getText())) {
                ...
                // check for "foo.bar" where "bar" refers to generated/synthetic "getBar()", "isBar()" or "setBar(...)"
                } else if (result.declaration instanceof MethodNode && (((MethodNode) result.declaration).isSynthetic())) {

Type inference sees a setter and the setter is synthetic (generated).

The other match comes from MethodReferenceSearchRequestor, which looks for accessor calls:

        if (methodName.equals(((MethodNode) result.declaration).getName())) {
            if (isDeclaration || node instanceof StaticMethodCallExpression) {
                ...
            // check for non-synthetic match; SyntheticAccessorSearchRequestor matches "foo.bar" to "getBar()", etc.
            } else if (methodName.equals(node.getText()) || (isNotSynthetic(node.getText(), result.declaringType) && !skipPseudoProperties)) {

Type inference sees a setter and isNotSynthetic returns true because there is a supporting field.

These checks are designed to be mutually exclusive. But there is a bit of overlap to keep rename working. Consider this variation:

class Bean {
  String foo
  void setFoo(String foo) { ... }
}

In this case, searches for the "foo" property match on "bean.foo = x" as potential. Renaming the property and not the setter must not change the assignment expression.

And there is the case where the is a "synthetic" property:

class Bean {
  String getFoo() { ... }
  void setFoo(String foo) { ... }
}

I need to figure out exactly which case isNotSynthetic is for, because if it returns false, you get the exact match that is desired and not the potential match this is problematic (for searching and renaming).

[eric-milles]

ready to test

[mauromol]

I tested this with 3.5.0.v201908101728-e1906 and now all references are shown as just one match, with no indication of "exact"/"potential", hence they must be exact, which is what I expect.

The only odd thing is that when I have for the same OrderPlacingOutcome.order property a match from Java code (like: outcome.getOrder()) the Search view shows the label "(1 match)" for it, which is strange because usually the number of occurrences is shown only when the number of matches is >1. See this screenshot:

I don't think it's an issue, however, so I'm closing this, thank you!

[mauromol]

Ouch, I can't close because you are the opener.