From dc37f19e2b79a9935eddc1a6d72adf486f4d04e1 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 12 Sep 2016 17:48:47 +0700 Subject: [PATCH] IDEA-161007 Add new inspection to make comparator lambdas use Comparator.comparing() combinators (Currently only simple comparators supported) --- .../ComparatorCombinatorsInspection.java | 123 ++++++++++++++++++ .../ConvertCompareToToEqualsIntention.java | 36 +---- .../com/intellij/psi/util/PsiMethodUtil.java | 35 ++++- .../comparatorCombinators/afterSimple.java | 14 ++ .../comparatorCombinators/beforeInverted.java | 13 ++ .../comparatorCombinators/beforeSimple.java | 13 ++ .../ComparatorCombinatorsInspectionTest.java | 38 ++++++ .../ComparatorCombinators.html | 10 ++ resources/src/META-INF/IdeaPlugin.xml | 4 + 9 files changed, 252 insertions(+), 34 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterSimple.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeInverted.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeSimple.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ComparatorCombinatorsInspectionTest.java create mode 100644 resources-en/src/inspectionDescriptions/ComparatorCombinators.html diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java new file mode 100644 index 000000000000..f73513ce19f8 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java @@ -0,0 +1,123 @@ +/* + * Copyright 2000-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection; + +import com.intellij.codeInsight.FileModificationService; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.codeStyle.CodeStyleManager; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.util.PsiMethodUtil; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +public class ComparatorCombinatorsInspection extends BaseJavaBatchLocalInspectionTool { + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitLambdaExpression(PsiLambdaExpression lambda) { + super.visitLambdaExpression(lambda); + PsiType type = lambda.getFunctionalInterfaceType(); + if(type instanceof PsiClassType && ((PsiClassType)type).rawType().equalsToText(CommonClassNames.JAVA_UTIL_COMPARATOR)) { + PsiElement body = lambda.getBody(); + if(body instanceof PsiMethodCallExpression) { + PsiMethodCallExpression methodCall = (PsiMethodCallExpression)body; + if(PsiMethodUtil.isCompareToCall(methodCall)) { + PsiExpression left = methodCall.getMethodExpression().getQualifierExpression(); + PsiExpression right = methodCall.getArgumentList().getExpressions()[0]; + if(left instanceof PsiMethodCallExpression && right instanceof PsiMethodCallExpression) { + PsiMethodCallExpression leftCall = (PsiMethodCallExpression)left; + PsiMethodCallExpression rightCall = (PsiMethodCallExpression)right; + if(leftCall.getArgumentList().getExpressions().length == 0 && + rightCall.getArgumentList().getExpressions().length == 0) { + PsiMethod leftMethod = leftCall.resolveMethod(); + PsiMethod rightMethod = rightCall.resolveMethod(); + if(leftMethod != null && rightMethod != null && leftMethod == rightMethod) { + if (areLambdaParameters(lambda, leftCall.getMethodExpression().getQualifierExpression(), + rightCall.getMethodExpression().getQualifierExpression())) { + //noinspection DialogTitleCapitalization + holder.registerProblem(lambda, "Can be replaced with Comparator.comparing", new ReplaceWithComparatorFix()); + } + } + } + } + } + } + } + } + }; + } + + private static boolean areLambdaParameters(PsiLambdaExpression lambda, PsiExpression left, PsiExpression right) { + PsiParameter[] parameters = lambda.getParameterList().getParameters(); + return left instanceof PsiReferenceExpression && + right instanceof PsiReferenceExpression && + ((PsiReferenceExpression)left).resolve() == parameters[0] && + ((PsiReferenceExpression)right).resolve() == parameters[1]; + } + + @Nls + @NotNull + @Override + public String getDisplayName() { + return "Use Comparator combinators"; + } + + static class ReplaceWithComparatorFix implements LocalQuickFix { + + @Nls + @NotNull + @Override + public String getName() { + return getFamilyName(); + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return "Replace with Comparator.comparing"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement element = descriptor.getStartElement(); + if (!(element instanceof PsiLambdaExpression)) return; + PsiLambdaExpression lambda = (PsiLambdaExpression)element; + PsiElement body = lambda.getBody(); + if (!(body instanceof PsiMethodCallExpression)) return; + PsiMethodCallExpression methodCall = (PsiMethodCallExpression)body; + if (!PsiMethodUtil.isCompareToCall(methodCall)) return; + PsiExpression qualifier = methodCall.getMethodExpression().getQualifierExpression(); + if (!(qualifier instanceof PsiMethodCallExpression)) return; + PsiMethodCallExpression call = (PsiMethodCallExpression)qualifier; + if (call.getArgumentList().getExpressions().length != 0) return; + PsiMethod method = call.resolveMethod(); + if (method == null) return; + PsiClass methodClass = method.getContainingClass(); + if (methodClass == null) return; + if (!FileModificationService.getInstance().preparePsiElementForWrite(element)) return; + PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); + PsiExpression replacement = + factory.createExpressionFromText("java.util.Comparator.comparing(" + methodClass.getQualifiedName() + "::" + method.getName() + ")", + element); + PsiElement result = lambda.replace(replacement); + CodeStyleManager.getInstance(project).reformat(JavaCodeStyleManager.getInstance(project).shortenClassReferences(result)); + } + } +} diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java index 88cd821cd19c..f81bfcd5684f 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2016 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,7 +22,7 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.Pair; import com.intellij.psi.*; -import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.util.PsiMethodUtil; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NotNull; @@ -80,7 +80,7 @@ public class ConvertCompareToToEqualsIntention extends BaseElementAtCaretIntenti PsiMethodCallExpression compareToExpression = null; boolean hasZero = false; for (PsiExpression psiExpression : binaryExpression.getOperands()) { - if (compareToExpression == null && detectCompareTo(psiExpression)) { + if (compareToExpression == null && PsiMethodUtil.isCompareToCall(psiExpression)) { compareToExpression = (PsiMethodCallExpression)psiExpression; continue; } @@ -95,36 +95,6 @@ public class ConvertCompareToToEqualsIntention extends BaseElementAtCaretIntenti return new ResolveResult(binaryExpression, compareToExpression, isEqEq); } - private static boolean detectCompareTo(final @NotNull PsiExpression expression) { - if (!(expression instanceof PsiMethodCallExpression)) { - return false; - } - final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression; - if (methodCallExpression.getMethodExpression().getQualifierExpression() == null) { - return false; - } - final PsiMethod psiMethod = methodCallExpression.resolveMethod(); - if (psiMethod == null || !"compareTo".equals(psiMethod.getName()) || psiMethod.getParameterList().getParametersCount() != 1) { - return false; - } - if (methodCallExpression.getArgumentList().getExpressions().length != 1) { - return false; - } - final PsiClass containingClass = psiMethod.getContainingClass(); - if (containingClass == null) { - return false; - } - final PsiClass javaLangComparable = JavaPsiFacade.getInstance(expression.getProject()).findClass(CommonClassNames.JAVA_LANG_COMPARABLE, GlobalSearchScope.allScope( - expression.getProject())); - if (javaLangComparable == null) { - return false; - } - if (!containingClass.isInheritor(javaLangComparable, true)) { - return false; - } - return true; - } - private static boolean detectZero(final @NotNull PsiExpression expression) { if (!(expression instanceof PsiLiteralExpression)) { return false; diff --git a/java/java-psi-api/src/com/intellij/psi/util/PsiMethodUtil.java b/java/java-psi-api/src/com/intellij/psi/util/PsiMethodUtil.java index 7a4d333b15f5..5082112471fa 100644 --- a/java/java-psi-api/src/com/intellij/psi/util/PsiMethodUtil.java +++ b/java/java-psi-api/src/com/intellij/psi/util/PsiMethodUtil.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2016 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import com.intellij.codeInsight.runner.JavaMainMethodProvider; import com.intellij.openapi.extensions.Extensions; import com.intellij.openapi.util.Condition; import com.intellij.psi.*; +import com.intellij.psi.search.GlobalSearchScope; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** @@ -89,4 +91,35 @@ public class PsiMethodUtil { } return findMainMethod(aClass); } + + public static boolean isCompareToCall(final @NotNull PsiExpression expression) { + if (!(expression instanceof PsiMethodCallExpression)) { + return false; + } + final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression; + if (methodCallExpression.getMethodExpression().getQualifierExpression() == null) { + return false; + } + final PsiMethod psiMethod = methodCallExpression.resolveMethod(); + if (psiMethod == null || !"compareTo".equals(psiMethod.getName()) || psiMethod.getParameterList().getParametersCount() != 1) { + return false; + } + if (methodCallExpression.getArgumentList().getExpressions().length != 1) { + return false; + } + final PsiClass containingClass = psiMethod.getContainingClass(); + if (containingClass == null) { + return false; + } + final PsiClass javaLangComparable = JavaPsiFacade.getInstance(expression.getProject()).findClass(CommonClassNames.JAVA_LANG_COMPARABLE, GlobalSearchScope + .allScope( + expression.getProject())); + if (javaLangComparable == null) { + return false; + } + if (!containingClass.isInheritor(javaLangComparable, true)) { + return false; + } + return true; + } } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterSimple.java new file mode 100644 index 000000000000..5b3790b30a90 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/afterSimple.java @@ -0,0 +1,14 @@ +// "Replace with Comparator.comparing" "true" + +import java.util.Comparator; +import java.util.List; + +public class Main { + interface Person { + String getName(); + } + + void sort(List persons) { + persons.sort(Comparator.comparing(Person::getName)); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeInverted.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeInverted.java new file mode 100644 index 000000000000..e9e3679ee35d --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeInverted.java @@ -0,0 +1,13 @@ +// "Replace with Comparator.comparing" "false" + +import java.util.List; + +public class Main { + interface Person { + String getName(); + } + + void sort(List persons) { + persons.sort((p1, p2) -> p2.getName().compareTo(p1.getName())); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeSimple.java new file mode 100644 index 000000000000..6a1e8856b116 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators/beforeSimple.java @@ -0,0 +1,13 @@ +// "Replace with Comparator.comparing" "true" + +import java.util.List; + +public class Main { + interface Person { + String getName(); + } + + void sort(List persons) { + persons.sort((p1, p2) -> p1.getName().compareTo(p2.getName())); + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ComparatorCombinatorsInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ComparatorCombinatorsInspectionTest.java new file mode 100644 index 000000000000..308d18534662 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/ComparatorCombinatorsInspectionTest.java @@ -0,0 +1,38 @@ +/* + * Copyright 2000-2016 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInsight.daemon.quickFix; + +import com.intellij.codeInspection.ComparatorCombinatorsInspection; +import com.intellij.codeInspection.LocalInspectionTool; +import org.jetbrains.annotations.NotNull; + + +public class ComparatorCombinatorsInspectionTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{ + new ComparatorCombinatorsInspection() + }; + } + + public void test() throws Exception { doAllTests(); } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCombinators"; + } +} \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/ComparatorCombinators.html b/resources-en/src/inspectionDescriptions/ComparatorCombinators.html new file mode 100644 index 000000000000..d1812075c369 --- /dev/null +++ b/resources-en/src/inspectionDescriptions/ComparatorCombinators.html @@ -0,0 +1,10 @@ + + +Inspection looks for Comparators defined as lambda expressions which could be expressed using +methods like Comparator.comparing(). + +Some comparators like (person1, person2) -> person1.getName().compareTo(person2.getName()) +could be simplified like this: Comparator.comparing(Person::getName). +New in 2016.3 + + \ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index e2d6a8c17c3e..655a60c72fea 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -820,6 +820,10 @@ groupKey="group.names.declaration.redundancy" enabledByDefault="true" level="WARNING" implementationClass="com.intellij.codeInspection.SimplifyStreamApiCallChainsInspection" displayName="Simplify stream API call chains"/> +