From b415bd616ac6f62de23fc74f383dcb4909b03c7e Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 20 May 2019 17:18:31 +0700 Subject: [PATCH] IDEA-209947 Try to deduce relation from contracts GitOrigin-RevId: 93c921e8b72b89658aadcc4a665cef820e25ebe7 --- .../dataFlow/DfaMemoryStateImpl.java | 4 +- .../dataFlow/TrackingRunner.java | 69 +++++++++++++------ .../tracker/BasedOnPreviousRelation.java | 2 +- .../BasedOnPreviousRelationContracts.java | 18 +++++ .../DataFlowInspectionTrackerTest.java | 6 +- 5 files changed, 75 insertions(+), 24 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelationContracts.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index bb86f2168e9a..e25386f97319 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -1342,7 +1342,9 @@ public class DfaMemoryStateImpl implements DfaMemoryState { @Override public void forceVariableFact(@NotNull DfaVariableValue var, @NotNull DfaFactType factType, @Nullable T value) { DfaVariableState state = getVariableState(var); - removeEquivalenceForVariableAndWrappers(var); + if (factType.equals(DfaFactType.NULLABILITY) && value == DfaNullability.NOT_NULL && isNull(var)) { + removeEquivalenceForVariableAndWrappers(var); + } setVariableState(var, state.withFact(factType, value)); updateEqClassesByState(var); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java index 26f640ccacab..9519cd655703 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java @@ -823,26 +823,31 @@ public class TrackingRunner extends StandardDataFlowRunner { } PsiExpression expression = change.getExpression(); if (expression != null) { + Collection relations = Collections.emptyList(); if (expression instanceof PsiBinaryExpression) { - DfaRelationValue rel = fromBinaryExpression(change, (PsiBinaryExpression)expression); + DfaRelationValue rel = getBinaryExpressionRelation(change, (PsiBinaryExpression)expression); if (rel != null) { if (isSameRelation(rel, value, relation)) { return new CauseItem(new CustomDfaProblemType("condition '" + condition + "' was checked before"), expression); } - List chain = findDeductionChain(change, Collections.singletonList(rel), value, relation); - if (!chain.isEmpty()) { - CauseItem[] result = new CauseItem[0]; - for (DfaRelationValue deduced : chain) { - CauseItem[] cause = - findRelationCause(deduced.getRelation(), change, deduced.getLeftOperand(), change, deduced.getRightOperand()); - result = ArrayUtil.mergeArrays(result, cause); - } - if (result.length > 1) { - CauseItem item = new CauseItem(new CustomDfaProblemType("condition '" + condition + "' was deduced"), (PsiElement)null); - item.addChildren(result); - return item; - } - } + relations = Collections.singleton(rel); + } + } + if (expression instanceof PsiCallExpression) { + relations = getCallRelations((PsiCallExpression)expression); + } + List chain = findDeductionChain(change, relations, value, relation); + if (!chain.isEmpty()) { + CauseItem[] result = new CauseItem[0]; + for (DfaRelationValue deduced : chain) { + CauseItem[] cause = + findRelationCause(deduced.getRelation(), change, deduced.getLeftOperand(), change, deduced.getRightOperand()); + result = ArrayUtil.mergeArrays(result, cause); + } + if (result.length > 1) { + CauseItem item = new CauseItem(new CustomDfaProblemType("condition '" + condition + "' was deduced"), (PsiElement)null); + item.addChildren(result); + return item; } } return new CauseItem(new CustomDfaProblemType("result of '" + condition + "' is known from #ref"), expression); @@ -851,16 +856,19 @@ public class TrackingRunner extends StandardDataFlowRunner { } private List findDeductionChain(MemoryStateChange change, - List knownRelations, + Collection knownRelations, DfaVariableValue value, Relation relation) { for (DfaRelationValue rel : knownRelations) { + if (isSameRelation(rel, value, relation)) { + continue; + } for (Map.Entry entry : change.myChanges.entrySet()) { DfaVariableValue actualVar = entry.getKey(); for (Relation actualRelation : entry.getValue().myAddedRelations) { if (isSameRelation(rel, actualVar, actualRelation)) { DfaValue left; - DfaValue right = actualRelation.myCounterpart; + DfaValue right; RelationType type; if (actualRelation.myRelationType == RelationType.EQ || (relation.myRelationType != RelationType.NE && relation.myRelationType == actualRelation.myRelationType)) { @@ -873,11 +881,20 @@ public class TrackingRunner extends StandardDataFlowRunner { continue; } if (actualVar == value) { - left = relation.myCounterpart; - type = Objects.requireNonNull(type.getFlipped()); + left = actualRelation.myCounterpart; + right = relation.myCounterpart; } else if (actualVar == relation.myCounterpart) { left = value; + right = actualRelation.myCounterpart; + } + else if (actualRelation.myCounterpart == relation.myCounterpart) { + left = value; + right = actualVar; + } + else if (actualRelation.myCounterpart == value) { + left = actualVar; + right = relation.myCounterpart; } else { continue; @@ -910,7 +927,7 @@ public class TrackingRunner extends StandardDataFlowRunner { } @Nullable - private DfaRelationValue fromBinaryExpression(MemoryStateChange change, PsiBinaryExpression binOp) { + private DfaRelationValue getBinaryExpressionRelation(MemoryStateChange change, PsiBinaryExpression binOp) { PsiExpression lOperand = binOp.getLOperand(); PsiExpression rOperand = binOp.getROperand(); MemoryStateChange leftPos = change.findExpressionPush(lOperand); @@ -926,6 +943,18 @@ public class TrackingRunner extends StandardDataFlowRunner { return null; } + private Collection getCallRelations(PsiCallExpression callExpression) { + List contracts = JavaMethodContractUtil.getMethodCallContracts(callExpression); + Set results = new LinkedHashSet<>(); + for (MethodContract contract : contracts) { + for (ContractValue condition : contract.getConditions()) { + DfaValue rel = condition.fromCall(getFactory(), callExpression); + ContainerUtil.addIfNotNull(results, ObjectUtils.tryCast(rel, DfaRelationValue.class)); + } + } + return results; + } + private CauseItem findNullabilityCause(MemoryStateChange factUse, DfaNullability nullability) { PsiExpression expression = factUse.getExpression(); if (expression instanceof PsiTypeCastExpression) { diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelation.java b/java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelation.java index cd7a9aee4c53..42d65d34f086 100644 --- a/java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelation.java +++ b/java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelation.java @@ -2,7 +2,7 @@ Value is always false (b == c; line#14) Condition 'b != c' was deduced Condition 'b != a' was checked before (a == b; line#11) - and condition 'c == a' was checked before (c == a; line#13) + and condition 'a == c' was checked before (c == a; line#13) */ public class T { diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelationContracts.java b/java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelationContracts.java new file mode 100644 index 000000000000..2b4051da678b --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/BasedOnPreviousRelationContracts.java @@ -0,0 +1,18 @@ +/* +Value is always false (b.equals("x"); line#15) + According to hard-coded contract, method 'equals' returns 'false' value when this != parameter (equals; line#15) + Condition 'b != "x"' was deduced + Result of 'b != a' is known from line #12 (a.equals(b); line#12) + and result of 'a == "x"' is known from line #14 (a.equals("x"); line#14) + */ + +public class T { + + void test(String a, String b) { + if(a.equals(b)) { + + } else if (a.equals("x")) { + if (b.equals("x")) {} + } + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java index 8de62dbb5693..6445f00ea60c 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java @@ -55,8 +55,9 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa assertTrue("Selected element is not an expression: " + selectedText, element instanceof PsiExpression); PsiExpression expression = (PsiExpression)element; TrackingRunner.DfaProblemType problemType = getProblemType(selectedText, expression); - List items = TrackingRunner.findProblemCause(true, false, expression, problemType); - String dump = StreamEx.of(items).map(item -> item.dump(getEditor().getDocument())).joining("\n----\n"); + TrackingRunner.CauseItem item = TrackingRunner.findProblemCause(true, false, expression, problemType); + assertNotNull(item); + String dump = item.dump(getEditor().getDocument()); PsiComment firstComment = PsiTreeUtil.findChildOfType(file, PsiComment.class); if (firstComment == null) { fail("Comment not found"); @@ -175,4 +176,5 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa public void testConstructorSimple() { doTest(); } public void testConstructorDependOnInitializer() { doTest(); } public void testBasedOnPreviousRelation() { doTest(); } + public void testBasedOnPreviousRelationContracts() { doTest(); } }