From 88614739d89f9e0253cbdcc8b318acc6d4a6e951 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 30 Aug 2016 19:04:12 +0300 Subject: [PATCH] IDEA-160442 Warn about excessive use of collectors --- ...SimplifyStreamApiCallChainsInspection.java | 131 ++++++++++++++++++ .../afterCollectorCounting.java | 10 ++ .../afterCollectorMapping.java | 10 ++ .../afterCollectorMaxBy.java | 10 ++ .../afterCollectorMinBy.java | 10 ++ .../afterCollectorReducing1.java | 10 ++ .../afterCollectorReducing2.java | 10 ++ .../afterCollectorReducing3.java | 10 ++ .../afterCollectorSummingInt.java | 10 ++ .../afterCollectorSummingLong.java | 10 ++ .../beforeCollectorCounting.java | 10 ++ .../beforeCollectorMapping.java | 10 ++ .../beforeCollectorMaxBy.java | 10 ++ .../beforeCollectorMinBy.java | 10 ++ .../beforeCollectorReducing1.java | 10 ++ .../beforeCollectorReducing3.java | 10 ++ .../beforeCollectorSummingInt.java | 10 ++ .../beforeCollectorSummingLong.java | 10 ++ 18 files changed, 301 insertions(+) create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorCounting.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMapping.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMaxBy.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMinBy.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing1.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing2.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing3.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingInt.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingLong.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorCounting.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMapping.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMaxBy.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMinBy.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing1.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing3.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingInt.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingLong.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java index 823513cdef5d..5dfb5cfd8ae6 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java @@ -45,6 +45,16 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns private static final String EMPTY_SET_METHOD = "emptySet"; private static final String SINGLETON_LIST_METHOD = "singletonList"; private static final String SINGLETON_METHOD = "singleton"; + private static final String COLLECT_METHOD = "collect"; + + private static final String COUNTING_COLLECTOR = "counting"; + private static final String MIN_BY_COLLECTOR = "minBy"; + private static final String MAX_BY_COLLECTOR = "maxBy"; + private static final String MAPPING_COLLECTOR = "mapping"; + private static final String REDUCING_COLLECTOR = "reducing"; + private static final String SUMMING_INT_COLLECTOR = "summingInt"; + private static final String SUMMING_LONG_COLLECTOR = "summingLong"; + private static final String SUMMING_DOUBLE_COLLECTOR = "summingDouble"; @Override public boolean isEnabledByDefault() { @@ -95,6 +105,44 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns holder.registerProblem(methodCall, null, fix.getMessage(), fix); } } + else if (isCallOf(method, CommonClassNames.JAVA_UTIL_STREAM_STREAM, COLLECT_METHOD, 1)) { + PsiElement parameter = methodCall.getArgumentList().getExpressions()[0]; + if(parameter instanceof PsiMethodCallExpression) { + PsiMethodCallExpression collectorCall = (PsiMethodCallExpression)parameter; + PsiMethod collectorMethod = collectorCall.resolveMethod(); + ReplaceCollectorFix fix = null; + if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, COUNTING_COLLECTOR, 0)) { + fix = new ReplaceCollectorFix(COUNTING_COLLECTOR, "count()", false); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, MIN_BY_COLLECTOR, 1)) { + fix = new ReplaceCollectorFix(MIN_BY_COLLECTOR, "min({1})", true); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, MAX_BY_COLLECTOR, 1)) { + fix = new ReplaceCollectorFix(MAX_BY_COLLECTOR, "max({1})", true); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, MAPPING_COLLECTOR, 2)) { + fix = new ReplaceCollectorFix(MAPPING_COLLECTOR, "map({1}).collect({2})", false); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, REDUCING_COLLECTOR, 1)) { + fix = new ReplaceCollectorFix(REDUCING_COLLECTOR, "reduce({1})", true); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, REDUCING_COLLECTOR, 2)) { + fix = new ReplaceCollectorFix(REDUCING_COLLECTOR, "reduce({1}, {2})", false); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, REDUCING_COLLECTOR, 3)) { + fix = new ReplaceCollectorFix(REDUCING_COLLECTOR, "map({2}).reduce({1}, {3})", false); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, SUMMING_INT_COLLECTOR, 1)) { + fix = new ReplaceCollectorFix(SUMMING_INT_COLLECTOR, "mapToInt({1}).sum()", false); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, SUMMING_LONG_COLLECTOR, 1)) { + fix = new ReplaceCollectorFix(SUMMING_LONG_COLLECTOR, "mapToLong({1}).sum()", false); + } else if(isCallOf(collectorMethod, CommonClassNames.JAVA_UTIL_STREAM_COLLECTORS, SUMMING_DOUBLE_COLLECTOR, 1)) { + fix = new ReplaceCollectorFix(SUMMING_DOUBLE_COLLECTOR, "mapToDouble({1}).sum()", false); + } + if (fix != null && + collectorCall.getArgumentList().getExpressions().length == collectorMethod.getParameterList().getParametersCount()) { + TextRange range = methodCall.getTextRange(); + PsiElement nameElement = methodCall.getMethodExpression().getReferenceNameElement(); + if(nameElement != null) { + range = new TextRange(nameElement.getTextOffset(), range.getEndOffset()); + } + holder.registerProblem(methodCall, range.shiftRight(-methodCall.getTextOffset()), fix.getMessage(), fix); + } + } + } else { final String name; if (isCallOf(method, CommonClassNames.JAVA_UTIL_STREAM_STREAM, FOR_EACH_METHOD, 1)) { @@ -390,4 +438,87 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns } } } + + private static class ReplaceCollectorFix implements LocalQuickFix { + private final String myCollector; + private final String myStreamSequence; + private final String myStreamSequenceStripped; + private final boolean myChangeSemantics; + + public ReplaceCollectorFix(String collector, String streamSequence, boolean changeSemantics) { + myCollector = collector; + myStreamSequence = streamSequence; + myStreamSequenceStripped = streamSequence.replaceAll("\\([^)]+\\)", "()"); + myChangeSemantics = changeSemantics; + } + + @Nls + @NotNull + @Override + public String getName() { + return getFamilyName(); + } + + @Nls + @NotNull + @Override + public String getFamilyName() { + return "Replace Stream.collect(" + myCollector + + "()) with Stream." + myStreamSequenceStripped + + (myChangeSemantics ? " (may change semantics when result is null)" : ""); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiElement element = descriptor.getStartElement(); + if (element instanceof PsiMethodCallExpression) { + PsiMethodCallExpression collectCall = (PsiMethodCallExpression)element; + PsiExpression qualifierExpression = collectCall.getMethodExpression().getQualifierExpression(); + if (qualifierExpression != null) { + PsiElement parameter = collectCall.getArgumentList().getExpressions()[0]; + if (parameter instanceof PsiMethodCallExpression) { + PsiMethodCallExpression collectorCall = (PsiMethodCallExpression)parameter; + PsiExpression[] collectorArgs = collectorCall.getArgumentList().getExpressions(); + String result = myStreamSequence; + for(int i=0; i .count() change the result type from boxed to primitive + // In rare cases it's necessary to add cast to return back to boxed type + // example: + // List intList; List stringList; + // intList.remove(stringList.stream().collect(summingInt(String::length)) -- remove given element + // intList.remove(stringList.stream().mapToInt(String::length).sum()) -- remove element by index + if(expression instanceof PsiExpression) { + PsiType type = ((PsiExpression)expression).getType(); + if(type instanceof PsiPrimitiveType) { + PsiClassType boxedType = ((PsiPrimitiveType)type).getBoxedType(expression); + if(boxedType != null) { + PsiExpression castExpression = + factory.createExpressionFromText("(" + boxedType.getCanonicalText() + ") " + expression.getText(), expression); + PsiElement cast = expression.replace(castExpression); + if (cast instanceof PsiTypeCastExpression && RedundantCastUtil.isCastRedundant((PsiTypeCastExpression)cast)) { + RedundantCastUtil.removeCast((PsiTypeCastExpression)cast); + } + } + } + } + } + } + } + } + + @NotNull + String getMessage() { + return "Stream.collect(" + myCollector + + "()) can be replaced with Stream." + myStreamSequenceStripped + "()" + + (myChangeSemantics ? " (may change semantics when result is null)" : ""); + } + } + } diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorCounting.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorCounting.java new file mode 100644 index 000000000000..71e3b19347f8 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorCounting.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(counting()) with Stream.count()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public long count(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).count(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMapping.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMapping.java new file mode 100644 index 000000000000..5947f90e9c27 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMapping.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(mapping()) with Stream.map().collect()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public List sizes(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).map(String::length).collect(Collectors.toList()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMaxBy.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMaxBy.java new file mode 100644 index 000000000000..d854a721537a --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMaxBy.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(maxBy()) with Stream.max() (may change semantics when result is null)" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public String max(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).max(String.CASE_INSENSITIVE_ORDER).orElse(""); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMinBy.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMinBy.java new file mode 100644 index 000000000000..c9feabac14e4 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorMinBy.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(minBy()) with Stream.min() (may change semantics when result is null)" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public Optional min(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).min(String.CASE_INSENSITIVE_ORDER); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing1.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing1.java new file mode 100644 index 000000000000..37374e316f13 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing1.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(reducing()) with Stream.reduce() (may change semantics when result is null)" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public Optional concat(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).reduce(String::concat); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing2.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing2.java new file mode 100644 index 000000000000..a5a5a24b9580 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing2.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(reducing()) with Stream.reduce()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public String concat(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).reduce("", String::concat); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing3.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing3.java new file mode 100644 index 000000000000..ab3b2ccea49c --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorReducing3.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(reducing()) with Stream.map().reduce()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public int sum(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).map(String::length).reduce(0, Integer::sum); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingInt.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingInt.java new file mode 100644 index 000000000000..249a0358d3e6 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingInt.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(summingInt()) with Stream.mapToInt().sum()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public void remove(List ints, List data) { + ints.remove((Integer) data.stream().mapToInt(String::length).sum()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingLong.java b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingLong.java new file mode 100644 index 000000000000..6b86f89a7693 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterCollectorSummingLong.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(summingLong()) with Stream.mapToLong().sum()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public void remove(List ints, List data) { + ints.remove(data.stream().mapToLong(String::length).sum()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorCounting.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorCounting.java new file mode 100644 index 000000000000..b40342231759 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorCounting.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(counting()) with Stream.count()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public long count(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).collect(Collectors.counting()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMapping.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMapping.java new file mode 100644 index 000000000000..a6642d790f95 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMapping.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(mapping()) with Stream.map().collect()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public List sizes(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).collect(Collectors.mapping(String::length, Collectors.toList())); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMaxBy.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMaxBy.java new file mode 100644 index 000000000000..e6c59cd4caa1 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMaxBy.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(maxBy()) with Stream.max() (may change semantics when result is null)" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public String max(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).collect(Collectors.maxBy(String.CASE_INSENSITIVE_ORDER)).orElse(""); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMinBy.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMinBy.java new file mode 100644 index 000000000000..7be18c5687b9 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorMinBy.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(minBy()) with Stream.min() (may change semantics when result is null)" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public Optional min(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).collect(Collectors.minBy(String.CASE_INSENSITIVE_ORDER)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing1.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing1.java new file mode 100644 index 000000000000..a10971f7ee78 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing1.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(reducing()) with Stream.reduce() (may change semantics when result is null)" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public Optional concat(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).collect(Collectors.reducing(String::concat)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing3.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing3.java new file mode 100644 index 000000000000..7ac6bff960d7 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorReducing3.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(reducing()) with Stream.map().reduce()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public int sum(List data) { + return data.stream().filter(x -> x.startsWith("xyz")).collect(Collectors.reducing(0, String::length, Integer::sum)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingInt.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingInt.java new file mode 100644 index 000000000000..0a0e77601350 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingInt.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(summingInt()) with Stream.mapToInt().sum()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public void remove(List ints, List data) { + ints.remove(data.stream().collect(Collectors.summingInt(String::length))); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingLong.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingLong.java new file mode 100644 index 000000000000..305cc7bc13d4 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeCollectorSummingLong.java @@ -0,0 +1,10 @@ +// "Replace Stream.collect(summingLong()) with Stream.mapToLong().sum()" "true" + +import java.util.List; +import java.util.stream.Collectors; + +public class Main { + public void remove(List ints, List data) { + ints.remove(data.stream().collect(Collectors.summingLong(String::length))); + } +} \ No newline at end of file