From 53f889314d8948e9784350310e680dbb37f28d4d Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Mon, 13 Jan 2020 12:03:01 -0600 Subject: [PATCH] Fix for #1019: improve accuracy of searches for simple method references --- .../search/MethodReferenceSearchTests.java | 80 ++++++++++++++++--- .../jdt/groovy/search/SimpleTypeLookup.java | 14 +++- .../TypeInferencingVisitorWithRequestor.java | 8 +- 3 files changed, 90 insertions(+), 12 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 c83c7ccc44..2f963715e3 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 @@ -428,10 +428,10 @@ public void testMethodWithDefaultParameters4() throws Exception { } @Test - public void testStaticMethodReferenceSearch() throws Exception { + public void testStaticMethodReferenceSearch1() throws Exception { String contents = "class Foo {\n" + - " static def bar() {}\n" + + " static bar() {}\n" + " static {\n" + " bar()\n" + " }\n" + @@ -444,9 +444,71 @@ public void testStaticMethodReferenceSearch() throws Exception { factory.createVisitor(match).visitCompilationUnit(new TypeRequestorFactory().createRequestor(match, pattern, searchRequestor)); assertEquals(1, searchRequestor.getMatches().size()); + assertEquals(SearchMatch.A_ACCURATE, searchRequestor.getMatch(0).getAccuracy()); assertLocation(searchRequestor.getMatch(0), contents.lastIndexOf("bar"), "bar".length()); } + @Test + public void testStaticMethodReferenceSearch2() throws Exception { + String contents = + "import static Foo.bar\n" + + "class Foo {\n" + + " static bar() {}\n" + + "}\n"; + GroovyCompilationUnit foo = createUnit("Foo", contents); + + IMethod method = foo.getType("Foo").getMethods()[0]; + MockPossibleMatch match = new MockPossibleMatch(foo); + SearchPattern pattern = SearchPattern.createPattern(method, IJavaSearchConstants.REFERENCES); + factory.createVisitor(match).visitCompilationUnit(new TypeRequestorFactory().createRequestor(match, pattern, searchRequestor)); + + assertEquals(1, searchRequestor.getMatches().size()); + assertEquals(SearchMatch.A_ACCURATE, searchRequestor.getMatch(0).getAccuracy()); + assertLocation(searchRequestor.getMatch(0), contents.indexOf("bar"), "bar".length()); + } + + @Test + public void testStaticMethodReferenceSearch3() throws Exception { + String contents = + "import static Foo.bar\n" + + "class Foo {\n" + + " static bar() {}\n" + + " static bar(int i) {}\n" + + "}\n"; + GroovyCompilationUnit foo = createUnit("Foo", contents); + + IMethod method = foo.getType("Foo").getMethods()[0]; + MockPossibleMatch match = new MockPossibleMatch(foo); + SearchPattern pattern = SearchPattern.createPattern(method, IJavaSearchConstants.REFERENCES); + factory.createVisitor(match).visitCompilationUnit(new TypeRequestorFactory().createRequestor(match, pattern, searchRequestor)); + + assertEquals(1, searchRequestor.getMatches().size()); + assertEquals(SearchMatch.A_INACCURATE, searchRequestor.getMatch(0).getAccuracy()); + assertLocation(searchRequestor.getMatch(0), contents.indexOf("bar"), "bar".length()); + } + + @Test + public void testStaticMethodReferenceSearch4() throws Exception { + String contents = + "import static Foo.bar\n" + + "class Foo extends Base {\n" + + " static bar() {}\n" + + "}\n" + + "class Base {\n" + + " static bar(int i) {}\n" + + "}\n"; + GroovyCompilationUnit foo = createUnit("Foo", contents); + + IMethod method = foo.getType("Foo").getMethods()[0]; + MockPossibleMatch match = new MockPossibleMatch(foo); + SearchPattern pattern = SearchPattern.createPattern(method, IJavaSearchConstants.REFERENCES); + factory.createVisitor(match).visitCompilationUnit(new TypeRequestorFactory().createRequestor(match, pattern, searchRequestor)); + + assertEquals(1, searchRequestor.getMatches().size()); + assertEquals(SearchMatch.A_INACCURATE, searchRequestor.getMatch(0).getAccuracy()); + assertLocation(searchRequestor.getMatch(0), contents.indexOf("bar"), "bar".length()); + } + @Test public void testExplicitPropertyGetterSearch1() throws Exception { GroovyCompilationUnit bar = createUnit("foo", "Bar", @@ -458,13 +520,13 @@ public void testExplicitPropertyGetterSearch1() throws Exception { "}\n"); GroovyCompilationUnit baz = createUnit("foo", "Baz", "package foo\n" + - "import static foo.Bar.getString as blah\n" + // potential - "import static foo.Bar.getString as getS\n" + // potential + "import static foo.Bar.getString as blah\n" + // exact + "import static foo.Bar.getString as getS\n" + // exact "Bar.getString()\n" + // exact "Bar.'getString'()\n" + // exact - "str = Bar.string\n" + // potential + "str = Bar.string\n" + // exact "str = Bar.@string\n" + - "fun = Bar.&getString\n" + // potential + "fun = Bar.&getString\n" + // potential (may refer to category method) "str = blah()\n" + // exact "str = s\n"); // exact @@ -478,16 +540,16 @@ public void testExplicitPropertyGetterSearch1() throws Exception { List matches = searchRequestor.getMatches(); assertEquals(8, matches.size()); - assertEquals(SearchMatch.A_INACCURATE, matches.get(0).getAccuracy()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(0).getAccuracy()); assertEquals(String.valueOf(baz.getContents()).indexOf("getString as blah"), matches.get(0).getOffset()); - assertEquals(SearchMatch.A_INACCURATE, matches.get(1).getAccuracy()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(1).getAccuracy()); assertEquals(String.valueOf(baz.getContents()).indexOf("getString as getS"), matches.get(1).getOffset()); assertEquals(SearchMatch.A_ACCURATE, matches.get(2).getAccuracy()); assertEquals(String.valueOf(baz.getContents()).indexOf("getString()"), matches.get(2).getOffset()); assertEquals(SearchMatch.A_ACCURATE, matches.get(3).getAccuracy()); assertEquals(String.valueOf(baz.getContents()).indexOf("'getString'"), matches.get(3).getOffset()); - assertEquals(SearchMatch.A_INACCURATE, matches.get(4).getAccuracy()); + assertEquals(SearchMatch.A_ACCURATE, matches.get(4).getAccuracy()); assertEquals(String.valueOf(baz.getContents()).indexOf("Bar.string") + 4, matches.get(4).getOffset()); assertEquals(SearchMatch.A_INACCURATE, matches.get(5).getAccuracy()); assertEquals(String.valueOf(baz.getContents()).indexOf("Bar.&getString") + 5, matches.get(5).getOffset()); 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 0f6845d4d2..4d0221739e 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 @@ -39,6 +39,7 @@ import org.codehaus.groovy.ast.ConstructorNode; import org.codehaus.groovy.ast.DynamicVariable; import org.codehaus.groovy.ast.FieldNode; +import org.codehaus.groovy.ast.ImportNode; import org.codehaus.groovy.ast.MethodNode; import org.codehaus.groovy.ast.Parameter; import org.codehaus.groovy.ast.PropertyNode; @@ -421,7 +422,9 @@ protected TypeLookupResult findTypeForNameWithKnownObjectExpression(final String } else if (method.isPrivate() && isThisObjectExpression(scope) && isNotThisOrOuterClass(declaringType, resolvedDeclaringType)) { // "this.method()" reference to private method of super class yields MissingMethodException; "super.method()" is okay confidence = TypeConfidence.UNKNOWN; - } else if (isLooseMatch(scope.getMethodCallArgumentTypes(), method.getParameters())) { + } else if (isLooseMatch(scope.getMethodCallArgumentTypes(), method.getParameters()) && + !(isStaticObjectExpression && isStaticReferenceToUnambiguousMethod(scope, name, declaringType)) && + !(AccessorSupport.isGetter(method) && !scope.isMethodCall() && scope.getEnclosingNode() instanceof PropertyExpression)) { // if arguments and parameters are mismatched, a category method may make a better match confidence = TypeConfidence.LOOSELY_INFERRED; } @@ -981,6 +984,15 @@ protected static boolean isStaticReferenceToInstanceMethod(final VariableScope s return false; } + protected static boolean isStaticReferenceToUnambiguousMethod(final VariableScope scope, final String name, final ClassNode type) { + if (scope.getEnclosingNode() instanceof ImportNode) { // import nodes can only refer to static methods of type + long staticMethodCount = getMethods(name, type).stream().filter(meth -> isCompatible(meth, true)).count(); + return (staticMethodCount == 1); + } + // TODO: Add case for PropertyExpression, MethodCallExpression or MethodPointerExpression? + return false; + } + protected static boolean isCompatible(final AnnotatedNode declaration, final boolean isStaticExpression) { if (declaration != null) { boolean isStatic = 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 820dbf557c..16c0a660fd 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 @@ -229,7 +229,7 @@ public void visitCompilationUnit(final ITypeRequestor requestor) { this.requestor = requestor; this.enclosingElement = unit; - VariableScope topLevelScope = new VariableScope(null, enclosingModule, false); + VariableScope topLevelScope = new VariableScope(null, enclosingModule, true); scopes.add(topLevelScope); for (ITypeLookup lookup : lookups) { @@ -531,9 +531,11 @@ public void visitImports(final ModuleNode node) { if (imp.getFieldNameExpr() != null) { completeExpressionStack.add(imp); try { + scope.setCurrentNode(imp); primaryTypeStack.add(type); - imp.getFieldNameExpr().visit(this); + visitConstantExpression(imp.getFieldNameExpr()); + scope.forgetCurrentNode(); dependentTypeStack.removeLast(); dependentDeclarationStack.removeLast(); } finally { @@ -2505,6 +2507,8 @@ private boolean hasStaticObjectExpression(final Expression node, final ClassNode } else if (objectExpression instanceof ClassExpression || VariableScope.CLASS_CLASS_NODE.equals(primaryType)) { staticObjectExpression = true; // separate lookup exists for non-static members of Class, Object, or GroovyObject } + } else if (complete instanceof ImportNode) { + staticObjectExpression = true; } } return staticObjectExpression;