From 4faa707528adc9aa96ec306ee92cfdac7696fa3e Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 28 Sep 2022 11:52:23 +0200 Subject: [PATCH] [java-dfa] Support groupingBy/partitioningBy/groupingByConcurrent collectors, to some extent (w/o downstream) Fixes IDEA-302543 'Optional.get()' without 'isPresent()' in groupingBy Collector despite checking it earlier with filter in the stream GitOrigin-RevId: c562ca1479e32c16517474db937528808c1f0fb8 --- .../java/inliner/StreamChainInliner.java | 43 +++++++++++++++++++ .../dataFlow/fixture/StreamGroupingBy.java | 31 +++++++++++++ .../optionalGet/StreamGroupingBy.java | 40 +++++++++++++++++ .../DataFlowInspection8Test.java | 1 + ...onalGetWithoutIsPresentInspectionTest.java | 1 + 5 files changed, 116 insertions(+) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/StreamGroupingBy.java create mode 100644 java/java-tests/testData/inspection/optionalGet/StreamGroupingBy.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/StreamChainInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/StreamChainInliner.java index 24e8efa71795..44651a8677ec 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/StreamChainInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/StreamChainInliner.java @@ -84,6 +84,8 @@ public class StreamChainInliner implements CallInliner { staticCall(JAVA_UTIL_STREAM_COLLECTORS, "toCollection").parameterCount(1)); private static final CallMatcher MAP_COLLECTOR = staticCall(JAVA_UTIL_STREAM_COLLECTORS, "toMap", "toConcurrentMap", "toUnmodifiableMap"); + private static final CallMatcher GROUPING_COLLECTOR = + staticCall(JAVA_UTIL_STREAM_COLLECTORS, "groupingBy", "partitioningBy", "groupingByConcurrent"); private static final CallMatcher NOT_NULL_COLLECTORS = staticCall(JAVA_UTIL_STREAM_COLLECTORS, "joining", "maxBy", "minBy", "averagingInt", "averagingLong", "averagingDouble", "summingInt", "summingLong", "summingDouble", "summarizingInt", "summarizingLong", "summarizingDouble"); @@ -689,6 +691,9 @@ public class StreamChainInliner implements CallInliner { } else { dfType = dfType.meet(DfTypes.LOCAL_OBJECT); } + if (SpecialField.fromQualifierType(dfType) == SpecialField.COLLECTION_SIZE) { + dfType = dfType.meet(SpecialField.COLLECTION_SIZE.asDfType(DfTypes.intValue(0))); + } builder.push(dfType); } } @@ -779,6 +784,35 @@ public class StreamChainInliner implements CallInliner { } } + static class GroupingStep extends AbstractCollectionStep { + private final @NotNull PsiExpression myKeyExtractor; + private final @Nullable PsiExpression myDownstream; + + GroupingStep(@NotNull PsiMethodCallExpression call, + @NotNull PsiExpression keyExtractor, + @Nullable PsiExpression downstream, + @Nullable PsiExpression supplier) { + super(call, supplier, false); + myKeyExtractor = keyExtractor; + myDownstream = downstream; + } + + @Override + void before(CFGBuilder builder) { + builder.evaluateFunction(myKeyExtractor) + .evaluateFunction(myDownstream); + super.before(builder); + } + + @Override + void iteration(CFGBuilder builder) { + // Null keys are not tolerated + builder.invokeFunction(1, myKeyExtractor, Nullability.NOT_NULL); + // Actual addition of Map element is unnecessary for current analysis + builder.flush(SpecialField.COLLECTION_SIZE.createValue(builder.getFactory(), myResult)).pop(); + } + } + @Override public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { if (TERMINAL_CALL.test(call)) { @@ -948,6 +982,15 @@ public class StreamChainInliner implements CallInliner { "toUnmodifiableMap".equals(collectorCall.getMethodExpression().getReferenceName())); } } + if (GROUPING_COLLECTOR.matches(collectorCall)) { + PsiExpression[] args = collectorCall.getArgumentList().getExpressions(); + if (args.length >= 1 && args.length <= 3) { + PsiExpression keyExtractor = args[0]; + PsiExpression downstream = args.length > 1 ? args[args.length - 1] : null; + PsiExpression supplier = args.length == 3 ? args[1] : null; + return new GroupingStep(call, keyExtractor, downstream, supplier); + } + } return new UnknownTerminalStep(call, NOT_NULL_COLLECTORS.test(collectorCall)); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamGroupingBy.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamGroupingBy.java new file mode 100644 index 000000000000..2c0cc4748f92 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamGroupingBy.java @@ -0,0 +1,31 @@ +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; + +class Demo { + void test(List list) { + Map> map1 = list.stream() + .filter(Objects::isNull) + .collect(Collectors.groupingBy(x -> x.trim())); + if (list.isEmpty() && map1.isEmpty()) {} + Map> map2 = list.stream() + .filter(Objects::isNull) + .collect(Collectors.groupingByConcurrent(x -> x.trim(), Collectors.toList())); + Map> map3 = list.stream() + .filter(Objects::isNull) + .collect(Collectors.groupingBy(null, null, null)); + Map> map4 = list.stream() + .filter(Objects::isNull) + .collect(Collectors.groupingBy(x -> null)); + Map> map5 = list.stream() + .filter(x -> !x.isEmpty()) + .collect(Collectors.partitioningBy(x -> x.isEmpty())); + Map> map6 = list.stream() + .filter(x -> x.isEmpty()) + .collect(Collectors.partitioningBy(x -> x.isEmpty(), Collectors.toList())); + Map> map7 = list.stream() + .filter(x -> !x.isEmpty()) + .collect(Collectors.partitioningBy(x -> list.isEmpty(), Collectors.toList())); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/optionalGet/StreamGroupingBy.java b/java/java-tests/testData/inspection/optionalGet/StreamGroupingBy.java new file mode 100644 index 000000000000..a1b52250865f --- /dev/null +++ b/java/java-tests/testData/inspection/optionalGet/StreamGroupingBy.java @@ -0,0 +1,40 @@ +import java.util.*; +import java.util.stream.*; + +class TestClassContainingOptional { + + void test() { + List list = new ArrayList<>(); + list.add(new TestClassContainingOptional("name1", "optional")); + list.add(new TestClassContainingOptional("name2")); + System.out.println(list.stream() + .filter(e -> e.getOptionalString().isPresent()) + .collect(Collectors.groupingBy(e -> e.getOptionalString().get())) + ); + System.out.println(list.stream() + .filter(e -> e.getOptionalString().isPresent()) + .collect(Collectors.groupingBy(e -> e.getOptionalString().get(), Collectors.toSet())) + ); + TreeMap> collect = list.stream() + .filter(e -> e.getOptionalString().isPresent()) + .collect(Collectors.groupingBy(e -> e.getOptionalString().get(), TreeMap::new, Collectors.toSet())); + System.out.println(collect); + } + + String name; + String optionalString = null; + + + public TestClassContainingOptional(String name) { + this.name = name; + } + + public TestClassContainingOptional(String name, String optionalString) { + this.name = name; + this.optionalString = optionalString; + } + + public Optional getOptionalString() { + return Optional.ofNullable(optionalString); + } +} \ 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 be03bc2673c9..fadf4b7ef366 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -218,6 +218,7 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { public void testStreamCustomSumMethod() { doTest(); } public void testStreamReduceLogicalAnd() { doTest(); } public void testStreamSingleElementReduce() { doTest(); } + public void testStreamGroupingBy() { doTest(); } public void testRequireNonNullMethodRef() { doTestWith((dfa, __) -> dfa.SUGGEST_NULLABLE_ANNOTATIONS = true); } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionTest.java index c9488f4359b3..88aa439948c0 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionTest.java @@ -18,6 +18,7 @@ public class OptionalGetWithoutIsPresentInspectionTest extends LightJavaCodeInsi public void testOptionalGet() { doTest(); } public void testOptionalGetInlineLambda() { doTest(); } public void testOptionalGetMethodReference() { doTest(); } + public void testStreamGroupingBy() { doTest(); } @NotNull @Override