IDEA-194142 IDEA should recognize min/max usage with reversed order

java/java-impl/src/META-INF/JavaPlugin.xml

@@ -789,7 +789,7 @@
     <localInspection groupPath="Java" language="JAVA" shortName="RedundantComparatorComparing"
                      groupBundle="messages.InspectionsBundle" groupKey="group.names.verbose.or.redundant.code.constructs"
                      enabledByDefault="true" level="WARNING"
-                     key="inspection.redundant.comparator.comparing.display.name" bundle="messages.InspectionsBundle"
+                     key="inspection.simplifiable.comparator.display.name" bundle="messages.InspectionsBundle"
                      implementationClass="com.intellij.codeInspection.RedundantComparatorComparingInspection"/>
     <localInspection groupPath="Java" language="JAVA" shortName="SuspiciousListRemoveInLoop"
                      groupBundle="messages.InspectionsBundle" groupKey="group.names.probable.bugs"

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/afterComparingMinMaxReversed.java

@@ -0,0 +1,12 @@
+// "Fix all 'Comparator can be simplified' problems in file" "true"
+import java.util.*;
+import java.util.stream.*;
+
+class Test {
+  void test(List<String> list) {
+    Collections.min(list, Comparator.naturalOrder());
+    list.stream().max(String.CASE_INSENSITIVE_ORDER);
+    list.stream().min(String.CASE_INSENSITIVE_ORDER);
+    Collector<String, ?, Optional<String>> c = Collectors.minBy(Comparator.comparing(String::length));
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/beforeComparingMinMaxReversed.java

@@ -0,0 +1,12 @@
+// "Fix all 'Comparator can be simplified' problems in file" "true"
+import java.util.*;
+import java.util.stream.*;
+
+class Test {
+  void test(List<String> list) {
+    Collections.m<caret>ax(list, Comparator.reverseOrder());
+    list.stream().min(String.CASE_INSENSITIVE_ORDER.reversed());
+    list.stream().max(Collections.reverseOrder(String.CASE_INSENSITIVE_ORDER));
+    Collector<String, ?, Optional<String>> c = Collectors.maxBy(Comparator.comparing(String::length, Comparator.reverseOrder()));
+  }
+}

java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/RedundantComparatorComparingInspectionTest.java

@@ -20,6 +20,6 @@ public class RedundantComparatorComparingInspectionTest extends LightQuickFixPar
 
   @Override
   protected String getBasePath() {
-    return "/codeInsight/daemonCodeAnalyzer/quickFix/redundantComparing";
+    return "/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified";
   }
 }

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -951,11 +951,14 @@ inspection.conditional.break.in.infinite.loop.description=Conditional break insi
 
 inspection.endless.stream.description=Non-short-circuit operation consumes the infinite stream
 
-inspection.redundant.comparator.comparing.display.name=Redundant Comparator.comparing
-inspection.redundant.comparator.comparing.message=Unnecessary ''{0}()'' call
-inspection.redundant.comparator.comparing.fix.remove.name=Remove ''{0}()'' call
-inspection.redundant.comparator.comparing.fix.replace.name=Remove ''{0}()'' call and use ''{1}()''
-inspection.redundant.comparator.comparing.fix.family.name=Remove redundant call
+inspection.simplifiable.comparator.display.name=Comparator can be simplified
+inspection.simplifiable.comparator.comparing.message=Unnecessary ''{0}()'' call
+inspection.simplifiable.comparator.reversed.message=Comparator can be simplified if ''{0}()'' call is replaced with ''{1}()''
+inspection.simplifiable.comparator.fix.comparing.family.name=Remove redundant call
+inspection.simplifiable.comparator.fix.remove.name=Remove ''{0}()'' call
+inspection.simplifiable.comparator.fix.replace.name=Remove ''{0}()'' call and use ''{1}()''
+inspection.simplifiable.comparator.fix.reversed.family.name=Simplify comparator replacing 'max' with 'min'
+inspection.simplifiable.comparator.fix.reversed.name=Replace with ''{0}'' simplifying the comparator
 
 inspection.capturing.cleaner=Runnable passed to Cleaner.register() captures ''{0}'' reference
 inspection.capturing.cleaner.description=Cleaner captures object reference

plugins/InspectionGadgets/src/com/intellij/codeInspection/RedundantComparatorComparingInspection.java

@@ -2,24 +2,23 @@
 package com.intellij.codeInspection;
 
 import com.intellij.openapi.project.Project;
-import com.intellij.psi.JavaElementVisitor;
-import com.intellij.psi.PsiElementVisitor;
-import com.intellij.psi.PsiExpressionList;
-import com.intellij.psi.PsiMethodCallExpression;
+import com.intellij.psi.*;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
-import com.intellij.util.ObjectUtils;
+import com.intellij.util.ArrayUtil;
 import com.siyeh.ig.callMatcher.CallMapper;
 import com.siyeh.ig.callMatcher.CallMatcher;
 import com.siyeh.ig.psiutils.CommentTracker;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
-import static com.intellij.psi.CommonClassNames.JAVA_UTIL_COMPARATOR;
-import static com.intellij.psi.CommonClassNames.JAVA_UTIL_FUNCTION_FUNCTION;
-import static com.siyeh.ig.callMatcher.CallMatcher.instanceCall;
-import static com.siyeh.ig.callMatcher.CallMatcher.staticCall;
+import java.util.Objects;
+
+import static com.intellij.psi.CommonClassNames.*;
+import static com.intellij.util.ObjectUtils.tryCast;
+import static com.siyeh.ig.callMatcher.CallMatcher.*;
 
 public class RedundantComparatorComparingInspection extends AbstractBaseJavaLocalInspectionTool {
   private static final CallMatcher THEN_COMPARING_COMPARATOR = instanceCall(JAVA_UTIL_COMPARATOR, "thenComparing")
@@ -27,10 +26,22 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca
   private static final CallMatcher THEN_COMPARING_FUNCTION = instanceCall(JAVA_UTIL_COMPARATOR, "thenComparing")
     .parameterTypes(JAVA_UTIL_FUNCTION_FUNCTION);
 
+  private static final CallMatcher COMPARATOR_REVERSED = instanceCall(JAVA_UTIL_COMPARATOR, "reversed").parameterCount(0);
+  private static final CallMatcher REVERSE_ORDER_FOR_COMPARATOR = staticCall(JAVA_UTIL_COLLECTIONS, "reverseOrder")
+    .parameterTypes(JAVA_UTIL_COMPARATOR);
+  private static final CallMatcher REVERSE_ORDER_FOR_NATURAL = staticCall(JAVA_UTIL_COMPARATOR, "reverseOrder").parameterCount(0);
+  private static final CallMatcher COMPARATOR_COMPARING_WITH_DOWNSTREAM =
+    staticCall(JAVA_UTIL_COMPARATOR, "comparing").parameterTypes(JAVA_UTIL_FUNCTION_FUNCTION, JAVA_UTIL_COMPARATOR);
+
+  private static final CallMatcher MIN_MAX = anyOf(
+    staticCall(JAVA_UTIL_COLLECTIONS, "min", "max").parameterTypes(JAVA_UTIL_COLLECTION, JAVA_UTIL_COMPARATOR),
+    instanceCall(JAVA_UTIL_STREAM_STREAM, "min", "max").parameterTypes(JAVA_UTIL_COMPARATOR),
+    staticCall(JAVA_UTIL_STREAM_COLLECTORS, "minBy", "maxBy").parameterTypes(JAVA_UTIL_COMPARATOR)
+  );
+
   private static final CallMapper<String> REPLACEMENTS = new CallMapper<String>()
     .register(staticCall(JAVA_UTIL_COMPARATOR, "comparing").parameterTypes(JAVA_UTIL_FUNCTION_FUNCTION), "thenComparing")
-    .register(staticCall(JAVA_UTIL_COMPARATOR, "comparing").parameterTypes(JAVA_UTIL_FUNCTION_FUNCTION, JAVA_UTIL_COMPARATOR),
-              "thenComparing")
+    .register(COMPARATOR_COMPARING_WITH_DOWNSTREAM, "thenComparing")
     .register(staticCall(JAVA_UTIL_COMPARATOR, "comparingInt").parameterCount(1), "thenComparingInt")
     .register(staticCall(JAVA_UTIL_COMPARATOR, "comparingLong").parameterCount(1), "thenComparingLong")
     .register(staticCall(JAVA_UTIL_COMPARATOR, "comparingDouble").parameterCount(1), "thenComparingDouble");
@@ -44,9 +55,26 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca
     return new JavaElementVisitor() {
       @Override
       public void visitMethodCallExpression(PsiMethodCallExpression call) {
-        if (!THEN_COMPARING_COMPARATOR.test(call)) return;
+        if (THEN_COMPARING_COMPARATOR.test(call)) {
+          checkThenComparing(call);
+        }
+        if (MIN_MAX.test(call)) {
+          PsiExpression arg = ArrayUtil.getLastElement(call.getArgumentList().getExpressions());
+          if (getPlainComparatorExpressionFromReversed(arg, new CommentTracker()) != null) {
+            PsiElement nameElement = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement());
+
+            String maxOrMin = nameElement.getText();
+            String replacement = getMaxMinReplacement(maxOrMin);
+            holder.registerProblem(nameElement,
+                                   InspectionsBundle.message("inspection.simplifiable.comparator.reversed.message", maxOrMin, replacement),
+                                   new ReplaceMaxMinFix(replacement));
+          }
+        }
+      }
+
+      private void checkThenComparing(@NotNull PsiMethodCallExpression call) {
         PsiMethodCallExpression comparingCall =
-          ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(call.getArgumentList().getExpressions()[0]), PsiMethodCallExpression.class);
+          tryCast(PsiUtil.skipParenthesizedExprDown(call.getArgumentList().getExpressions()[0]), PsiMethodCallExpression.class);
         String targetMethod = REPLACEMENTS.mapFirst(comparingCall);
         if (targetMethod == null) return;
         PsiExpressionList comparingArgs = comparingCall.getArgumentList();
@@ -59,12 +87,41 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca
         String name = comparingCall.getMethodExpression().getReferenceName();
         holder
           .registerProblem(comparingCall.getMethodExpression(),
-                           InspectionsBundle.message("inspection.redundant.comparator.comparing.message", name),
+                           InspectionsBundle.message("inspection.simplifiable.comparator.comparing.message", name),
                            ProblemHighlightType.LIKE_UNUSED_SYMBOL, new DeleteComparingCallFix(name, targetMethod));
       }
     };
   }
 
+  @NotNull
+  private static String getMaxMinReplacement(@NotNull String maxOrMin) {
+    return (maxOrMin.startsWith("max") ? "min" : "max")+maxOrMin.substring(3);
+  }
+
+  @Nullable
+  static String getPlainComparatorExpressionFromReversed(PsiExpression expression, CommentTracker ct) {
+    PsiMethodCallExpression call = tryCast(PsiUtil.skipParenthesizedExprDown(expression), PsiMethodCallExpression.class);
+    if (call == null) return null;
+    if (COMPARATOR_REVERSED.test(call)) {
+      PsiExpression qualifier = call.getMethodExpression().getQualifierExpression();
+      return qualifier == null ? null : ct.text(qualifier);
+    }
+    if (REVERSE_ORDER_FOR_COMPARATOR.test(call)) {
+      PsiExpression arg = call.getArgumentList().getExpressions()[0];
+      if (call.getType() != null && call.getType().equals(arg.getType())) {
+        return ct.text(arg);
+      }
+    }
+    if (REVERSE_ORDER_FOR_NATURAL.test(call)) {
+      PsiReferenceParameterList parameterList = call.getMethodExpression().getParameterList();
+      return JAVA_UTIL_COMPARATOR + "."+(parameterList == null ? "" : ct.text(parameterList))+"naturalOrder()";
+    }
+    if (COMPARATOR_COMPARING_WITH_DOWNSTREAM.test(call) && REVERSE_ORDER_FOR_NATURAL.matches(call.getArgumentList().getExpressions()[1])) {
+      return ct.text(call.getMethodExpression()) + "(" + ct.text(call.getArgumentList().getExpressions()[0]) + ")";
+    }
+    return null;
+  }
+
   static class DeleteComparingCallFix implements LocalQuickFix {
     private final String mySourceMethod;
     private final String myTargetMethod;
@@ -79,15 +136,15 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca
     @Override
     public String getName() {
       return myTargetMethod.equals("thenComparing")
-             ? InspectionsBundle.message("inspection.redundant.comparator.comparing.fix.remove.name", mySourceMethod)
-             : InspectionsBundle.message("inspection.redundant.comparator.comparing.fix.replace.name", mySourceMethod, myTargetMethod);
+             ? InspectionsBundle.message("inspection.simplifiable.comparator.fix.remove.name", mySourceMethod)
+             : InspectionsBundle.message("inspection.simplifiable.comparator.fix.replace.name", mySourceMethod, myTargetMethod);
     }
 
     @Nls
     @NotNull
     @Override
     public String getFamilyName() {
-      return InspectionsBundle.message("inspection.redundant.comparator.comparing.fix.family.name");
+      return InspectionsBundle.message("inspection.simplifiable.comparator.fix.comparing.family.name");
     }
 
     @Override
@@ -101,4 +158,39 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca
       thenComparingCall.getArgumentList().replace(ct.markUnchanged(comparingCall.getArgumentList()));
     }
   }
+
+  private static class ReplaceMaxMinFix implements LocalQuickFix {
+    private final String myReplacement;
+
+    public ReplaceMaxMinFix(String replacement) {
+      myReplacement = replacement;
+    }
+
+    @Nls(capitalization = Nls.Capitalization.Sentence)
+    @NotNull
+    @Override
+    public String getName() {
+      return InspectionsBundle.message("inspection.simplifiable.comparator.fix.reversed.name", myReplacement);
+    }
+
+    @Nls(capitalization = Nls.Capitalization.Sentence)
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return InspectionsBundle.message("inspection.simplifiable.comparator.fix.reversed.family.name");
+    }
+
+    @Override
+    public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
+      PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class);
+      if (call == null) return;
+      PsiExpression comparator = ArrayUtil.getLastElement(call.getArgumentList().getExpressions());
+      if (comparator == null) return;
+      CommentTracker ct = new CommentTracker();
+      String reversed = getPlainComparatorExpressionFromReversed(comparator, ct);
+      if (reversed == null) return;
+      ct.replaceAndRestoreComments(comparator, reversed);
+      ExpressionUtils.bindCallTo(call, myReplacement);
+    }
+  }
 }