From dce1711a8830ef9cd0b5535e74c5e21da490108c Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Sat, 4 Nov 2017 11:34:25 -0500 Subject: [PATCH] Fix for issue #366: check static compatibility when inferring expression --- .../groovy/tests/search/InferencingTests.java | 16 ++++++ .../jdt/groovy/search/SimpleTypeLookup.java | 30 ++++++---- .../TypeInferencingVisitorWithRequestor.java | 55 +++++++++++++++---- .../jdt/groovy/search/VariableScope.java | 16 +++++- .../resources/RenameField/test9/in/A.groovy | 21 +++---- .../resources/RenameField/test9/out/A.groovy | 21 +++---- 6 files changed, 112 insertions(+), 47 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java index 739748b349..d4ef5e5e5e 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java @@ -472,6 +472,22 @@ public void testClosure10() { assertDeclaration(contents, start, end, "java.lang.Object", "equals", DeclarationKind.METHOD); } + @Test + public void testClosure11() { + String contents = + "class A {\n" + + " String b\n" + + " static void main(args) {\n" + + " def c = {\n" + + " b\n" + // unknown because enclosing declaration is static + " }\n" + + " }\n" + + "}"; + + int offset = contents.lastIndexOf('b'); + assertUnknownConfidence(contents, offset, offset + 1, "A", false); + } + @Test public void testSpread1() { String contents = "def z = [1,2]*.value"; diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java index b872fbacba..126b7a8f6c 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java @@ -476,7 +476,7 @@ protected ASTNode findDeclarationForDynamicVariable(VariableExpression var, Clas } } if (candidate == null && resolveStrategy < Closure.DELEGATE_ONLY) { - candidate = findDeclaration(var.getName(), owner, isLhsExpr, isOwnerStatic(scope), callArgs); + candidate = findDeclaration(var.getName(), owner, isLhsExpr, scope.isOwnerStatic(), callArgs); if (candidate == null && resolveStrategy < Closure.OWNER_FIRST) { candidate = findDeclaration(var.getName(), scope.getDelegate(), isLhsExpr, false, callArgs); } @@ -524,14 +524,14 @@ protected ASTNode findDeclaration(String name, ClassNode declaringType, boolean // look for property for (ClassNode type : typeHierarchy) { PropertyNode property = type.getProperty(name); - if (property != null) { + if (isCompatible(property, isStaticExpression)) { return property; } } // look for field FieldNode field = declaringType.getField(name); - if (field != null) { + if (isCompatible(field, isStaticExpression)) { return field; } @@ -550,7 +550,7 @@ protected ASTNode findDeclaration(String name, ClassNode declaringType, boolean } // look for static or synthetic accessor - if (accessor != null) { + if (isCompatible(accessor, isStaticExpression)) { return accessor; } @@ -736,15 +736,21 @@ protected static ClassNode getDeclaringTypeFromDeclaration(ASTNode declaration, } } - protected static boolean isOwnerStatic(VariableScope scope) { - AnnotatedNode enclosing; - boolean isOwnerStatic = false; - if ((enclosing = scope.getEnclosingMethodDeclaration()) != null) { - isOwnerStatic = ((MethodNode) enclosing).isStatic(); - } else if ((enclosing = scope.getEnclosingFieldDeclaration()) != null) { - isOwnerStatic = ( (FieldNode) enclosing).isStatic(); + protected static boolean isCompatible(AnnotatedNode declaration, boolean isStaticExpression) { + if (declaration != null) { + boolean isStatic = false; + if (declaration instanceof FieldNode) { + isStatic = ((FieldNode) declaration).isStatic(); + } else if (declaration instanceof MethodNode) { + isStatic = ((MethodNode) declaration).isStatic(); + } else if (declaration instanceof PropertyNode) { + isStatic = ((PropertyNode) declaration).isStatic(); + } + if (!isStaticExpression || isStatic || declaration.getDeclaringClass().equals(VariableScope.CLASS_CLASS_NODE)) { + return true; + } } - return isOwnerStatic; + return false; } /** diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java index 9728560b75..c4532c4958 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/TypeInferencingVisitorWithRequestor.java @@ -367,17 +367,17 @@ public void visitCompilationUnit(ITypeRequestor requestor) { } public void visitJDT(IType type, ITypeRequestor requestor) { - IJavaElement oldEnclosing = enclosingElement; - ASTNode oldEnclosingNode = enclosingDeclarationNode; - enclosingElement = type; ClassNode node = findClassNode(createName(type)); if (node == null) { // probably some sort of AST transformation is making this node invisible return; } + scopes.add(new VariableScope(scopes.getLast(), node, false)); + ASTNode enclosingDeclaration0 = enclosingDeclarationNode; + IJavaElement enclosingElement0 = enclosingElement; + enclosingDeclarationNode = node; + enclosingElement = type; try { - scopes.add(new VariableScope(scopes.getLast(), node, false)); - enclosingDeclarationNode = node; visitClassInternal(node); try { @@ -400,7 +400,7 @@ public void visitJDT(IType type, ITypeRequestor requestor) { // visit fields created by @Field for (FieldNode field : node.getFields()) { if (field.getEnd() > 0) { - visitField(field); + visitFieldInternal(field); } } } else { @@ -409,14 +409,14 @@ public void visitJDT(IType type, ITypeRequestor requestor) { List traitFields = (List) node.getNodeMetaData("trait.fields"); if (traitFields != null) { for (FieldNode field : traitFields) { - visitField(field); + visitFieldInternal(field); } } @SuppressWarnings("unchecked") List traitMethods = (List) node.getNodeMetaData("trait.methods"); if (traitMethods != null) { for (MethodNode method : traitMethods) { - visitConstructorOrMethod(method, false); + visitConstructorOrMethodInternal(method, false); } } } @@ -426,7 +426,7 @@ public void visitJDT(IType type, ITypeRequestor requestor) { if (!type.getMethod(type.getElementName(), NO_PARAMS).exists()) { ConstructorNode defConstructor = findDefaultConstructor(node); if (defConstructor != null) { - visitConstructorOrMethod(defConstructor, true); + visitConstructorOrMethodInternal(defConstructor, true); } } } @@ -434,7 +434,14 @@ public void visitJDT(IType type, ITypeRequestor requestor) { // visit relocated @Memoized method bodies for (MethodNode method : node.getMethods()) { if (method.getName().startsWith("memoizedMethodPriv$")) { - visitClassCodeContainer(method.getCode()); + scopes.add(new VariableScope(scopes.getLast(), method, method.isStatic())); + enclosingDeclarationNode = method; + try { + visitClassCodeContainer(method.getCode()); + } finally { + scopes.removeLast(); + enclosingDeclarationNode = node; + } } } @@ -446,9 +453,9 @@ public void visitJDT(IType type, ITypeRequestor requestor) { throw vc; } } finally { - enclosingElement = oldEnclosing; - enclosingDeclarationNode = oldEnclosingNode; scopes.removeLast(); + enclosingElement = enclosingElement0; + enclosingDeclarationNode = enclosingDeclaration0; } } @@ -747,6 +754,30 @@ private void visitClassReference(ClassNode node) { } } + private void visitFieldInternal(FieldNode field) { + scopes.add(new VariableScope(scopes.getLast(), field, field.isStatic())); + ASTNode enclosingDeclarationNode0 = enclosingDeclarationNode; + enclosingDeclarationNode = field; + try { + visitField(field); + } finally { + scopes.removeLast(); + enclosingDeclarationNode = enclosingDeclarationNode0; + } + } + + private void visitConstructorOrMethodInternal(MethodNode method, boolean isCtor) { + scopes.add(new VariableScope(scopes.getLast(), method, method.isStatic())); + ASTNode enclosingDeclarationNode0 = enclosingDeclarationNode; + enclosingDeclarationNode = method; + try { + visitConstructorOrMethod(method, isCtor); + } finally { + scopes.removeLast(); + enclosingDeclarationNode = enclosingDeclarationNode0; + } + } + // @Override diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/VariableScope.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/VariableScope.java index bc802c9986..a3c91dbbe6 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/VariableScope.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/VariableScope.java @@ -515,6 +515,11 @@ public ClassNode getDelegate() { return delegate != null ? delegate.type : null; } + public ClassNode getDelegateOrThis() { + VariableInfo info = getDelegateOrThisInfo(); + return info != null ? info.type : null; + } + /** * @return the current delegate type if exists, or this type if exists, or * Object. Returns null if in top level scope (i.e. in import statement). @@ -530,9 +535,14 @@ public VariableInfo getDelegateOrThisInfo() { return info; } - public ClassNode getDelegateOrThis() { - VariableInfo info = getDelegateOrThisInfo(); - return info != null ? info.type : null; + public boolean isOwnerStatic() { + FieldNode field; MethodNode method; + if (isStatic() || + ((field = getEnclosingFieldDeclaration()) != null && field.isStatic()) || + ((method = getEnclosingMethodDeclaration()) != null && method.isStatic())) { + return true; + } + return false; } public void addVariable(String name, ClassNode type, ClassNode declaringType) { diff --git a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/in/A.groovy b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/in/A.groovy index 179f24074f..ae06ae3ab8 100755 --- a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/in/A.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/in/A.groovy @@ -1,18 +1,19 @@ package p + +class A { + public static def f + static def s() { + f = A.f + } +} + class B extends A { - def f - void s() { + public def f + void m() { f = A.f f = super.f } - static s2() { + static def s2() { f = A.f } } - -class A { - static f = 7 - static s() { - f = A.f - } -} \ No newline at end of file diff --git a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/out/A.groovy b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/out/A.groovy index 8f534b55b5..27cfea4dcf 100755 --- a/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/out/A.groovy +++ b/ide-test/org.codehaus.groovy.eclipse.refactoring.test/resources/RenameField/test9/out/A.groovy @@ -1,18 +1,19 @@ package p + +class A { + public static def g + static def s() { + g = A.g + } +} + class B extends A { - def f - void s() { + public def f + void m() { f = A.g f = super.g } - static s2() { + static def s2() { f = A.g } } - -class A { - static g = 7 - static s() { - g = A.g - } -} \ No newline at end of file