diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java index b915cb30b502..9592c32b1833 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java @@ -27,7 +27,9 @@ import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.psiutils.MethodCallUtils; import com.siyeh.ig.psiutils.StreamApiUtil; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import java.util.Objects; import java.util.function.UnaryOperator; import static com.intellij.psi.CommonClassNames.*; @@ -49,6 +51,16 @@ public class StreamChainInliner implements CallInliner { instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, "findFirst", "findAny").parameterCount(0)); private static final CallMatcher MIN_MAX_TERMINAL = instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, "min", "max", "reduce").parameterCount(1); + private static final CallMatcher COLLECT_TERMINAL = + instanceCall(JAVA_UTIL_STREAM_STREAM, "collect").parameterTypes("java.util.stream.Collector"); + + private static final CallMatcher COUNTING_COLLECTOR = + staticCall(JAVA_UTIL_STREAM_COLLECTORS, "counting").parameterCount(0); + private static final CallMatcher COLLECTION_COLLECTOR = + anyOf(staticCall(JAVA_UTIL_STREAM_COLLECTORS, "toList", "toSet", "toImmutableList", "toImmutableSet").parameterCount(0), + staticCall(JAVA_UTIL_STREAM_COLLECTORS, "toCollection").parameterCount(1)); + private static final CallMatcher MAP_COLLECTOR = + staticCall(JAVA_UTIL_STREAM_COLLECTORS, "toMap", "toConcurrentMap", "toImmutableMap"); private static final CallMatcher SKIP_STEP = instanceCall(JAVA_UTIL_STREAM_BASE_STREAM, "unordered", "parallel", "sequential").parameterCount(0); @@ -101,7 +113,8 @@ public class StreamChainInliner implements CallInliner { .register(MATCH_TERMINAL, MatchTerminalStep::new) .register(SUM_TERMINAL, SumTerminalStep::new) .register(MIN_MAX_TERMINAL, MinMaxTerminalStep::new) - .register(OPTIONAL_TERMINAL, OptionalTerminalStep::new); + .register(OPTIONAL_TERMINAL, OptionalTerminalStep::new) + .register(COLLECT_TERMINAL, StreamChainInliner::createTerminalFromCollector); private static final Step NULL_TERMINAL_STEP = new Step(null, null, null) { @Override @@ -221,6 +234,9 @@ public class StreamChainInliner implements CallInliner { @Override protected void pushInitialValue(CFGBuilder builder) { PsiType type = myCall.getType(); + if (!(type instanceof PsiPrimitiveType)) { + type = PsiPrimitiveType.getUnboxedType(type); + } Object value = PsiTypesUtil.getDefaultValue(type); builder.push(builder.getFactory().getConstFactory().createFromValue(value, type, null)); } @@ -498,6 +514,94 @@ public class StreamChainInliner implements CallInliner { } } + abstract static class AbstractCollectionStep extends TerminalStep { + final boolean myImmutable; + + AbstractCollectionStep(@NotNull PsiMethodCallExpression call, @Nullable PsiExpression supplier, boolean immutable) { + super(call, supplier); + myImmutable = immutable; + } + + @Override + protected void pushInitialValue(CFGBuilder builder) { + if (myFunction != null) { + builder.invokeFunction(0, myFunction, Nullness.NOT_NULL); + } + else { + DfaValue value = builder.getFactory().createTypeValue(myCall.getType(), Nullness.NOT_NULL); + if (myImmutable) { + value = builder.getFactory().withFact(value, DfaFactType.MUTABILITY, Mutability.UNMODIFIABLE); + } + builder.push(value); + } + } + } + + static class ToCollectionStep extends AbstractCollectionStep { + ToCollectionStep(@NotNull PsiMethodCallExpression call, @Nullable PsiExpression supplier, boolean immutable) { + super(call, supplier, immutable); + } + + @Override + void iteration(CFGBuilder builder) { + // do nothing currently: we can emulate calling collection.add, + // but it's unnecessary for current analysis + builder.pop(); + } + + @Override + boolean expectNotNull() { + return myImmutable; + } + } + + static class ToMapStep extends AbstractCollectionStep { + private final @NotNull PsiExpression myKeyExtractor; + private final @NotNull PsiExpression myValueExtractor; + private final @Nullable PsiExpression myMerger; + + ToMapStep(@NotNull PsiMethodCallExpression call, + @NotNull PsiExpression keyExtractor, + @NotNull PsiExpression valueExtractor, + @Nullable PsiExpression merger, + @Nullable PsiExpression supplier, + boolean immutable) { + super(call, supplier, immutable); + myKeyExtractor = keyExtractor; + myValueExtractor = valueExtractor; + myMerger = merger; + } + + @Override + void before(CFGBuilder builder) { + builder.evaluateFunction(myKeyExtractor) + .evaluateFunction(myValueExtractor); + if (myMerger != null) { + builder.evaluateFunction(myMerger); + } + super.before(builder); + } + + @Override + void iteration(CFGBuilder builder) { + // Null values are not tolerated + // Null keys are not tolerated for immutable maps + builder.dup() + .invokeFunction(1, myKeyExtractor, myImmutable ? Nullness.NOT_NULL : Nullness.NULLABLE) + .pop() + .invokeFunction(1, myValueExtractor, Nullness.NOT_NULL); + if (myMerger != null) { + builder.pushUnknown() + .ifConditionIs(true) + .push(builder.getFactory().getFactValue(DfaFactType.CAN_BE_NULL, false)) + .invokeFunction(2, myMerger) + .endIf(); + } + // Actual addition of Map element is unnecessary for current analysis + builder.pop(); + } + } + @Override public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { if (TERMINAL_CALL.test(call)) { @@ -659,4 +763,29 @@ public class StreamChainInliner implements CallInliner { Step step = TERMINAL_STEP_MAPPER.mapFirst(call); return step == null ? new UnknownTerminalStep(call) : step; } + + private static Step createTerminalFromCollector(PsiMethodCallExpression call) { + PsiMethodCallExpression collectorCall = + ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(call.getArgumentList().getExpressions()[0]), PsiMethodCallExpression.class); + if (COUNTING_COLLECTOR.matches(collectorCall)) { + return new SumTerminalStep(call); + } + if (COLLECTION_COLLECTOR.matches(collectorCall)) { + String name = Objects.requireNonNull(collectorCall.getMethodExpression().getReferenceName()); + return new ToCollectionStep(call, ArrayUtil.getFirstElement(collectorCall.getArgumentList().getExpressions()), + name.startsWith("toImmutable")); + } + if (MAP_COLLECTOR.matches(collectorCall)) { + PsiExpression[] args = collectorCall.getArgumentList().getExpressions(); + if (args.length >= 2 && args.length <= 4) { + PsiExpression keyExtractor = args[0]; + PsiExpression valueExtractor = args[1]; + PsiExpression merger = args.length >= 3 ? args[2] : null; + PsiExpression supplier = args.length >= 4 ? args[3] : null; + return new ToMapStep(call, keyExtractor, valueExtractor, merger, supplier, + "toImmutableMap".equals(collectorCall.getMethodExpression().getReferenceName())); + } + } + return new UnknownTerminalStep(call); + } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollector10Inlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollector10Inlining.java new file mode 100644 index 000000000000..e957fc7b83b5 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollector10Inlining.java @@ -0,0 +1,49 @@ +package java.util.stream; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.function.*; +import java.util.stream.*; + +// Mock +class Collectors { + public static Collector> toList() {} + public static Collector> toImmutableList() {} + public static Collector> toImmutableSet() {} + public static Collector> toImmutableMap(Function keyMapper, Function valueMapper) {} +} + +public class StreamCollector10Inlining { + + @Nullable + final String convert(String s) { + return s.isEmpty() ? null : s; + } + + void testToList() { + List list = Stream.of("foo", "bar", "baz").map(this::convert).collect(Collectors.toList()); + list.sort(null); + } + + void testToImmutableList() { + List list = Stream.of("foo", "bar", "baz") + .map(this::convert) + .collect(Collectors.toImmutableList()); + list.sort(null); + } + + void testToImmutableSet() { + Set set = Stream.of("foo", "bar", "baz") + .map(this::convert) + .collect(Collectors.toImmutableSet()); + set.add("qux"); + } + + void testToImmutableMap() { + Map map = Stream.of("foo", "bar", "baz", "") + .collect(Collectors.toImmutableMap(this::convert, this::convert)); + map.put("qux", "qux"); + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollectorInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollectorInlining.java new file mode 100644 index 000000000000..c82328acd924 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollectorInlining.java @@ -0,0 +1,55 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.function.*; +import java.util.stream.*; + +public class StreamCollectorInlining { + + void testCounting() { + long count = Stream.empty().collect(Collectors.counting()); + if(count > 0) { + System.out.println("impossible"); + } + } + + void testToListNoSideEffect(List list) { + if(list.isEmpty()) return; + List other = Stream.of("foo", "bar", "baz").collect(Collectors.toList()); + // We can conclude now that this stream produces no side effect, thus list.isEmpty() result is still valid + if(list.isEmpty()) return; + } + + void testToCollection(List list) { + list.stream().collect(Collectors.toCollection(() -> null)); + } + + @Nullable + final String convert(String s) { + return s.isEmpty() ? null : s; + } + + Map testToMapNullableValue(List list) { + return list.stream().collect(Collectors.toMap( + this::convert, this::convert)); + } + + Map testToMapNullableValueMerger(List list) { + return list.stream().collect( + Collectors.toMap(this::convert, this::convert, + (a, b) -> a == null ? b : a)); + } + + Map testToMapNullableValueMerger2(List list) { + return list.stream().collect( + Collectors.toMap(this::convert, this::convert, + (a, b) -> b == null ? b : a)); + } + + Map testToMapNullableValueSupplier(List list) { + return list.stream().collect( + Collectors.toMap(this::convert, this::convert, + (a, b) -> b, () -> null)); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection10Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection10Test.java new file mode 100644 index 000000000000..d5899568ff71 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection10Test.java @@ -0,0 +1,32 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.java.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.openapi.projectRoots.Sdk; +import com.intellij.testFramework.IdeaTestUtil; +import com.intellij.testFramework.LightProjectDescriptor; +import com.intellij.testFramework.PsiTestUtil; +import com.intellij.testFramework.fixtures.DefaultLightProjectDescriptor; +import org.jetbrains.annotations.NotNull; + +public class DataFlowInspection10Test extends DataFlowInspectionTestCase { + private static final DefaultLightProjectDescriptor PROJECT_DESCRIPTOR = new DefaultLightProjectDescriptor() { + @Override + public Sdk getSdk() { + return PsiTestUtil.addJdkAnnotations(IdeaTestUtil.getMockJdk9()); + } + }; + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return PROJECT_DESCRIPTOR; + } + + @Override + protected String getTestDataPath() { + return JavaTestUtil.getJavaTestDataPath() + "/inspection/dataFlow/fixture/"; + } + + public void testStreamCollector10Inlining() { doTest(); } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java index 6777aa2d2c29..a09377c4e004 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -195,6 +195,7 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { doTest(); } public void testStreamInlining() { doTest(); } + public void testStreamCollectorInlining() { doTest(); } public void testStreamComparatorInlining() { doTest(); } public void testStreamKnownSource() { doTest(); } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java index 3cc8e73df49b..66b453295857 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java @@ -30,6 +30,7 @@ public class DataFlowInspectionTestSuite { suite.addTestSuite(DataFlowInspectionTest.class); suite.addTestSuite(DataFlowInspection8Test.class); suite.addTestSuite(DataFlowInspection9Test.class); + suite.addTestSuite(DataFlowInspection10Test.class); suite.addTestSuite(DataFlowInspectionHeavyTest.class); suite.addTestSuite(DataFlowInspectionAncientTest.class); suite.addTestSuite(ContractCheckTest.class);