From 8d9a69225a326568fdb91f6a4e99cab693e0a179 Mon Sep 17 00:00:00 2001 From: Eric Milles Date: Wed, 22 Apr 2020 14:35:36 -0500 Subject: [PATCH] Fix for #1090: apply same method selection algorithm to constructors --- .../ConstructorReferenceSearchTests.java | 17 +++++++++++++-- .../groovy/tests/search/InferencingTests.java | 16 ++++++++++++++ .../jdt/groovy/search/SimpleTypeLookup.java | 21 +++++-------------- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/ConstructorReferenceSearchTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/ConstructorReferenceSearchTests.java index 67afb34113..f006b26992 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/ConstructorReferenceSearchTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/ConstructorReferenceSearchTests.java @@ -340,7 +340,7 @@ public void testConstructorReferences13() throws Exception { " Foo(String s) {}\n" + "}"); createUnit("", "Bar", "import p.Foo\n" + - "def bar = new Foo(0) {\n" + // yes + "def foo = new Foo(0) {\n" + // yes "}\n"); long ctorRefs = searchForReferences(foo.getType("Foo").getMethods()[0]).stream() @@ -357,7 +357,7 @@ public void testConstructorReferences14() throws Exception { " Foo(String s) {}\n" + // search for this "}"); createUnit("", "Bar", "import p.Foo\n" + - "def bar = new Foo(0) {\n" + // no + "def foo = new Foo(0) {\n" + // no "}\n"); long ctorRefs = searchForReferences(foo.getType("Foo").getMethods()[1]).stream() @@ -366,6 +366,19 @@ public void testConstructorReferences14() throws Exception { assertEquals(0, ctorRefs); } + @Test + public void testConstructorReferences15() throws Exception { + GroovyCompilationUnit foo = createUnit("p", "Foo", "package p\n" + + "class Foo {\n" + + " Foo(String s, Map m) {}\n" + + " Foo(String s, ... v) {}\n" + // search for this + "}"); + createUnit("", "Bar", "def foo = new p.Foo('')"); // yes + + long ctorRefs = searchForReferences(foo.getType("Foo").getMethods()[1]).stream().count(); + assertEquals(1, ctorRefs); + } + @Test // https://github.com/groovy/groovy-eclipse/issues/796 public void testNewifyConstructorReferences1() throws Exception { GroovyCompilationUnit foo = createUnit("p", "Foo", "package p\n" + 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 4c841dc4dd..a797072e41 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 @@ -3813,4 +3813,20 @@ public void testMethodOverloadsArgumentMatching10() { assertType(contents, "meth", "java.lang.Character"); } + + @Test // https://github.com/groovy/groovy-eclipse/issues/1090 + public void testMethodOverloadsArgumentMatching11() { + String contents = + "class C {\n" + + " C(String s, Map m) {\n" + + " }\n" + + " C(String s, ... v) {\n" + + " }\n" + + "}\n" + + "new C('')\n"; + + int offset = contents.indexOf("new"); + MethodNode m = assertDeclaration(contents, offset, offset + 9, "C", "", DeclarationKind.METHOD); + assertTrue("Expected array, but was " + m.getParameters()[1].getType().getNameWithoutPackage(), m.getParameters()[1].getType().isArray()); + } } 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 0bcf7a8962..b1e2fc5f07 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 @@ -287,6 +287,7 @@ protected TypeLookupResult findType(final Expression node, final ClassNode decla resolvedDeclaringType = resolvedDeclaringType.getUnresolvedSuperClass(false); } + ASTNode declaration; // try to find best match if there is more than one constructor to choose from List declaredConstructors = resolvedDeclaringType.getDeclaredConstructors(); if (declaredConstructors.size() > 1 && call.getArguments() instanceof ArgumentListExpression) { @@ -297,23 +298,11 @@ protected TypeLookupResult findType(final Expression node, final ClassNode decla callTypes.remove(0); } } - - List looseMatches = new ArrayList<>(); - for (ConstructorNode ctor : declaredConstructors) { - if (callTypes != null && callTypes.size() == ctor.getParameters().length) { - if (Boolean.TRUE.equals(isTypeCompatible(callTypes, ctor.getParameters()))) { - return new TypeLookupResult(nodeType, resolvedDeclaringType, ctor, confidence, scope); - } - // argument types may not be fully resolved; at least the number of arguments matched - looseMatches.add(ctor); - } - } - if (!looseMatches.isEmpty()) { - declaredConstructors = looseMatches; - } + declaration = findMethodDeclaration0(declaredConstructors, callTypes, false); + } else { + declaration = (!declaredConstructors.isEmpty() ? declaredConstructors.get(0) : resolvedDeclaringType); } - ASTNode declaration = (!declaredConstructors.isEmpty() ? declaredConstructors.get(0) : resolvedDeclaringType); return new TypeLookupResult(nodeType, resolvedDeclaringType, declaration, confidence, scope); } else if (node instanceof StaticMethodCallExpression) { @@ -753,7 +742,7 @@ protected MethodNode findMethodDeclaration(final String name, final ClassNode de return outerCandidate; } - protected static MethodNode findMethodDeclaration0(final List candidates, final List argumentTypes, final boolean isStaticExpression) { + protected static MethodNode findMethodDeclaration0(final List candidates, final List argumentTypes, final boolean isStaticExpression) { int argumentCount = (argumentTypes == null ? -1 : argumentTypes.size()); MethodNode closestMatch = null;