Prep for #678: separation of synthetic accessors from pseudo properties

- renaming only field will not rename pseudo-property accesses (aka
calls to provided getter/setter/isser using the property style)

base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/BinarySearchTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2017 the original author or authors.
+ * Copyright 2009-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -174,7 +174,7 @@ public void testFieldDecl1() throws Exception {
         String toFind = "age_1";
         IField field = type.getField(toFind);
         MockSearchRequestor requestor = performSearch(field);
-        assertMatches(toFind, requestor, 4, 2);
+        assertMatches(toFind, requestor, 2, 2); // all was 4, but in binary synthetic accessor is indistinguishable from source method
     }
 
     @Test
@@ -183,7 +183,7 @@ public void testFieldDecl2() throws Exception {
         String toFind = "name_1";
         IField field = type.getField(toFind);
         MockSearchRequestor requestor = performSearch(field);
-        assertMatches(toFind, requestor, 4, 2);
+        assertMatches(toFind, requestor, 2, 2); // all was 4, but in binary synthetic accessor is indistinguishable from source method
     }
 
     @Test
@@ -201,7 +201,7 @@ public void testFieldRefInInitializer() throws Exception {
         String toFind = "fieldInInitializer";
         IField method = type.getField(toFind);
         MockSearchRequestor requestor = performSearch(method);
-        assertMatches(toFind, requestor, 2, 1);
+        assertMatches(toFind, requestor, 1, 1); // all was 2, but in binary synthetic accessor is indistinguishable from source method
     }
 
     @Test

base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyClassScope.java

@@ -35,6 +35,7 @@
 import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
 import org.eclipse.jdt.internal.compiler.lookup.AnnotationBinding;
 import org.eclipse.jdt.internal.compiler.lookup.BinaryTypeBinding;
+import org.eclipse.jdt.internal.compiler.lookup.Binding;
 import org.eclipse.jdt.internal.compiler.lookup.ClassScope;
 import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
 import org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers;
@@ -76,7 +77,7 @@ protected MethodBinding[] augmentMethodBindings(MethodBinding[] methodBindings)
             return methodBindings;
         }
 
-        ReferenceBinding[] superInterfaces = (typeBinding.superInterfaces != null ? typeBinding.superInterfaces : new ReferenceBinding[0]);
+        ReferenceBinding[] superInterfaces = (typeBinding.superInterfaces != null ? typeBinding.superInterfaces : Binding.NO_SUPERINTERFACES);
 
         boolean implementsGroovyLangObject = false;
         for (ReferenceBinding face : superInterfaces) {
@@ -111,7 +112,7 @@ protected MethodBinding[] augmentMethodBindings(MethodBinding[] methodBindings)
                 .ifPresent(groovyMethods::add);
             createMethod("setProperty", false, new TypeBinding[] {bindingJLS, bindingJLO}, TypeBinding.VOID, methodBindings)
                 .ifPresent(groovyMethods::add);
-            createMethod("getMetaClass", false, new TypeBinding[] {}, bindingGLM, methodBindings)
+            createMethod("getMetaClass", false, Binding.NO_TYPES, bindingGLM, methodBindings)
                 .ifPresent(groovyMethods::add);
             createMethod("setMetaClass", false, new TypeBinding[] {bindingGLM}, TypeBinding.VOID, methodBindings)
                 .ifPresent(groovyMethods::add);

base/org.eclipse.jdt.groovy.core/src/org/codehaus/jdt/groovy/internal/compiler/ast/JDTClassNode.java

@@ -62,6 +62,7 @@
 import org.eclipse.jdt.internal.compiler.lookup.ClassScope;
 import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
 import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
+import org.eclipse.jdt.internal.compiler.lookup.LazilyResolvedMethodBinding;
 import org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment;
 import org.eclipse.jdt.internal.compiler.lookup.MemberTypeBinding;
 import org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
@@ -346,6 +347,7 @@ private MethodNode methodBindingToMethodNode(MethodBinding methodBinding) {
 
             MethodNode methodNode = new JDTMethodNode(methodBinding, resolver, String.valueOf(methodBinding.selector), modifiers, returnType, parameters, exceptions, null);
             methodNode.setGenericsTypes(new JDTClassNodeBuilder(resolver).configureTypeVariables(methodBinding.typeVariables()));
+            methodNode.setSynthetic(methodBinding instanceof LazilyResolvedMethodBinding); // see GroovyClassScope
             return methodNode;
         } catch (AbortCompilation e) {
             throw e;

base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/FieldReferenceSearchRequestor.java

@@ -21,6 +21,7 @@
 import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.FieldExpression;
@@ -48,7 +49,7 @@ public class FieldReferenceSearchRequestor implements ITypeRequestor {
 
     protected final String fieldName, declaringQualifiedName;
     protected final Set<Position> acceptedPositions = new HashSet<>();
-    protected final boolean readAccess, writeAccess, findReferences, findDeclarations;
+    protected final boolean readAccess, writeAccess, findReferences, findDeclarations, findPseudoProperties;
 
     public FieldReferenceSearchRequestor(FieldPattern pattern, SearchRequestor requestor, SearchParticipant participant) {
         this.requestor = requestor;
@@ -66,6 +67,7 @@ public FieldReferenceSearchRequestor(FieldPattern pattern, SearchRequestor reque
         writeAccess = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "writeAccess", pattern);
         findReferences = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "findReferences", pattern);
         findDeclarations = (Boolean) ReflectionUtils.getPrivateField(VariablePattern.class, "findDeclarations", pattern);
+        findPseudoProperties = requestor.getClass().getName().contains("SyntheticAccessorSearch"); // accessor as property
     }
 
     @Override
@@ -76,38 +78,28 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
 
         boolean doCheck = false;
         boolean isAssignment = false;
-        boolean isDeclaration = false;
+        boolean isDeclaration = (node instanceof FieldNode);
         int start = 0;
         int end = 0;
 
-        if (node instanceof ConstantExpression) {
-            // check for "foo.bar" where "bar" refers to "getBar()" or "setBar(...)" with backing field or property
-            if (fieldName.equals(((ConstantExpression) node).getText()) && (result.confidence == TypeConfidence.UNKNOWN ||
-                    result.declaringType.getField(fieldName) != null || result.declaringType.getProperty(fieldName) != null)) {
+        if (isDeclaration) {
+            FieldNode fieldNode = (FieldNode) node;
+            if (fieldName.equals(fieldNode.getName())) {
                 doCheck = true;
-                isAssignment = EqualityVisitor.checkForAssignment(node, result.enclosingAssignment);
-                start = node.getStart();
-                end = node.getEnd();
+                // assume all FieldNodes are assignments -- not true if there is no initializer, but we
+                // can't know this at this point since initializer has already been moved to the <init>
+                isAssignment = true; //fieldNode.hasInitialExpression();
+                start = fieldNode.getNameStart();
+                end = fieldNode.getNameEnd() + 1;
             }
         } else if (node instanceof FieldExpression) {
             if (fieldName.equals(((FieldExpression) node).getFieldName())) {
                 doCheck = true;
                 isAssignment = EqualityVisitor.checkForAssignment(node, result.enclosingAssignment);
-                // fully-qualified field expressions in static contexts will have an sloc of the entire qualified name
+                // fully-qualified field expressions in static contexts will have sloc of the entire qualified name
                 start = node.getEnd() - fieldName.length();
                 end = node.getEnd();
             }
-        } else if (node instanceof FieldNode) {
-            FieldNode fnode = (FieldNode) node;
-            if (fieldName.equals(fnode.getName())) {
-                doCheck = true;
-                // assume all FieldNodes are assignments -- not true if there is no initializer, but we
-                // can't know this at this point since initializer has already been moved to the <init>
-                isAssignment = true;
-                isDeclaration = true;
-                start = fnode.getNameStart();
-                end = fnode.getNameEnd() + 1; // arrrgh...why +1?
-            }
         } else if (node instanceof VariableExpression) {
             if (fieldName.equals(((VariableExpression) node).getName()) &&
                     (result.declaration instanceof FieldNode || result.declaration instanceof PropertyNode)) {
@@ -116,6 +108,22 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
                 start = node.getStart();
                 end = node.getEnd();
             }
+        } else if (node instanceof ConstantExpression) {
+            if (fieldName.equals(((ConstantExpression) node).getText())) {
+                if (result.declaration instanceof FieldNode || result.declaration instanceof PropertyNode || result.confidence == TypeConfidence.UNKNOWN) {
+                    doCheck = true;
+                    isAssignment = EqualityVisitor.checkForAssignment(node, result.enclosingAssignment);
+                    start = node.getStart();
+                    end = node.getEnd();
+
+                // check for "foo.bar" where "bar" refers to generated/synthetic "getBar()", "isBar()" or "setBar(...)"
+                } else if (result.declaration instanceof MethodNode && (((MethodNode) result.declaration).isSynthetic() || findPseudoProperties)) {
+                    doCheck = true;
+                    isAssignment = ((MethodNode) result.declaration).getName().startsWith("set");
+                    start = node.getStart();
+                    end = node.getEnd();
+                }
+            }
         }
 
         if (doCheck && end > 0) {
@@ -156,11 +164,8 @@ private boolean qualifiedNameMatches(ClassNode declaringType) {
         } else if (declaringQualifiedName.isEmpty()) {
             // no type specified, accept all
             return true;
-        } else if (declaringType.getName().equals(declaringQualifiedName)) {
-            return true;
-        } else {
-            return false;
         }
+        return declaringType.getName().equals(declaringQualifiedName);
     }
 
     private int getAccuracy(TypeConfidence confidence, boolean isCompleteMatch) {

base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java

@@ -197,7 +197,6 @@ protected static boolean isPrimitiveType(char[] name) {
     @Override
     public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaElement enclosingElement) {
         if (result.declaringType == null) {
-            // GRECLIPSE-1180: probably a literal of some kind
             return VisitStatus.CONTINUE;
         }
 
@@ -210,7 +209,7 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
                 start = ((AnnotatedNode) node).getNameStart();
                 end = ((AnnotatedNode) node).getNameEnd() + 1;
 
-            // check for non-synthetic match; SyntheticAccessorSearchRequestor matches "foo.bar" to "getBar()" w/o backing field
+            // check for non-synthetic match; SyntheticAccessorSearchRequestor matches "foo.bar" to "getBar()", etc.
             } else if (node.getText().equals(methodName) || isNotSynthetic(node.getText(), result.declaringType)) {
                 start = node.getStart();
                 end = node.getEnd();
@@ -221,8 +220,8 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
             // don't want to double accept nodes; this could happen with field and object initializers can get pushed into multiple constructors
             Position position = new Position(start, end - start);
             if (!acceptedPositions.contains(position)) {
-                if (nameAndArgsMatch(GroovyUtils.getBaseType(result.declaringType), isDeclaration ?
-                        GroovyUtils.getParameterTypes(((MethodNode) node).getParameters()) : result.scope.getMethodCallArgumentTypes())) {
+                if (nameAndArgsMatch(GroovyUtils.getBaseType(result.declaringType), isDeclaration
+                        ? GroovyUtils.getParameterTypes(((MethodNode) node).getParameters()) : result.scope.getMethodCallArgumentTypes())) {
                     if (enclosingElement.getOpenable() instanceof GroovyClassFileWorkingCopy) {
                         enclosingElement = ((GroovyClassFileWorkingCopy) enclosingElement.getOpenable()).convertToBinary(enclosingElement);
                     }

base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java

@@ -60,13 +60,11 @@
 import org.codehaus.groovy.ast.expr.VariableExpression;
 import org.codehaus.groovy.classgen.asm.OptimizingStatementWriter.StatementMeta;
 import org.codehaus.groovy.transform.trait.Traits;
-import org.codehaus.jdt.groovy.internal.compiler.ast.JDTMethodNode;
 import org.codehaus.jdt.groovy.model.GroovyCompilationUnit;
 import org.eclipse.jdt.groovy.core.util.GroovyUtils;
 import org.eclipse.jdt.groovy.core.util.ReflectionUtils;
 import org.eclipse.jdt.groovy.search.TypeLookupResult.TypeConfidence;
 import org.eclipse.jdt.groovy.search.VariableScope.VariableInfo;
-import org.eclipse.jdt.internal.compiler.lookup.LazilyResolvedMethodBinding;
 
 /**
  * Determines types using AST inspection.
@@ -862,8 +860,7 @@ protected static boolean isCompatible(AnnotatedNode declaration, boolean isStati
      */
     protected static boolean isSynthetic(MethodNode method) {
         // TODO: What about 'method.getDeclaringClass().equals(ClassHelper.GROOVY_OBJECT_TYPE)'?
-        return method.isSynthetic() || method.getDeclaringClass().equals(VariableScope.CLOSURE_CLASS_NODE) ||
-            (method instanceof JDTMethodNode && ((JDTMethodNode) method).getJdtBinding() instanceof LazilyResolvedMethodBinding);
+        return method.isSynthetic() || method.getDeclaringClass().equals(VariableScope.CLOSURE_CLASS_NODE);
     }
 
     /**

ide-test/org.codehaus.groovy.eclipse.refactoring.test/src/org/codehaus/groovy/eclipse/refactoring/test/rename/RenameRefactoringTestSuite.groovy

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2017 the original author or authors.
+ * Copyright 2009-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -50,35 +50,20 @@ abstract class RenameRefactoringTestSuite extends GroovyEclipseTestSuite {
 
     protected void assertContents(ICompilationUnit[] existingUnits, List<String> expectedContents) {
         def sb = new StringBuilder()
-        existingUnits.eachWithIndex { unit, i ->
+        existingUnits.eachWithIndex { existingUnit, i ->
             if (expectedContents[i] != null) {
-                String actualContents = String.valueOf(unit.contents)
-                if (!actualContents.equals(expectedContents[i])) {
-                    sb.append('\n-----EXPECTING-----\n')
-                    sb.append(expectedContents[i])
-                    sb.append('\n--------WAS--------\n')
-                    sb.append(actualContents)
-                }
-            } else if (existingUnits[i].exists()) {
-                // unit should have been deleted
-                sb.append('\nUnit ' + unit.elementName + ' should have been deleted.\n')
-                sb.append('Instead had the following contents:\n')
-                sb.append(unit.contents)
+                assertContents(existingUnit, expectedContents[i])
+            } else if (existingUnits[i].exists()) { // unit should have been deleted
+                sb.append('\nUnit ' + existingUnit.elementName + ' should have been deleted.\n')
+                sb.append('Instead had the following contents:\n').append(existingUnit.contents)
+                Assert.fail('Refactoring produced unexpected results:' + sb)
             }
         }
-        if (sb.length() > 0) Assert.fail('Refactoring produced unexpected results:' + sb)
     }
 
-    protected void assertContents(ICompilationUnit existingUnits, String expectedContents) {
-        def sb = new StringBuilder()
-        String actualContents = String.valueOf(existingUnits.contents)
-        if (!actualContents.equals(expectedContents)) {
-            sb.append('\n-----EXPECTING-----\n')
-            sb.append(expectedContents)
-            sb.append('\n--------WAS--------\n')
-            sb.append(actualContents)
-        }
-        if (sb.length() > 0) Assert.fail('Refactoring produced unexpected results:' + sb)
+    protected void assertContents(ICompilationUnit existingUnit, String expectedContents) {
+        String actualContents = String.valueOf(existingUnit.contents)
+        Assert.assertEquals('Refactoring produced unexpected results:', expectedContents, actualContents)
     }
 
     protected RefactoringStatus performRefactoring(Refactoring refactor, boolean providesUndo) {
@@ -89,7 +74,7 @@ abstract class RenameRefactoringTestSuite extends GroovyEclipseTestSuite {
         change.setUndoManager(undoer, refactor.name)
 undoer.flush()
         executePerformOperation(change, ResourcesPlugin.getWorkspace())
-        RefactoringStatus status = create.getConditionCheckingStatus()
+        RefactoringStatus status = create.conditionCheckingStatus
         assert change.changeExecuted() || !change.changeExecutionFailed() : 'Change was not executed'
         if (providesUndo) {
             assert change.undoChange != null : 'Undo does not exist'