From ed28503329f60b73f40fa1dfc596842171c740e0 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Sat, 9 Oct 2021 11:07:06 +0700 Subject: [PATCH] [java-inspections] ComparatorCombinatorsInspection: add explicit lambda parameter type when necessary Fixes IDEA-279693 Replace with Comparator.comparing leads to red code Also: improve RedundantLambdaParameterTypeInspection via isSafeLambdaReplacement GitOrigin-RevId: 8125436ff758fe3e4a770b33933a94e44be199fb --- .../ComparatorCombinatorsInspection.java | 19 ++++---- ...edundantLambdaParameterTypeInspection.java | 48 +++++++++---------- .../comparatorCombinators/afterCast.java | 15 ++++++ .../comparatorCombinators/beforeCast.java | 16 +++++++ .../CallNoTypeArgs.java | 2 +- .../CallNoTypeArgs_after.java | 16 +++++++ ...dantLambdaParameterTypeInspectionTest.java | 2 +- 7 files changed, 83 insertions(+), 35 deletions(-) rename {plugins/InspectionGadgets => java/java-impl-inspections}/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java (97%) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterCast.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeCast.java create mode 100644 java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs_after.java diff --git a/plugins/InspectionGadgets/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java similarity index 97% rename from plugins/InspectionGadgets/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java rename to java/java-impl-inspections/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java index 022e4a79e7b1..d9c873b71085 100644 --- a/plugins/InspectionGadgets/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java @@ -1,6 +1,7 @@ -// Copyright 2000-2020 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. +// Copyright 2000-2021 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.codeInspection; +import com.intellij.codeInspection.lambda.RedundantLambdaParameterTypeInspection; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.text.StringUtil; @@ -21,10 +22,7 @@ import com.siyeh.ig.psiutils.*; import one.util.streamex.StreamEx; import org.jetbrains.annotations.*; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Objects; +import java.util.*; import static com.intellij.util.ObjectUtils.tryCast; @@ -347,7 +345,8 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec else { String parameterName = leftVar.getName(); PsiTypeElement typeElement = leftVar.getTypeElement(); - String parameterDeclaration = typeElement == null ? parameterName : "(" + typeElement.getText() + " " + parameterName + ")"; + String typeText = typeElement == null ? leftVar.getType().getCanonicalText() : typeElement.getText(); + String parameterDeclaration = "(" + typeText + " " + parameterName + ")"; text = "java.util.Comparator." + methodName + "(" + (parameterDeclaration + " -> " + left.getText()) + ")"; } @@ -540,6 +539,7 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec PsiLambdaExpression lambda = (PsiLambdaExpression)element; PsiParameter[] parameters = lambda.getParameterList().getParameters(); if (parameters.length != 2) return; + boolean keepParameterTypes = parameters[0].getTypeElement() != null; if (lambda.getBody() instanceof PsiCodeBlock) { PsiStatement[] statements = ((PsiCodeBlock)lambda.getBody()).getStatements(); if(statements.length > 1) { @@ -559,11 +559,11 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); PsiExpression replacement = factory.createExpressionFromText(text, element); PsiMethodCallExpression result = (PsiMethodCallExpression)lambda.replace(replacement); - normalizeLambda(ArrayUtil.getFirstElement(result.getArgumentList().getExpressions()), factory); + normalizeLambda(ArrayUtil.getFirstElement(result.getArgumentList().getExpressions()), factory, keepParameterTypes); CodeStyleManager.getInstance(project).reformat(JavaCodeStyleManager.getInstance(project).shortenClassReferences(result)); } - private static void normalizeLambda(PsiExpression expression, PsiElementFactory factory) { + private static void normalizeLambda(PsiExpression expression, PsiElementFactory factory, boolean keepParameterTypes) { if (!(expression instanceof PsiLambdaExpression)) return; PsiLambdaExpression lambda = (PsiLambdaExpression)expression; PsiParameter[] parameters = lambda.getParameterList().getParameters(); @@ -578,6 +578,9 @@ public class ComparatorCombinatorsInspection extends AbstractBaseJavaLocalInspec .nonNull().forEach(nameElement -> nameElement.replace(factory.createIdentifier(name))); parameter.setName(name); } + if (!keepParameterTypes) { + RedundantLambdaParameterTypeInspection.removeLambdaParameterTypesIfPossible(lambda); + } } } } diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/lambda/RedundantLambdaParameterTypeInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/lambda/RedundantLambdaParameterTypeInspection.java index bb13ff71a1f7..c4aed0c64c1f 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/lambda/RedundantLambdaParameterTypeInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/lambda/RedundantLambdaParameterTypeInspection.java @@ -8,12 +8,13 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.util.PsiUtil; +import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.psiutils.CommentTracker; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; -import java.util.Arrays; import java.util.Collections; +import java.util.Objects; public class RedundantLambdaParameterTypeInspection extends AbstractBaseJavaLocalInspectionTool { public static final Logger LOG = Logger.getInstance(RedundantLambdaParameterTypeInspection.class); @@ -48,36 +49,34 @@ public class RedundantLambdaParameterTypeInspection extends AbstractBaseJavaLoca } if (parameters.length == 0) return false; final PsiType functionalInterfaceType = expression.getFunctionalInterfaceType(); - if (functionalInterfaceType != null) { - final PsiElement lambdaParent = expression.getParent(); - if (lambdaParent instanceof PsiExpressionList) { - final PsiElement gParent = lambdaParent.getParent(); - if (gParent instanceof PsiCallExpression && ((PsiCallExpression)gParent).getTypeArguments().length == 0) { - final JavaResolveResult resolveResult = ((PsiCallExpression)gParent).resolveMethodGenerics(); - final PsiMethod method = (PsiMethod)resolveResult.getElement(); - if (method == null) return false; - final int idx = LambdaUtil.getLambdaIdx((PsiExpressionList)lambdaParent, expression); - if (idx < 0) return false; - - PsiCallExpression copy = (PsiCallExpression)gParent.copy(); - PsiLambdaExpression lambdaToStripTypeParameters = (PsiLambdaExpression)copy.getArgumentList().getExpressions()[idx]; - for (PsiParameter parameter : lambdaToStripTypeParameters.getParameterList().getParameters()) { - parameter.getTypeElement().delete(); - } - - return functionalInterfaceType.equals(lambdaToStripTypeParameters.getFunctionalInterfaceType()); - } + if (functionalInterfaceType == null) return false; + return LambdaUtil.isSafeLambdaReplacement(expression, () -> { + PsiLambdaExpression lambdaWithoutParameters = (PsiLambdaExpression)expression.copy(); + for (PsiParameter parameter : lambdaWithoutParameters.getParameterList().getParameters()) { + PsiTypeElement typeElement = Objects.requireNonNull(parameter.getTypeElement()); + typeElement.delete(); } - return true; + return lambdaWithoutParameters; + }); + } + + /** + * Removes lambda parameter types when possible + * + * @param lambdaExpression lambda expression to process + */ + public static void removeLambdaParameterTypesIfPossible(@NotNull PsiLambdaExpression lambdaExpression) { + PsiParameterList list = lambdaExpression.getParameterList(); + if (isApplicable(list)) { + removeTypes(lambdaExpression); } - return false; } private static void removeTypes(PsiLambdaExpression lambdaExpression) { if (lambdaExpression != null) { final PsiParameter[] parameters = lambdaExpression.getParameterList().getParameters(); if (PsiUtil.isLanguageLevel11OrHigher(lambdaExpression) && - Arrays.stream(parameters).anyMatch(parameter -> keepVarType(parameter))) { + ContainerUtil.exists(parameters, parameter -> keepVarType(parameter))) { for (PsiParameter parameter : parameters) { PsiTypeElement element = parameter.getTypeElement(); if (element != null) { @@ -101,8 +100,7 @@ public class RedundantLambdaParameterTypeInspection extends AbstractBaseJavaLoca } private static boolean keepVarType(PsiParameter parameter) { - return parameter.hasModifierProperty(PsiModifier.FINAL) || - parameter.getAnnotations().length > 0; + return parameter.hasModifierProperty(PsiModifier.FINAL) || parameter.getAnnotations().length > 0; } private static class LambdaParametersFix implements LocalQuickFix { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterCast.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterCast.java new file mode 100644 index 000000000000..64c8ae7b2401 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterCast.java @@ -0,0 +1,15 @@ +// "Replace with 'Comparator.comparing'" "true" + +import java.util.Comparator; + +class CodeSample { + public void foo() { + final Comparator CMP = (Comparator) Comparator.comparing((Entity o) -> o.getUuid()); + } + + private class Entity { + public Comparable getUuid() { + return null; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeCast.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeCast.java new file mode 100644 index 000000000000..be2e0f486f1d --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeCast.java @@ -0,0 +1,16 @@ +// "Replace with 'Comparator.comparing'" "true" + +import java.util.Comparator; + +class CodeSample { + public void foo() { + final Comparator CMP = (Comparator) (o1, o2) -> o1.getUuid() + .compareTo(o2.getUuid()); + } + + private class Entity { + public Comparable getUuid() { + return null; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs.java b/java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs.java index 094dbbe61308..45c351bb61c3 100644 --- a/java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs.java +++ b/java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs.java @@ -1,4 +1,4 @@ -// "Remove redundant types" "false" +// "Remove redundant types" "true" class Test2 { class Y{ T t; diff --git a/java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs_after.java b/java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs_after.java new file mode 100644 index 000000000000..f21c4f1aabb7 --- /dev/null +++ b/java/java-tests/testData/codeInspection/redundantLambdaParameterType/CallNoTypeArgs_after.java @@ -0,0 +1,16 @@ +// "Remove redundant types" "true" +class Test2 { + class Y{ + T t; + } + + interface I { + X foo(Y list); + } + + static I bar(I i){return i;} + + { + Test2.bar(y -> y.t); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantLambdaParameterTypeInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantLambdaParameterTypeInspectionTest.java index fc001fe49362..021543b02c70 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantLambdaParameterTypeInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/RedundantLambdaParameterTypeInspectionTest.java @@ -61,7 +61,7 @@ public class RedundantLambdaParameterTypeInspectionTest extends LightJavaCodeIns } public void testCallNoTypeArgs() { - assertIntentionNotAvailable(); + doTest(); } public void testCallNoTypeArgs1() {