diff --git a/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java b/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java index 79ae3bb9a5f1..443d58011862 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamToLoop/FunctionHelper.java @@ -34,6 +34,7 @@ import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.text.MessageFormat; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -159,10 +160,18 @@ abstract class FunctionHelper { if (expression instanceof PsiReferenceExpression && ExpressionUtils.isSimpleExpression(expression)) { return new SimpleReferenceFunctionHelper(returnType, expression, interfaceMethod.getName()); } - if (expression instanceof PsiMethodCallExpression && - MethodCallUtils - .isCallToStaticMethod((PsiMethodCallExpression)expression, CommonClassNames.JAVA_UTIL_FUNCTION_FUNCTION, "identity", 0)) { - return paramCount == 1 ? new IdentityFunctionHelper(returnType) : null; + if (expression instanceof PsiMethodCallExpression) { + PsiMethodCallExpression call = (PsiMethodCallExpression)expression; + if (MethodCallUtils.isCallToStaticMethod(call, CommonClassNames.JAVA_UTIL_FUNCTION_FUNCTION, "identity", 0)) { + return paramCount == 1 ? new InlinedFunctionHelper(returnType, 1, "{0}") : null; + } + if (MethodCallUtils.isCallToStaticMethod(call, CommonClassNames.JAVA_UTIL_COMPARATOR, "naturalOrder", 0)) { + return paramCount == 2 ? new InlinedFunctionHelper(returnType, 2, "{0}.compareTo({1})") : null; + } + if (MethodCallUtils.isCallToStaticMethod(call, CommonClassNames.JAVA_UTIL_COMPARATOR, "reverseOrder", 0) || + MethodCallUtils.isCallToStaticMethod(call, CommonClassNames.JAVA_UTIL_COLLECTIONS, "reverseOrder", 0)) { + return paramCount == 2 ? new InlinedFunctionHelper(returnType, 2, "{1}.compareTo({0})") : null; + } } return new ComplexExpressionFunctionHelper(returnType, type, interfaceMethod.getName(), expression); } @@ -444,11 +453,15 @@ abstract class FunctionHelper { } } - private static class IdentityFunctionHelper extends FunctionHelper { + private static class InlinedFunctionHelper extends FunctionHelper { + private final int myArgCount; + private final String myTemplate; private PsiExpression myExpression; - public IdentityFunctionHelper(PsiType type) { + public InlinedFunctionHelper(PsiType type, int argCount, String template) { super(type); + myArgCount = argCount; + myTemplate = template; } @Override @@ -459,8 +472,8 @@ abstract class FunctionHelper { @Override void transform(StreamToLoopReplacementContext context, String... argumentValues) { - LOG.assertTrue(argumentValues.length == 1); - myExpression = context.createExpression(argumentValues[0]); + LOG.assertTrue(argumentValues.length == myArgCount); + myExpression = context.createExpression(MessageFormat.format(myTemplate, (Object[])argumentValues)); } } diff --git a/java/java-impl/src/com/intellij/codeInspection/streamToLoop/TerminalOperation.java b/java/java-impl/src/com/intellij/codeInspection/streamToLoop/TerminalOperation.java index 294e6150d40d..62c467b057c6 100644 --- a/java/java-impl/src/com/intellij/codeInspection/streamToLoop/TerminalOperation.java +++ b/java/java-impl/src/com/intellij/codeInspection/streamToLoop/TerminalOperation.java @@ -19,12 +19,10 @@ import com.intellij.codeInspection.streamToLoop.StreamToLoopInspection.StreamToL import com.intellij.codeInspection.util.OptionalUtil; import com.intellij.openapi.project.Project; import com.intellij.psi.*; -import com.intellij.psi.util.InheritanceUtil; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import com.siyeh.ig.psiutils.BoolUtils; -import com.siyeh.ig.psiutils.ExpressionUtils; import one.util.streamex.StreamEx; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; @@ -684,43 +682,36 @@ abstract class TerminalOperation extends Operation { private String myType; private String myTemplate; private String myComparatorType; - private @Nullable PsiExpression myComparator; + private @Nullable FunctionHelper myComparator; - public MinMaxTerminalOperation(String type, String template, @Nullable PsiExpression comparator) { + public MinMaxTerminalOperation(String type, String template, @Nullable FunctionHelper comparator) { myType = type; myTemplate = template; myComparator = comparator; if(comparator != null) { - PsiType comparatorType = comparator.getType(); - if(comparatorType != null) { - myComparatorType = comparatorType.getCanonicalText(); - } else { - myComparatorType = CommonClassNames.JAVA_UTIL_COMPARATOR+"<"+myType+">"; - } + myComparatorType = CommonClassNames.JAVA_UTIL_COMPARATOR+"<"+myType+">"; } } @Override public void registerUsedNames(Consumer usedNameConsumer) { if(myComparator != null) { - FunctionHelper.processUsedNames(myComparator, usedNameConsumer); + myComparator.registerUsedNames(usedNameConsumer); } } @Override String generate(StreamVariable inVar, StreamToLoopReplacementContext context) { - String comparator = ""; - if(myComparator != null) { - comparator = myComparator.getText(); - if(!ExpressionUtils.isSimpleExpression(context.createExpression(comparator))) { - comparator = context.declare("comparator", myComparatorType, comparator); - } - } String seen = context.declare("seen", "boolean", "false"); String best = context.declareResult("best", myType, TypeConversionUtil.isPrimitive(myType) ? "0" : "null", false); - String type = myType; - context.setFinisher(new ConditionalExpression.Optional(type, seen, best)); - String comparePredicate = myTemplate.replace("{best}", best).replace("{item}", inVar.getName()).replace("{comparator}", comparator); + context.setFinisher(new ConditionalExpression.Optional(myType, seen, best)); + String comparePredicate; + if(myComparator != null) { + myComparator.transform(context, inVar.getName(), best); + comparePredicate = myTemplate.replace("{comparator}", myComparator.getText()); + } else { + comparePredicate = myTemplate.replace("{best}", best).replace("{item}", inVar.getName()); + } return "if(!" + seen + " || " + comparePredicate + ") {\n" + seen + "=true;\n" + best + "=" + inVar + ";\n}\n"; @@ -736,8 +727,12 @@ abstract class TerminalOperation extends Operation { if (PsiType.DOUBLE.equalsToText(elementType)) { return new MinMaxTerminalOperation(elementType, "java.lang.Double.compare({item},{best})" + sign + "0", null); } - } else if(InheritanceUtil.isInheritor(comparator.getType(), CommonClassNames.JAVA_UTIL_COMPARATOR)) { - return new MinMaxTerminalOperation(elementType, "{comparator}.compare({item},{best})" + sign + "0", comparator); + } + else { + FunctionHelper fn = FunctionHelper.create(comparator, 2); + if(fn != null) { + return new MinMaxTerminalOperation(elementType, "{comparator}"+sign+"0", fn); + } } return null; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectMaxBy.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectMaxBy.java index f70e1b5d26e4..89b306734674 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectMaxBy.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterCollectMaxBy.java @@ -5,12 +5,11 @@ import java.util.stream.Collectors; public class Main { public static Optional test(List strings) { - Comparator comparator = Comparator.naturalOrder(); boolean seen = false; String best = null; for (String s : strings) { if (!s.isEmpty()) { - if (!seen || comparator.compare(s, best) > 0) { + if (!seen || s.compareTo(best) > 0) { seen = true; best = s; } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxComparator.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxComparator.java index 2cdef07902a1..8f7da89361bb 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxComparator.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxComparator.java @@ -6,9 +6,9 @@ import java.util.List; public class Main { public static String test(List strings) { - Comparator comparator = Comparator.comparing(String::length); boolean seen = false; String best = null; + Comparator comparator = Comparator.comparing(String::length); for (String string : strings) { if (!seen || comparator.compare(string, best) > 0) { seen = true; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxLambda.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxLambda.java new file mode 100644 index 000000000000..104a93c4a8c1 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxLambda.java @@ -0,0 +1,17 @@ +// "Replace Stream API chain with loop" "true" + +import java.util.*; + +public class Main { + private static Optional max(Map> dependencies, String fruits, Map weights) { + boolean seen = false; + String best = null; + for (String s : dependencies.get(fruits)) { + if (!seen || weights.get(s).compareTo(weights.get(best)) > 0) { + seen = true; + best = s; + } + } + return seen ? Optional.of(best) : Optional.empty(); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxReverseOrder.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxReverseOrder.java new file mode 100644 index 000000000000..961b312b7055 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMaxReverseOrder.java @@ -0,0 +1,17 @@ +// "Replace Stream API chain with loop" "true" + +import java.util.*; + +public class Main { + private static Optional max(Map> dependencies, String fruits, Map weights) { + boolean seen = false; + String best = null; + for (String s : dependencies.get(fruits)) { + if (!seen || best.compareTo(s) > 0) { + seen = true; + best = s; + } + } + return seen ? Optional.of(best) : Optional.empty(); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMinComparatorReversed.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMinComparatorReversed.java index 39f7df3d6bfc..7a50e057f8a9 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMinComparatorReversed.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/afterMinComparatorReversed.java @@ -6,9 +6,9 @@ import java.util.List; public class Main { public static String test(List strings, Comparator comparator) { - Comparator comparator1 = comparator.reversed(); boolean seen = false; String best = null; + Comparator comparator1 = comparator.reversed(); for (String string : strings) { if (!seen || comparator1.compare(string, best) < 0) { seen = true; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeMaxLambda.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeMaxLambda.java new file mode 100644 index 000000000000..5fe009e4e49a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeMaxLambda.java @@ -0,0 +1,9 @@ +// "Replace Stream API chain with loop" "true" + +import java.util.*; + +public class Main { + private static Optional max(Map> dependencies, String fruits, Map weights) { + return dependencies.get(fruits).stream().max((o1, o2) -> weights.get(o1).compareTo(weights.get(o2))); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeMaxReverseOrder.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeMaxReverseOrder.java new file mode 100644 index 000000000000..32c834373f87 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/streamToLoop/beforeMaxReverseOrder.java @@ -0,0 +1,9 @@ +// "Replace Stream API chain with loop" "true" + +import java.util.*; + +public class Main { + private static Optional max(Map> dependencies, String fruits, Map weights) { + return dependencies.get(fruits).stream().max(Collections.reverseOrder()); + } +}