MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-209947 Explain boolean result from trivial contract

Browse Source 
GitOrigin-RevId: 78eef614c45e43e307f8986c79005f2ca6a1fe01
This commit is contained in:
Tagir Valeev
2019-05-07 13:03:41 +03:00
committed by intellij-monorepo-bot
parent b87b7c31c1
commit 22624a343b
11 changed files with 257 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java
+12 -2
View File
@@ -200,6 +200,9 @@ public class DataFlowRunner {
LOG.trace("Too complex because too many different possible states");
return RunnerResult.TOO_COMPLEX;
}
assert !states.isEmpty();
Instruction instruction = states.get(0).getInstruction();
beforeInstruction(instruction);
for (DfaInstructionState instructionState : states) {
lastInstructionState = instructionState;
if (count++ > stateLimit) {
@@ -214,8 +217,6 @@ public class DataFlowRunner {
// useful for quick debugging by uncommenting and hot-swapping
//System.out.println(instructionState.toString());
Instruction instruction = instructionState.getInstruction();
if (instruction instanceof BranchingInstruction) {
BranchingInstruction branching = (BranchingInstruction)instruction;
Collection<DfaMemoryState> processed = processedStates.get(branching);
@@ -271,6 +272,7 @@ public class DataFlowRunner {
queue.offer(state);
}
}
afterInstruction(instruction);
if (myCancelled) {
return RunnerResult.CANCELLED;
}
@@ -295,6 +297,14 @@ public class DataFlowRunner {
}
}
protected void beforeInstruction(Instruction instruction) {
}
protected void afterInstruction(Instruction instruction) {
}
@NotNull
private DfaInstructionState mergeBackBranches(DfaInstructionState instructionState, Collection<DfaMemoryState> processed) {
DfaMemoryStateImpl curState = (DfaMemoryStateImpl)instructionState.getMemoryState();
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
+18 -3
View File
@@ -27,11 +27,20 @@ import java.util.*;
*/
public class StandardInstructionVisitor extends InstructionVisitor {
private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.StandardInstructionVisitor");
private final boolean myStopAnalysisOnNpe;
private final Set<InstanceofInstruction> myReachable = new THashSet<>();
private final Set<InstanceofInstruction> myCanBeNullInInstanceof = new THashSet<>();
private final Set<InstanceofInstruction> myUsefulInstanceofs = new THashSet<>();
public StandardInstructionVisitor() {
myStopAnalysisOnNpe = false;
}
StandardInstructionVisitor(boolean stopAnalysisOnNpe) {
myStopAnalysisOnNpe = stopAnalysisOnNpe;
}
@Override
public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
DfaValue dfaSource = memState.pop();
@@ -597,14 +606,20 @@ public class StandardInstructionVisitor extends InstructionVisitor {
checkNotNullable(memState, memState.peek(), problem);
} else {
DfaControlTransferValue transfer = instruction.getOnNullTransfer();
DfaValue value = memState.pop();
boolean isNull = myStopAnalysisOnNpe && memState.isNull(value);
if (transfer == null) {
memState.push(dereference(memState, memState.pop(), problem));
memState.push(dereference(memState, value, problem));
if (isNull) {
return DfaInstructionState.EMPTY_ARRAY;
}
} else {
DfaValue value = memState.pop();
List<DfaInstructionState> result = new ArrayList<>();
DfaMemoryState nullState = memState.createCopy();
memState.push(dereference(memState, value, problem));
result.add(new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), memState));
if (!isNull) {
result.add(new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), memState));
}
DfaValueFactory factory = runner.getFactory();
if (nullState.applyCondition(factory.createCondition(value, RelationType.EQ, factory.getConstFactory().getNull()))) {
List<DfaInstructionState> dispatched = transfer.dispatch(nullState, runner);
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java
+117 -20
View File
@@ -11,6 +11,7 @@ import com.intellij.psi.PsiElement;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.psiutils.ExpressionUtils;
import one.util.streamex.EntryStream;
import one.util.streamex.StreamEx;
@@ -115,7 +116,14 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
void recordChange(Instruction instruction, TrackingDfaMemoryState previous) {
Map<DfaVariableValue, Change> result = new HashMap<>();
Map<DfaVariableValue, Change> result = getChangeMap(previous);
DfaValue value = isEmptyStack() ? DfaUnknownValue.getInstance() : peek();
myHistory.replaceAll(prev -> MemoryStateChange.create(prev, instruction, result, value));
}
@NotNull
private Map<DfaVariableValue, Change> getChangeMap(TrackingDfaMemoryState previous) {
Map<DfaVariableValue, Change> changeMap = new HashMap<>();
Set<DfaVariableValue> varsToCheck = new HashSet<>();
previous.forVariableStates((value, state) -> varsToCheck.add(value));
forVariableStates((value, state) -> varsToCheck.add(value));
@@ -135,7 +143,7 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
removed = removed.with((DfaFactType<Object>)type, oldVal);
}
}
result.put(value, new Change(Collections.emptySet(), Collections.emptySet(), removed, added));
changeMap.put(value, new Change(Collections.emptySet(), Collections.emptySet(), removed, added));
}
}
Map<DfaVariableValue, Set<Relation>> oldRelations = previous.getRelations();
@@ -151,20 +159,63 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
added.removeAll(oldValueRelations);
Set<Relation> removed = new HashSet<>(oldValueRelations);
removed.removeAll(newValueRelations);
result.compute(
changeMap.compute(
value, (v, change) -> change == null
? Change.create(removed, added, DfaFactMap.EMPTY, DfaFactMap.EMPTY)
: Change.create(removed, added, change.myRemovedFacts, change.myAddedFacts));
}
}
DfaValue value = isEmptyStack() ? DfaUnknownValue.getInstance() : peek();
myHistory.replaceAll(prev -> MemoryStateChange.create(prev, instruction, result, value));
return changeMap;
}
List<MemoryStateChange> getHistory() {
return myHistory;
}
/**
* Records a bridge changes. A bridge states are states which process the same input instruction,
* but in result jump to another place in the program (other than this state target).
* A bridge change is the difference between this state and all states which have different
* target instruction. Bridges allow to track what else is processed in parallel with current state,
* including states which may not arrive into target place. E.g. consider two states like this:
*
* <pre>
* this_state other_state
* | |
* some_condition <-- bridge is recorded here
* |(true) |(false)
* | return
* |
* always_true_condition <-- explanation is requested here
* </pre>
*
* Thanks to the bridge we know that {@code some_condition} could be important for
* {@code always_true_condition} explanation.
*
* @param instruction instruction which
* @param bridgeStates
*/
void addBridge(Instruction instruction, List<TrackingDfaMemoryState> bridgeStates) {
Map<DfaVariableValue, Change> changeMap = null;
for (TrackingDfaMemoryState bridge : bridgeStates) {
Map<DfaVariableValue, Change> newChangeMap = getChangeMap(bridge);
if (changeMap == null) {
changeMap = newChangeMap;
} else {
changeMap.keySet().retainAll(newChangeMap.keySet());
changeMap.replaceAll((var, old) -> old.unite(newChangeMap.get(var)));
changeMap.values().removeIf(Objects::isNull);
}
if (changeMap.isEmpty()) {
break;
}
}
if (changeMap != null && !changeMap.isEmpty()) {
Map<DfaVariableValue, Change> finalChangeMap = changeMap;
myHistory.replaceAll(s -> s.withBridge(instruction, finalChangeMap));
}
}
static class Relation {
final @NotNull RelationType myRelationType;
final @NotNull DfaValue myCounterpart;
@@ -207,6 +258,7 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
myAddedFacts = addedFacts;
}
@Nullable
static Change create(Set<Relation> removedRelations, Set<Relation> addedRelations, DfaFactMap removedFacts, DfaFactMap addedFacts) {
if (removedRelations.isEmpty() && addedRelations.isEmpty() && removedFacts == DfaFactMap.EMPTY && addedFacts == DfaFactMap.EMPTY) {
return null;
@@ -214,6 +266,20 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
return new Change(removedRelations, addedRelations, removedFacts, addedFacts);
}
/**
* Creates a Change which reflects changes actual for both this and other change
* @param other other change to unite with
* @return new change or null if this and other change has nothing in common
*/
@Nullable
Change unite(Change other) {
Set<Relation> added = new HashSet<>(ContainerUtil.intersection(myAddedRelations, other.myAddedRelations));
Set<Relation> removed = new HashSet<>(ContainerUtil.intersection(myRemovedRelations, other.myRemovedRelations));
DfaFactMap addedFacts = myAddedFacts.unite(other.myAddedFacts);
DfaFactMap removedFacts = myRemovedFacts.unite(other.myRemovedFacts);
return create(removed, added, removedFacts, addedFacts);
}
@Override
public String toString() {
String removed = StreamEx.of(myRemovedRelations).map(Object::toString).append(myRemovedFacts.toString())
@@ -229,15 +295,18 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
final @NotNull Instruction myInstruction;
final @NotNull Map<DfaVariableValue, Change> myChanges;
final @NotNull DfaValue myTopOfStack;
final @NotNull Map<DfaVariableValue, Change> myBridgeChanges;
private MemoryStateChange(@Nullable MemoryStateChange previous,
@NotNull Instruction instruction,
@NotNull Map<DfaVariableValue, Change> changes,
@NotNull DfaValue topOfStack) {
@NotNull DfaValue topOfStack,
@NotNull Map<DfaVariableValue, Change> bridgeChanges) {
myPrevious = previous;
myInstruction = instruction;
myChanges = changes;
myTopOfStack = topOfStack;
myBridgeChanges = bridgeChanges;
}
@Contract("null -> null")
@@ -264,7 +333,9 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
MemoryStateChange findRelation(DfaVariableValue value, @NotNull Predicate<Relation> relationPredicate, boolean startFromSelf) {
return findChange(change -> {
Change varChange = change.myChanges.get(value);
return varChange != null && varChange.myAddedRelations.stream().anyMatch(relationPredicate);
if (varChange != null && varChange.myAddedRelations.stream().anyMatch(relationPredicate)) return true;
Change bridgeVarChange = change.myBridgeChanges.get(value);
return bridgeVarChange != null && bridgeVarChange.myAddedRelations.stream().anyMatch(relationPredicate);
}, startFromSelf);
}
@@ -272,22 +343,32 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
<T> Pair<MemoryStateChange, T> findFact(DfaValue value, DfaFactType<T> type) {
if (value instanceof DfaVariableValue) {
for (MemoryStateChange change = this; change != null; change = change.myPrevious) {
Change varChange = change.myChanges.get(value);
if (varChange != null) {
T added = varChange.myAddedFacts.get(type);
if (added != null) {
return Pair.create(change, added);
}
if (varChange.myRemovedFacts.get(type) != null) {
return Pair.create(change, null);
}
}
Pair<MemoryStateChange, T> factPair = factFromChange(type, change, change.myChanges.get(value));
if (factPair != null) return factPair;
factPair = factFromChange(type, change, change.myBridgeChanges.get(value));
if (factPair != null) return factPair;
}
return Pair.create(null, ((DfaVariableValue)value).getInherentFacts().get(type));
}
return Pair.create(null, type.fromDfaValue(value));
}
@Nullable
private static <T> Pair<MemoryStateChange, T> factFromChange(DfaFactType<T> type,
MemoryStateChange change,
Change varChange) {
if (varChange != null) {
T added = varChange.myAddedFacts.get(type);
if (added != null) {
return Pair.create(change, added);
}
if (varChange.myRemovedFacts.get(type) != null) {
return Pair.create(change, null);
}
}
return null;
}
@Nullable
private MemoryStateChange findChange(@NotNull Predicate<MemoryStateChange> predicate, boolean startFromSelf) {
for (MemoryStateChange change = startFromSelf ? this : myPrevious; change != null; change = change.myPrevious) {
@@ -318,12 +399,13 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
return myInstruction.equals(change.myInstruction) &&
myTopOfStack.equals(change.myTopOfStack) &&
myChanges.equals(change.myChanges) &&
myBridgeChanges.equals(change.myBridgeChanges) &&
Objects.equals(myPrevious, change.myPrevious);
}
@Override
public int hashCode() {
return Objects.hash(myPrevious, myInstruction, myChanges, myTopOfStack);
return Objects.hash(myPrevious, myInstruction, myChanges, myBridgeChanges, myTopOfStack);
}
@Nullable
@@ -371,6 +453,19 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
}
}
MemoryStateChange withBridge(@NotNull Instruction instruction, @NotNull Map<DfaVariableValue, Change> bridge) {
if (myInstruction != instruction) {
if (instruction instanceof ConditionalGotoInstruction &&
getExpression() == ((ConditionalGotoInstruction)instruction).getPsiAnchor()) {
instruction = myInstruction;
} else {
return new MemoryStateChange(this, instruction, Collections.emptyMap(), DfaUnknownValue.getInstance(), bridge);
}
}
assert myBridgeChanges.isEmpty();
return new MemoryStateChange(myPrevious, instruction, myChanges, myTopOfStack, bridge);
}
@Nullable
static MemoryStateChange create(@Nullable MemoryStateChange previous,
@NotNull Instruction instruction,
@@ -379,7 +474,7 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
if (result.isEmpty() && value == DfaUnknownValue.getInstance()) {
return previous;
}
return new MemoryStateChange(previous, instruction, result, value);
return new MemoryStateChange(previous, instruction, result, value, Collections.emptyMap());
}
MemoryStateChange[] flatten() {
@@ -396,7 +491,9 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
public String toString() {
return myInstruction.getIndex() + " " + myInstruction + ": " + myTopOfStack +
(myChanges.isEmpty() ? "" :
"; Changes: " + EntryStream.of(myChanges).join(": ", "\n\t", "").joining());
"; Changes: " + EntryStream.of(myChanges).join(": ", "\n\t", "").joining()) +
(myBridgeChanges.isEmpty() ? "" :
"; Bridge changes: " + EntryStream.of(myBridgeChanges).join(": ", "\n\t", "").joining());
}
}
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java
+34 -1
View File
@@ -38,12 +38,38 @@ import java.util.stream.Stream;
public class TrackingRunner extends StandardDataFlowRunner {
private final List<MemoryStateChange> myHistoryForContext = new ArrayList<>();
private final PsiExpression myExpression;
private final List<DfaInstructionState> afterStates = new ArrayList<>();
private final List<TrackingDfaMemoryState> killedStates = new ArrayList<>();
private TrackingRunner(boolean unknownMembersAreNullable, @Nullable PsiElement context, PsiExpression expression) {
super(unknownMembersAreNullable, context);
myExpression = expression;
}
@Override
protected void beforeInstruction(Instruction instruction) {
afterStates.clear();
killedStates.clear();
}
@Override
protected void afterInstruction(Instruction instruction) {
if (afterStates.size() <= 1 && killedStates.isEmpty()) return;
Map<Instruction, List<TrackingDfaMemoryState>> instructionToState =
StreamEx.of(afterStates).mapToEntry(s -> s.getInstruction(), s -> (TrackingDfaMemoryState)s.getMemoryState()).grouping();
if (instructionToState.size() <= 1 && killedStates.isEmpty()) return;
instructionToState.forEach((target, memStates) -> {
List<TrackingDfaMemoryState> bridgeChanges =
StreamEx.of(afterStates).filter(s -> s.getInstruction() != target)
.map(s -> ((TrackingDfaMemoryState)s.getMemoryState()))
.append(killedStates)
.toList();
for (TrackingDfaMemoryState state : memStates) {
state.addBridge(instruction, bridgeChanges);
}
});
}
@NotNull
@Override
protected DfaMemoryState createMemoryState() {
@@ -57,8 +83,12 @@ public class TrackingRunner extends StandardDataFlowRunner {
TrackingDfaMemoryState memState = (TrackingDfaMemoryState)instructionState.getMemoryState().createCopy();
DfaInstructionState[] states = super.acceptInstruction(visitor, instructionState);
for (DfaInstructionState state : states) {
afterStates.add(state);
((TrackingDfaMemoryState)state.getMemoryState()).recordChange(instruction, memState);
}
if (states.length == 0) {
killedStates.add(memState);
}
if (instruction instanceof ExpressionPushingInstruction) {
ExpressionPushingInstruction pushing = (ExpressionPushingInstruction)instruction;
if (pushing.getExpression() == myExpression && pushing.getExpressionRange() == null) {
@@ -77,7 +107,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
PsiElement body = DfaUtil.getDataflowContext(expression);
if (body == null) return Collections.emptyList();
TrackingRunner runner = new TrackingRunner(unknownAreNullables, body, expression);
StandardInstructionVisitor visitor = new StandardInstructionVisitor();
StandardInstructionVisitor visitor = new StandardInstructionVisitor(true);
RunnerResult result = runner.analyzeMethodRecursively(body, visitor, ignoreAssertions);
if (result != RunnerResult.OK) return Collections.emptyList();
CauseItem cause = null;
@@ -508,6 +538,9 @@ public class TrackingRunner extends StandardDataFlowRunner {
}
}
}
if (expression instanceof PsiMethodCallExpression) {
return new CauseItem[]{fromCallContract(history, (PsiMethodCallExpression)expression, ContractReturnValue.returnBoolean(value))};
}
return new CauseItem[0];
}
java/java-tests/testData/inspection/dataFlow/tracker/ArrayLengthCollectionSize.java
+1 -1
View File
@@ -3,7 +3,7 @@ Value is always false (s.length == list.size(); line#15)
Left operand is >= 1 (s.length; line#15)
Range is known from line #12 (s[0]; line#12)
and right operand is 0 (list.size(); line#15)
Range is known from line #13 (list.isEmpty(); line#13)
Range is known from line #13 (!list.isEmpty(); line#13)
*/
import java.util.List;
java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfSecondCheck.java
+19
View File
@@ -0,0 +1,19 @@
/*
Value is always true (!isString; line#13)
Value 'isString' is always 'false' (isString; line#13)
'isString == false' was established from condition (isString; line#10)
*/
public class ExplainMe {
public int foo(Object a) {
boolean isString = a instanceof String;
if (isString) {
return 0;
}
if (<selection>!isString</selection>) {
return 1;
}
return 0;
}
}
java/java-tests/testData/inspection/dataFlow/tracker/ListAddContract.java
+12
View File
@@ -0,0 +1,12 @@
/*
Value is always true (list.add("foo"); line#9)
According to contract, method 'add' always returns 'true' value (add; line#9)
*/
import java.util.List;
class Test {
void test(List<String> list) {
if(<selection>list.add("foo")</selection>) {}
}
}
java/java-tests/testData/inspection/dataFlow/tracker/NullCheckNpeNullCheck.java
+22
View File
@@ -0,0 +1,22 @@
/*
Value is always true (map != null; line#18)
'map' was dereferenced (map; line#14)
*/
import java.util.Map;
public class ExplainMe {
void checkTheMap(Map<String, Integer> map) {
if (map == null) {
System.out.println("that's null");
}
if (map.get("ONE") == null) {
System.out.println("Okay");
}
if (<selection>map != null</selection>) {
System.out.println("not null");
}
}
}
java/java-tests/testData/inspection/dataFlow/tracker/RepeatingIntegerComparisonThreeBranches.java
+3 -3
View File
@@ -1,12 +1,12 @@
/*
Value is always true (x < y; line#13)
Condition 'x < y' was checked before (x > y; line#7)
Condition 'x < y' was checked before (x == y; line#9)
*/
class Test {
void test(int x, int y) {
if (x > y) return;
if (x > y) return; // would be better to point also here, but acceptable
if (x == y) { // explanation doesn't point here: not entirely correct, but hard to fix; postponed
if (x == y) {
return;
}
java/java-tests/testData/inspection/dataFlow/tracker/TripleCheck.java
+15
View File
@@ -0,0 +1,15 @@
/*
Value is always false (x; line#12)
'x == false' was established from condition (x; line#9)
*/
class Test {
void test(boolean x) {
if(x) {}
if(x) {
} else {
if(<selection>x</selection>) {}
}
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java
+4
View File
@@ -152,4 +152,8 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa
public void testAssignTernaryNotNull() { doTest(); }
public void testAssignTernaryNumeric() { doTest(); }
public void testTrivialContract() { doTest(); }
public void testTripleCheck() { doTest(); }
public void testInstanceOfSecondCheck() { doTest(); }
public void testNullCheckNpeNullCheck() { doTest(); }
public void testListAddContract() { doTest(); }
}