From 6d13de38dcb350a39fea8d3b64529b81363216a8 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Thu, 31 Jan 2019 17:52:50 +0300 Subject: [PATCH] [groovy] prefer overload with 0 distance instead of -1, because -1 means mismatch This fixes overload resolution for null argument. --- .../groovy/lang/resolve/impl/distance.kt | 7 +++- .../resolve/ResolveMethodOverloadsTest.groovy | 42 +++++++++++++++++++ .../lang/resolve/ResolveMethodTest.groovy | 14 ------- .../plugins/groovy/util/ResolveTest.java | 9 ++-- 4 files changed, 52 insertions(+), 20 deletions(-) create mode 100644 plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodOverloadsTest.groovy diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/impl/distance.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/impl/distance.kt index 7e6f6a171c2c..4afd2842df22 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/impl/distance.kt +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/lang/resolve/impl/distance.kt @@ -41,10 +41,13 @@ fun compare(left: ArgumentMapping, right: ArgumentMapping): Int { return -1 } - // prefer one will less distance val leftDistance = (left as PositionalArgumentMapping).distance val rightDistance = (right as PositionalArgumentMapping).distance - return leftDistance.compareTo(rightDistance) + return when { + leftDistance == 0L -> -1 + rightDistance == 0L -> 1 + else -> leftDistance.compareTo(rightDistance) // prefer one with less distance + } } fun positionalParametersDistance(map: Map, context: PsiElement): Long { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodOverloadsTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodOverloadsTest.groovy new file mode 100644 index 000000000000..9a8801fc20c9 --- /dev/null +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodOverloadsTest.groovy @@ -0,0 +1,42 @@ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package org.jetbrains.plugins.groovy.lang.resolve + +import com.intellij.psi.PsiMethod +import groovy.transform.CompileStatic +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod +import org.jetbrains.plugins.groovy.util.GroovyLatestTest +import org.jetbrains.plugins.groovy.util.ResolveTest +import org.junit.Test + +import static com.intellij.psi.CommonClassNames.JAVA_LANG_OBJECT +import static com.intellij.psi.CommonClassNames.JAVA_UTIL_LIST + +@CompileStatic +class ResolveMethodOverloadsTest extends GroovyLatestTest implements ResolveTest { + + @Test + void 'void argument List vs Object'() { + def method = resolveTest 'def foo(Object o); def foo(List l); void bar(); foo(bar())', GrMethod + assert method.parameterList.parameters.first().type.equalsToText(JAVA_LANG_OBJECT) + } + + @Test + void 'null argument List vs Object'() { + def method = resolveTest 'def foo(Object o); def foo(List l); foo(null)', GrMethod + assert method.parameterList.parameters.first().type.equalsToText(JAVA_LANG_OBJECT) + } + + @Test + void 'list equals null'() { + def method = resolveTest 'void usage(List l) { l.equals(null) }', PsiMethod + assert method.containingClass.qualifiedName == JAVA_UTIL_LIST + assert method.parameterList.parameters.last().type.equalsToText(JAVA_LANG_OBJECT) + } + + @Test + void 'list == null'() { + def method = resolveTest 'void usage(List l) { l == null }', PsiMethod + assert method.containingClass.qualifiedName == JAVA_UTIL_LIST + assert method.parameterList.parameters.last().type.equalsToText(JAVA_LANG_OBJECT) + } +} diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodTest.groovy index 9cddede19b19..53ebf706a1fc 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/resolve/ResolveMethodTest.groovy @@ -2108,20 +2108,6 @@ class Fo { assertEquals('C', clazz.qualifiedName) } - void 'test list equals null'() { - def method = resolveByText '''\ -void usage(List l) { l.equals(null) } -''', GrGdkMethod - assert method.staticMethod.parameterList.parameters.last().type.equalsToText("java.util.List") - } - - void 'test list == null'() { - def method = resolveByText '''\ -void usage(List l) { l == null } -''', GrGdkMethod - assert method.staticMethod.parameterList.parameters.last().type.equalsToText("java.util.List") - } - void testSuperReferenceWithTraitQualifier() { def method = resolveByText(''' trait A { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/util/ResolveTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/util/ResolveTest.java index 237a3d08fc00..d0dc290b0381 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/util/ResolveTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/util/ResolveTest.java @@ -35,18 +35,19 @@ public interface ResolveTest extends BaseTest { return referenceByText(text).resolve(false); } - default void resolveTest(@NotNull String text, @Nullable Class clazz) { - resolveTest(referenceByText(text), clazz); + default T resolveTest(@NotNull String text, @Nullable Class clazz) { + return resolveTest(referenceByText(text), clazz); } - default void resolveTest(@NotNull GroovyReference reference, @Nullable Class clazz) { + default T resolveTest(@NotNull GroovyReference reference, @Nullable Class clazz) { Collection results = reference.resolve(false); if (clazz == null) { assertEmpty(results); + return null; } else { PsiElement resolved = assertOneElement(results).getElement(); - assertInstanceOf(resolved, clazz); + return assertInstanceOf(resolved, clazz); } } }