Skip to content

Commit

Permalink
Fix for non-static inner class constructor reference searching
Browse files Browse the repository at this point in the history 
#768
  • Loading branch information
@eric-milles
eric-milles committed Nov 26, 2018
1 parent 6a76cae commit 232a365
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 31 deletions.
24 changes: 22 additions & 2 deletions ...builder/src/org/eclipse/jdt/core/groovy/tests/search/ConstructorReferenceSearchTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public final class ConstructorReferenceSearchTests extends SearchTestSuite {
public void testConstructorReferences1() throws Exception {
GroovyCompilationUnit foo = createUnit("p", "Foo", "package p\n" +
"class Foo {\n" +
" Foo() {\n" +
" Foo() {\n" + // search for this
" new Foo()\n" + // yes
" }\n" +
" Foo(a) {\n" +
Expand Down Expand Up @@ -78,7 +78,7 @@ public void testConstructorReferences2() throws Exception {
"class Foo {\n" +
" static class Bar {\n" +
" static class Baz {\n" +
" Baz() {\n" +
" Baz() {\n" + // search for this
" this(null)\n" + // no
" }\n" +
" Baz(def arg) {\n" +
Expand Down Expand Up @@ -238,6 +238,26 @@ public void testConstructorReferences9() throws Exception {
assertEquals(0, ctorRefs);
}

@Test // non-static inner class constructors may have implicit parameter
public void testConstructorReferences10() throws Exception {
GroovyCompilationUnit foo = createUnit("p", "Foo", "package p\n" +
"class Foo {\n" +
" class FooFoo {\n" +
" FooFoo(int i) {}\n" + // search for this
" FooFoo(String s) {}\n" +
" }\n" +
"}");
createUnit("", "Bar", "import p.Foo\n" +
"def foo = new Foo()\n" +
"new Foo.FooFoo(foo, 0)\n" + // yes
"new Foo.FooFoo(foo, '')\n"); // no

long ctorRefs = searchForReferences(foo.getType("Foo").getType("FooFoo").getMethods()[0]).stream()
.filter(match -> ((IMethod) match.getElement()).getResource().getName().equals("Bar.groovy"))
.count();
assertEquals(1, ctorRefs);
}

//--------------------------------------------------------------------------

List<SearchMatch> searchForReferences(IMethod method) throws CoreException {
Expand Down
44 changes: 21 additions & 23 deletions ...dt.groovy.core/src/org/eclipse/jdt/groovy/search/ConstructorReferenceSearchRequestor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.eclipse.jdt.groovy.search;

import java.util.Optional;
import java.util.function.Function;

import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.ClassHelper;
Expand Down Expand Up @@ -93,17 +94,8 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
ConstructorNode decl = (ConstructorNode) node;
String typeName = result.declaringType.getName().replace('$', '.');
if (typeName.equals(declaringQualifiedName) && hasMatchingParameters(decl.getParameters())) {

if (enclosingElement.getOpenable() instanceof GroovyClassFileWorkingCopy) {
enclosingElement = ((GroovyClassFileWorkingCopy) enclosingElement.getOpenable()).convertToBinary(enclosingElement);
}
try {
requestor.acceptSearchMatch(new MethodDeclarationMatch(
enclosingElement, SearchMatch.A_ACCURATE, decl.getNameStart(), decl.getNameEnd() + 1 - decl.getNameStart(),
participant, enclosingElement.getResource()));
} catch (CoreException e) {
Util.log(e, "Error reporting search match inside of " + enclosingElement + " in resource " + enclosingElement.getResource());
}
reportSearchMatch(enclosingElement, element -> new MethodDeclarationMatch(element, SearchMatch.A_ACCURATE,
decl.getNameStart(), decl.getNameEnd() + 1 - decl.getNameStart(), participant, element.getResource()));
}
}

Expand All @@ -112,24 +104,30 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
String typeName = result.declaringType.getName().replace('$', '.');
Parameter[] parameters = ((ConstructorNode) result.declaration).getParameters();
if (typeName.equals(declaringQualifiedName) && hasMatchingParameters(parameters)) {
boolean isConstructor = true, isSynthetic = false, isSuperInvocation = call.isSuperCall(), isWithinComment = false;

if (enclosingElement.getOpenable() instanceof GroovyClassFileWorkingCopy) {
enclosingElement = ((GroovyClassFileWorkingCopy) enclosingElement.getOpenable()).convertToBinary(enclosingElement);
}
try {
requestor.acceptSearchMatch(new MethodReferenceMatch(
enclosingElement, SearchMatch.A_ACCURATE, call.getNameStart(), call.getNameEnd() + 1 - call.getNameStart(),
isConstructor, isSynthetic, isSuperInvocation, isWithinComment, participant, enclosingElement.getResource()));
} catch (CoreException e) {
Util.log(e, "Error reporting search match inside of " + enclosingElement + " in resource " + enclosingElement.getResource());
}
reportSearchMatch(enclosingElement, element -> {
boolean isConstructor = true, isSynthetic = false, isSuperInvocation = call.isSuperCall(), isWithinComment = false;
return new MethodReferenceMatch(
element, SearchMatch.A_ACCURATE, call.getNameStart(), call.getNameEnd() + 1 - call.getNameStart(),
isConstructor, isSynthetic, isSuperInvocation, isWithinComment, participant, element.getResource());
});
}
}
}
return VisitStatus.CONTINUE;
}

protected void reportSearchMatch(IJavaElement element, Function<IJavaElement, SearchMatch> producer) {
if (element.getOpenable() instanceof GroovyClassFileWorkingCopy) {
element = ((GroovyClassFileWorkingCopy) element.getOpenable()).convertToBinary(element);
}
SearchMatch match = producer.apply(element);
try {
requestor.acceptSearchMatch(match);
} catch (CoreException e) {
Util.log(e, "Error reporting search match inside of " + element + " in resource " + element.getResource());
}
}

protected boolean hasMatchingParameters(Parameter[] declarationParameters) {
if (declarationParameters.length == parameterQualifiedNames.length) {
for (int i = 0; i < parameterQualifiedNames.length; i += 1) {
Expand Down
21 changes: 15 additions & 6 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 @@ -258,18 +258,27 @@ protected TypeLookupResult findType(Expression node, ClassNode declaringType, Va

} else if (node instanceof ConstructorCallExpression) {
ConstructorCallExpression constructorCall = (ConstructorCallExpression) node;
MethodNode constructorDecl = scope.getEnclosingMethodDeclaration(); // watch for initializers but no constructor
if (constructorCall.isThisCall()) {
declaringType = constructorDecl != null ? constructorDecl.getDeclaringClass() : scope.getEnclosingTypeDeclaration();
declaringType = scope.getEnclosingMethodDeclaration().getDeclaringClass();
} else if (constructorCall.isSuperCall()) {
declaringType = constructorDecl != null ? constructorDecl.getDeclaringClass().getUnresolvedSuperClass() : scope.getEnclosingTypeDeclaration();
declaringType = scope.getEnclosingMethodDeclaration().getDeclaringClass().getUnresolvedSuperClass();
} else if (constructorCall.isUsingAnonymousInnerClass()) {
//declaringType = declaringType.getUnresolvedSuperClass() || declaringType.getUnresolvedInterfaces()[0]?
}

// try to find best match if there is more than one constructor to choose from
List<ConstructorNode> declaredConstructors = declaringType.getDeclaredConstructors();
if (constructorCall.getArguments() instanceof ArgumentListExpression && declaredConstructors.size() > 1) {
List<ConstructorNode> looseMatches = new ArrayList<>();
if (declaredConstructors.size() > 1 && constructorCall.getArguments() instanceof ArgumentListExpression) {
List<ClassNode> callTypes = scope.getMethodCallArgumentTypes();
if (callTypes.size() > 1) {
// non-static inner types may have extra argument for enclosing type
if (callTypes.get(0).equals(declaringType.getOuterClass()) &&
(declaringType.getModifiers() & ClassNode.ACC_STATIC) == 0) {
callTypes.remove(0);
}
}

List<ConstructorNode> looseMatches = new ArrayList<>();
for (ConstructorNode ctor : declaredConstructors) {
if (callTypes.size() == ctor.getParameters().length) {
if (Boolean.TRUE.equals(isTypeCompatible(callTypes, ctor.getParameters()))) {
Expand All @@ -284,7 +293,7 @@ protected TypeLookupResult findType(Expression node, ClassNode declaringType, Va
}
}

ASTNode declaration = !declaredConstructors.isEmpty() ? declaredConstructors.get(0) : declaringType;
ASTNode declaration = (!declaredConstructors.isEmpty() ? declaredConstructors.get(0) : declaringType);
return new TypeLookupResult(nodeType, declaringType, declaration, confidence, scope);

} else if (node instanceof StaticMethodCallExpression) {
Expand Down

0 comments on commit 232a365

Please sign in to comment.