From 1c312d3e13d90b2991b86d75cb4c25d19d49de34 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 18 Apr 2017 17:55:48 +0700 Subject: [PATCH] IDEA-170626 Suggest to use Stream.peek instead of Stream.map if the lambda parameter is not reassigned during the operation. --- ...SimplifyStreamApiCallChainsInspection.java | 52 ++++++++++++++++++- .../streamApiCallChains/afterMapToPeek.java | 14 +++++ .../afterMapToPeekPrimitive.java | 15 ++++++ .../streamApiCallChains/beforeMapToPeek.java | 16 ++++++ .../beforeMapToPeekPrimitive.java | 18 +++++++ .../beforeMapToPeekWritten.java | 16 ++++++ .../SimplifyStreamApiCallChains.html | 11 ++-- 7 files changed, 136 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeek.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeekPrimitive.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeek.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekPrimitive.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekWritten.java diff --git a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java index b9ac3870e9c4..1da24a31cc28 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java @@ -61,6 +61,8 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns instanceCall(CommonClassNames.JAVA_UTIL_STREAM_STREAM, "filter").parameterTypes(CommonClassNames.JAVA_UTIL_FUNCTION_PREDICATE); private static final CallMatcher STREAM_MAP = instanceCall(CommonClassNames.JAVA_UTIL_STREAM_STREAM, "map").parameterTypes(CommonClassNames.JAVA_UTIL_FUNCTION_FUNCTION); + private static final CallMatcher BASE_STREAM_MAP = + instanceCall(CommonClassNames.JAVA_UTIL_STREAM_BASE_STREAM, "map").parameterCount(1); private static final CallMatcher STREAM_ANY_MATCH = instanceCall(CommonClassNames.JAVA_UTIL_STREAM_BASE_STREAM, "anyMatch").parameterCount(1); private static final CallMatcher STREAM_NONE_MATCH = @@ -81,7 +83,8 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns ReplaceWithBoxedFix.handler(), ReplaceWithElementIterationFix.handler(), ReplaceForEachMethodFix.handler(), - RemoveBooleanIdentityFix.handler() + RemoveBooleanIdentityFix.handler(), + ReplaceWithPeekFix.handler() ).registerAll(SimplifyMatchNegationFix.handlers()); private static final Logger LOG = Logger.getInstance("#" + SimplifyStreamApiCallChainsInspection.class.getName()); @@ -720,6 +723,53 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns } } + private static class ReplaceWithPeekFix implements CallChainSimplification { + + @Override + public String getName() { + return "Replace with 'peek'"; + } + + @Override + public String getMessage() { + return "Can be replaced with 'peek'"; + } + + @Override + public PsiElement simplify(PsiMethodCallExpression call) { + PsiLambdaExpression lambda = + tryCast(PsiUtil.skipParenthesizedExprDown(call.getArgumentList().getExpressions()[0]), PsiLambdaExpression.class); + if (lambda == null) return null; + PsiCodeBlock block = tryCast(lambda.getBody(), PsiCodeBlock.class); + if (block == null) return null; + PsiReturnStatement statement = tryCast(ArrayUtil.getLastElement(block.getStatements()), PsiReturnStatement.class); + if (statement == null) return null; + ExpressionUtils.bindCallTo(call, "peek"); + new CommentTracker().deleteAndRestoreComments(statement); + LambdaRefactoringUtil.simplifyToExpressionLambda(lambda); + LambdaCanBeMethodReferenceInspection.replaceLambdaWithMethodReference(lambda); + return call; + } + + static CallHandler handler() { + return CallHandler.of(BASE_STREAM_MAP, call -> { + PsiLambdaExpression lambda = + tryCast(PsiUtil.skipParenthesizedExprDown(call.getArgumentList().getExpressions()[0]), PsiLambdaExpression.class); + if (lambda == null) return null; + PsiParameter[] parameters = lambda.getParameterList().getParameters(); + if (parameters.length != 1) return null; + PsiCodeBlock block = tryCast(lambda.getBody(), PsiCodeBlock.class); + if (block == null) return null; + PsiStatement[] statements = block.getStatements(); + if (statements.length != 2) return null; + PsiReturnStatement returnStatement = tryCast(statements[1], PsiReturnStatement.class); + if (returnStatement == null || !ExpressionUtils.isReferenceTo(returnStatement.getReturnValue(), parameters[0])) return null; + if (VariableAccessUtils.variableIsAssigned(parameters[0]) || !(statements[0] instanceof PsiExpressionStatement)) return null; + return new ReplaceWithPeekFix(); + }); + } + } + private static class ReplaceWithBoxedFix implements CallChainSimplification { private static final CallMatcher MAP_TO_OBJ = instanceCall(CommonClassNames.JAVA_UTIL_STREAM_BASE_STREAM, "mapToObj").parameterCount(1); diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeek.java b/java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeek.java new file mode 100644 index 000000000000..0b20bc1610ae --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeek.java @@ -0,0 +1,14 @@ +// "Replace with 'peek'" "true" + +import java.util.List; + +public class Main { + void test(List list) { + // hello +/* in return */ + long count = list.stream() + .peek(System.out::println) + .count(); + System.out.println(count); + } +} diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeekPrimitive.java b/java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeekPrimitive.java new file mode 100644 index 000000000000..bb6fe48b2116 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterMapToPeekPrimitive.java @@ -0,0 +1,15 @@ +// "Replace with 'peek'" "true" + +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.IntStream; + +public class Main { + void test() { + AtomicInteger counter = new AtomicInteger(); + int[] ints = IntStream.range(0, 100) + .filter(x -> x % 3 == 0) + .peek((x -> counter.incrementAndGet())) + .toArray(); + System.out.println(counter.get()); + } +} diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeek.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeek.java new file mode 100644 index 000000000000..f1bf03fc5751 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeek.java @@ -0,0 +1,16 @@ +// "Replace with 'peek'" "true" + +import java.util.List; + +public class Main { + void test(List list) { + long count = list.stream() + .map(e -> { + System.out.println(e); + // hello + return /* in return */ e; + }) + .count(); + System.out.println(count); + } +} diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekPrimitive.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekPrimitive.java new file mode 100644 index 000000000000..3e45e0d136c9 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekPrimitive.java @@ -0,0 +1,18 @@ +// "Replace with 'peek'" "true" + +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.IntStream; + +public class Main { + void test() { + AtomicInteger counter = new AtomicInteger(); + int[] ints = IntStream.range(0, 100) + .filter(x -> x % 3 == 0) + .map((x -> { + counter.incrementAndGet(); + return x; + })) + .toArray(); + System.out.println(counter.get()); + } +} diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekWritten.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekWritten.java new file mode 100644 index 000000000000..45b10e940bbc --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeMapToPeekWritten.java @@ -0,0 +1,16 @@ +// "Replace with 'peek'" "false" + +import java.util.stream.IntStream; + +public class Main { + void test() { + int[] ints = IntStream.range(0, 100) + .filter(x -> x % 3 == 0) + .map(x -> { + x++; + return x; + }) + .toArray(); + System.out.println(ints.length); + } +} diff --git a/resources-en/src/inspectionDescriptions/SimplifyStreamApiCallChains.html b/resources-en/src/inspectionDescriptions/SimplifyStreamApiCallChains.html index c08bbd1db67e..95696480578b 100644 --- a/resources-en/src/inspectionDescriptions/SimplifyStreamApiCallChains.html +++ b/resources-en/src/inspectionDescriptions/SimplifyStreamApiCallChains.html @@ -15,12 +15,13 @@ It allows to avoid creating redundant temporary objects when traversing a collec
  • Collections.singleton().stream() → Stream.of()
  • Collections.emptyList().stream() → Stream.empty()
  • stream.filter().findFirst().isPresent() → stream.anyMatch()
    • -
  • stream.collect(Collectors.counting()) → stream.count()
    • -
  • stream.collect(Collectors.maxBy()) → stream.max()
    • -
  • stream.collect(Collectors.mapping()) → stream.map().collect()
    • -
  • stream.collect(Collectors.reducing()) → stream.reduce()
    • -
  • stream.collect(Collectors.summingInt()) → stream.mapToInt().sum()
    • +
  • stream.collect(counting()) → stream.count()
    • +
  • stream.collect(maxBy()) → stream.max()
    • +
  • stream.collect(mapping()) → stream.map().collect()
    • +
  • stream.collect(reducing()) → stream.reduce()
    • +
  • stream.collect(summingInt()) → stream.mapToInt().sum()
  • stream.mapToObj(x -> x) → stream.boxed()
    • +
  • stream.map(x -> {...; return x;}) → stream.peek(x -> ...)
  • !stream.anyMatch() → stream.noneMatch()
  • !stream.anyMatch(x -> !(...)) → stream.allMatch()
  • stream.map().anyMatch(Boolean::booleanValue) -> stream.anyMatch()