From d2e2a474ad52e0a46432f18c58fd953219b20b63 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 18 Jan 2022 16:26:24 +0700 Subject: [PATCH] [java-dfa] First class support for Collection.removeIf() Fixes IDEA-286737 Side-effects from removeIf predicate are not taken into account GitOrigin-RevId: 6efc71f98d10824a288e4510a209da34ad381bbb --- .../dataFlow/java/ControlFlowAnalyzer.java | 2 +- .../java/inliner/CollectionUpdateInliner.java | 62 +++++++++++++++++++ .../dataFlow/fixture/CollectionRemoveIf.java | 36 +++++++++++ .../DataFlowRangeAnalysisTest.java | 3 + java/jdkAnnotations/java/util/annotations.xml | 5 -- 5 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/CollectionUpdateInliner.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/CollectionRemoveIf.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java index 0ba5345edddb..861579a25ec5 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java @@ -2394,7 +2394,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } private static final CallInliner[] INLINERS = { - new OptionalChainInliner(), new LambdaInliner(), + new OptionalChainInliner(), new LambdaInliner(), new CollectionUpdateInliner(), new StreamChainInliner(), new MapUpdateInliner(), new AssumeInliner(), new ClassMethodsInliner(), new AssertAllInliner(), new BoxingInliner(), new SimpleMethodInliner(), new TransformInliner(), new EnumCompareInliner(), new IndexOfInliner() diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/CollectionUpdateInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/CollectionUpdateInliner.java new file mode 100644 index 000000000000..de8d607d6168 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/CollectionUpdateInliner.java @@ -0,0 +1,62 @@ +/* + * Copyright 2000-2017 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.codeInspection.dataFlow.java.inliner; + +import com.intellij.codeInspection.dataFlow.DfaPsiUtil; +import com.intellij.codeInspection.dataFlow.Mutability; +import com.intellij.codeInspection.dataFlow.NullabilityUtil; +import com.intellij.codeInspection.dataFlow.java.CFGBuilder; +import com.intellij.codeInspection.dataFlow.jvm.SpecialField; +import com.intellij.codeInspection.dataFlow.jvm.problems.MutabilityProblem; +import com.intellij.codeInspection.dataFlow.types.DfType; +import com.intellij.codeInspection.dataFlow.types.DfTypes; +import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; +import com.intellij.codeInspection.dataFlow.value.RelationType; +import com.intellij.psi.CommonClassNames; +import com.intellij.psi.PsiExpression; +import com.intellij.psi.PsiMethodCallExpression; +import com.intellij.psi.PsiType; +import com.intellij.psi.util.PsiUtil; +import com.siyeh.ig.callMatcher.CallMatcher; +import org.jetbrains.annotations.NotNull; + +public class CollectionUpdateInliner implements CallInliner { + private static final CallMatcher COLLECTION_REMOVEIF = CallMatcher.instanceCall( + CommonClassNames.JAVA_UTIL_COLLECTION, "removeIf").parameterTypes(CommonClassNames.JAVA_UTIL_FUNCTION_PREDICATE); + + @Override + public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { + if (COLLECTION_REMOVEIF.test(call)) { + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return false; + PsiType elementType = PsiUtil.substituteTypeParameter(qualifier.getType(), CommonClassNames.JAVA_UTIL_COLLECTION, 0, true); + DfType elementDfType = DfTypes.typedObject(elementType, DfaPsiUtil.getTypeNullability(elementType)); + PsiExpression predicate = call.getArgumentList().getExpressions()[0]; + DfaVariableValue result = builder.createTempVariable(PsiType.BOOLEAN); + builder + .assignAndPop(result, DfTypes.FALSE) + .pushExpression(qualifier) // stack: qualifier + .ensure(RelationType.IS, Mutability.MUTABLE.asDfType(), new MutabilityProblem(call, true), null) + .evaluateFunction(predicate) + .unwrap(SpecialField.COLLECTION_SIZE) // stack: qualifier.size + .dup() // stack: qualifier.size qualifier.size + .push(DfTypes.intValue(0)) // stack: qualifier.size qualifier.size 0 + .ifCondition(RelationType.GT) + .doWhileUnknown() + .push(elementDfType) + .invokeFunction(1, predicate) + .ifConditionIs(true) + .pushUnknown() + .assign() + .assignAndPop(result, DfTypes.TRUE) + .end() + .end() + .end() + .pop() + .push(result); + return true; + } + return false; + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/CollectionRemoveIf.java b/java/java-tests/testData/inspection/dataFlow/fixture/CollectionRemoveIf.java new file mode 100644 index 000000000000..e154052292c8 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/CollectionRemoveIf.java @@ -0,0 +1,36 @@ +import java.util.Collection; +import java.util.Collections; + +class RemoveIf +{ + void unmodifiable(Collection c) { + c = Collections.unmodifiableCollection(c); + c.removeIf(x -> true); + } + + void falsePredicate(Collection c) { + int size = c.size(); + c.removeIf(x -> false); + if (size == c.size()) {} + c.removeIf(String::isEmpty); + if (size == c.size()) {} + } + + void empty(Collection c) { + if (!c.isEmpty()) return; + c.removeIf(x -> c.add(x)); + if (c.isEmpty()) {} + } + + int x; + + void returnValueAndSideEffect(Collection c) { + x = 0; + boolean ret = c.removeIf(v -> { + x = 1; + return v.isEmpty(); + }); + if (x == 0 && ret) {} + if (x == 1 && ret) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java index 30e781019609..be408aeb330c 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowRangeAnalysisTest.java @@ -78,6 +78,9 @@ public class DataFlowRangeAnalysisTest extends DataFlowInspectionTestCase { public void testWidenMismatch() { doTest(); } public void testDontWidenPlusInLoop() { doTest(); } public void testCollectionAddRemove() { doTest(); } + + public void testCollectionRemoveIf() { doTest(); } + public void testRelationsOnAddition() { doTest(); } public void testModSpecialCase() { doTest(); } public void testArrayAccessWithCastInCountedLoop() { doTest(); } diff --git a/java/jdkAnnotations/java/util/annotations.xml b/java/jdkAnnotations/java/util/annotations.xml index 6f04844607e5..96f33690769a 100644 --- a/java/jdkAnnotations/java/util/annotations.xml +++ b/java/jdkAnnotations/java/util/annotations.xml @@ -1899,11 +1899,6 @@ - - - - -