From efcbb2042270780e0f4951cdfb59e8f3b48ad345 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 25 Apr 2019 13:11:36 +0700 Subject: [PATCH] IDEA-209947 Better type constraint explanation GitOrigin-RevId: bc97ff49908ffee155459fb81dc5e46e4fb12c59 --- .../dataFlow/TrackingRunner.java | 14 +++---- .../dataFlow/TypeConstraint.java | 41 +++++++++++++++++++ .../dataFlow/tracker/InstanceOfChain.java | 17 ++++++++ .../dataFlow/tracker/InstanceOfChain2.java | 21 ++++++++++ .../dataFlow/tracker/InstanceOfConflict.java | 2 +- .../tracker/InstanceOfPreviousCast.java | 16 ++++++++ .../dataFlow/tracker/InstanceOfRedundant.java | 2 +- .../dataFlow/tracker/NotInstanceOf.java | 17 ++++++++ .../DataFlowInspectionTrackerTest.java | 4 ++ 9 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain.java create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain2.java create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfPreviousCast.java create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/NotInstanceOf.java 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 c526d3b2531e..df9c818918d4 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 @@ -349,23 +349,21 @@ public class TrackingRunner extends StandardDataFlowRunner { 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) { + String explanation = fact.second == null ? null : fact.second.getAssignabilityExplanation(wanted, isInstance); + while (explanation != null) { 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); + String prevExplanation = prevConstraint.getAssignabilityExplanation(wanted, isInstance); + if (prevExplanation == null) { + CauseItem causeItem = new CauseItem(explanation, operand); causeItem.addChildren(new CauseItem("Type of '" + operand.getText() + "' is known from #ref", causeLocation)); return new CauseItem[]{causeItem}; } - constraint = prevConstraint; + explanation = prevExplanation; } return null; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TypeConstraint.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TypeConstraint.java index 8393fcc2cbe3..7b7dd5779542 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TypeConstraint.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TypeConstraint.java @@ -69,6 +69,7 @@ public abstract class TypeConstraint { public abstract boolean isExact(String typeName); + public abstract String getAssignabilityExplanation(DfaPsiType otherType, boolean expectedAssignable); static final class Exact extends TypeConstraint { final @NotNull DfaPsiType myType; @@ -95,6 +96,21 @@ public abstract class TypeConstraint { return type.isAssignableFrom(myType) ? null : this; } + @Override + public String getAssignabilityExplanation(DfaPsiType otherType, boolean expectedAssignable) { + boolean actual = otherType.isAssignableFrom(myType); + if (actual != expectedAssignable) return null; + if (expectedAssignable) { + if (myType == otherType) { + return "An object is already known to be " + myType; + } + return "An object type is exactly " + myType + " which is a subtype of " + otherType; + } + else { + return "An object type is exactly " + myType + " which is not a subtype of " + otherType; + } + } + @NotNull @Override TypeConstraint withoutType(@NotNull DfaPsiType type) { @@ -411,6 +427,31 @@ public abstract class TypeConstraint { return false; } + @Override + public String getAssignabilityExplanation(DfaPsiType otherType, boolean expectedAssignable) { + if (expectedAssignable) { + for (DfaPsiType dfaTypeValue : myInstanceofValues) { + if (otherType.isAssignableFrom(dfaTypeValue)) { + return "An object is already known to be " + dfaTypeValue + + (otherType == dfaTypeValue ? "" : " which is a subtype of " + otherType); + } + } + } else { + for (DfaPsiType dfaTypeValue : myNotInstanceofValues) { + if (dfaTypeValue.isAssignableFrom(otherType)) { + return "An object is known to be not " + dfaTypeValue + + (otherType == dfaTypeValue ? "" : " which is a supertype of " + otherType); + } + } + for (DfaPsiType dfaTypeValue : myInstanceofValues) { + if (!otherType.isConvertibleFrom(dfaTypeValue)) { + return "An object is known to be " + dfaTypeValue + " which is definitely incompatible with " + otherType; + } + } + } + return null; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain.java b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain.java new file mode 100644 index 000000000000..63b4ed824c55 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain.java @@ -0,0 +1,17 @@ +/* +Value is always false (s instanceof String) + An object is known to be Number which is definitely incompatible with String (s) + Type of 's' is known from line #10 (s instanceof Number) + */ +import java.util.List; + +class Test { + void test(Object s) { + if (s instanceof Number) { + if (s instanceof Integer) { + if (s instanceof String){ + } + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain2.java b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain2.java new file mode 100644 index 000000000000..81a2f9a8083c --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfChain2.java @@ -0,0 +1,21 @@ +/* +Value is always true (s instanceof RandomAccess) + An object is already known to be ArrayList which is a subtype of RandomAccess (s) + Type of 's' is known from line #12 (s instanceof ArrayList) + */ +import java.util.*; + +class Test { + void test(Object s) { + if (s instanceof Map) { + if (s instanceof List) { + if (s instanceof ArrayList) { + if (s instanceof CharSequence) { + if (s instanceof RandomAccess){ + } + } + } + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfConflict.java b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfConflict.java index a8e7a9c35acd..45b89131e375 100644 --- a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfConflict.java +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfConflict.java @@ -1,6 +1,6 @@ /* Value is always false (s instanceof Integer) - Type of 's' is exactly String (s) + An object type is exactly String which is not a subtype of Integer (s) Type of 's' is known from line #10 (s instanceof String) */ import java.util.List; diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfPreviousCast.java b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfPreviousCast.java new file mode 100644 index 000000000000..aa3478595c03 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfPreviousCast.java @@ -0,0 +1,16 @@ +/* +Value is always true (s instanceof String) + An object is already known to be String (s) + Type of 's' is known from line #10 ((String)s) + */ +import java.util.List; + +class Test { + void test(Object s) { + System.out.println(((String)s).trim()); + + + if (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 index 4a60d04259ec..a300e4664b71 100644 --- a/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfRedundant.java +++ b/java/java-tests/testData/inspection/dataFlow/tracker/InstanceOfRedundant.java @@ -1,6 +1,6 @@ /* Value is always true (s instanceof CharSequence) - Type of 's' is exactly String (s) + An object type is exactly String which is a subtype of CharSequence (s) Type of 's' is known from line #10 (s instanceof String) */ import java.util.List; diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NotInstanceOf.java b/java/java-tests/testData/inspection/dataFlow/tracker/NotInstanceOf.java new file mode 100644 index 000000000000..26cd4c055bf6 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NotInstanceOf.java @@ -0,0 +1,17 @@ +/* +Value is always false (s instanceof String) + An object is known to be not CharSequence which is a supertype of String (s) + Type of 's' is known from line #10 (s instanceof CharSequence) + */ +import java.util.List; + +class Test { + void test(Object s) { + if (!(s instanceof CharSequence)) { + if (s instanceof Integer) { + if (s instanceof String){ + } + } + } + } +} \ 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 ceb8ee9e9bdf..45538a75ee43 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java @@ -117,4 +117,8 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa public void testInstanceOfNull() { doTest(); } public void testInstanceOfConflict() { doTest(); } public void testInstanceOfRedundant() { doTest(); } + public void testInstanceOfChain() { doTest(); } + public void testInstanceOfChain2() { doTest(); } + public void testNotInstanceOf() { doTest(); } + public void testInstanceOfPreviousCast() { doTest(); } }