Skip to content

Commit

Permalink
Fix for #1131: infer direct access for super object expression
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Jun 30, 2020
1 parent 3b27a52 commit 0b1ffde
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 15 deletions.
90 changes: 90 additions & 0 deletions ...sts.builder/src/org/eclipse/jdt/core/groovy/tests/search/DeclarationInferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,96 @@ public void testGetterAndField15a() {
assertDeclaration(contents, offset, offset + 3, "Bar", "xxx", DeclarationKind.FIELD);
}

@Test // https://github.com/groovy/groovy-eclipse/issues/1131
public void testGetterAndField16() {
String contents =
//@formatter:off
"import groovy.transform.PackageScope\n" +
"class Foo {\n" +
" def a\n" +
" private b\n" +
" public c\n" +
" protected d\n" +
" @PackageScope e\n" +
" def getA() { 'A' }\n" +
" def getB() { 'B' }\n" +
" def getC() { 'C' }\n" +
" def getD() { 'D' }\n" +
" def getE() { 'E' }\n" +
"}\n" +
"class Bar extends Foo {\n" +
" void meth() {\n" +
" super.a\n" +
" super.b\n" +
" super.c\n" +
" super.d\n" +
" super.e\n" +
" }\n" +
"}\n";
//@formatter:on

int offset = contents.lastIndexOf("a");
assertDeclaration(contents, offset, offset + 1, "Foo", "getA", DeclarationKind.METHOD);

offset = contents.lastIndexOf("b");
assertDeclaration(contents, offset, offset + 1, "Foo", "getB", DeclarationKind.METHOD);

offset = contents.lastIndexOf("c");
assertDeclaration(contents, offset, offset + 1, "Foo", "c", DeclarationKind.FIELD);

offset = contents.lastIndexOf("d");
assertDeclaration(contents, offset, offset + 1, "Foo", "d", DeclarationKind.FIELD);

offset = contents.lastIndexOf("e");
assertDeclaration(contents, offset, offset + 1, "Foo", "e", DeclarationKind.FIELD);
}

@Test
public void testGetterAndField16a() {
String contents =
//@formatter:off
"import groovy.transform.PackageScope\n" +
"class Foo {\n" +
" def a\n" +
" private b\n" +
" public c\n" +
" protected d\n" +
" @PackageScope e\n" +
" def getA() { 'A' }\n" +
" def getB() { 'B' }\n" +
" def getC() { 'C' }\n" +
" def getD() { 'D' }\n" +
" def getE() { 'E' }\n" +
"}\n" +
"class Bar extends Foo {\n" +
" void meth() {\n" +
" { ->\n" +
" super.a\n" +
" super.b\n" +
" super.c\n" +
" super.d\n" +
" super.e\n" +
" }\n" +
" }\n" +
"}\n";
//@formatter:on

int offset = contents.lastIndexOf("a");
assertDeclaration(contents, offset, offset + 1, "Foo", "getA", DeclarationKind.METHOD);

offset = contents.lastIndexOf("b");
assertDeclaration(contents, offset, offset + 1, "Foo", "getB", DeclarationKind.METHOD);

offset = contents.lastIndexOf("c");
assertDeclaration(contents, offset, offset + 1, "Foo", "getC", DeclarationKind.METHOD);

offset = contents.lastIndexOf("d");
assertDeclaration(contents, offset, offset + 1, "Foo", "getD", DeclarationKind.METHOD);

offset = contents.lastIndexOf("e");
assertDeclaration(contents, offset, offset + 1, "Foo", "getE", DeclarationKind.METHOD);
}

@Test
public void testSetterAndField1() {
String contents =
Expand Down
31 changes: 16 additions & 15 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,8 @@ protected TypeLookupResult findType(final Expression node, final ClassNode decla
protected TypeLookupResult findTypeForNameWithKnownObjectExpression(final String name, final ClassNode type, final ClassNode declaringType, final VariableScope scope, final boolean isLhsExpression, final boolean isStaticObjectExpression) {

TypeConfidence confidence = TypeConfidence.EXACT;
boolean isFieldAccessDirect = (isThisObjectExpression(scope) ? scope.isFieldAccessDirect() : false);
ASTNode declaration = findDeclaration(name, declaringType, isLhsExpression, isStaticObjectExpression, isFieldAccessDirect, scope.getMethodCallArgumentTypes());
int fieldAccessPolicy = (!scope.isFieldAccessDirect() || !(isThisObjectExpression(scope) || isSuperObjectExpression(scope)) ? 0 : isThisObjectExpression(scope) ? 1 : 2);
ASTNode declaration = findDeclaration(name, declaringType, isLhsExpression, isStaticObjectExpression, fieldAccessPolicy, scope.getMethodCallArgumentTypes());
if (declaration instanceof MethodNode && scope.getEnclosingNode() instanceof PropertyExpression && !scope.isMethodCall() &&
(!AccessorSupport.isGetter((MethodNode) declaration) || name.equals(((MethodNode) declaration).getName()))) {
declaration = null; // property expression "foo.bar" does not resolve to "bar(...)" or "setBar(x)" w/o call args
Expand Down Expand Up @@ -428,7 +428,7 @@ protected TypeLookupResult findTypeForNameWithKnownObjectExpression(final String
if (declaration instanceof MethodNode) {
confidence = TypeConfidence.LOOSELY_INFERRED; // setter for write; field or property or accessor for read
} else if (findPropertyAccessorMethod(name, declaringType, false, isStaticObjectExpression, null).filter(getter ->
!isSynthetic(getter) && !(isFieldAccessDirect && declaringType.equals(getter.getDeclaringClass()))
!isSynthetic(getter) && !(fieldAccessPolicy != 0 && declaringType.equals(getter.getDeclaringClass()))
).isPresent()) {
confidence = TypeConfidence.LOOSELY_INFERRED; // field or property for write; accessor for read
}
Expand Down Expand Up @@ -555,7 +555,7 @@ protected ASTNode findDeclarationForDynamicVariable(final VariableExpression var

if (resolveStrategy == Closure.DELEGATE_FIRST || resolveStrategy == Closure.DELEGATE_ONLY) {
// TODO: If strategy is DELEGATE_ONLY and delegate is enclosing closure, do outer search.
candidate = findDeclaration(var.getName(), scope.getDelegate(), isAssignTarget, false, false, callArgs);
candidate = findDeclaration(var.getName(), scope.getDelegate(), isAssignTarget, false, 0, callArgs);
}
if (candidate == null && resolveStrategy < Closure.DELEGATE_ONLY) {
VariableScope outer = owner.getNodeMetaData("outer.scope");
Expand All @@ -567,10 +567,10 @@ protected ASTNode findDeclarationForDynamicVariable(final VariableExpression var
outer.setMethodCallArgumentTypes(null);
}
} else {
candidate = findDeclaration(var.getName(), owner, isAssignTarget, scope.isOwnerStatic(), scope.isFieldAccessDirect(), callArgs);
candidate = findDeclaration(var.getName(), owner, isAssignTarget, scope.isOwnerStatic(), scope.isFieldAccessDirect() ? 1 : 0, callArgs);
}
if (candidate == null && resolveStrategy < Closure.DELEGATE_FIRST && scope.getEnclosingClosure() != null) {
candidate = findDeclaration(var.getName(), scope.getDelegate(), isAssignTarget, false, false, callArgs);
candidate = findDeclaration(var.getName(), scope.getDelegate(), isAssignTarget, false, 0, callArgs);
}
if (candidate == null && scope.getEnclosingClosure() == null && scope.getEnclosingMethodDeclaration() != null) {
for (Parameter parameter : scope.getEnclosingMethodDeclaration().getParameters()) {
Expand All @@ -582,7 +582,7 @@ protected ASTNode findDeclarationForDynamicVariable(final VariableExpression var
}
}
if (candidate == null && resolveStrategy <= Closure.TO_SELF && (resolveStrategy > 0 || scope.getEnclosingClosure() != null)) {
candidate = findDeclaration(var.getName(), VariableScope.CLOSURE_CLASS_NODE, isAssignTarget, false, false, callArgs);
candidate = findDeclaration(var.getName(), VariableScope.CLOSURE_CLASS_NODE, isAssignTarget, false, 0, callArgs);
}

return candidate;
Expand All @@ -596,18 +596,19 @@ protected ASTNode findDeclarationForDynamicVariable(final VariableExpression var
* @param declaringType the type in which the named member's declaration resides
* @param isLhsExpression {@code true} if named member is being assigned a value
* @param isStaticExpression {@code true} if member is being accessed statically
* @param directFieldAccess {@code false} if accessor methods may take precedence
* @param methodCallArgumentTypes types of arguments to the associated method call
* (or {@code null} if not a method call)
* @param directFieldAccess {@code 1}: access fields from {@code declaringType} directly;
* {@code 2}: access non-private fields from {@code declaringType} directly;
* {@code 0}: prefer accessor methods over fields/properties of {@code declaringType}
* @param methodCallArgumentTypes types of arguments to the associated method call (or {@code null} if not a method call)
*/
protected ASTNode findDeclaration(final String name, final ClassNode declaringType, final boolean isLhsExpression, final boolean isStaticExpression, final boolean directFieldAccess, final List<ClassNode> methodCallArgumentTypes) {
protected ASTNode findDeclaration(final String name, final ClassNode declaringType, final boolean isLhsExpression, final boolean isStaticExpression, final int directFieldAccess, final List<ClassNode> methodCallArgumentTypes) {
if (declaringType.isArray()) {
// only length exists on arrays
if ("length".equals(name)) {
return createLengthField(declaringType);
}
// otherwise search on object
return findDeclaration(name, VariableScope.OBJECT_CLASS_NODE, isLhsExpression, isStaticExpression, directFieldAccess, methodCallArgumentTypes);
return findDeclaration(name, VariableScope.OBJECT_CLASS_NODE, isLhsExpression, isStaticExpression, 0, methodCallArgumentTypes);
}

if (!isLhsExpression && methodCallArgumentTypes != null) {
Expand All @@ -620,7 +621,7 @@ protected ASTNode findDeclaration(final String name, final ClassNode declaringTy

// look for canonical accessor method
Optional<MethodNode> accessor = findPropertyAccessorMethod(name, declaringType, isLhsExpression, isStaticExpression, methodCallArgumentTypes).filter(it -> !isSynthetic(it));
if (accessor.isPresent() && !directFieldAccess) {
if (accessor.isPresent() && directFieldAccess == 0) {
return accessor.get();
}

Expand All @@ -635,7 +636,7 @@ protected ASTNode findDeclaration(final String name, final ClassNode declaringTy
.flatMap(list -> list.stream().filter(prop -> prop.getName().equals(name)).findFirst()).orElse(null);
}
if (isCompatible(property, isStaticExpression) && (!accessor.isPresent() ||
directFieldAccess && declaringType.equals(property.getDeclaringClass()))) {
directFieldAccess == 1 && declaringType.equals(property.getDeclaringClass()))) {
return property;
}
if (property != null) break;
Expand All @@ -644,7 +645,7 @@ protected ASTNode findDeclaration(final String name, final ClassNode declaringTy
// look for field
FieldNode field = declaringType.getField(name);
if (isCompatible(field, isStaticExpression) && (!accessor.isPresent() ||
directFieldAccess && declaringType.equals(field.getDeclaringClass()))) {
directFieldAccess >= 1 && declaringType.equals(field.getDeclaringClass()) && (directFieldAccess == 1 || !field.isPrivate()))) {
return field;
}

Expand Down

0 comments on commit 0b1ffde

Please sign in to comment.