From 9567645279fa05079c6b4125e17cea41c87708fe Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 5 Dec 2017 17:53:29 +0700 Subject: [PATCH] LambdaUtil#isSafeLambdaReplacement; used in ComparatorCombinators Fixes IDEA-183048 stream replacing issue --- .../RedundantLambdaCodeBlockInspection.java | 2 +- .../SimplifyBooleanExpressionFix.java | 2 +- .../src/com/intellij/psi/LambdaUtil.java | 92 +++++--- .../beforeUnchecked.java | 14 ++ .../controlflow/DoubleNegationInspection.java | 2 +- ...essaryConditionalExpressionInspection.java | 2 +- .../UnnecessaryBoxingInspection.java | 2 +- .../UnnecessaryUnboxingInspection.java | 2 +- .../ComparatorCombinatorsInspection.java | 218 ++++++++---------- .../PointlessBooleanExpressionInspection.java | 4 +- 10 files changed, 178 insertions(+), 162 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeUnchecked.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java index 8b44f2238864..4be1b21ccbef 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/RedundantLambdaCodeBlockInspection.java @@ -72,7 +72,7 @@ public class RedundantLambdaCodeBlockInspection extends AbstractBaseJavaLocalIns PsiExpression psiExpression = LambdaUtil.extractSingleExpressionFromBody(body); if (psiExpression != null && !findCommentsOutsideExpression(body, psiExpression)) { if (LambdaUtil.isExpressionStatementExpression(psiExpression) && - !LambdaUtil.isSameOverloadAfterReplacement((PsiLambdaExpression)body.getParent(), () -> psiExpression)) { + !LambdaUtil.isSafeLambdaBodyReplacement((PsiLambdaExpression)body.getParent(), () -> psiExpression)) { return null; } return psiExpression; diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java index bfffdc0f4d06..e34e8587f3df 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/SimplifyBooleanExpressionFix.java @@ -348,7 +348,7 @@ public class SimplifyBooleanExpressionFix extends LocalQuickFixOnPsiElement { if (!(expression instanceof PsiConditionalExpression) && !PsiType.BOOLEAN.equals(expression.getType())) return false; PsiElement parent = expression.getParent(); if (parent instanceof PsiLambdaExpression && - !LambdaUtil.isSameOverloadAfterReplacement((PsiLambdaExpression)parent, () -> createSimplifiedReplacement(expression))) { + !LambdaUtil.isSafeLambdaBodyReplacement((PsiLambdaExpression)parent, () -> createSimplifiedReplacement(expression))) { return false; } 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 81c8f3ee4eaa..a48780e164e7 100644 --- a/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/LambdaUtil.java @@ -12,6 +12,7 @@ import com.intellij.psi.infos.MethodCandidateInfo; import com.intellij.psi.util.*; import com.intellij.util.Consumer; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.ObjectUtils; import com.intellij.util.Producer; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.HashMap; @@ -20,6 +21,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; +import java.util.function.Function; import java.util.function.Supplier; /** @@ -848,8 +850,52 @@ public class LambdaUtil { ((PsiReferenceExpression)expression).isReferenceTo(parameters.getParameters()[0]); } + private static boolean isSafeLambdaReplacement(PsiLambdaExpression lambda, Function replacer) { + PsiElement body = lambda.getBody(); + if (body == null) return false; + final PsiCall call = treeWalkUp(body); + PsiMethod oldTarget; + if (call != null && (oldTarget = call.resolveMethod()) != 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; + if (copyCall.resolveMethod() != oldTarget) return false; + if (function instanceof PsiFunctionalExpression && ((PsiFunctionalExpression)function).getFunctionalInterfaceType() == null) { + return false; + } + PsiType newType = copyCall instanceof PsiExpression ? ((PsiExpression)copyCall).getType() : null; + if (origType != null && !origType.equals(newType)) { + return false; + } + } + return true; + } + /** - * Returns false if after suggested replacement of lambda body, containing method call would resolve to something else. + * Returns false if after suggested replacement of lambda body, containing method call would resolve to something else + * or its return type will change. + * + * @param lambda a lambda whose body is going to be replaced + * @param newFunctionSupplier replacement for lambda to check, + * lazy computed for lambdas in invocation context only. + * Replacement evaluated to {@code null} is treated as invalid overload + */ + public static boolean isSafeLambdaReplacement(PsiLambdaExpression lambda, Supplier newFunctionSupplier) { + return isSafeLambdaReplacement(lambda, l -> { + PsiExpression replacement = newFunctionSupplier.get(); + return replacement == null ? null : (PsiExpression)l.replace(replacement); + }); + } + + /** + * Returns false if after suggested replacement of lambda body, containing method call would resolve to something else + * or its return type will change. * *

* True will be returned for lambdas in non-invocation context as well as for lambdas in invocation context, @@ -866,44 +912,28 @@ public class LambdaUtil { * lazy computed for lambdas in invocation context only. * Replacement evaluated to {@code null} is treated as invalid overload */ - public static boolean isSameOverloadAfterReplacement(PsiLambdaExpression lambda, Supplier newBodySupplier) { - PsiElement body = lambda.getBody(); - if (body == null) return false; - final PsiCall call = treeWalkUp(body); - PsiMethod oldTarget; - if (call != null && (oldTarget = call.resolveMethod()) != null) { - Object marker = new Object(); - PsiTreeUtil.mark(body, marker); - PsiCall copyCall = copyTopLevelCall(call); - if (copyCall == null) return false; - final PsiElement bodyCopy = PsiTreeUtil.releaseMark(copyCall, marker); - if (bodyCopy != null) { - final PsiElement parent = bodyCopy.getParent(); - if (parent instanceof PsiLambdaExpression) { - PsiElement replacement = newBodySupplier.get(); - if (replacement == null) { - return false; - } - bodyCopy.replace(replacement); - if (copyCall.resolveMethod() != oldTarget || ((PsiLambdaExpression)parent).getFunctionalInterfaceType() == null) { - return false; - } - } - } - } - return true; + public static boolean isSafeLambdaBodyReplacement(PsiLambdaExpression lambda, Supplier newBodySupplier) { + return isSafeLambdaReplacement(lambda, l -> { + PsiElement oldBody = l.getBody(); + PsiElement newBody = newBodySupplier.get(); + if (oldBody == null || newBody == null) return null; + oldBody.replace(newBody); + return l; + }); } /** - * {@link #isSameOverloadAfterReplacement(PsiLambdaExpression, Supplier)} overload to test the same lambda body, + * {@link #isSafeLambdaBodyReplacement(PsiLambdaExpression, Supplier)} overload to test the same lambda body, * but with only return value {@code expression} changed to {@code replacement} + * @param lambdaReturnExpression a return expression inside lambda body + * @param replacement a replacement for return expression */ - public static boolean isSameOverloadAfterReplacement(@NotNull PsiExpression lambdaReturnExpression, - @NotNull PsiExpression replacement) { + public static boolean isSafeLambdaReturnValueReplacement(@NotNull PsiExpression lambdaReturnExpression, + @NotNull PsiExpression replacement) { if (lambdaReturnExpression.getParent() instanceof PsiReturnStatement || lambdaReturnExpression.getParent() instanceof PsiLambdaExpression) { PsiLambdaExpression lambdaExpression = PsiTreeUtil.getParentOfType(lambdaReturnExpression, PsiLambdaExpression.class, true, PsiMethod.class); if (lambdaExpression != null && - !isSameOverloadAfterReplacement(lambdaExpression, () -> { + !isSafeLambdaBodyReplacement(lambdaExpression, () -> { PsiLambdaExpression lambdaExpression1 = PsiTreeUtil.getParentOfType(lambdaReturnExpression, PsiLambdaExpression.class); if (lambdaExpression1 == null) return null; PsiElement body = lambdaExpression1.getBody(); diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeUnchecked.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeUnchecked.java new file mode 100644 index 000000000000..df06687898db --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeUnchecked.java @@ -0,0 +1,14 @@ +// "Replace with Comparator.comparing" "false" + +import java.util.*; + +public class Main { + public void sortByValues(Map inMap, + HashMap outMap) { + + inMap.entrySet() + .stream() + .sorted((it1, it2) -> it1.getValue().compareTo(it2.getValue())) + .forEachOrdered(it -> outMap.put((Character) it.getKey(), (Float) it.getValue())); + } +} diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/DoubleNegationInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/DoubleNegationInspection.java index 9252782d3ea2..aba3cc2f9388 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/DoubleNegationInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/DoubleNegationInspection.java @@ -126,7 +126,7 @@ public class DoubleNegationInspection extends BaseInspection { PsiExpression nestedOperand = ParenthesesUtils.stripParentheses(operand); if (nestedOperand instanceof PsiPrefixExpression) { PsiExpression nestedPrefixOperand = ((PsiPrefixExpression)nestedOperand).getOperand(); - if (nestedPrefixOperand == null || !LambdaUtil.isSameOverloadAfterReplacement(expression, nestedPrefixOperand)) { + if (nestedPrefixOperand == null || !LambdaUtil.isSafeLambdaReturnValueReplacement(expression, nestedPrefixOperand)) { return; } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspection.java index 311dd39f90cf..51d1225df84d 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/UnnecessaryConditionalExpressionInspection.java @@ -114,7 +114,7 @@ public class UnnecessaryConditionalExpressionInspection extends BaseInspection i } else if (BoolUtils.isTrue(thenExpression) && BoolUtils.isFalse(elseExpression)) { if (!(parent instanceof PsiLambdaExpression) || - LambdaUtil.isSameOverloadAfterReplacement((PsiLambdaExpression)parent, () -> condition)) { + LambdaUtil.isSafeLambdaBodyReplacement((PsiLambdaExpression)parent, () -> condition)) { registerError(expression, condition.getText()); } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java index f346896b564f..36dbe552dfe0 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryBoxingInspection.java @@ -275,7 +275,7 @@ public class UnnecessaryBoxingInspection extends BaseInspection { return isPossibleObjectComparison(boxingExpression, polyadicExpression); } return MethodCallUtils.isNecessaryForSurroundingMethodCall(boxingExpression, boxedExpression) || - !LambdaUtil.isSameOverloadAfterReplacement(boxingExpression, boxedExpression); + !LambdaUtil.isSafeLambdaReturnValueReplacement(boxingExpression, boxedExpression); } private boolean isPossibleObjectComparison(PsiExpression expression, PsiPolyadicExpression polyadicExpression) { diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryUnboxingInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryUnboxingInspection.java index 082df387d171..e5d1d59e133c 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryUnboxingInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/UnnecessaryUnboxingInspection.java @@ -208,7 +208,7 @@ public class UnnecessaryUnboxingInspection extends BaseInspection { return true; } - if (!LambdaUtil.isSameOverloadAfterReplacement(expression, unboxedExpression)) return true; + if (!LambdaUtil.isSafeLambdaReturnValueReplacement(expression, unboxedExpression)) return true; if (onlyReportSuperfluouslyUnboxed) { final PsiType expectedType = ExpectedTypeUtils.findExpectedType(expression, false, true); diff --git a/plugins/InspectionGadgets/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java b/plugins/InspectionGadgets/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java index a97849a3a253..b6c0dbb0cd60 100644 --- a/plugins/InspectionGadgets/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java +++ b/plugins/InspectionGadgets/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java @@ -3,6 +3,7 @@ package com.intellij.codeInspection; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; @@ -26,12 +27,12 @@ import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Objects; import static com.intellij.util.ObjectUtils.tryCast; public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspectionTool { private static final Logger LOG = Logger.getInstance(ComparatorCombinatorsInspection.class); - private static final EquivalenceChecker ourEquivalence = EquivalenceChecker.getCanonicalPsiEquivalence(); @NotNull @@ -49,62 +50,32 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec !((PsiClassType)type).rawType().equalsToText(CommonClassNames.JAVA_UTIL_COMPARATOR)) { return; } - PsiElement body = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); - String methodName = null; - if (body instanceof PsiMethodCallExpression) { - PsiMethodCallExpression methodCall = (PsiMethodCallExpression)body; - PsiExpression[] args = methodCall.getArgumentList().getExpressions(); - if (args.length == 1 && MethodCallUtils.isCompareToCall(methodCall)) { - PsiExpression left = methodCall.getMethodExpression().getQualifierExpression(); - if (left == null) { - return; - } - PsiExpression right = args[0]; - if (left instanceof PsiReferenceExpression && right instanceof PsiReferenceExpression) { - PsiElement leftElement = ((PsiReferenceExpression)left).resolve(); - PsiElement rightElement = ((PsiReferenceExpression)right).resolve(); - if (leftElement == parameters[0] && rightElement == parameters[1]) { - methodName = "naturalOrder"; - } - else if (leftElement == parameters[1] && rightElement == parameters[0]) { - methodName = "reverseOrder"; - } - } - if (methodName == null && areEquivalent(parameters, left, right)) { - methodName = "comparing"; - } - } - else { - PsiMethod method = methodCall.resolveMethod(); - if (args.length == 2 && method != null && method.getName().equals("compare")) { - PsiClass compareClass = method.getContainingClass(); - if (compareClass != null) { - methodName = getComparingMethodName(compareClass.getQualifiedName()); - if (!areEquivalent(parameters, args[0], args[1])) return; - } - } - } - } else if (body instanceof PsiBinaryExpression) { - PsiBinaryExpression binOp = (PsiBinaryExpression)body; - if (binOp.getOperationTokenType().equals(JavaTokenType.MINUS) && - areEquivalent(parameters, binOp.getLOperand(), binOp.getROperand())) { - PsiType opType = binOp.getLOperand().getType(); - if(opType == null) return; - methodName = getComparingMethodName(opType.getCanonicalText()); - } + String replacementText = generateSimpleCombinator(lambda, parameters[0], parameters[1]); + if (replacementText != null) { + if (!isSafeReplacement(lambda, replacementText)) return; + String qualifiedName = Objects.requireNonNull(StringUtil.substringBefore(replacementText, "(")); + String methodName = StringUtil.getShortName(qualifiedName); + holder + .registerProblem(lambda, "Can be replaced with Comparator." + methodName, + ProblemHighlightType.LIKE_UNUSED_SYMBOL, new ReplaceWithComparatorFix("Comparator." + methodName)); + return; } - else if (lambda.getBody() instanceof PsiCodeBlock) { + if (lambda.getBody() instanceof PsiCodeBlock) { PsiStatement[] statements = ((PsiCodeBlock)lambda.getBody()).getStatements(); - if (extractComparisonChain(statements, parameters[0], parameters[1]) == null) return; + List blocks = extractComparisonChain(statements, parameters[0], parameters[1]); + if (blocks == null) return; + String chainCombinator = generateChainCombinator(blocks, parameters[0], parameters[1]); + if (chainCombinator == null) return; + if (!isSafeReplacement(lambda, chainCombinator)) return; holder .registerProblem(lambda, "Can be replaced with Comparator chain", ProblemHighlightType.LIKE_UNUSED_SYMBOL, new ReplaceWithComparatorFix("Comparator chain")); } - if (methodName != null) { - holder - .registerProblem(lambda, "Can be replaced with Comparator." + methodName, - ProblemHighlightType.LIKE_UNUSED_SYMBOL, new ReplaceWithComparatorFix("Comparator." + methodName)); - } + } + + private boolean isSafeReplacement(PsiLambdaExpression lambda, String replacementText) { + return LambdaUtil.isSafeLambdaReplacement(lambda, () -> JavaPsiFacade.getElementFactory(holder.getProject()) + .createExpressionFromText(replacementText, lambda.getParent())); } }; } @@ -299,21 +270,7 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec PsiExpression second) { String secondParamName = secondParam.getName(); if (secondParamName == null) return false; - PsiExpression firstCopy = replaceVariableInExpression(firstParam, first, secondParamName); - return ourEquivalence.expressionsAreEquivalent(firstCopy, second); - } - - @NotNull - private static PsiExpression replaceVariableInExpression(@NotNull PsiVariable variable, - PsiExpression expression, - String newName) { - PsiExpression qualifierCopy = (PsiExpression)expression.copy(); - for (PsiReference ref : ReferencesSearch.search(variable, new LocalSearchScope(qualifierCopy))) { - if (ref instanceof PsiReferenceExpression) { - ExpressionUtils.bindReferenceTo((PsiReferenceExpression)ref, newName); - } - } - return qualifierCopy; + return areEquivalent(firstParam, first, secondParam, second); } private static boolean usagesAreAllowed(@NotNull PsiVariable firstParam, @@ -339,9 +296,64 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec } @Nullable - private static String generateCode(@NotNull List blocks, - @NotNull PsiVariable firstVar, - @NotNull PsiVariable secondVar) { + private static String generateSimpleCombinator(PsiLambdaExpression lambda, + PsiParameter leftVar, PsiParameter rightVar) { + PsiElement body = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); + PsiExpression left; + String methodName = null; + if (body instanceof PsiMethodCallExpression) { + PsiMethodCallExpression methodCall = (PsiMethodCallExpression)body; + if (MethodCallUtils.isCompareToCall(methodCall)) { + left = methodCall.getMethodExpression().getQualifierExpression(); + PsiExpression right = ArrayUtil.getFirstElement(methodCall.getArgumentList().getExpressions()); + if (ExpressionUtils.isReferenceTo(left, leftVar) && ExpressionUtils.isReferenceTo(right, rightVar)) { + methodName = "naturalOrder"; + } + else if (ExpressionUtils.isReferenceTo(right, leftVar) && ExpressionUtils.isReferenceTo(left, rightVar)) { + methodName = "reverseOrder"; + } + else if (areEquivalent(leftVar, left, rightVar, right)) { + methodName = "comparing"; + } + } + else { + PrimitiveComparison comparison = PrimitiveComparison.from(methodCall); + if (comparison == null) return null; + PsiExpression[] args = methodCall.getArgumentList().getExpressions(); + if (!areEquivalent(leftVar, args[0], rightVar, args[1])) return null; + methodName = comparison.getMethodName(); + left = comparison.getKeyExtractor(); + } + } + else if (body instanceof PsiBinaryExpression) { + PsiBinaryExpression binOp = (PsiBinaryExpression)body; + if (!binOp.getOperationTokenType().equals(JavaTokenType.MINUS)) return null; + left = binOp.getLOperand(); + PsiType type = left.getType(); + if (type == null) return null; + if (areEquivalent(leftVar, left, rightVar, binOp.getROperand())) { + methodName = getComparingMethodName(type.getCanonicalText()); + } + } + else { + return null; + } + if (methodName == null) return null; + String parameterName = leftVar.getName(); + String text; + if (!methodName.startsWith("comparing")) { + text = "java.util.Comparator." + methodName + "()"; + } + else { + text = "java.util.Comparator." + methodName + "(" + (parameterName + " -> " + left.getText()) + ")"; + } + return text; + } + + @Nullable + private static String generateChainCombinator(@NotNull List blocks, + @NotNull PsiVariable firstVar, + @NotNull PsiVariable secondVar) { if (blocks.size() < 2) return null; ComparisonBlock first = blocks.get(0); StringBuilder builder = new StringBuilder(); @@ -436,20 +448,19 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec return null; } - @Contract("_, null, _ -> false; _, !null, null -> false") - private static boolean areEquivalent(@NotNull PsiParameter[] parameters, @Nullable PsiExpression left, @Nullable PsiExpression right) { + @Contract("_, null, _, _ -> false; _, !null, _, null -> false") + private static boolean areEquivalent(PsiVariable leftVar, @Nullable PsiExpression left, + PsiVariable rightVar, @Nullable PsiExpression right) { if (left == null || right == null) return false; - if (!PsiTreeUtil.processElements(left, e -> !(e instanceof PsiReferenceExpression) || - ((PsiReferenceExpression)e).resolve() != parameters[1]) || - !PsiTreeUtil.processElements(right, e -> !(e instanceof PsiReferenceExpression) || - ((PsiReferenceExpression)e).resolve() != parameters[0])) { + if (VariableAccessUtils.variableIsUsed(rightVar, left) || + VariableAccessUtils.variableIsUsed(leftVar, right)) { return false; } PsiExpression copy = (PsiExpression)right.copy(); PsiElement[] rightRefs = PsiTreeUtil.collectElements(copy, e -> e instanceof PsiReferenceExpression && - ((PsiReferenceExpression)e).resolve() == parameters[1]); + ((PsiReferenceExpression)e).resolve() == rightVar); PsiElementFactory factory = JavaPsiFacade.getElementFactory(left.getProject()); - String paramName = parameters[0].getName(); + String paramName = leftVar.getName(); if (paramName == null) return false; for (PsiElement ref : rightRefs) { PsiElement nameElement = ((PsiReferenceExpression)ref).getReferenceNameElement(); @@ -527,12 +538,12 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec PsiLambdaExpression lambda = (PsiLambdaExpression)element; PsiParameter[] parameters = lambda.getParameterList().getParameters(); if (parameters.length != 2) return; - if (lambda.getBody() instanceof PsiCodeBlock ) { + if (lambda.getBody() instanceof PsiCodeBlock) { PsiStatement[] statements = ((PsiCodeBlock)lambda.getBody()).getStatements(); if(statements.length > 1) { List chain = extractComparisonChain(statements, parameters[0], parameters[1]); if (chain == null) return; - String code = generateCode(chain, parameters[0], parameters[1]); + String code = generateChainCombinator(chain, parameters[0], parameters[1]); if (code == null) return; PsiElement result = new CommentTracker().replaceAndRestoreComments(lambda, code); PsiDiamondTypeUtil.removeRedundantTypeArguments(result); @@ -541,51 +552,12 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec return; } } - PsiElement body = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); - PsiExpression keyExtractor = null; - String methodName = null; - if (body instanceof PsiMethodCallExpression) { - PsiMethodCallExpression methodCall = (PsiMethodCallExpression)body; - if (MethodCallUtils.isCompareToCall(methodCall)) { - methodName = "comparing"; - keyExtractor = methodCall.getMethodExpression().getQualifierExpression(); - if (keyExtractor instanceof PsiReferenceExpression) { - PsiElement keyElement = ((PsiReferenceExpression)keyExtractor).resolve(); - if (keyElement == parameters[0]) { - methodName = "naturalOrder"; - } - else if (keyElement == parameters[1]) { - methodName = "reverseOrder"; - } - } - } - else { - PrimitiveComparison comparison = PrimitiveComparison.from(methodCall); - if(comparison == null) return; - methodName = comparison.getMethodName(); - keyExtractor = comparison.getKeyExtractor(); - } - } else if(body instanceof PsiBinaryExpression) { - PsiBinaryExpression binOp = (PsiBinaryExpression)body; - if(!binOp.getOperationTokenType().equals(JavaTokenType.MINUS)) return; - keyExtractor = binOp.getLOperand(); - PsiType type = keyExtractor.getType(); - if(type == null) return; - methodName = getComparingMethodName(type.getCanonicalText()); - } - if (methodName == null || keyExtractor == null) return; - String parameterName = parameters[0].getName(); + String text = generateSimpleCombinator(lambda, parameters[0], parameters[1]); + if (text == null) return; PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); - PsiElement result; - if (!methodName.startsWith("comparing")) { - result = lambda.replace(factory.createExpressionFromText("java.util.Comparator." + methodName + "()", element)); - } - else { - String newLambda = parameterName + " -> " + keyExtractor.getText(); - PsiExpression replacement = factory.createExpressionFromText("java.util.Comparator." + methodName + "(" + newLambda + ")", element); - result = lambda.replace(replacement); - normalizeLambda(((PsiMethodCallExpression)result).getArgumentList().getExpressions()[0], factory); - } + PsiExpression replacement = factory.createExpressionFromText(text, element); + PsiMethodCallExpression result = (PsiMethodCallExpression)lambda.replace(replacement); + normalizeLambda(ArrayUtil.getFirstElement(result.getArgumentList().getExpressions()), factory); CodeStyleManager.getInstance(project).reformat(JavaCodeStyleManager.getInstance(project).shortenClassReferences(result)); } diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java index 97f5129caf14..0ab19a9489ad 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java @@ -374,8 +374,8 @@ public class PointlessBooleanExpressionInspection extends BaseInspection { } if (parent instanceof PsiLambdaExpression && - !LambdaUtil.isSameOverloadAfterReplacement((PsiLambdaExpression)parent, - () -> { + !LambdaUtil.isSafeLambdaBodyReplacement((PsiLambdaExpression)parent, + () -> { String simplifiedReturnExpression = buildSimplifiedExpression(expression, new StringBuilder(), new CommentTracker()).toString(); return JavaPsiFacade.getElementFactory(expression.getProject()).createExpressionFromText(simplifiedReturnExpression, expression); })) {