From 071634d77eff05eead4266c466d974e6a3ce05b6 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Mon, 5 Jun 2017 15:04:40 +0300 Subject: [PATCH] Java: Navigate to proper overload of a method being accessed via reflection (IDEA-172319) --- ...vaLangInvokeHandleSignatureInspection.java | 2 +- .../JavaReflectionInvocationInspection.java | 35 ++++++--- .../JavaReflectionMemberAccessInspection.java | 5 ++ .../impl/JavaLangClassMemberReference.java | 47 ++++++++---- .../impl/JavaLangInvokeHandleReference.java | 25 ++++++- .../JavaLangInvokeHandleNavigationTest.kt | 64 ++++++++++++++++- .../JavaReflectionNavigationTest.java | 71 +++++++++++++++++-- 7 files changed, 217 insertions(+), 32 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaLangInvokeHandleSignatureInspection.java b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaLangInvokeHandleSignatureInspection.java index 4db61720ca88..e71c07de7568 100644 --- a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaLangInvokeHandleSignatureInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaLangInvokeHandleSignatureInspection.java @@ -309,7 +309,7 @@ public class JavaLangInvokeHandleSignatureInspection extends BaseJavaBatchLocalI * from arguments of MethodType.methodType(Class...) and MethodType.genericMethodType(int, boolean?) */ @Nullable - private static ReflectiveSignature composeMethodSignature(@Nullable PsiExpression methodTypeExpression) { + public static ReflectiveSignature composeMethodSignature(@Nullable PsiExpression methodTypeExpression) { final PsiExpression typeDefinition = findDefinition(methodTypeExpression); if (typeDefinition instanceof PsiMethodCallExpression) { final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)typeDefinition; diff --git a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java index 04bd9a3dca84..6f3515fceafe 100644 --- a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionInvocationInspection.java @@ -20,6 +20,7 @@ import com.intellij.codeInspection.InspectionsBundle; import com.intellij.codeInspection.ProblemsHolder; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.psiutils.ParenthesesUtils; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; @@ -131,22 +132,34 @@ public class JavaReflectionInvocationInspection extends BaseJavaBatchLocalInspec if (definition instanceof PsiMethodCallExpression) { final PsiMethodCallExpression definitionCall = (PsiMethodCallExpression)definition; if (methodPredicate.test(definitionCall)) { - final PsiExpression[] arguments = definitionCall.getArgumentList().getExpressions(); - - if (arguments.length == argumentOffset + 1) { - final PsiExpression[] arrayElements = getVarargAsArray(arguments[argumentOffset]); - if (arrayElements != null) { - return Arrays.asList(arrayElements); - } - } - if (arguments.length >= argumentOffset) { - return Arrays.asList(arguments).subList(argumentOffset, arguments.length); - } + return getRequiredMethodArguments(definitionCall, argumentOffset); } } return null; } + private static List getRequiredMethodArguments(@NotNull PsiMethodCallExpression definitionCall, int argumentOffset) { + final PsiExpression[] arguments = definitionCall.getArgumentList().getExpressions(); + + if (arguments.length == argumentOffset + 1) { + final PsiExpression[] arrayElements = getVarargAsArray(arguments[argumentOffset]); + if (arrayElements != null) { + return Arrays.asList(arrayElements); + } + } + if (arguments.length >= argumentOffset) { + return Arrays.asList(arguments).subList(argumentOffset, arguments.length); + } + return null; + } + + @Nullable + public static List getReflectionMethodParameterTypes(@NotNull PsiMethodCallExpression definitionCall, + int argumentOffset) { + List arguments = getRequiredMethodArguments(definitionCall, argumentOffset); + return arguments != null ? ContainerUtil.map(arguments, type -> getReflectiveType(type)) : null; + } + @Nullable static Arguments getActualMethodArguments(PsiExpression[] arguments, int argumentOffset, boolean allowVarargAsArray) { if (allowVarargAsArray && arguments.length == argumentOffset + 1) { diff --git a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java index 951770eb6d56..90213c251cfa 100644 --- a/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/reflectiveAccess/JavaReflectionMemberAccessInspection.java @@ -279,6 +279,11 @@ public class JavaReflectionMemberAccessInspection extends BaseJavaBatchLocalInsp final List argumentTypes = ContainerUtil.map(methodArguments.expressions, JavaReflectionReferenceUtil::getReflectiveType); + return matchMethod(methods, argumentTypes); + } + + @Nullable + public static PsiMethod matchMethod(@NotNull PsiMethod[] methods, @NotNull List argumentTypes) { int mismatchCount = Integer.MAX_VALUE; PsiMethod bestGuess = null; for (PsiMethod method : methods) { diff --git a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangClassMemberReference.java b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangClassMemberReference.java index 67629f036bce..00ba95c77149 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangClassMemberReference.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangClassMemberReference.java @@ -18,9 +18,11 @@ package com.intellij.psi.impl.source.resolve.reference.impl; import com.intellij.codeInsight.completion.InsertHandler; import com.intellij.codeInsight.completion.InsertionContext; import com.intellij.codeInsight.lookup.LookupElement; -import com.intellij.codeInsight.lookup.LookupElementBuilder; +import com.intellij.codeInspection.reflectiveAccess.JavaReflectionInvocationInspection; +import com.intellij.codeInspection.reflectiveAccess.JavaReflectionMemberAccessInspection; import com.intellij.psi.*; import com.intellij.psi.util.MethodSignatureBackedByPsiMethod; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ContainerUtil; import gnu.trove.THashSet; @@ -28,10 +30,7 @@ import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.Arrays; -import java.util.Comparator; -import java.util.Objects; -import java.util.Set; +import java.util.*; import static com.intellij.psi.impl.source.resolve.reference.impl.JavaReflectionReferenceUtil.*; @@ -73,17 +72,29 @@ public class JavaLangClassMemberReference extends PsiReferenceBase isRegularMethod(method) && isPublic(method)); - if (publicMethod != null) { - return publicMethod; + PsiMethod[] methods = psiClass.findMethodsByName(name, true); + if (methods.length > 1) { + methods = + ContainerUtil.filter(methods, method -> isRegularMethod(method) && isPublic(method)) + .toArray(PsiMethod.EMPTY_ARRAY); + if (methods.length > 1) { + return findOverloadedMethod(methods); + } } - return ContainerUtil.find(methods, method -> isRegularMethod(method)); + return methods.length != 0 ? methods[0] : null; } case GET_DECLARED_METHOD: { - final PsiMethod[] methods = psiClass.findMethodsByName(name, false); - return ContainerUtil.find(methods, method -> isRegularMethod(method) && isPotentiallyAccessible(method, psiClass)); + PsiMethod[] methods = psiClass.findMethodsByName(name, false); + if (methods.length > 1) { + methods = + ContainerUtil.filter(methods, method -> isRegularMethod(method) && isPotentiallyAccessible(method, psiClass)) + .toArray(PsiMethod.EMPTY_ARRAY); + if (methods.length > 1) { + return findOverloadedMethod(methods); + } + } + return methods.length != 0 ? methods[0] : null; } } } @@ -155,6 +166,18 @@ public class JavaLangClassMemberReference extends PsiReferenceBase parameterTypes = JavaReflectionInvocationInspection.getReflectionMethodParameterTypes(definitionCall, 1); + if (parameterTypes != null) { + return JavaReflectionMemberAccessInspection.matchMethod(methods, parameterTypes); + } + } + return null; + } + @Override public void handleInsert(InsertionContext context, LookupElement item) { final Object object = item.getObject(); diff --git a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangInvokeHandleReference.java b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangInvokeHandleReference.java index 838ee3abe01e..2574d768a80e 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangInvokeHandleReference.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/resolve/reference/impl/JavaLangInvokeHandleReference.java @@ -19,13 +19,16 @@ import com.intellij.codeInsight.completion.InsertHandler; import com.intellij.codeInsight.completion.InsertionContext; import com.intellij.codeInsight.completion.JavaLookupElementBuilder; import com.intellij.codeInsight.lookup.LookupElement; +import com.intellij.codeInspection.reflectiveAccess.JavaLangInvokeHandleSignatureInspection; import com.intellij.openapi.util.Condition; import com.intellij.psi.*; import com.intellij.psi.util.MethodSignatureBackedByPsiMethod; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.ProcessingContext; import com.intellij.util.containers.ContainerUtil; +import com.siyeh.ig.psiutils.ParenthesesUtils; import gnu.trove.THashSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -97,9 +100,25 @@ public class JavaLangInvokeHandleReference extends PsiReferenceBase filter) { - final PsiMethod[] methods = psiClass.findMethodsByName(name, true); - return ContainerUtil.find(methods, filter); + private PsiElement resolveMethod(@NotNull String name, @NotNull PsiClass psiClass, Condition filter) { + PsiMethod[] methods = psiClass.findMethodsByName(name, true); + if (methods.length != 0) { + methods = ContainerUtil.filter(methods, filter).toArray(PsiMethod.EMPTY_ARRAY); + if (methods.length > 1) { + final PsiMethodCallExpression definitionCall = PsiTreeUtil.getParentOfType(myElement, PsiMethodCallExpression.class); + if (definitionCall != null) { + final PsiExpression[] arguments = definitionCall.getArgumentList().getExpressions(); + if (arguments.length > 2) { + final PsiExpression typeExpression = ParenthesesUtils.stripParentheses(arguments[2]); + final ReflectiveSignature expectedSignature = JavaLangInvokeHandleSignatureInspection.composeMethodSignature(typeExpression); + if (expectedSignature != null) { + return ContainerUtil.find(methods, method -> expectedSignature.equals(getMethodSignature(method))); + } + } + } + } + } + return methods.length != 0 ? methods[0] : null; } @NotNull diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaLangInvokeHandleNavigationTest.kt b/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaLangInvokeHandleNavigationTest.kt index 73e7aa0e2f4c..b8f0b87796bb 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaLangInvokeHandleNavigationTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaLangInvokeHandleNavigationTest.kt @@ -16,10 +16,13 @@ package com.intellij.java.codeInsight.navigation import com.intellij.psi.PsiMember +import com.intellij.psi.PsiMethod import com.intellij.psi.PsiReference import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase import junit.framework.TestCase +import org.intellij.lang.annotations.Language import org.intellij.lang.annotations.MagicConstant +import org.jetbrains.annotations.NonNls /** * @author Pavel.Dolgov @@ -74,6 +77,64 @@ class JavaLangInvokeHandleNavigationTest : LightCodeInsightFixtureTestCase() { fun testStaticSetter6() = doNegativeTest("pf1", STATIC_SETTER) fun testStaticSetter7() = doNegativeTest("m1", STATIC_SETTER) + fun testOverloadedBothPublic() = doTestOverloaded( + """public class Overloaded { + public void foo(int n) {} + public void foo(String s) {} +}""", VIRTUAL, "java.lang.String") + + fun testOverloadedFirstPublic() = doTestOverloaded( + """public class Overloaded { + public void foo(int n) {} + void foo(String s) {} +}""", VIRTUAL, "int") + + fun testOverloadedSecondPublic() = doTestOverloaded( + """public class Overloaded { + void foo(int n) {} + public void foo(String s) {} +}""", VIRTUAL, "java.lang.String") + + fun testOverloadedInherited() { + myFixture.addClass("""public class OverloadedParent { + public static void foo(String s) {} +}""") + + doTestOverloaded( + """public class Overloaded extends OverloadedParent { + public static void foo(int n) {} +}""", STATIC, "java.lang.String") + } + + fun testOverloadedStatic() = doTestOverloaded( + """public class Overloaded { + public static void foo(int n) {} + public static void foo(String s) {} +}""", STATIC, "java.lang.String") + + + private fun doTestOverloaded(@NonNls @Language("JAVA") classText: String, function: String, vararg expectedParameterTypes: String) { + myFixture.addClass(classText) + + val methodType = arrayOf("void", *expectedParameterTypes).map { "$it.class" }.joinToString(", ") + val member = doTestImpl("foo", """ +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +class Main { + void foo() throws ReflectiveOperationException { + MethodHandles.Lookup lookup = MethodHandles.lookup(); + lookup.$function(Overloaded.class, "foo", MethodType.methodType($methodType)); + } +}""") + + TestCase.assertTrue("Is method", member is PsiMethod) + val parameters = (member as PsiMethod).parameterList.parameters + TestCase.assertEquals("Parameter count", expectedParameterTypes.size, parameters.size) + for (i in 0 until expectedParameterTypes.size) { + TestCase.assertEquals("Parameter $i", expectedParameterTypes[i], parameters[i].type.canonicalText) + } + } + private fun doTest(name: String, @MagicConstant(stringValues = arrayOf(VIRTUAL, STATIC, SPECIAL, @@ -83,7 +144,7 @@ class JavaLangInvokeHandleNavigationTest : LightCodeInsightFixtureTestCase() { doTestImpl(name, getMainClassText(name, function)) } - private fun doTestImpl(name: String, mainClassText: String) { + private fun doTestImpl(name: String, mainClassText: String): PsiMember { val reference = getReference(mainClassText) TestCase.assertEquals("Reference text", name, reference.canonicalText) val resolved = reference.resolve() @@ -91,6 +152,7 @@ class JavaLangInvokeHandleNavigationTest : LightCodeInsightFixtureTestCase() { TestCase.assertTrue("Target is a member", resolved is PsiMember) val member = resolved as PsiMember? TestCase.assertEquals("Target name", name, member!!.name) + return member } private fun doNegativeTest(name: String, diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaReflectionNavigationTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaReflectionNavigationTest.java index e904b9b9deb1..1102cb973937 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaReflectionNavigationTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/navigation/JavaReflectionNavigationTest.java @@ -15,9 +15,7 @@ */ package com.intellij.java.codeInsight.navigation; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiMember; -import com.intellij.psi.PsiReference; +import com.intellij.psi.*; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; import org.intellij.lang.annotations.Language; import org.intellij.lang.annotations.MagicConstant; @@ -93,6 +91,70 @@ public class JavaReflectionNavigationTest extends LightCodeInsightFixtureTestCas "}"); } + public void testOverloadedMethodBothPublic() { + doTestOverloadedMethod("foo", + "class Overloaded {\n" + + " public void foo() {}\n" + + " public void foo(String s) {}\n" + + "}", false, "java.lang.String"); + } + + public void testOverloadedMethodPublicFirst() { + doTestOverloadedMethod("foo", + "class Overloaded {\n" + + " public void foo() {}\n" + + " void foo(String s) {}\n" + + "}", false); + } + + public void testOverloadedMethodPublicSecond() { + doTestOverloadedMethod("foo", + "class Overloaded {\n" + + " void foo() {}\n" + + " public void foo(String s) {}\n" + + "}", false, "java.lang.String"); + } + + public void testOverloadedDeclaredMethod() { + doTestOverloadedMethod("foo", + "class Overloaded {\n" + + " public void foo() {}\n" + + " public void foo(String s) {}\n" + + "}", true, "java.lang.String"); + } + + public void testOverloadedInheritedMethod() { + doTestOverloadedMethod("bar", + "class OverloadedParent {" + + " public void bar(String s) {}\n" + + "}" + + "" + + "class Overloaded extends OverloadedParent {\n" + + " public void bar() {}\n" + + "}", false, "java.lang.String"); + } + + private void doTestOverloadedMethod(String name, + @NotNull @NonNls @Language("JAVA") String classText, + boolean isDeclared, + String... expectedParameterTypes) { + myFixture.addClass(classText); + + PsiMember member = doTestImpl(name, + "class Main {" + + " void main() {" + + " Overloaded.class.get" + (isDeclared?"Declared":"") + "Method(\""+name+"\", String.class);" + + " }" + + "}"); + assertTrue("Target is a method", member instanceof PsiMethod); + PsiMethod method = (PsiMethod)member; + PsiParameter[] parameters = method.getParameterList().getParameters(); + assertEquals("Parameter count", expectedParameterTypes.length, parameters.length); + for (int i = 0; i < expectedParameterTypes.length; i++) { + assertEquals("Parameter type " + i, expectedParameterTypes[i], parameters[0].getType().getCanonicalText()); + } + } + private void doTest(String name, @MagicConstant(stringValues = {FIELD, METHOD, DF, DM}) String type) { @@ -104,7 +166,7 @@ public class JavaReflectionNavigationTest extends LightCodeInsightFixtureTestCas doTestImpl(name, mainClassText); } - private void doTestImpl(String name, String mainClassText) { + private PsiMember doTestImpl(String name, @NotNull @NonNls @Language("JAVA") String mainClassText) { PsiReference reference = getReference(mainClassText); assertEquals("Reference text", name, reference.getCanonicalText()); PsiElement resolved = reference.resolve(); @@ -112,6 +174,7 @@ public class JavaReflectionNavigationTest extends LightCodeInsightFixtureTestCas assertTrue("Target is a member", resolved instanceof PsiMember); PsiMember member = (PsiMember)resolved; assertEquals("Target name", name, member.getName()); + return member; } private void doNegativeTest(String name,