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 e255dc6e05cf..545483d676f9 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 @@ -7,11 +7,8 @@ import com.intellij.codeInspection.dataFlow.TrackingDfaMemoryState.MemoryStateCh import com.intellij.codeInspection.dataFlow.TrackingDfaMemoryState.Relation; import com.intellij.codeInspection.dataFlow.instructions.*; import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet; -import com.intellij.codeInspection.dataFlow.value.DfaConstValue; +import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType; -import com.intellij.codeInspection.dataFlow.value.DfaValue; -import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; -import com.intellij.codeInspection.dataFlow.value.VariableDescriptor; import com.intellij.openapi.editor.Document; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.Segment; @@ -270,7 +267,7 @@ public class TrackingRunner extends StandardDataFlowRunner { } return new CauseItem[]{item}; } - else if (expectedValue instanceof Boolean && varSourceExpression != null) { + else if (varSourceExpression != null) { return new CauseItem[]{new CauseItem("'" + value + " == "+expectedValue+"' was established from condition", varSourceExpression)}; } } @@ -320,9 +317,59 @@ public class TrackingRunner extends StandardDataFlowRunner { } } } + if (expression instanceof PsiInstanceOfExpression) { + PsiInstanceOfExpression instanceOfExpression = (PsiInstanceOfExpression)expression; + PsiExpression operand = instanceOfExpression.getOperand(); + MemoryStateChange operandHistory = history.findExpressionPush(operand); + if (operandHistory != null) { + DfaValue operandValue = operandHistory.myTopOfStack; + if (!value) { + Pair nullability = operandHistory.findFact(operandValue, DfaFactType.NULLABILITY); + if (nullability.second == DfaNullability.NULL) { + CauseItem causeItem = new CauseItem("Value '" + operand.getText() + "' is always 'null'", operand); + causeItem.addChildren(findConstantValueCause(operand, operandHistory, null)); + return new CauseItem[]{causeItem}; + } + } + PsiTypeElement typeElement = instanceOfExpression.getCheckType(); + if (typeElement != null) { + PsiType type = typeElement.getType(); + CauseItem[] causeItem = findTypeCause(operandHistory, type, value); + if (causeItem != null) return causeItem; + } + } + } return new CauseItem[0]; } + @Nullable + private static CauseItem[] findTypeCause(MemoryStateChange operandHistory, PsiType type, boolean isInstance) { + PsiExpression operand = Objects.requireNonNull(operandHistory.getExpression()); + DfaValue operandValue = operandHistory.myTopOfStack; + DfaPsiType wanted = operandValue.getFactory().createDfaType(type); + + Pair fact = operandHistory.findFact(operandValue, DfaFactType.TYPE_CONSTRAINT); + TypeConstraint constraint = fact.second == null ? TypeConstraint.empty() : fact.second; + boolean stillSatisfied = (isInstance ? constraint.withNotInstanceofValue(wanted) : constraint.withInstanceofValue(wanted)) == null; + while (stillSatisfied) { + MemoryStateChange causeLocation = fact.first; + if (causeLocation == null) break; + MemoryStateChange prevHistory = causeLocation.myPrevious; + if (prevHistory == null) break; + fact = prevHistory.findFact(operandValue, DfaFactType.TYPE_CONSTRAINT); + TypeConstraint prevConstraint = fact.second == null ? TypeConstraint.empty() : fact.second; + stillSatisfied = (isInstance ? prevConstraint.withNotInstanceofValue(wanted) : prevConstraint.withInstanceofValue(wanted)) == null; + if (!stillSatisfied) { + CauseItem causeItem = + new CauseItem("Type of '" + operand.getText() + "' is " + constraint.getPresentationText(operand.getType()), operand); + causeItem.addChildren(new CauseItem("Type of '" + operand.getText() + "' is known from #ref", causeLocation)); + return new CauseItem[]{causeItem}; + } + constraint = prevConstraint; + } + return null; + } + @NotNull private static CauseItem[] findRelationCause(RelationType relationType, MemoryStateChange leftChange, diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfConflict.java b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfConflict.java new file mode 100644 index 000000000000..a8e7a9c35acd --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfConflict.java @@ -0,0 +1,14 @@ +/* +Value is always false (s instanceof Integer) + Type of 's' is exactly String (s) + Type of 's' is known from line #10 (s instanceof String) + */ +import java.util.List; + +class Test { + void test(Object s) { + if (s instanceof String) { + if (s instanceof Integer) {} + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfNull.java b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfNull.java new file mode 100644 index 000000000000..9ad93e2bd309 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfNull.java @@ -0,0 +1,12 @@ +/* +Value is always false (s instanceof String) + Value 's' is always 'null' (s) + 's == null' was established from condition (s == null) + */ +import java.util.List; + +class Test { + void test(Object s) { + if (s == null && s instanceof String) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfRedundant.java b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfRedundant.java new file mode 100644 index 000000000000..4a60d04259ec --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfRedundant.java @@ -0,0 +1,14 @@ +/* +Value is always true (s instanceof CharSequence) + Type of 's' is exactly String (s) + Type of 's' is known from line #10 (s instanceof String) + */ +import java.util.List; + +class Test { + void test(Object s) { + if (s instanceof String) { + if (s instanceof CharSequence) {} + } + } +} \ No newline at end of file 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 fe254cbd1720..ceb8ee9e9bdf 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java @@ -114,4 +114,7 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa public void testBooleanUnderNegation() { doTest(); } public void testArrayLength() { doTest(); } public void testArrayLengthCollectionSize() { doTest(); } + public void testInstanceOfNull() { doTest(); } + public void testInstanceOfConflict() { doTest(); } + public void testInstanceOfRedundant() { doTest(); } }