Rename of accessor method improperly renames property-style accesses

[eric-milles]

Consider the following:

class A {
  private int foo
  int getFoo() { return this.foo }
  void setFoo(int foo) { this.foo = foo }
}

class B {
  def m(A a) {
    int i = a.@foo
    i = a.foo
    a.foo = i
  }
}

Rename of accessor getFoo improperly replaces i = a.foo with i = a.getNewName. Same goes for rename of setFoo (and probably isFoo if one existed).

Originally seen as part of #672

[eric-milles]

During a rename refactor, there is a tricky balance set up between FieldReferenceSearchRequestor, MethodReferenceSearchRequestor and SyntheticAccessorSearchRequestor.

Property-style accesses that resolve to a source method must not be seen as field accesses to prevent being renamed with the field. This is accomplished here in FieldReferenceSearchRequestor:

    public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaElement enclosingElement) {
        ...
        } else if (node instanceof ConstantExpression) {
            if (fieldName.equals(((ConstantExpression) node).getText())) {
                if (result.declaration instanceof FieldNode || result.declaration instanceof PropertyNode) {
                    doCheck = true;
                    isAssignment = EqualityVisitor.checkForAssignment(node, result.enclosingAssignment);
                    start = node.getStart();
                    end = node.getEnd();

                // check for "foo.bar" where "bar" refers to synthetic "getBar()", "isBar()" or "setBar(...)"
                } else if (result.declaration instanceof MethodNode && result.declaration.getEnd() < 1) {
                    doCheck = true;
                    isAssignment = ((MethodNode) result.declaration).getName().startsWith("set");
                    start = node.getStart();
                    end = node.getEnd();
                }
            }

These same accesses must not be seen as method references by MethodReferenceSearchRequestor or else the standard method rename refactoring will be applied:

    public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaElement enclosingElement) {
        ...
        if (result.declaration instanceof MethodNode && ((MethodNode) result.declaration).getName().equals(methodName)) {
            if (isDeclaration || node instanceof StaticMethodCallExpression) {
                start = ((AnnotatedNode) node).getNameStart();
                end = ((AnnotatedNode) node).getNameEnd() + 1;

            // check for non-synthetic match; SyntheticAccessorSearchRequestor matches "foo.bar" to "getBar()" w/o backing field
            } else if (node.getText().equals(methodName) || isNotSynthetic(node.getText(), result.declaringType)) {
                start = node.getStart();
                end = node.getEnd();
            }
        }

However, when renaming an accessor, the property-style access needs to be seen by SyntheticAccessorSearchRequestor so that SyntheticAccessorsRenameParticipant can add the extra renaming to the overall refactor.

The check for method reference in MethodReferenceSearchRequestor appears to be letting the property-style access through and so the normal rename path is applying the new accessor name instead of the new property name.

[eric-milles]

See #489 (comment) for more info on method references in call hierarchy. Not all the same participants are invoked and so search for references, rename refactoring, find occurrences and call hierarchy can be difficult to get working all at the same time.

[eric-milles]

Ready to test

[eric-milles]

Ready to test

[mauromol]

This works for me in 3.1.0.xx-201808191927-e47 in the example I was testing 👍