From 77241416e2cbe9caa8dc6540b03af1695beb0cb7 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Tue, 15 Jan 2019 16:10:24 -0600 Subject: [PATCH] Fix for #773, #775 and #776: increase accuracy of overload method match --- .../search/MethodReferenceSearchTests.java | 111 ++++++++++--- .../MethodReferenceSearchRequestor.java | 155 +++++++++--------- .../jdt/groovy/search/SimpleTypeLookup.java | 16 +- .../requestor/CodeSelectRequestor.java | 3 +- .../search/FindAllReferencesRequestor.java | 64 +++----- 5 files changed, 205 insertions(+), 144 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java index d88f973abd..0c44992afd 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/MethodReferenceSearchTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2018 the original author or authors. + * Copyright 2009-2019 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. @@ -31,6 +31,7 @@ import org.eclipse.jdt.core.search.SearchParticipant; import org.eclipse.jdt.core.search.SearchPattern; import org.eclipse.jdt.groovy.search.TypeRequestorFactory; +import org.junit.Ignore; import org.junit.Test; /** @@ -138,7 +139,7 @@ public void testMethodReferencesInClass4() throws Exception { @Test public void testOverloadedMethodReferences1() throws Exception { - // should match on the method reference with precise # of args as well as method reference with unmatched number of args + // search for "First.xxx()" should match on the method reference with precise # of args as well as method reference with unmatched number of args doTestForTwoMethodReferences( "interface First {\n" + " void xxx()\n" + @@ -146,13 +147,13 @@ public void testOverloadedMethodReferences1() throws Exception { "}", "public class Second implements First {\n" + " public void other() {\n" + - " xxx()\n" + + " xxx()\n" + //yes " }\n" + " public void xxx() {\n" + - " xxx(a)\n" + + " xxx(a)\n" + //no! " }\n" + " void xxx(a) {\n" + - " xxx(a,b)\n" + + " xxx(a,b)\n" + //yes " }\n" + "}", false, 0, "xxx"); @@ -160,7 +161,7 @@ public void testOverloadedMethodReferences1() throws Exception { @Test public void testOverloadedMethodReferences2() throws Exception { - // should match on the method reference with precise # of args as well as method reference with unmatched number of args + // search for "First.xxx(a)" should match on the method reference with precise # of args as well as method reference with unmatched number of args doTestForTwoMethodReferences( "interface First {\n" + " void xxx(a)\n" + @@ -168,13 +169,13 @@ public void testOverloadedMethodReferences2() throws Exception { "}", "public class Second implements First {\n" + " public void other() {\n" + - " xxx(a)\n" + + " xxx(a)\n" + //yes " }\n" + " public void xxx() {\n" + - " xxx()\n" + + " xxx()\n" + //no! " }\n" + " void xxx(a) {\n" + - " xxx(a,b)\n" + + " xxx(a,b)\n" + //yes " }\n" + "}", false, 0, "xxx"); @@ -182,7 +183,7 @@ public void testOverloadedMethodReferences2() throws Exception { @Test public void testOverloadedMethodReferences3() throws Exception { - // should match on the method reference with precise # of args as well as method reference with unmatched number of args + // search for "First.xxx(a)" should match on the method reference with precise # of args as well as method reference with unmatched number of args createUnit("Sub", "interface Sub extends First {\n" + " void xxx(a)\n" + @@ -194,13 +195,13 @@ public void testOverloadedMethodReferences3() throws Exception { "}", "public class Second implements Sub {\n" + " public void other() {\n" + - " xxx(a)\n" + + " xxx(a)\n" + //yes " }\n" + " public void xxx() {\n" + - " xxx()\n" + + " xxx()\n" + //no! " }\n" + " void xxx(a) {\n" + - " xxx(a,b)\n" + + " xxx(a,b)\n" + //yes " }\n" + "}", false, 0, "xxx"); @@ -208,7 +209,7 @@ public void testOverloadedMethodReferences3() throws Exception { @Test public void testOverloadedMethodReferences4() throws Exception { - // should match on the method reference with precise # of args as well as method reference with unmatched number of args + // search for "First.xxx(a,b)" should match on the method reference with precise # of args as well as method reference with unmatched number of args createUnit("Sub", "interface Sub extends First {\n" + " void xxx(a)\n" + @@ -222,18 +223,18 @@ public void testOverloadedMethodReferences4() throws Exception { "public class Second implements Sub {\n" + " public void other() {\n" + " First f\n" + - " f.xxx(a,b,c)\n" + + " f.xxx(a,b,c)\n" + //yes " }\n" + " public void xxx() {\n" + - " xxx(a)\n" + - " xxx(a,b,c)\n" + + " xxx(a)\n" + //no! + " xxx(a,b,c)\n" + //no! " Sub s\n" + - " s.xxx(a)\n" + - " s.xxx(a,b,c)\n" + + " s.xxx(a)\n" + //no! + " s.xxx(a,b,c)\n" + //no! " }\n" + " void xxx(a) {\n" + " Sub s\n" + - " s.xxx(a,b)\n" + + " s.xxx(a,b)\n" + //yes " }\n" + "}", false, 0, "xxx"); @@ -327,6 +328,74 @@ public void testMethodWithDefaultParameters2() throws Exception { false, 0, "xxx"); } + @Test // https://github.com/groovy/groovy-eclipse/issues/776 + public void testMethodWithDefaultParameters3() throws Exception { + GroovyCompilationUnit groovyUnit = createUnit("foo", "Bar", + "package foo\n" + + "class Bar {\n" + + " void doSomething() {}\n" + + " void doSomething(String one, String two = 'x') {}\n" + + "}"); + createUnit("foo", "Baz", + "package foo;\n" + + "public class Baz {\n" + + " void test(Bar bar) {\n" + + " bar.doSomething();\n" + //no! + " bar.doSomething(\"one\");\n" + //yes + " bar.doSomething(\"one\", \"two\");\n" + //yes + " }\n" + + "}"); + + IMethod method = groovyUnit.getType("Bar").getMethods()[1]; + new SearchEngine().search( + SearchPattern.createPattern(method, IJavaSearchConstants.REFERENCES), + new SearchParticipant[] {SearchEngine.getDefaultSearchParticipant()}, + SearchEngine.createJavaSearchScope(new IJavaElement[] {groovyUnit.getPackageFragmentRoot()}, false), + searchRequestor, new NullProgressMonitor()); + + List matches = searchRequestor.getMatches(); + + assertEquals(2, matches.size()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(0).getAccuracy()); + assertEquals("Baz.groovy", ((IJavaElement) matches.get(0).getElement()).getResource().getName()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(1).getAccuracy()); + assertEquals("Baz.groovy", ((IJavaElement) matches.get(1).getElement()).getResource().getName()); + } + + @Test @Ignore("Only one method signature is searched and Java lacks link to original method") + public void testMethodWithDefaultParameters4() throws Exception { + GroovyCompilationUnit groovyUnit = createUnit("foo", "Bar", + "package foo\n" + + "class Bar {\n" + + " void doSomething() {}\n" + + " void doSomething(String one, String two = 'x') {}\n" + + "}"); + createJavaUnit("foo", "Baz", + "package foo;\n" + + "public class Baz {\n" + + " void test(Bar bar) {\n" + + " bar.doSomething();\n" + //no! + " bar.doSomething(\"one\");\n" + //no! (want to be yes) + " bar.doSomething(\"one\", \"two\");\n" + //yes + " }\n" + + "}"); + + IMethod method = groovyUnit.getType("Bar").getMethods()[1]; + new SearchEngine().search( + SearchPattern.createPattern(method, IJavaSearchConstants.REFERENCES), + new SearchParticipant[] {SearchEngine.getDefaultSearchParticipant()}, + SearchEngine.createJavaSearchScope(new IJavaElement[] {groovyUnit.getPackageFragmentRoot()}, false), + searchRequestor, new NullProgressMonitor()); + + List matches = searchRequestor.getMatches(); + + assertEquals(2, matches.size()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(0).getAccuracy()); + assertEquals("Baz.java", ((IJavaElement) matches.get(0).getElement()).getResource().getName()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(1).getAccuracy()); + assertEquals("Baz.java", ((IJavaElement) matches.get(1).getElement()).getResource().getName()); + } + @Test public void testStaticMethodReferenceSearch() throws Exception { String contents = @@ -535,7 +604,7 @@ public void testGenericsMethodReferenceSearch() throws Exception { "}"); @SuppressWarnings("unused") ICompilationUnit javaUnit = createJavaUnit("foo", "Baz", - "package foo\n" + + "package foo;\n" + "import java.util.Date;\n" + "public class Baz {\n" + " private T test;\n" + diff --git a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java index 9b248bbe95..0d7c6cccf3 100644 --- a/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java +++ b/base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/MethodReferenceSearchRequestor.java @@ -15,6 +15,7 @@ */ package org.eclipse.jdt.groovy.search; +import java.util.BitSet; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -27,6 +28,7 @@ import org.codehaus.groovy.ast.AnnotatedNode; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; @@ -67,8 +69,8 @@ public class MethodReferenceSearchRequestor implements ITypeRequestor { protected final boolean findReferences, findDeclarations, skipPseudoProperties; protected final Set acceptedPositions = new HashSet<>(); - protected static final int MAX_PARAMS = 10; // indices available in each array of: - protected final Map cachedParameterCounts = new HashMap<>(); + protected static final int MAX_PARAMS = 20; // indexes available in each of: + protected final Map cachedParameterCounts = new HashMap<>(); protected final Map cachedDeclaringNameMatches = new HashMap<>(); public MethodReferenceSearchRequestor(MethodPattern pattern, SearchRequestor requestor, SearchParticipant participant) { @@ -168,7 +170,7 @@ protected static String[] getParameterTypeSignatures(MethodPattern pattern) { protected static boolean isPrimitiveType(char[] name) { // adapted from ASTConverter - switch(name[0]) { + switch (name[0]) { case 'i': return (name.length >= 3 && name[1] == 'n' && name[2] == 't' && (name.length == 3 || name[3] == '[')); case 'l': @@ -214,7 +216,8 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle // check for "foo.bar" where "bar" refers to "getBar()", "isBar()" or "setBar(...)" if (!isDeclaration && (end - start) < ((StaticMethodCallExpression) node).getMethod().length() && skipPseudoProperties) { - start = end = 0; + start = 0; + end = 0; } // check for non-synthetic match; SyntheticAccessorSearchRequestor matches "foo.bar" to "getBar()", etc. @@ -232,12 +235,17 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle } if (end > 0) { // name matches, now check declaring type and parameter types - // don't want to double accept nodes; this could happen with field and object initializers can get pushed into multiple constructors + // don't want to double accept nodes; this could happen if field and object initializers 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 (!acceptedPositions.contains(position) && declaringTypeMatches(result.declaringType)) { + boolean acceptable; + if (isDeclaration || result.confidence == TypeConfidence.EXACT) { + acceptable = parameterTypesMatch((MethodNode) result.declaration); + } else { + acceptable = argumentTypesMatch(result.scope.getMethodCallArgumentTypes(), result.declaringType); + } + if (acceptable) { if (enclosingElement.getOpenable() instanceof GroovyClassFileWorkingCopy) { enclosingElement = ((GroovyClassFileWorkingCopy) enclosingElement.getOpenable()).convertToBinary(enclosingElement); } @@ -262,8 +270,46 @@ public VisitStatus acceptASTNode(ASTNode node, TypeLookupResult result, IJavaEle return VisitStatus.CONTINUE; } + private boolean declaringTypeMatches(ClassNode declaringType) { + if (declaringType == null) { + return false; + } + String declaringTypeName = declaringType.getName().replace('$', '.'); + if ("java.lang.Object".equals(declaringTypeName) && declaringType.getDeclaredMethods(methodName).isEmpty()) { + // local variables have a declaring type of Object; don't accidentally return them as a match + return false; + } + if (this.declaringTypeName == null || this.declaringTypeName.isEmpty()) { + // no type specified, accept all + return true; + } + + Boolean maybeMatch = cachedDeclaringNameMatches.get(declaringType); + if (maybeMatch != null) { + return maybeMatch; + } + + if (declaringTypeName.equals(this.declaringTypeName)) { + cachedDeclaringNameMatches.put(declaringType, Boolean.TRUE); + return true; + } else { // check the supers + maybeMatch = declaringTypeMatches(declaringType.getSuperClass()); + if (!maybeMatch) { + for (ClassNode face : declaringType.getInterfaces()) { + maybeMatch = declaringTypeMatches(face); + if (maybeMatch) { + break; + } + } + } + cachedDeclaringNameMatches.put(declaringType, maybeMatch); + return maybeMatch; + } + } + private boolean parameterTypesMatch(MethodNode methodNode) { - List parameterTypes = GroovyUtils.getParameterTypes(methodNode.getParameters()); + Parameter[] parameters = methodNode.getOriginal().getParameters(); + List parameterTypes = GroovyUtils.getParameterTypes(parameters); int n; if ((n = parameterTypes.size()) != parameterTypeSignatures.length) { return false; } @@ -301,86 +347,45 @@ private boolean parameterTypesMatch(MethodNode methodNode) { * and return a match * */ - private boolean nameAndArgsMatch(ClassNode declaringType, List argumentTypes) { - if (matchOnName(declaringType)) { - 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 = ConstructorReferenceSearchRequestor.makeType(parameterTypeNames[i]); - if (Boolean.FALSE.equals(SimpleTypeLookup.isTypeCompatible(source, target))) { - return false; - } - } - return true; - } else { - boolean[] foundParameterNumbers = cachedParameterCounts.get(declaringType); - if (foundParameterNumbers == null) { - foundParameterNumbers = new boolean[MAX_PARAMS + 1]; - gatherParameters(declaringType, foundParameterNumbers); - cachedParameterCounts.put(declaringType, foundParameterNumbers); - } - // now, if we find a method that has the same number of parameters in the call, - // then assume the call is for this target method (and therefore there is no match) - return !foundParameterNumbers[Math.min(MAX_PARAMS, argumentTypes.size())]; - } - } - return false; - } - - private boolean matchOnName(ClassNode declaringType) { - if (declaringType == null) { - return false; - } - String declaringTypeName = declaringType.getName().replace('$', '.'); - if ("java.lang.Object".equals(declaringTypeName) && declaringType.getDeclaredMethods(methodName).isEmpty()) { - // local variables have a declaring type of Object; don't accidentally return them as a match - return false; - } - if (this.declaringTypeName == null || this.declaringTypeName.isEmpty()) { - // no type specified, accept all + private boolean argumentTypesMatch(List argumentTypes, ClassNode declaringType) { + if (argumentTypes == null) { return true; } - Boolean maybeMatch = cachedDeclaringNameMatches.get(declaringType); - if (maybeMatch != null) { - return maybeMatch; - } - - if (declaringTypeName.equals(this.declaringTypeName)) { - cachedDeclaringNameMatches.put(declaringType, true); - return true; - } else { // check the supers - maybeMatch = matchOnName(declaringType.getSuperClass()); - if (!maybeMatch) { - for (ClassNode iface : declaringType.getInterfaces()) { - maybeMatch = matchOnName(iface); - if (maybeMatch) { - break; - } + 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 = ConstructorReferenceSearchRequestor.makeType(parameterTypeNames[i]); + if (Boolean.FALSE.equals(SimpleTypeLookup.isTypeCompatible(source, target))) { + return false; } } - cachedDeclaringNameMatches.put(declaringType, maybeMatch); - return maybeMatch; + return true; } + + BitSet foundParameterCounts = cachedParameterCounts.computeIfAbsent(declaringType, t -> { + BitSet parameterCounts = new BitSet(MAX_PARAMS + 1); + gatherParameters(declaringType, parameterCounts); + return parameterCounts; + }); + // now, if we find a method that has the same number of parameters in the call, + // then assume the call is for this target method (and therefore there is no match) + return !foundParameterCounts.get(Math.min(MAX_PARAMS, argumentTypes.size())); } - private void gatherParameters(ClassNode declaringType, boolean[] foundParameterNumbers) { + private void gatherParameters(ClassNode declaringType, BitSet foundParameterCounts) { if (declaringType == null) { return; } declaringType = findWrappedNode(declaringType.redirect()); List methods = declaringType.getMethods(methodName); for (MethodNode method : methods) { - // GRECLIPSE-1233: ensure default parameters are ignored - method = method.getOriginal(); - foundParameterNumbers[Math.min(method.getParameters().length, MAX_PARAMS)] = true; + Parameter[] parameters = method.getParameters(); + foundParameterCounts.set(Math.min(parameters.length, MAX_PARAMS)); } - gatherParameters(declaringType.getSuperClass(), foundParameterNumbers); - for (ClassNode iface : declaringType.getInterfaces()) { - gatherParameters(iface, foundParameterNumbers); + gatherParameters(declaringType.getSuperClass(), foundParameterCounts); + for (ClassNode face : declaringType.getInterfaces()) { + gatherParameters(face, foundParameterCounts); } } @@ -403,7 +408,7 @@ private ClassNode findWrappedNode(ClassNode declaringType) { } } } - return wrappedNode == null ? declaringType : wrappedNode; + return (wrappedNode == null ? declaringType : wrappedNode); } private int getAccuracy(TypeConfidence confidence) { 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 e7641d484c..2399b060ee 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 @@ -338,7 +338,7 @@ protected TypeLookupResult findType(Expression node, ClassNode declaringType, Va confidence = TypeConfidence.LOOSELY_INFERRED; } - return new TypeLookupResult(closestMatch.getReturnType(), closestMatch.getDeclaringClass(), closestMatch.getOriginal(), confidence, scope); + return new TypeLookupResult(closestMatch.getReturnType(), closestMatch.getDeclaringClass(), closestMatch, confidence, scope); } } @@ -472,6 +472,13 @@ protected TypeLookupResult findTypeForVariable(VariableExpression var, VariableS if (field.getName().contains("__") && implementsTrait(owner)) { candidate = findTraitField(field.getName(), owner).orElse(field); } + } else if (candidate instanceof MethodNode) { + // check for call "method(1,2,3)" matched to decl "method(int)" + List argumentTypes = scope.getMethodCallArgumentTypes(); + Parameter[] parameterNodes = ((MethodNode) candidate).getParameters(); + if (argumentTypes != null && isLooseMatch(argumentTypes, parameterNodes)) { + newConfidence = TypeConfidence.findLessPrecise(confidence, TypeConfidence.INFERRED); + } } decl = candidate; declaringType = getDeclaringTypeFromDeclaration(decl, variableInfo != null ? variableInfo.declaringType : VariableScope.OBJECT_CLASS_NODE); @@ -625,7 +632,7 @@ protected MethodNode findMethodDeclaration(String name, ClassNode declaringType, if (!declaringType.isAbstract() && !declaringType.isInterface() && !implementsTrait(declaringType)) { List candidates = declaringType.getMethods(name); if (!candidates.isEmpty()) { - return findMethodDeclaration0(candidates, argumentTypes, isStaticExpression).getOriginal(); + return findMethodDeclaration0(candidates, argumentTypes, isStaticExpression); } return null; } @@ -642,7 +649,10 @@ protected MethodNode findMethodDeclaration(String name, ClassNode declaringType, MethodNode innerCandidate = null; List candidates = getMethods(name, type); if (!candidates.isEmpty()) { - innerCandidate = findMethodDeclaration0(candidates, argumentTypes, isStaticExpression).getOriginal(); + innerCandidate = findMethodDeclaration0(candidates, argumentTypes, isStaticExpression); + if (innerCandidate.getOriginal() == null) { + innerCandidate = null; // trait bridge + } if (outerCandidate == null) { outerCandidate = innerCandidate; } diff --git a/ide/org.codehaus.groovy.eclipse.codebrowsing/src/org/codehaus/groovy/eclipse/codebrowsing/requestor/CodeSelectRequestor.java b/ide/org.codehaus.groovy.eclipse.codebrowsing/src/org/codehaus/groovy/eclipse/codebrowsing/requestor/CodeSelectRequestor.java index f9fa1382d6..7e35daedc8 100644 --- a/ide/org.codehaus.groovy.eclipse.codebrowsing/src/org/codehaus/groovy/eclipse/codebrowsing/requestor/CodeSelectRequestor.java +++ b/ide/org.codehaus.groovy.eclipse.codebrowsing/src/org/codehaus/groovy/eclipse/codebrowsing/requestor/CodeSelectRequestor.java @@ -16,6 +16,7 @@ package org.codehaus.groovy.eclipse.codebrowsing.requestor; import java.util.List; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -467,7 +468,7 @@ else if (existsOnlyInGroovyModel(node.getField(), name, declaringType, jdtDeclar Parameter[] parameters = null; if (declaration instanceof MethodNode) { name = ((MethodNode) declaration).getName(); - parameters = ((MethodNode) declaration).getParameters(); + parameters = Optional.ofNullable(((MethodNode) declaration).getOriginal()).orElse((MethodNode) declaration).getParameters(); } maybeRequested = findElement(jdtDeclaringType, name, parameters); } diff --git a/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/FindAllReferencesRequestor.java b/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/FindAllReferencesRequestor.java index 4fc7fc487b..7928355c58 100644 --- a/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/FindAllReferencesRequestor.java +++ b/ide/org.codehaus.groovy.eclipse.core/src/org/codehaus/groovy/eclipse/core/search/FindAllReferencesRequestor.java @@ -1,5 +1,5 @@ /* - * Copyright 2009-2018 the original author or authors. + * Copyright 2009-2019 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. @@ -17,6 +17,7 @@ import java.util.Comparator; import java.util.Map; +import java.util.Optional; import java.util.TreeMap; import org.codehaus.groovy.ast.ASTNode; @@ -31,7 +32,6 @@ import org.codehaus.groovy.ast.PropertyNode; import org.codehaus.groovy.ast.expr.ClassExpression; import org.codehaus.jdt.groovy.internal.compiler.ast.JDTClassNode; -import org.codehaus.jdt.groovy.internal.compiler.ast.JDTMethodNode; import org.eclipse.jdt.core.IJavaElement; import org.eclipse.jdt.groovy.search.EqualityVisitor; import org.eclipse.jdt.groovy.search.ITypeRequestor; @@ -104,30 +104,16 @@ private boolean isEquivalent(ASTNode maybeDeclaration) { if (maybeDeclaration == declaration) { return true; } - // here we need to test for dynamically added fields and methods - // they will not be the same instance, so we need to check - // for equivalence some other way + // test for dynamically added fields and methods; they will not be the same instance, so check for equivalence some other way if (maybeDeclaration instanceof FieldNode && declaration instanceof FieldNode) { - FieldNode maybeField = (FieldNode) maybeDeclaration; - FieldNode field = (FieldNode) declaration; + FieldNode maybeField = (FieldNode) maybeDeclaration, field = (FieldNode) declaration; return maybeField.getName().equals(field.getName()) && maybeField.getDeclaringClass().equals(field.getDeclaringClass()); - } else if (declaration instanceof MethodNode) { - if (maybeDeclaration instanceof JDTMethodNode) { - // GRECLIPSE-1255 Catches the case where the node comes from - // JDT, we do not check for parameter count since JDT is - // ignorant of possible default parameters - MethodNode maybeMethod = (MethodNode) maybeDeclaration; - MethodNode method = (MethodNode) declaration; - return maybeMethod.getName().equals(method.getName()) && - maybeMethod.getDeclaringClass().equals(method.getDeclaringClass()); - } else if (maybeDeclaration instanceof MethodNode) { - MethodNode maybeMethod = (MethodNode) maybeDeclaration; - MethodNode method = (MethodNode) declaration; - return checkParamLength(maybeMethod, method) && maybeMethod.getName().equals(method.getName()) && - maybeMethod.getDeclaringClass().equals(method.getDeclaringClass()) && checkParams(maybeMethod, method); - } + } else if (maybeDeclaration instanceof MethodNode && declaration instanceof MethodNode) { + MethodNode maybeMethod = (MethodNode) maybeDeclaration, method = (MethodNode) declaration; + return maybeMethod.getName().equals(method.getName()) && maybeMethod.getDeclaringClass().equals(method.getDeclaringClass()) && + checkParams(Optional.ofNullable(maybeMethod.getOriginal()).orElse(maybeMethod).getParameters(), method.getParameters()); } - // here check for inner class nodes + // check for inner class nodes if ((maybeDeclaration instanceof InnerClassNode && declaration instanceof JDTClassNode) || (declaration instanceof InnerClassNode && maybeDeclaration instanceof JDTClassNode)) { return ((ClassNode) maybeDeclaration).getName().equals(((ClassNode) declaration).getName()); @@ -135,18 +121,24 @@ private boolean isEquivalent(ASTNode maybeDeclaration) { return false; } - private boolean checkParams(MethodNode maybeMethod, MethodNode method) { - Parameter[] maybeParameters = maybeMethod.getParameters(); - Parameter[] parameters = method.getParameters(); + private static boolean checkParams(Parameter[] maybeParameters, Parameter[] parameters) { + if (maybeParameters == null) { + return parameters == null; + } else if (parameters == null) { + return false; + } + if (maybeParameters.length != parameters.length) { + return false; + } for (int i = 0; i < parameters.length; i += 1) { - if (!maybeParameters[i].getName().equals(parameters[i].getName()) || !typeEquals(maybeParameters[i], parameters[i])) { + if (!typeEquals(maybeParameters[i], parameters[i])) { return false; } } return true; } - private boolean typeEquals(Parameter maybeParameter, Parameter parameter) { + private static boolean typeEquals(Parameter maybeParameter, Parameter parameter) { ClassNode maybeType = maybeParameter.getType(); ClassNode type = parameter.getType(); if (maybeType == null) { @@ -156,20 +148,4 @@ private boolean typeEquals(Parameter maybeParameter, Parameter parameter) { } return maybeType.getName().equals(type.getName()); } - - private boolean checkParamLength(MethodNode maybeMethod, MethodNode method) { - Parameter[] maybeParameters = maybeMethod.getParameters(); - Parameter[] parameters = method.getParameters(); - if (maybeParameters == null) { - return parameters == null; - } else if (parameters == null) { - return false; - } - - if (maybeParameters.length != parameters.length) { - return false; - } - - return true; - } }