DFA: special support of some collectors

Supported: counting, toList, toSet, toCollection, toMap, toImmutableList, toImmutableSet, toImmutableMap
Fixes IDEA-187215 Support Java10 toImmutableXYZ collectors in dataflow
This commit is contained in:
Tagir Valeev
2018-02-26 17:34:38 +07:00
parent 7f87c4a20f
commit 2505f31197
6 changed files with 268 additions and 1 deletions
@@ -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);
}
}
@@ -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 <T> Collector<T, ?, List<T>> toList() {<error descr="Missing return statement">}</error>
public static <T> Collector<T, ?, List<T>> toImmutableList() {<error descr="Missing return statement">}</error>
public static <T> Collector<T, ?, Set<T>> toImmutableSet() {<error descr="Missing return statement">}</error>
public static <T, K, U> Collector<T, ?, Map<K,U>> toImmutableMap(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends U> valueMapper) {<error descr="Missing return statement">}</error>
}
public class StreamCollector10Inlining {
@Nullable
final String convert(String s) {
return s.isEmpty() ? null : s;
}
void testToList() {
List<String> list = Stream.of("foo", "bar", "baz").map(this::convert).collect(Collectors.toList());
list.sort(null);
}
void testToImmutableList() {
List<String> list = Stream.of("foo", "bar", "baz")
.map(<warning descr="Function may return null, but it's not allowed here">this::convert</warning>)
.collect(Collectors.toImmutableList());
list.<warning descr="Immutable object is modified">sort</warning>(null);
}
void testToImmutableSet() {
Set<String> set = Stream.of("foo", "bar", "baz")
.map(<warning descr="Function may return null, but it's not allowed here">this::convert</warning>)
.collect(Collectors.toImmutableSet());
set.<warning descr="Immutable object is modified">add</warning>("qux");
}
void testToImmutableMap() {
Map<String, String> map = Stream.of("foo", "bar", "baz", "")
.collect(Collectors.toImmutableMap(<warning descr="Function may return null, but it's not allowed here">this::convert</warning>, <warning descr="Function may return null, but it's not allowed here">this::convert</warning>));
map.<warning descr="Immutable object is modified">put</warning>("qux", "qux");
}
}
@@ -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(<warning descr="Condition 'count > 0' is always 'false'">count > 0</warning>) {
System.out.println("impossible");
}
}
void testToListNoSideEffect(List<String> list) {
if(list.isEmpty()) return;
List<String> 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(<warning descr="Condition 'list.isEmpty()' is always 'false'">list.isEmpty()</warning>) return;
}
void testToCollection(List<String> list) {
list.stream().collect(Collectors.toCollection(() -> <warning descr="Function may return null, but it's not allowed here">null</warning>));
}
@Nullable
final String convert(String s) {
return s.isEmpty() ? null : s;
}
Map<String, String> testToMapNullableValue(List<String> list) {
return list.stream().collect(Collectors.toMap(
this::convert, <warning descr="Function may return null, but it's not allowed here">this::convert</warning>));
}
Map<String, String> testToMapNullableValueMerger(List<String> list) {
return list.stream().collect(
Collectors.toMap(this::convert, <warning descr="Function may return null, but it's not allowed here">this::convert</warning>,
(a, b) -> <warning descr="Condition 'a == null' is always 'false'">a == null</warning> ? b : a));
}
Map<String, String> testToMapNullableValueMerger2(List<String> list) {
return list.stream().collect(
Collectors.toMap(this::convert, <warning descr="Function may return null, but it's not allowed here">this::convert</warning>,
(a, b) -> <warning descr="Condition 'b == null' is always 'false'">b == null</warning> ? b : a));
}
Map<String, String> testToMapNullableValueSupplier(List<String> list) {
return list.stream().collect(
Collectors.toMap(this::convert, <warning descr="Function may return null, but it's not allowed here">this::convert</warning>,
(a, b) -> b, () -> <warning descr="Function may return null, but it's not allowed here">null</warning>));
}
}
@@ -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(); }
}
@@ -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(); }
@@ -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);