diff --git a/java/java-impl/src/inspectionDescriptions/RedundantComparatorComparing.html b/java/java-impl/src/inspectionDescriptions/RedundantComparatorComparing.html index a2330ecd4917..0d695851674f 100644 --- a/java/java-impl/src/inspectionDescriptions/RedundantComparatorComparing.html +++ b/java/java-impl/src/inspectionDescriptions/RedundantComparatorComparing.html @@ -1,7 +1,11 @@ -Reports redundant Comparator combinator constructs like thenComparing(Comparator.comparing(function)) -which can be simplified to thenComparing(function). +Reports redundant Comparator combinator constructs which can be simplified. Examples: +

New in 2018.1

diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/afterComparingMapEntry.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/afterComparingMapEntry.java new file mode 100644 index 000000000000..06171dcb6760 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/afterComparingMapEntry.java @@ -0,0 +1,26 @@ +// "Fix all 'Comparator can be simplified' problems in file" "true" +import java.util.*; +import java.util.Map.Entry; +import java.util.stream.Collectors; + +class Test { + void test() { + Map unsortMap = new HashMap<>(); + unsortMap.put("z", 10); + unsortMap.put("b", 5); + unsortMap.put("a", 6); + unsortMap.put("c", 20); + unsortMap.put("d", 1); + + Map result = unsortMap.entrySet().stream() + .sorted(Entry.comparingByValue(Comparator.reverseOrder())) + .sorted(Entry.comparingByValue()) + .sorted(Entry.comparingByKey()) + .sorted(Entry.comparingByKey(String.CASE_INSENSITIVE_ORDER)) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, + (oldValue, newValue) -> oldValue, LinkedHashMap::new)); + + System.out.println("Sorted..."); + System.out.println(result); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/beforeComparingMapEntry.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/beforeComparingMapEntry.java new file mode 100644 index 000000000000..294cf7fdcbc0 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/comparatorCanBeSimplified/beforeComparingMapEntry.java @@ -0,0 +1,26 @@ +// "Fix all 'Comparator can be simplified' problems in file" "true" +import java.util.*; +import java.util.Map.Entry; +import java.util.stream.Collectors; + +class Test { + void test() { + Map unsortMap = new HashMap<>(); + unsortMap.put("z", 10); + unsortMap.put("b", 5); + unsortMap.put("a", 6); + unsortMap.put("c", 20); + unsortMap.put("d", 1); + + Map result = unsortMap.entrySet().stream() + .sorted(Comparator.comparing(Map.Entry::getValue, Comparator.reverseOrder()) ) + .sorted(Comparator.comparing(Map.Entry::getValue) ) + .sorted(Comparator.comparing(Map.Entry::getKey) ) + .sorted(Comparator.comparing(Entry::getKey, String.CASE_INSENSITIVE_ORDER) ) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, + (oldValue, newValue) -> oldValue, LinkedHashMap::new)); + + System.out.println("Sorted..."); + System.out.println(result); + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index cccbb85d78d6..56516c07061c 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -992,11 +992,13 @@ inspection.endless.stream.description=Non-short-circuit operation consumes the i 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.entry.comparator.message=''{0}'' can be used instead 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.simplifiable.comparator.fix.entry.comparator.family.name=Use predefined Map.Entry comparator inspection.capturing.cleaner=Runnable passed to Cleaner.register() captures ''{0}'' reference inspection.capturing.cleaner.description=Cleaner captures object reference diff --git a/plugins/InspectionGadgets/src/com/intellij/codeInspection/RedundantComparatorComparingInspection.java b/plugins/InspectionGadgets/src/com/intellij/codeInspection/RedundantComparatorComparingInspection.java index 408ba1479959..122f2827a5b6 100644 --- a/plugins/InspectionGadgets/src/com/intellij/codeInspection/RedundantComparatorComparingInspection.java +++ b/plugins/InspectionGadgets/src/com/intellij/codeInspection/RedundantComparatorComparingInspection.java @@ -11,6 +11,7 @@ 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 com.siyeh.ig.psiutils.FunctionalExpressionUtils; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -35,6 +36,9 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca staticCall(JAVA_UTIL_COLLECTIONS, "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 COMPARATOR_COMPARING = anyOf( + staticCall(JAVA_UTIL_COMPARATOR, "comparing").parameterTypes(JAVA_UTIL_FUNCTION_FUNCTION), + COMPARATOR_COMPARING_WITH_DOWNSTREAM); private static final CallMatcher MIN_MAX = anyOf( staticCall(JAVA_UTIL_COLLECTIONS, "min", "max").parameterTypes(JAVA_UTIL_COLLECTION, JAVA_UTIL_COMPARATOR), @@ -43,8 +47,7 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca ); private static final CallMapper REPLACEMENTS = new CallMapper() - .register(staticCall(JAVA_UTIL_COMPARATOR, "comparing").parameterTypes(JAVA_UTIL_FUNCTION_FUNCTION), "thenComparing") - .register(COMPARATOR_COMPARING_WITH_DOWNSTREAM, "thenComparing") + .register(COMPARATOR_COMPARING, "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"); @@ -61,6 +64,19 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca if (THEN_COMPARING_COMPARATOR.test(call)) { checkThenComparing(call); } + if (COMPARATOR_COMPARING.test(call)) { + PsiExpression arg = call.getArgumentList().getExpressions()[0]; + PsiElement nameElement = Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()); + for (String suffix : new String[]{"Key", "Value"}) { + if (FunctionalExpressionUtils.isFunctionalReferenceTo(arg, JAVA_UTIL_MAP_ENTRY, null, "get"+suffix, PsiType.EMPTY_ARRAY)) { + String replacementMethod = "comparingBy" + suffix; + holder.registerProblem(nameElement, + InspectionsBundle.message("inspection.simplifiable.comparator.entry.comparator.message", + "Entry." + replacementMethod + "()"), + new ReplaceWithEntryComparatorFix(replacementMethod)); + } + } + } if (MIN_MAX.test(call)) { PsiExpression arg = ArrayUtil.getLastElement(call.getArgumentList().getExpressions()); if (getPlainComparatorExpressionFromReversed(arg, new CommentTracker()) != null) { @@ -201,4 +217,36 @@ public class RedundantComparatorComparingInspection extends AbstractBaseJavaLoca ExpressionUtils.bindCallTo(call, myReplacement); } } + + private static class ReplaceWithEntryComparatorFix implements LocalQuickFix { + private final String myReplacementMethod; + + ReplaceWithEntryComparatorFix(String replacementMethod) { + myReplacementMethod = replacementMethod; + } + + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getName() { + return CommonQuickFixBundle.message("fix.replace.with.x", "Entry."+myReplacementMethod+"()"); + } + + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.simplifiable.comparator.fix.entry.comparator.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[] args = call.getArgumentList().getExpressions(); + CommentTracker ct = new CommentTracker(); + String replacement = JAVA_UTIL_MAP_ENTRY + "." + myReplacementMethod + "(" + (args.length == 2 ? ct.text(args[1]) : "") + ")"; + ct.replaceAndRestoreComments(call, replacement); + } + } }