From 242cd356a7a1c101ac36930001eb1204327c6db9 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Tue, 29 Dec 2015 13:50:49 +0100 Subject: [PATCH] method reference: additional diagnostics for invalid method references (IDEA-149688) --- .../impl/analysis/HighlightVisitorImpl.java | 11 ++-- .../intellij/psi/PsiMethodReferenceUtil.java | 22 ------- .../tree/java/MethodReferenceResolver.java | 57 ++++++++++++++++--- .../lambda/methodRef/Ambiguity.java | 8 +-- .../lambda/methodRef/Assignability.java | 2 +- .../lambda/methodRef/Varargs.java | 4 +- .../ApplicabilityConflictMessage.java | 15 +++++ ...CapturedTypesOfImplicitParameterTypes.java | 2 +- ...erenceWhenContradictBoundsAreInferred.java | 12 ++-- ...odReferenceTypeArgumentsApplicability.java | 2 +- .../lambda/NewMethodRefHighlightingTest.java | 4 ++ 11 files changed, 89 insertions(+), 50 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/ApplicabilityConflictMessage.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index f1b899bb91b9..97c7a57ad88a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -1343,26 +1343,29 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh !((MethodCandidateInfo)results[0]).isApplicable() && expression.getFunctionalInterfaceType() != null) { String description = null; + if (results.length == 1) { + description = ((MethodCandidateInfo)results[0]).getInferenceErrorMessage(); + } if (expression.isConstructor()) { final PsiClass containingClass = PsiMethodReferenceUtil.getQualifierResolveResult(expression).getContainingClass(); if (containingClass != null) { if (!myHolder.add(HighlightClassUtil.checkInstantiationOfAbstractClass(containingClass, expression)) && !myHolder.add(GenericsHighlightUtil.checkEnumInstantiation(expression, containingClass)) && - containingClass.isPhysical()) { + containingClass.isPhysical() && + description == null) { description = JavaErrorMessages.message("cannot.resolve.constructor", containingClass.getName()); } } } - else { + else if (description == null){ description = JavaErrorMessages.message("cannot.resolve.method", expression.getReferenceName()); } if (description != null) { final PsiElement referenceNameElement = expression.getReferenceNameElement(); final HighlightInfo highlightInfo = - HighlightInfo.newHighlightInfo(results.length == 0 ? HighlightInfoType.WRONG_REF - : HighlightInfoType.ERROR) + HighlightInfo.newHighlightInfo(results.length == 0 ? HighlightInfoType.WRONG_REF : HighlightInfoType.ERROR) .descriptionAndTooltip(description).range(referenceNameElement).create(); myHolder.add(highlightInfo); final TextRange fixRange = HighlightMethodUtil.getFixRange(referenceNameElement); diff --git a/java/java-psi-api/src/com/intellij/psi/PsiMethodReferenceUtil.java b/java/java-psi-api/src/com/intellij/psi/PsiMethodReferenceUtil.java index 1b5b53ffa06b..0d570d1c02fc 100644 --- a/java/java-psi-api/src/com/intellij/psi/PsiMethodReferenceUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/PsiMethodReferenceUtil.java @@ -54,28 +54,6 @@ public class PsiMethodReferenceUtil { isSecondSearchPossible(functionalMethodParameterTypes, qualifierResolveResult, methodRef); } - public static boolean isCorrectAssignment(PsiType[] parameterTypes, - PsiType[] argTypes, - boolean varargs, - int offset) { - final int min = Math.min(parameterTypes.length, argTypes.length - offset); - for (int i = 0; i < min; i++) { - final PsiType argType = argTypes[i + offset]; - PsiType parameterType = parameterTypes[i]; - parameterType = GenericsUtil.getVariableTypeByExpressionType(parameterType, true); - if (varargs && i == parameterTypes.length - 1) { - if (!TypeConversionUtil.isAssignable(parameterType, argType) && - !TypeConversionUtil.isAssignable(((PsiArrayType)parameterType).getComponentType(), argType)) { - return false; - } - } - else if (!TypeConversionUtil.isAssignable(parameterType, argType)) { - return false; - } - } - return !varargs || parameterTypes.length - 1 <= argTypes.length - offset; - } - @Nullable public static PsiType getQualifierType(PsiMethodReferenceExpression expression) { PsiType qualifierType = null; diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/MethodReferenceResolver.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/MethodReferenceResolver.java index 2f7be250fc27..a63b075a712e 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/MethodReferenceResolver.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/java/MethodReferenceResolver.java @@ -140,7 +140,7 @@ public class MethodReferenceResolver implements ResolveCache.PolyVariantContextR final PsiType[] argTypes = signature.getParameterTypes(); boolean hasReceiver = PsiMethodReferenceUtil.isSecondSearchPossible(argTypes, qualifierResolveResult, reference); - return MethodReferenceConflictResolver.isApplicableByFirstSearch(this, argTypes, hasReceiver, interfaceMethod.isVarArgs()) != null; + return MethodReferenceConflictResolver.isApplicableByFirstSearch(this, argTypes, hasReceiver, reference, interfaceMethod.isVarArgs()) != null; } }; } @@ -237,7 +237,7 @@ public class MethodReferenceResolver implements ResolveCache.PolyVariantContextR for (CandidateInfo conflict : conflicts) { if (!(conflict instanceof MethodCandidateInfo)) continue; - final Boolean applicableByFirstSearch = isApplicableByFirstSearch(conflict, argTypes, hasReceiver, myFunctionalMethodVarArgs); + final Boolean applicableByFirstSearch = isApplicableByFirstSearch(conflict, argTypes, hasReceiver, myReferenceExpression, myFunctionalMethodVarArgs); if (applicableByFirstSearch != null) { (applicableByFirstSearch ? firstCandidates : secondCandidates).add(conflict); } @@ -275,9 +275,12 @@ public class MethodReferenceResolver implements ResolveCache.PolyVariantContextR return null; } - private static Boolean isApplicableByFirstSearch(CandidateInfo conflict, PsiType[] argTypes, - boolean hasReceiver, - boolean functionalMethodVarArgs) { + private static Boolean isApplicableByFirstSearch(CandidateInfo conflict, + PsiType[] functionalInterfaceParamTypes, + boolean hasReceiver, + PsiMethodReferenceExpression referenceExpression, + boolean functionalMethodVarArgs) { + final PsiMethod psiMethod = ((MethodCandidateInfo)conflict).getElement(); final PsiSubstitutor substitutor = ((MethodCandidateInfo)conflict).getSubstitutor(false); @@ -288,19 +291,55 @@ public class MethodReferenceResolver implements ResolveCache.PolyVariantContextR return null; } - if ((varargs || argTypes.length == parameterTypes.length) && - PsiMethodReferenceUtil.isCorrectAssignment(parameterTypes, argTypes, varargs, 0)) { + if ((varargs || functionalInterfaceParamTypes.length == parameterTypes.length) && + isCorrectAssignment(parameterTypes, functionalInterfaceParamTypes, varargs, referenceExpression, conflict, 0)) { return true; } if (hasReceiver && - (varargs || argTypes.length == parameterTypes.length + 1) && - PsiMethodReferenceUtil.isCorrectAssignment(parameterTypes, argTypes, varargs, 1)) { + (varargs || functionalInterfaceParamTypes.length == parameterTypes.length + 1) && + isCorrectAssignment(parameterTypes, functionalInterfaceParamTypes, varargs, referenceExpression, conflict, 1)) { return false; } return null; } + private static boolean isCorrectAssignment(PsiType[] parameterTypes, + PsiType[] functionalInterfaceParamTypes, + boolean varargs, + PsiMethodReferenceExpression referenceExpression, + CandidateInfo conflict, + int offset) { + final int min = Math.min(parameterTypes.length, functionalInterfaceParamTypes.length - offset); + for (int i = 0; i < min; i++) { + final PsiType argType = PsiUtil.captureToplevelWildcards(functionalInterfaceParamTypes[i + offset], referenceExpression); + final PsiType parameterType = parameterTypes[i]; + if (varargs && i == parameterTypes.length - 1) { + if (!TypeConversionUtil.isAssignable(parameterType, argType) && + !TypeConversionUtil.isAssignable(((PsiArrayType)parameterType).getComponentType(), argType)) { + reportParameterConflict(referenceExpression, conflict, argType, parameterType); + return false; + } + } + else if (!TypeConversionUtil.isAssignable(parameterType, argType)) { + reportParameterConflict(referenceExpression, conflict, argType, parameterType); + return false; + } + } + return !varargs || parameterTypes.length - 1 <= functionalInterfaceParamTypes.length - offset; + } + + private static void reportParameterConflict(PsiMethodReferenceExpression referenceExpression, + CandidateInfo conflict, + PsiType argType, + PsiType parameterType) { + if (conflict instanceof MethodCandidateInfo) { + ((MethodCandidateInfo)conflict).setInferenceError("Invalid " + + (referenceExpression.isConstructor() ? "constructor" :"method") + + " reference: " + argType.getPresentableText() + " cannot be converted to " + parameterType.getPresentableText()); + } + } + private boolean resolveConflicts(List firstCandidates, List secondCandidates, int applicabilityLevel) { final int firstApplicability = checkApplicability(firstCandidates); diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Ambiguity.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Ambiguity.java index 475ae43cdd78..71c39f4362c9 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Ambiguity.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Ambiguity.java @@ -54,7 +54,7 @@ class MyTest1 { } { - Bar1 b1 = MyTest2 :: foo; + Bar1 b1 = MyTest2 :: foo; bar(MyTest1 :: foo); } } @@ -80,7 +80,7 @@ class MyTest2 { }*/ { - Bar1 b1 = MyTest2 :: foo; + Bar1 b1 = MyTest2 :: foo; bar(MyTest2 :: foo); } } @@ -106,8 +106,8 @@ class MyTest3 { } { - Bar1 b1 = MyTest2 :: foo; - bar(MyTest3 :: foo); + Bar1 b1 = MyTest2 :: foo; + bar(MyTest3 :: foo); } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Assignability.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Assignability.java index de25cbcb6d90..b50f2a3dcf33 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Assignability.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Assignability.java @@ -2,7 +2,7 @@ class Test { { Runnable b = Test :: length; Comparable c = Test :: length; - Comparable c1 = Test :: length; + Comparable c1 = Test :: length; } public static Integer length(String s) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Varargs.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Varargs.java index 4f69a80fcddc..9587d691376d 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Varargs.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/methodRef/Varargs.java @@ -49,7 +49,7 @@ class MyTest { static { I1 i1 = MyTest::static_1; I1 i2 = MyTest::static_2; - I1 i3 = MyTest::static_3; + I1 i3 = MyTest::static_3; I1 i4 = MyTest::static_4; } @@ -62,7 +62,7 @@ class MyTest { I1 i1 = this::_1; I1 i2 = this::_2; - I1 i3 = this::_3; + I1 i3 = this::_3; I1 i4 = this::_4; I2 i21 = MyTest::m1; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/ApplicabilityConflictMessage.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/ApplicabilityConflictMessage.java new file mode 100644 index 000000000000..b15a13ece669 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/ApplicabilityConflictMessage.java @@ -0,0 +1,15 @@ +class A { + static class B { + int foo(T x) {return 0;} + } + public static void main(String[] args) { + B q = new B<>(); + + Func x = q::foo; + x.invoke(""); + } + + interface Func { + int invoke(CharSequence x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/CapturedTypesOfImplicitParameterTypes.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/CapturedTypesOfImplicitParameterTypes.java index 1abedc8e0c2d..6b3dc0f89e6a 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/CapturedTypesOfImplicitParameterTypes.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/CapturedTypesOfImplicitParameterTypes.java @@ -7,7 +7,7 @@ interface B { class Test { public static void test() { - method1(Test::method2); + method1(Test::method2); } static void method1(B> arg) { } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/HighlightReferenceWhenContradictBoundsAreInferred.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/HighlightReferenceWhenContradictBoundsAreInferred.java index 0814550220a9..4e1ef88f8844 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/HighlightReferenceWhenContradictBoundsAreInferred.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/HighlightReferenceWhenContradictBoundsAreInferred.java @@ -29,14 +29,14 @@ class Test { static void meth4(I3 s) { } static { - meth1(Foo::new); + meth1(Foo::new); meth2(Foo::new); - meth3(Foo::new); + meth3(Foo::new); meth4(Foo::new); - meth1(Test::foo); + meth1(Test::foo); meth2(Test::foo); - meth3(Test::foo); + meth3(Test::foo); meth4(Test::foo); } @@ -55,8 +55,8 @@ class Test { } void test() { - II1 i1 = this::fooInstance; + II1 i1 = this::fooInstance; II2 i2 = this::fooInstance; - II3 i3 = this::fooInstance; + II3 i3 = this::fooInstance; } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/MethodReferenceTypeArgumentsApplicability.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/MethodReferenceTypeArgumentsApplicability.java index 4d2d0b1ef6a7..af7c60b674d7 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/MethodReferenceTypeArgumentsApplicability.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/lambda/newMethodRef/MethodReferenceTypeArgumentsApplicability.java @@ -17,7 +17,7 @@ abstract class Test { { I i = Test::foo; - I i1 = Test::foo; + I i1 = Test::foo; I i2 = Test::foo; } diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/lambda/NewMethodRefHighlightingTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/lambda/NewMethodRefHighlightingTest.java index f26556dd5d20..ef2524f9e848 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/lambda/NewMethodRefHighlightingTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/lambda/NewMethodRefHighlightingTest.java @@ -474,6 +474,10 @@ public class NewMethodRefHighlightingTest extends LightDaemonAnalyzerTestCase { doTest(); } + public void testApplicabilityConflictMessage() throws Exception { + doTest(); + } + private void doTest() { doTest(false); }