IDEA-161007 Add new inspection to make comparator lambdas use Comparator.comparing() combinators (Currently only simple comparators supported)

2026-09-13 09:19:13 +07:00 · 2016-09-12 17:50:47 +07:00
parent 34ed877b9d
commit dc37f19e2b
9 changed files with 252 additions and 34 deletions
@@ -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));
+    }
+  }
+}
@@ -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;
@@ -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;
+  }
 }
@@ -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<Person> persons) {
+    persons.sort(Comparator.comparing(Person::getName));
+  }
+}
@@ -0,0 +1,13 @@
+// "Replace with Comparator.comparing" "false"
+
+import java.util.List;
+
+public class Main {
+  interface Person {
+    String getName();
+  }
+
+  void sort(List<Person> persons) {
+    persons.sort((p1, p2) -> p2.getNam<caret>e().compareTo(p1.getName()));
+  }
+}
@@ -0,0 +1,13 @@
+// "Replace with Comparator.comparing" "true"
+
+import java.util.List;
+
+public class Main {
+  interface Person {
+    String getName();
+  }
+
+  void sort(List<Person> persons) {
+    persons.sort((p1, p2) -> p1.getNam<caret>e().compareTo(p2.getName()));
+  }
+}
@@ -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";
+  }
+}
@@ -0,0 +1,10 @@
+<html>
+<body>
+Inspection looks for Comparators defined as lambda expressions which could be expressed using
+methods like <code>Comparator.comparing()</code>.
+<!-- tooltip end -->
+Some comparators like <code>(person1, person2) -> person1.getName().compareTo(person2.getName())</code>
+could be simplified like this: <code>Comparator.comparing(Person::getName)</code>.
+<small>New in 2016.3</small>
+</body>
+</html>
@@ -820,6 +820,10 @@
                     groupKey="group.names.declaration.redundancy" enabledByDefault="true" level="WARNING"
                     implementationClass="com.intellij.codeInspection.SimplifyStreamApiCallChainsInspection"
                     displayName="Simplify stream API call chains"/>
+    <localInspection groupPath="Java" language="JAVA" shortName="ComparatorCombinatorsInspection"
+                     groupBundle="messages.InspectionsBundle"
+                     groupKey="group.names.language.level.specific.issues.and.migration.aids" enabledByDefault="true" level="WARNING"
+                     implementationClass="com.intellij.codeInspection.ComparatorCombinatorsInspection"/>
    <localInspection groupPath="Java" language="JAVA" shortName="ReplaceInefficientStreamCount"
                     groupBundle="messages.InspectionsBundle"
                     groupKey="group.names.performance.issues" enabledByDefault="true" level="WARNING"