From 5ae32b3ec9d0aa35865bcffcd102cd74d761941d Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 15 Apr 2024 16:36:43 +0200 Subject: [PATCH] [java-analysis] LambdaUtil.isSafeLambdaReplacement: check all calls in-between; check applicability Fixes IDEA-350194 Inspection QuickFix results in compilation error GitOrigin-RevId: fb83bfcb4e4aef9dc87a5c84de6d08202dd52ec6 --- .../src/com/intellij/psi/LambdaUtil.java | 84 +++++++++++-------- .../beforeInferenceFailAfterConversion.java | 10 +++ .../findFirst/afterFindFirstField.java | 11 ++- .../findFirst/afterFindFirstStaticField.java | 11 ++- .../findFirst/beforeFindFirstCall.java | 12 ++- .../findFirst/beforeFindFirstField.java | 11 ++- .../findFirst/beforeFindFirstFieldOther.java | 11 ++- .../findFirst/beforeFindFirstStaticField.java | 11 ++- 8 files changed, 120 insertions(+), 41 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/lambda2methodReference/beforeInferenceFailAfterConversion.java diff --git a/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java b/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java index eb720e31e9da..aaa99f54b212 100644 --- a/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java @@ -999,42 +999,43 @@ public final class LambdaUtil { if (body == null) return false; final PsiCall call = treeWalkUp(body); if (call != null) { - JavaResolveResult result = call.resolveMethodGenerics(); - PsiElement oldTarget = result.getElement(); - if (oldTarget != null) { - String origErrorMessage = result instanceof MethodCandidateInfo ? ((MethodCandidateInfo)result).getInferenceErrorMessage() : null; - Object marker = new Object(); - PsiTreeUtil.mark(lambda, marker); - PsiType origType = call instanceof PsiExpression ? ((PsiExpression)call).getType() : null; - PsiCall copyCall = copyTopLevelCall(call); - if (copyCall == null) return false; - PsiLambdaExpression lambdaCopy = ObjectUtils.tryCast(PsiTreeUtil.releaseMark(copyCall, marker), PsiLambdaExpression.class); - if (lambdaCopy == null) return false; - PsiExpression function = replacer.apply(lambdaCopy); - if (function == null) return false; - JavaResolveResult resultCopy = copyCall.resolveMethodGenerics(); - if (!oldTarget.getManager().areElementsEquivalent(resultCopy.getElement(), oldTarget)) return false; - String copyMessage = resultCopy instanceof MethodCandidateInfo ? ((MethodCandidateInfo)resultCopy).getInferenceErrorMessage() : null; - if (!Objects.equals(origErrorMessage, copyMessage)) return false; - if (function instanceof PsiFunctionalExpression) { - PsiType functionalType = ((PsiFunctionalExpression)function).getFunctionalInterfaceType(); - if (functionalType == null) return false; - PsiType lambdaFunctionalType = lambda.getFunctionalInterfaceType(); - if (lambdaFunctionalType != null && !functionalType.getCanonicalText().equals(lambdaFunctionalType.getCanonicalText())) { - return false; - } + Object marker = new Object(); + PsiTreeUtil.mark(lambda, marker); + PsiType origType = call instanceof PsiExpression ? ((PsiExpression)call).getType() : null; + PsiCall copyCall = copyTopLevelCall(call); + if (copyCall == null) return false; + PsiLambdaExpression lambdaCopy = ObjectUtils.tryCast(PsiTreeUtil.releaseMark(copyCall, marker), PsiLambdaExpression.class); + if (lambdaCopy == null) return false; + PsiExpression function = replacer.apply(lambdaCopy); + if (function == null) return false; + PsiElement copyCur = function, cur = lambda; + while(cur != call) { + cur = cur.getParent(); + copyCur = copyCur.getParent(); + if (cur instanceof PsiCall && copyCur instanceof PsiCall) { + JavaResolveResult result = ((PsiCall)cur).resolveMethodGenerics(); + JavaResolveResult resultCopy = ((PsiCall)copyCur).resolveMethodGenerics(); + if (!equalResolveResult(result, resultCopy)) return false; } - if (origType instanceof PsiClassType && !((PsiClassType)origType).isRaw() && - //when lambda has no formal parameter types, it's ignored during applicability check - //so unchecked warnings inside lambda's body won't lead to erasure of the type of the containing call - //but after replacement of lambda with the equivalent method call, unchecked warning won't be ignored anymore - //and the type of the call would be erased => red code may appear - !lambda.hasFormalParameterTypes()) { - PsiExpression expressionFromBody = extractSingleExpressionFromBody(body); - if (expressionFromBody instanceof PsiMethodCallExpression && - PsiTypesUtil.isUncheckedCall(((PsiMethodCallExpression)expressionFromBody).resolveMethodGenerics())) { - return false; - } + } + if (function instanceof PsiFunctionalExpression) { + PsiType functionalType = ((PsiFunctionalExpression)function).getFunctionalInterfaceType(); + PsiType lambdaFunctionalType = lambda.getFunctionalInterfaceType(); + boolean sameType = functionalType == null ? lambdaFunctionalType == null : + lambdaFunctionalType != null && + functionalType.getCanonicalText().equals(lambdaFunctionalType.getCanonicalText()); + if (!sameType) return false; + } + if (origType instanceof PsiClassType && !((PsiClassType)origType).isRaw() && + //when lambda has no formal parameter types, it's ignored during applicability check + //so unchecked warnings inside lambda's body won't lead to erasure of the type of the containing call + //but after replacement of lambda with the equivalent method call, unchecked warning won't be ignored anymore + //and the type of the call would be erased => red code may appear + !lambda.hasFormalParameterTypes()) { + PsiExpression expressionFromBody = extractSingleExpressionFromBody(body); + if (expressionFromBody instanceof PsiMethodCallExpression && + PsiTypesUtil.isUncheckedCall(((PsiMethodCallExpression)expressionFromBody).resolveMethodGenerics())) { + return false; } } } @@ -1050,6 +1051,19 @@ public final class LambdaUtil { return true; } + private static boolean equalResolveResult(JavaResolveResult r1, JavaResolveResult r2) { + PsiElement target1 = r1.getElement(); + PsiElement target2 = r2.getElement(); + boolean targetMatch = target1 == null ? target2 == null : target1.getManager().areElementsEquivalent(target2, target1); + if (!targetMatch) return false; + boolean applicable1 = !(r1 instanceof MethodCandidateInfo) || ((MethodCandidateInfo)r1).isApplicable(); + boolean applicable2 = !(r2 instanceof MethodCandidateInfo) || ((MethodCandidateInfo)r2).isApplicable(); + if (applicable1 != applicable2) return false; + String message1 = r1 instanceof MethodCandidateInfo ? ((MethodCandidateInfo)r1).getInferenceErrorMessage() : null; + String message2 = r2 instanceof MethodCandidateInfo ? ((MethodCandidateInfo)r2).getInferenceErrorMessage() : null; + return Objects.equals(message1, message2); + } + /** * Returns false if after suggested replacement of lambda body, containing method call would resolve to something else * or its return type will change. diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/lambda2methodReference/beforeInferenceFailAfterConversion.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/lambda2methodReference/beforeInferenceFailAfterConversion.java new file mode 100644 index 000000000000..58d764bf7be2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/lambda2methodReference/beforeInferenceFailAfterConversion.java @@ -0,0 +1,10 @@ +// "Replace lambda with method reference" "false" +import java.util.*; +import java.util.stream.*; + +class Test { + void test() { + Set lorem = Collections.unmodifiableSet(Stream.of("Lorem") + .collect(Collectors.toCollection(() -> new TreeSet<>()))); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstField.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstField.java index 1316bf4d0877..da7761e2356c 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstField.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstField.java @@ -1,6 +1,5 @@ // "Collapse loop with stream 'findFirst()'" "true-preview" -import java.awt.*; import java.util.List; import java.util.Objects; @@ -10,4 +9,14 @@ public class Main { public Point find(List points) { return points.stream().filter(Objects::nonNull).findFirst().orElse(field); } + + static class Point { + private int x; + private int y; + + Point(int x, int y) { + this.x = x; + this.y = y; + } + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstStaticField.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstStaticField.java index 8f4b3d8e1479..73ed797e436f 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstStaticField.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/afterFindFirstStaticField.java @@ -1,6 +1,5 @@ // "Collapse loop with stream 'findFirst()'" "true-preview" -import java.awt.*; import java.util.List; import java.util.Objects; @@ -10,4 +9,14 @@ public class Main { public static Point find(List points) { return points.stream().filter(Objects::nonNull).findFirst().orElse(ZERO); } + + static class Point { + private int x; + private int y; + + Point(int x, int y) { + this.x = x; + this.y = y; + } + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstCall.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstCall.java index 92ebea4b8b74..d6ed7cb3a800 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstCall.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstCall.java @@ -1,6 +1,5 @@ // "Collapse loop with stream 'findFirst()'" "false" -import java.awt.*; import java.util.List; public class Main { @@ -10,4 +9,15 @@ public class Main { } return new Point(0, 0); } + + + static class Point { + private int x; + private int y; + + Point(int x, int y) { + this.x = x; + this.y = y; + } + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstField.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstField.java index cf54eb6b7f6e..b5b88ca03e67 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstField.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstField.java @@ -1,6 +1,5 @@ // "Collapse loop with stream 'findFirst()'" "true-preview" -import java.awt.*; import java.util.List; public class Main { @@ -12,4 +11,14 @@ public class Main { } return field; } + + static class Point { + private int x; + private int y; + + Point(int x, int y) { + this.x = x; + this.y = y; + } + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstFieldOther.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstFieldOther.java index e2f51e5fc6e3..7dfe021594e3 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstFieldOther.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstFieldOther.java @@ -1,6 +1,5 @@ // "Collapse loop with stream 'findFirst()'" "false" -import java.awt.*; import java.util.List; public class Main { @@ -12,4 +11,14 @@ public class Main { } return other.field; } + + static class Point { + private int x; + private int y; + + Point(int x, int y) { + this.x = x; + this.y = y; + } + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstStaticField.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstStaticField.java index 36f932858856..bd4f01b57a08 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstStaticField.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamApiMigration/findFirst/beforeFindFirstStaticField.java @@ -1,6 +1,5 @@ // "Collapse loop with stream 'findFirst()'" "true-preview" -import java.awt.*; import java.util.List; public class Main { @@ -12,4 +11,14 @@ public class Main { } return ZERO; } + + static class Point { + private int x; + private int y; + + Point(int x, int y) { + this.x = x; + this.y = y; + } + } } \ No newline at end of file