[java-dfa] Check mutability for inlined Map methods

Fixes IDEA-285183 Enhance dfa: Add check mutabilaty for Map.merge and Map.compute
Also fix the lattice for Mutability
Tests from PR#1877

Co-authored-by: pyltsin-m <pyltsin-m@yandex.ru>

GitOrigin-RevId: 9b29b2ccf3d56092e0ef5ecf1a00243cfeb1e493
This commit is contained in:
Tagir Valeev
2021-12-20 07:01:11 +00:00
committed by intellij-monorepo-bot
co-authored by pyltsin-m
parent 5a9337a78c
commit 973ebe7d60
6 changed files with 63 additions and 15 deletions
@@ -35,9 +35,21 @@ import org.jetbrains.annotations.PropertyKey;
import java.util.Collections;
import java.util.List;
/**
* Lattice:
* UNKNOWN
* | \
* MUTABLE MUST_NOT_MODIFY
* | |
* | UNMODIFIABLE_VIEW
* | |
* | UNMODIFIABLE
* \ /
* BOTTOM (null)
*/
public enum Mutability {
/**
* Mutability is not known; probably value can be mutated
* Mutability is not known; probably value can be mutated; TOP
*/
UNKNOWN("mutability.unknown", null),
/**
@@ -96,23 +108,28 @@ public enum Mutability {
}
@NotNull
public Mutability unite(Mutability other) {
public Mutability join(@NotNull Mutability other) {
if (this == other) return this;
if (this == UNKNOWN || other == UNKNOWN) return UNKNOWN;
if (this == MUTABLE || other == MUTABLE) return MUTABLE;
if (this == MUTABLE || other == MUTABLE) return UNKNOWN;
if (this == MUST_NOT_MODIFY || other == MUST_NOT_MODIFY) return MUST_NOT_MODIFY;
if (this == UNMODIFIABLE_VIEW || other == UNMODIFIABLE_VIEW) return UNMODIFIABLE_VIEW;
return UNMODIFIABLE;
}
@NotNull
public Mutability intersect(Mutability other) {
/**
* @param other mutability to meet
* @return resulting mutability; null if bottom
*/
@Nullable
public Mutability meet(@NotNull Mutability other) {
if (this == other) return this;
if (this == UNKNOWN) return other;
if (other == UNKNOWN) return this;
if (this == MUTABLE || other == MUTABLE) return null;
if (this == UNMODIFIABLE || other == UNMODIFIABLE) return UNMODIFIABLE;
if (this == UNMODIFIABLE_VIEW || other == UNMODIFIABLE_VIEW) return UNMODIFIABLE_VIEW;
if (this == MUST_NOT_MODIFY || other == MUST_NOT_MODIFY) return MUST_NOT_MODIFY;
if (this == MUTABLE || other == MUTABLE) return MUTABLE;
return UNKNOWN;
return MUST_NOT_MODIFY;
}
@Nullable
@@ -197,7 +214,7 @@ public enum Mutability {
newMutability = method == null ? UNKNOWN : getMutability(method);
}
}
mutability = mutability.unite(newMutability);
mutability = mutability.join(newMutability);
if (!mutability.isUnmodifiable()) break;
}
return mutability;
@@ -128,7 +128,7 @@ interface MethodReturnInferenceResult {
return Mutability.UNKNOWN
}
return delegateCalls.stream().map { range -> getDelegateMutability(method, range, body()) }.reduce(
Mutability::unite).orElse(
Mutability::join).orElse(
Mutability.UNKNOWN)
}
@@ -4,9 +4,12 @@
package com.intellij.codeInspection.dataFlow.java.inliner;
import com.intellij.codeInsight.Nullability;
import com.intellij.codeInspection.dataFlow.Mutability;
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.DfTypes;
import com.intellij.codeInspection.dataFlow.value.RelationType;
import com.intellij.psi.CommonClassNames;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiMethodCallExpression;
@@ -35,6 +38,7 @@ public class MapUpdateInliner implements CallInliner {
PsiExpression function = args[1];
builder
.pushExpression(qualifier) // stack: .. qualifier
.ensure(RelationType.IS, Mutability.MUTABLE.asDfType(), new MutabilityProblem(call, true), null)
.pushExpression(key) // stack: .. qualifier; key
.evaluateFunction(function);
String name = Objects.requireNonNull(call.getMethodExpression().getReferenceName());
@@ -65,6 +69,7 @@ public class MapUpdateInliner implements CallInliner {
PsiExpression function = args[2];
builder
.pushExpression(qualifier) // stack: .. qualifier
.ensure(RelationType.IS, Mutability.MUTABLE.asDfType(), new MutabilityProblem(call, true), null)
.pushExpression(key) // stack: .. qualifier; key
.pop() // stack: .. qualifier
.pushExpression(value) // stack: .. qualifier; value
@@ -163,7 +163,7 @@ final class DfGenericObjectType extends DfAntiConstantType<Object> implements Df
if (type.getNullability() != getNullability() && getNullability() != DfaNullability.UNKNOWN &&
type.getNullability() != DfaNullability.NOT_NULL) return false;
if (!getConstraint().isSuperConstraintOf(type.getConstraint())) return false;
if (getMutability().ordinal() > type.getMutability().ordinal()) return false;
if (getMutability().join(type.getMutability()) != getMutability()) return false;
SpecialField sf = getSpecialField();
if (sf != null) {
if (sf != type.getSpecialField()) return false;
@@ -197,7 +197,7 @@ final class DfGenericObjectType extends DfAntiConstantType<Object> implements Df
throw new AssertionError("Join failed: " + this + " | " + other);
}
DfaNullability nullability = getNullability().unite(type.getNullability());
Mutability mutability = getMutability().unite(type.getMutability());
Mutability mutability = getMutability().join(type.getMutability());
boolean locality = isLocal() && type.isLocal();
SpecialField sf = Objects.equals(getSpecialField(), type.getSpecialField()) ? getSpecialField() : null;
DfType sfType = sf == null ? BOTTOM : getSpecialFieldType().join(type.getSpecialFieldType());
@@ -266,7 +266,7 @@ final class DfGenericObjectType extends DfAntiConstantType<Object> implements Df
mySpecialField, mySpecialFieldType, myLocal);
}
case MUTABILITY:
return new DfGenericObjectType(myNotValues, myConstraint, myNullability, myMutability.unite(otherMutability),
return new DfGenericObjectType(myNotValues, myConstraint, myNullability, myMutability.join(otherMutability),
mySpecialField, mySpecialFieldType, myLocal);
case NULLABILITY:
return new DfGenericObjectType(myNotValues, myConstraint, myNullability.unite(otherNullability), myMutability,
@@ -304,7 +304,8 @@ final class DfGenericObjectType extends DfAntiConstantType<Object> implements Df
}
DfaNullability nullability = getNullability().intersect(type.getNullability());
if (nullability == null) return BOTTOM;
Mutability mutability = getMutability().intersect(type.getMutability());
Mutability mutability = getMutability().meet(type.getMutability());
if (mutability == null) return BOTTOM;
boolean locality = isLocal() || type.isLocal();
SpecialField sf;
DfType sfType;
@@ -108,7 +108,7 @@ public class DfReferenceConstantType extends DfConstantType<Object> implements D
DfReferenceType type = (DfReferenceType)other;
TypeConstraint constraint = getConstraint().join(type.getConstraint());
DfaNullability nullability = getNullability().unite(type.getNullability());
Mutability mutability = getMutability().unite(type.getMutability());
Mutability mutability = getMutability().join(type.getMutability());
boolean locality = isLocal() && type.isLocal();
SpecialField sf = Objects.equals(getSpecialField(), type.getSpecialField()) ? getSpecialField() : null;
DfType sfType = sf == null ? BOTTOM : getSpecialFieldType().join(type.getSpecialFieldType());
@@ -54,4 +54,29 @@ public class MutabilityJdk9 {
list.<warning descr="Immutable object is modified">add</warning>("foo");
}
}
void testMapOfEntriesMerge() {
Map<String, Integer> map = Map.ofEntries(Map.entry("x", 1), Map.entry("y", 2));
map.<warning descr="Immutable object is modified">merge</warning>("a", 1, (x, y)->y);
}
void testMapCompute() {
Map<String, Integer> map = Map.of("a", 1, "b", 2, "c", 3, "d", 4, "e", 5);
map.<warning descr="Immutable object is modified">compute</warning>("a", (x, y)->y);
}
void testMapComputeIfPresent() {
Map<String, Integer> map = Map.of("a", 1, "b", 2, "c", 3, "d", 4, "e", 5);
map.<warning descr="Immutable object is modified">computeIfPresent</warning>("a", (x, y)->y);
}
void testMapComputeIfAbsent() {
Map<String, Integer> map = Map.of("a", 1, "b", 2, "c", 3, "d", 4, "e", 5);
map.<warning descr="Immutable object is modified">computeIfAbsent</warning>("a", x->1);
}
void testMapMerge() {
Map<String, Integer> map = Map.of("a", 1, "b", 2, "c", 3, "d", 4, "e", 5);
map.<warning descr="Immutable object is modified">merge</warning>("a", 1, (x, y)->y);
}
}