Fix for #1090: apply same method selection algorithm to constructors

groovy · Apr 22, 2020 · 8d9a692 · 8d9a692
1 parent d751dcf
commit 8d9a692
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 18 deletions.
diff --git a/...builder/src/org/eclipse/jdt/core/groovy/tests/search/ConstructorReferenceSearchTests.java b/...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/...ovy.core.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/InferencingTests.java b/...ovy.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", "<init>", 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
@@ -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<ConstructorNode> 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<ConstructorNode> 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<MethodNode> candidates, final List<ClassNode> argumentTypes, final boolean isStaticExpression) {
+    protected static MethodNode findMethodDeclaration0(final List<? extends MethodNode> candidates, final List<ClassNode> argumentTypes, final boolean isStaticExpression) {
         int argumentCount = (argumentTypes == null ? -1 : argumentTypes.size());
 
         MethodNode closestMatch = null;