From 1137e3ab2a59e4fdb856854c102e7c8ee07ca354 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 14 Nov 2022 12:34:02 +0100 Subject: [PATCH] [java-dfa] DfaMemoryState.updateDfType is introduced to properly update the aliases Fixes IDEA-304297 Incorrect empty collection inspection with ad hoc collector GitOrigin-RevId: 496e7aef00680696561508ad33670f833bab15ee --- .../dataFlow/java/JavaDfaHelpers.java | 12 ++++------ .../java/inst/MethodCallInstruction.java | 18 ++++++--------- .../fixture/StreamCollectInlining.java | 14 +++++++---- .../dataFlow/memory/DfaMemoryState.java | 18 +++++++++++++++ .../dataFlow/memory/DfaMemoryStateImpl.java | 23 ++++++++++++++++--- 5 files changed, 59 insertions(+), 26 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaHelpers.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaHelpers.java index c3ab76cddc9c..1f3a5a370de1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaHelpers.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaHelpers.java @@ -16,6 +16,7 @@ import com.intellij.psi.util.PsiUtil; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; +import java.util.function.UnaryOperator; /** * Utility class to help interpreting the Java DFA @@ -35,15 +36,10 @@ public class JavaDfaHelpers { return value; } DfaVariableValue var = (DfaVariableValue)value; - DfType dfType = state.getDfType(var); - if (dfType instanceof DfReferenceType) { - state.setDfType(var, ((DfReferenceType)dfType).dropLocality()); - } + UnaryOperator<@NotNull DfType> updater = dfType -> dfType instanceof DfReferenceType refType ? refType.dropLocality() : dfType; + state.updateDfType(var, updater); for (DfaVariableValue v : new ArrayList<>(var.getDependentVariables())) { - dfType = state.getDfType(v); - if (dfType instanceof DfReferenceType) { - state.setDfType(v, ((DfReferenceType)dfType).dropLocality()); - } + state.updateDfType(v, updater); } return value; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java index 0c3edfe730a5..a7a545f86b9f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java @@ -442,11 +442,9 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { @NotNull DfaMemoryState memState, DfaValue @Nullable [] argValues) { DfaValue value = memState.pop(); - if (getContext() instanceof PsiMethodReferenceExpression) { - PsiMethodReferenceExpression context = (PsiMethodReferenceExpression)getContext(); - if (MethodReferenceInstruction.isQualifierDereferenced(context)) { - value = CheckNotNullInstruction.dereference(interpreter, memState, value, NullabilityProblemKind.callMethodRefNPE.problem(context, null)); - } + if (getContext() instanceof PsiMethodReferenceExpression context && MethodReferenceInstruction.isQualifierDereferenced(context)) { + value = CheckNotNullInstruction.dereference( + interpreter, memState, value, NullabilityProblemKind.callMethodRefNPE.problem(context, null)); } DfType dfType = memState.getDfType(value); if (getMutationSignature().mutatesThis() && !Mutability.fromDfType(dfType).canBeModified()) { @@ -455,9 +453,8 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { // So let's conservatively skip the warning here. Such contract is still useful because it assures that nothing else is mutated. if (method != null && JavaMethodContractUtil.hasExplicitContractAnnotation(method)) { interpreter.getListener().onCondition(new MutabilityProblem(getContext(), true), value, ThreeState.YES, memState); - if (dfType instanceof DfReferenceType) { - memState.setDfType(value, ((DfReferenceType)dfType).dropMutability().meet(Mutability.MUTABLE.asDfType())); - } + memState.updateDfType( + value, type -> type instanceof DfReferenceType refType ? refType.dropMutability().meet(Mutability.MUTABLE.asDfType()) : type); } } TypeConstraint constraint = TypeConstraint.fromDfType(dfType); @@ -509,9 +506,8 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { !memState.getDfType(SpecialField.ARRAY_LENGTH.createValue(interpreter.getFactory(), arg)).equals(intValue(0))) { PsiElement anchor = getArgumentAnchor(paramIndex); interpreter.getListener().onCondition(new MutabilityProblem(anchor, false), arg, ThreeState.YES, memState); - if (dfType instanceof DfReferenceType) { - memState.setDfType(arg, ((DfReferenceType)dfType).dropMutability().meet(Mutability.MUTABLE.asDfType())); - } + memState.updateDfType( + arg, type -> type instanceof DfReferenceType refType ? refType.dropMutability().meet(Mutability.MUTABLE.asDfType()) : type); } } if (argValues != null) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollectInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollectInlining.java index 5a33060613e4..fe31f476ae50 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollectInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamCollectInlining.java @@ -1,12 +1,18 @@ import foo.NotNull; import foo.Nullable; -import java.util.Collection; -import java.util.HashMap; -import java.util.Map; -import java.util.Objects; +import java.util.*; class Clazz { + // IDEA-304297 + void customCollectToHashMap(List list2) { + Collection list = Arrays.asList("test"); + Map aux = list.stream().collect(() -> new HashMap<>(), (a, b) -> a.put(b, b.length()), Map::putAll); + if (aux.isEmpty()) {} + Map aux2 = list2.stream().collect(() -> new HashMap<>(), (a, b) -> a.put(b, b.length()), Map::putAll); + if (aux.isEmpty()) {} + } + public void passNull(Collection c) { c.stream().collect(null, null, diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java index 9c49aad68a27..ee01b172b59d 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryState.java @@ -11,6 +11,7 @@ import org.jetbrains.annotations.Nullable; import java.util.Set; import java.util.function.Predicate; +import java.util.function.UnaryOperator; /** * Represents a memory state of abstract interpreter. @@ -132,12 +133,29 @@ public interface DfaMemoryState { * Forcibly sets the supplied dfType to given value if given value state can be memoized. * This is necessary to override some knowledge about the variable state. In most of the cases * {@link #meetDfType(DfaValue, DfType)} should be used as it narrows existing type. + *

+ * Use of this method is in general discouraged, as it doesn't update the type of known aliases, + * which may cause subtle bugs. Consider using {@link #updateDfType(DfaValue, UnaryOperator)} instead. + *

* * @param value value to update. * @param dfType type to assign to value. Note that type might be adjusted, e.g. to be compatible with value declared PsiType. */ void setDfType(@NotNull DfaValue value, @NotNull DfType dfType); + /** + * Forcibly updates the dfType for given value if given value state can be memoized. + * Known aliases are updated as well. This is necessary to override some knowledge about the variable state. + * This may happen if contradiction is found and reported, but you want to continue the analysis, or + * if you need to adjust mutable property like object locality. In most of the cases {@link #meetDfType(DfaValue, DfType)} + * should be used as it narrows existing type. + * + * @param value value to update. + * @param updater a function that accepts the current dfType and returns the updated one. May be called + * several times for every alias. + */ + void updateDfType(@NotNull DfaValue value, @NotNull UnaryOperator<@NotNull DfType> updater); + /** * @param value value to get the type of * @return the DfType of the value within this memory state diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java index e4b4cf57a36a..baaa823293ac 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/dataFlow/memory/DfaMemoryStateImpl.java @@ -82,11 +82,11 @@ public class DfaMemoryStateImpl implements DfaMemoryState { copy.flushFields(new QualifierStatusMap(null, true)); copy.emptyStack(); for (DfaValue value : getFactory().getValues().toArray(new DfaValue[0])) { - if (value instanceof DfaVariableValue) { - DfType type = copy.getDfType(value); + if (value instanceof DfaVariableValue var) { + DfType type = copy.getDfType(var); DfType newType = type.correctForClosure(); if (newType != type) { - copy.setDfType(value, newType); + copy.recordVariableType(var, newType); } } } @@ -752,6 +752,23 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } } + @Override + public void updateDfType(@NotNull DfaValue value, @NotNull UnaryOperator<@NotNull DfType> updater) { + if (!(value instanceof DfaVariableValue var)) return; + EqClass values = getEqClass(var); + Iterable vars = values == null ? List.of(var) : values; + for (@NotNull DfaVariableValue eqVar : vars) { + DfType type = getRecordedType(eqVar); + if (type == null) { + type = eqVar.getInherentType(); + } + DfType newType = updater.apply(type); + if (!newType.equals(type)) { + recordVariableType(eqVar, newType); + } + } + } + @Override public boolean meetDfType(@NotNull DfaValue value, @NotNull DfType dfType) { if (dfType == DfType.TOP) return true;