Skip to content

Commit

Permalink
Fix for issue #373: factor in parameter compatibility for method search
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Nov 26, 2017
1 parent cc7f36a commit c10ed64
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 25 deletions.
40 changes: 40 additions & 0 deletions ...ests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,46 @@ public void testOverloadedMethodReferences4() throws Exception {
false, 0, "xxx");
}

@Test // https://github.com/groovy/groovy-eclipse/issues/373
public void testOverloadedMethodReferences5() throws Exception {
doTestForTwoMethodReferences(
"class First {\n" +
" URL doSomething(String s, URL u) { }\n" + // search for references
" URL doSomething(Integer i, URL u) { }\n" +
"}",
"class Second {\n" +
" First first\n" +
" def other\n" +
" void xxx() {\n" +
" URL u = new URL('www.example.com')\n" +
" first.doSomething('ciao', u)\n" + //yes
" first.doSomething(1L, u)\n" + //no!
" first.&doSomething\n" + //yes
" }\n" +
"}",
true, 2, "doSomething"); // "true, 2" says both matches are in xxx() (aka Second.children[2])
}

@Test // https://github.com/groovy/groovy-eclipse/issues/373
public void testOverloadedMethodReferences6() throws Exception {
doTestForTwoMethodReferences(
"class First {\n" +
" URL doSomething(Integer i, URL u) { }\n" + // search for references
" URL doSomething(String s, URL u) { }\n" +
"}",
"class Second {\n" +
" First first\n" +
" def other\n" +
" void xxx() {\n" +
" URL u = 'www.example.com'.toURL()\n" +
" first.doSomething(1, u)\n" + //yes
" first.doSomething('', u)\n" + //no!
" first.&doSomething\n" + //yes
" }\n" +
"}",
true, 2, "doSomething");// "true, 2" says both matches are in xxx() (aka Second.children[2])
}

@Test
public void testMethodWithDefaultParameters1() throws Exception {
doTestForTwoMethodReferences(
Expand Down
81 changes: 56 additions & 25 deletions ...pse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Set;

import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.expr.StaticMethodCallExpression;
Expand Down Expand Up @@ -87,9 +88,9 @@ public MethodReferenceSearchRequestor(MethodPattern pattern, SearchRequestor req
}
}
if (!methods.isEmpty()) {
declaringType = methods.getLast().getDeclaringType();
char[] superTypeName = declaringType.getElementName().toCharArray();
char[] packageName = declaringType.getPackageFragment().getElementName().toCharArray();
IType type = methods.getLast().getDeclaringType();
char[] superTypeName = type.getElementName().toCharArray();
char[] packageName = type.getPackageFragment().getElementName().toCharArray();
declaringQualifiedName = CharOperation.concat(packageName, superTypeName, '.');
}
}
Expand Down Expand Up @@ -117,24 +118,35 @@ public MethodReferenceSearchRequestor(MethodPattern pattern, SearchRequestor req
protected static String[] getParameterTypeNames(MethodPattern pattern, String[] parameterTypeSignatures, IType declaringType) {
int n = parameterTypeSignatures.length;
String[] typeNames = new String[n];
for (int i = 0; i < n; i += 1) {
if (pattern.parameterQualifications[i] != null || isPrimitiveType(pattern.parameterSimpleNames[i])) {
typeNames[i] = String.valueOf(CharOperation.concat(pattern.parameterQualifications[i], pattern.parameterSimpleNames[i], '.'));
} else {
int arrayCount = Signature.getArrayCount(parameterTypeSignatures[i]);
try {
String[][] resolved = declaringType.resolveType(String.valueOf(pattern.parameterSimpleNames[i], 0, pattern.parameterSimpleNames[i].length - (2*arrayCount)));
typeNames[i] = Signature.toQualifiedName(resolved[0]);
if (typeNames[i].charAt(0) == '.') { // default pkg
typeNames[i] = typeNames[i].substring(1);
if (declaringType != null) // TODO: Should searches like "main(String[])", which have null declaring type, check param types?
try {
int candidates = 0;
if (n > 0) { // check for method overloads
for (IMethod m : declaringType.getMethods()) {
if (equal(pattern.selector, m.getElementName()) && n == m.getNumberOfParameters()) { // TODO: What about variadic methods?
candidates += 1;
}
while (arrayCount-- > 0) {
typeNames[i] += "[]";
}
}
if (candidates > 1) {
for (int i = 0; i < n; i += 1) {
if (pattern.parameterQualifications[i] != null || isPrimitiveType(pattern.parameterSimpleNames[i])) {
typeNames[i] = String.valueOf(CharOperation.concat(pattern.parameterQualifications[i], pattern.parameterSimpleNames[i], '.'));
} else {
int arrayCount = Signature.getArrayCount(parameterTypeSignatures[i]);
String[][] resolved = declaringType.resolveType(String.valueOf(pattern.parameterSimpleNames[i], 0, pattern.parameterSimpleNames[i].length - (2*arrayCount)));
typeNames[i] = Signature.toQualifiedName(resolved[0]);
if (typeNames[i].charAt(0) == '.') { // default pkg
typeNames[i] = typeNames[i].substring(1);
}
while (arrayCount-- > 0) {
typeNames[i] += "[]";
}
}
} catch (Exception e) {
Util.log(e);
}
}
} catch (Exception e) {
Util.log(e);
}
return typeNames;
}
Expand Down Expand Up @@ -237,9 +249,10 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
}

/**
* When matching method references and declarations, we can't actually match
* on parameter types. Instead, we match on the number of parameterrs and
* assume that it is slightly more preceise than just matching on name.
* When matching method references and declarations, match on the number of
* parameterrs and assume that it is slightly more accurate than matching on
* name alone. If parameters and arguments are equal length, check the type
* compatibility of each pair.
*
* The heuristic that is used in this method is this:
* <ol>
Expand All @@ -254,14 +267,20 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle
* assume that current method call is an alternative way of calling the method
* and return a match
* </ol>
*
* @return true if there is a precise match between number of arguments and
* numner of parameters. false if there exists a different method with same
* number of arguments in current type, or true otherwise
*/
private boolean nameAndArgsMatch(ClassNode declaringType, List<ClassNode> argumentTypes) {
if (matchOnName(declaringType)) {
if (argumentTypes == null || argumentTypes.size() == parameterTypeNames.length) {
if (argumentTypes == null) {
return true;
}
if (argumentTypes.size() == parameterTypeNames.length) {
for (int i = 0; i < parameterTypeNames.length; i += 1) {
if (parameterTypeNames[i] == null) continue; // skip check
ClassNode source = argumentTypes.get(i), target = ClassHelper.makeWithoutCaching(parameterTypeNames[i]);
if (Boolean.FALSE.equals(SimpleTypeLookup.isTypeCompatible(source, target))) {
return false;
}
}
return true;
} else {
boolean[] foundParameterNumbers = cachedParameterCounts.get(declaringType);
Expand Down Expand Up @@ -374,4 +393,16 @@ private static boolean supportsOverride(IMethod method) throws JavaModelExceptio
int flags = method.getFlags();
return !(Flags.isPrivate(flags) || Flags.isStatic(flags));
}

private static boolean equal(char[] arr, CharSequence seq) {
if (arr.length != seq.length()) {
return false;
}
for (int i = 0, n = arr.length; i < n; i += 1) {
if (arr[i] != seq.charAt(i)) {
return false;
}
}
return true;
}
}

0 comments on commit c10ed64

Please sign in to comment.