IDEA-209947 Add and/or when several children are displayed (IDEA-CR-46720)

GitOrigin-RevId: 2556ab90f89dccbf9f2b3a820e5894c2b0fcf9a6
This commit is contained in:
Tagir Valeev
2019-04-28 18:04:06 +03:00
committed by intellij-monorepo-bot
parent fc79a9d931
commit 7786fe9165
16 changed files with 87 additions and 70 deletions
@@ -131,20 +131,24 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (o == null || getClass() != o.getClass()) return false;
CauseItem item = (CauseItem)o;
return myChildren.equals(item.myChildren) &&
myProblem.toString().equals(item.myProblem.toString()) &&
getProblemName().equals(item.getProblemName()) &&
Objects.equals(myTarget, item.myTarget);
}
private String getProblemName() {
return myProblem.toString();
}
@Override
public int hashCode() {
return Objects.hash(myChildren, myProblem, myTarget);
return Objects.hash(myChildren, getProblemName(), myTarget);
}
public String dump(Document doc) {
return dump(doc, 0);
return dump(doc, 0, null);
}
private String dump(Document doc, int indent) {
private String dump(Document doc, int indent, CauseItem parent) {
String text = null;
if (myTarget != null) {
Segment range = myTarget.getRange();
@@ -152,8 +156,8 @@ public class TrackingRunner extends StandardDataFlowRunner {
text = doc.getText(TextRange.create(range));
}
}
return StringUtil.repeat(" ", indent) + render(doc) + (text == null ? "" : " (" + text + ")") + "\n" +
StreamEx.of(myChildren).map(child -> child.dump(doc, indent + 1)).joining();
return StringUtil.repeat(" ", indent) + render(doc, parent) + (text == null ? "" : " (" + text + ")") + "\n" +
StreamEx.of(myChildren).map(child -> child.dump(doc, indent + 1, this)).joining();
}
public Stream<CauseItem> children() {
@@ -169,27 +173,37 @@ public class TrackingRunner extends StandardDataFlowRunner {
return myTarget == null ? null : myTarget.getRange();
}
public String render(Document doc) {
public String render(Document doc, CauseItem parent) {
String title = null;
Segment range = getTargetSegment();
if (range != null) {
String cause = myProblem.toString();
String cause = getProblemName();
if (cause.endsWith("#ref")) {
int offset = range.getStartOffset();
int number = doc.getLineNumber(offset);
return cause.replaceFirst("#ref$", "line #" + (number + 1));
title = cause.replaceFirst("#ref$", "line #" + (number + 1));
}
}
return toString();
if (title == null) {
title = toString();
}
int childIndex = parent == null ? 0 : parent.myChildren.indexOf(this);
if (childIndex > 0) {
title = (parent.myProblem instanceof PossibleExecutionDfaProblemType ? "or " : "and ") + title;
} else {
title = StringUtil.capitalize(title);
}
return title;
}
@Override
public String toString() {
return myProblem.toString().replaceFirst("#ref$", "here");
return getProblemName().replaceFirst("#ref$", "here");
}
public CauseItem merge(CauseItem other) {
if (this.equals(other)) return this;
if (Objects.equals(this.myTarget, other.myTarget) && this.myProblem.toString().equals(other.myProblem.toString())) {
if (Objects.equals(this.myTarget, other.myTarget) && getProblemName().equals(other.getProblemName())) {
if(tryMergeChildren(other.myChildren)) return this;
if(other.tryMergeChildren(this.myChildren)) return other;
}
@@ -230,7 +244,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
public static class CastDfaProblemType extends DfaProblemType {
public String toString() {
return "Cast may fail";
return "cast may fail";
}
}
@@ -239,7 +253,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
@Override
public String toString() {
return myComplete ? "One of the following happens:" : "An execution might exist where...";
return myComplete ? "one of the following happens:" : "an execution might exist where:";
}
}
@@ -253,7 +267,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
@Override
public String toString() {
return "Value is always " + myValue;
return "value is always " + myValue;
}
}
@@ -314,7 +328,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
Object constantExpressionValue = ExpressionUtils.computeConstantExpression(expression);
DfaValue value = history.myTopOfStack;
if (constantExpressionValue != null && constantExpressionValue.equals(expectedValue)) {
return new CauseItem[]{new CauseItem("It's compile-time constant which evaluates to '" + value + "'", expression)};
return new CauseItem[]{new CauseItem("it's compile-time constant which evaluates to '" + value + "'", expression)};
}
if (value instanceof DfaConstValue) {
Object constValue = ((DfaConstValue)value).getValue();
@@ -354,7 +368,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (negated != null) {
MemoryStateChange negatedPush = history.findExpressionPush(negated);
if (negatedPush != null) {
CauseItem cause = new CauseItem("Value '" + negated.getText() + "' is always '" + !value + "'", negated);
CauseItem cause = new CauseItem("value '" + negated.getText() + "' is always '" + !value + "'", negated);
cause.addChildren(findConstantValueCause(negated, negatedPush, !value));
return new CauseItem[]{cause};
}
@@ -374,7 +388,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
((ConditionalGotoInstruction)push.myInstruction).isTarget(value, history.myInstruction)) ||
(push.myTopOfStack instanceof DfaConstValue &&
Boolean.valueOf(value).equals(((DfaConstValue)push.myTopOfStack).getValue())))) {
CauseItem cause = new CauseItem("Operand #" + (i + 1) + " of " + (and ? "&&" : "||") + "-chain is " + value, operand);
CauseItem cause = new CauseItem("operand #" + (i + 1) + " of " + (and ? "&&" : "||") + "-chain is " + value, operand);
cause.addChildren(findBooleanResultCauses(operand, push, value));
return new CauseItem[]{cause};
}
@@ -402,12 +416,12 @@ public class TrackingRunner extends StandardDataFlowRunner {
}
if (leftValue == rightValue &&
(leftValue instanceof DfaVariableValue || leftValue instanceof DfaConstValue)) {
return new CauseItem[]{new CauseItem("Comparison arguments are the same", binOp.getOperationSign())};
return new CauseItem[]{new CauseItem("comparison arguments are the same", binOp.getOperationSign())};
}
if (leftValue != rightValue && relationType.isInequality() &&
leftValue instanceof DfaConstValue && rightValue instanceof DfaConstValue) {
return new CauseItem[]{
new CauseItem("Comparison arguments are different constants", binOp.getOperationSign())};
new CauseItem("comparison arguments are different constants", binOp.getOperationSign())};
}
}
}
@@ -421,7 +435,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (!value) {
Pair<MemoryStateChange, DfaNullability> nullability = operandHistory.findFact(operandValue, DfaFactType.NULLABILITY);
if (nullability.second == DfaNullability.NULL) {
CauseItem causeItem = new CauseItem("Value '" + operand.getText() + "' is always 'null'", operand);
CauseItem causeItem = new CauseItem("value '" + operand.getText() + "' is always 'null'", operand);
causeItem.addChildren(findConstantValueCause(operand, operandHistory, null));
return new CauseItem[]{causeItem};
}
@@ -455,7 +469,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
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));
causeItem.addChildren(new CauseItem("type of '" + operand.getText() + "' is known from #ref", causeLocation));
return causeItem;
}
explanation = prevExplanation;
@@ -513,8 +527,8 @@ public class TrackingRunner extends StandardDataFlowRunner {
LongRangeSet fromRelation = rightRange.second.fromRelation(relationType.getNegated());
if (fromRelation != null && !fromRelation.intersects(leftRange.second)) {
return new CauseItem[]{
findRangeCause(leftChange, leftRange.first, leftRange.second, "Left operand is %s"),
findRangeCause(rightChange, rightRange.first, rightRange.second, "Right operand is %s")};
findRangeCause(leftChange, leftRange.first, leftRange.second, "left operand is %s"),
findRangeCause(rightChange, rightRange.first, rightRange.second, "right operand is %s")};
}
}
return new CauseItem[0];
@@ -534,7 +548,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
}
if (instruction instanceof InstanceofInstruction) {
PsiExpression expression = ((InstanceofInstruction)instruction).getExpression();
return new CauseItem("The 'instanceof' check implies non-nullity", expression);
return new CauseItem("the 'instanceof' check implies non-nullity", expression);
}
}
if (instruction instanceof AssignInstruction) {
@@ -564,11 +578,11 @@ public class TrackingRunner extends StandardDataFlowRunner {
DfaValue rightValue = rightPos.myTopOfStack;
if (leftValue == value && rightValue == relation.myCounterpart ||
rightValue == value && leftValue == relation.myCounterpart) {
return new CauseItem(new CustomDfaProblemType(condition + " was checked before"), expression);
return new CauseItem(new CustomDfaProblemType("condition '" + condition + "' was checked before"), expression);
}
}
}
return new CauseItem(new CustomDfaProblemType(condition + " is known from #ref"), expression);
return new CauseItem(new CustomDfaProblemType("result of '" + condition + "' is known from #ref"), expression);
}
return null;
}
@@ -584,24 +598,24 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (expression instanceof PsiMethodCallExpression) {
PsiMethodCallExpression call = (PsiMethodCallExpression)expression;
PsiMethod method = call.resolveMethod();
return fromMemberNullability(nullability, method, "Method", call.getMethodExpression().getReferenceNameElement());
return fromMemberNullability(nullability, method, "method", call.getMethodExpression().getReferenceNameElement());
}
if (expression instanceof PsiReferenceExpression) {
PsiVariable variable = ObjectUtils.tryCast(((PsiReferenceExpression)expression).resolve(), PsiVariable.class);
if (variable instanceof PsiField) {
return fromMemberNullability(nullability, variable, "Field", ((PsiReferenceExpression)expression).getReferenceNameElement());
return fromMemberNullability(nullability, variable, "field", ((PsiReferenceExpression)expression).getReferenceNameElement());
}
if (variable instanceof PsiParameter) {
return fromMemberNullability(nullability, variable, "Parameter", ((PsiReferenceExpression)expression).getReferenceNameElement());
return fromMemberNullability(nullability, variable, "parameter", ((PsiReferenceExpression)expression).getReferenceNameElement());
}
if (variable != null) {
return fromMemberNullability(nullability, variable, "Variable", ((PsiReferenceExpression)expression).getReferenceNameElement());
return fromMemberNullability(nullability, variable, "variable", ((PsiReferenceExpression)expression).getReferenceNameElement());
}
}
if (nullability == DfaNullability.NOT_NULL) {
String explanation = getObviouslyNonNullExplanation(expression);
if (explanation != null) {
return new CauseItem("Expression cannot be null as it's " + explanation, expression);
return new CauseItem("expression cannot be null as it's " + explanation, expression);
}
}
return null;
@@ -665,11 +679,11 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (descriptor instanceof SpecialField && range.equals(LongRangeSet.indexRange())) {
switch (((SpecialField)descriptor)) {
case ARRAY_LENGTH:
return new CauseItem("Array length is always non-negative", factUse);
return new CauseItem("array length is always non-negative", factUse);
case STRING_LENGTH:
return new CauseItem("String length is always non-negative", factUse);
return new CauseItem("string length is always non-negative", factUse);
case COLLECTION_SIZE:
return new CauseItem("Collection size is always non-negative", factUse);
return new CauseItem("collection size is always non-negative", factUse);
default:
}
}
@@ -685,7 +699,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (method != null) {
LongRangeSet fromAnnotation = LongRangeSet.fromPsiElement(method);
if (fromAnnotation.equals(range)) {
return new CauseItem("The range of '" + method.getName() + "' is specified by annotation as " + range,
return new CauseItem("the range of '" + method.getName() + "' is specified by annotation as " + range,
call.getMethodExpression().getReferenceNameElement());
}
}
@@ -712,14 +726,14 @@ public class TrackingRunner extends StandardDataFlowRunner {
}
LongRangeSet result = leftSet.second.binOpFromToken(binOp.getOperationTokenType(), rightSet.second, isLong);
if (range.equals(result)) {
CauseItem cause = new CauseItem("Result of '" + binOp.getOperationSign().getText() +
CauseItem cause = new CauseItem("result of '" + binOp.getOperationSign().getText() +
"' is " + range.getPresentationText(expression.getType()), factUse);
CauseItem leftCause = null, rightCause = null;
if (!leftSet.second.equals(fromType)) {
leftCause = findRangeCause(leftPush, leftSet.first, leftSet.second, "Left operand is %s");
leftCause = findRangeCause(leftPush, leftSet.first, leftSet.second, "left operand is %s");
}
if (!rightSet.second.equals(fromType)) {
rightCause = findRangeCause(rightPush, rightSet.first, rightSet.second, "Right operand is %s");
rightCause = findRangeCause(rightPush, rightSet.first, rightSet.second, "right operand is %s");
}
cause.addChildren(leftCause, rightCause);
return cause;
@@ -745,7 +759,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
}
PsiExpression defExpression = factDef.getExpression();
if (defExpression != null) {
item.addChildren(new CauseItem("Range is known from #ref", defExpression));
item.addChildren(new CauseItem("range is known from #ref", defExpression));
}
}
return item;
@@ -102,12 +102,12 @@ public abstract class TypeConstraint {
if (actual != expectedAssignable) return null;
if (expectedAssignable) {
if (myType == otherType) {
return "An object is already known to be " + myType;
return "an object is already known to be " + myType;
}
return "An object type is exactly " + myType + " which is a subtype of " + otherType;
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;
return "an object type is exactly " + myType + " which is not a subtype of " + otherType;
}
}
@@ -432,20 +432,20 @@ public abstract class TypeConstraint {
if (expectedAssignable) {
for (DfaPsiType dfaTypeValue : myInstanceofValues) {
if (otherType.isAssignableFrom(dfaTypeValue)) {
return "An object is already known to be " + 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 +
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 "an object is known to be " + dfaTypeValue + " which is definitely incompatible with " + otherType;
}
}
}
@@ -30,7 +30,7 @@ import com.intellij.psi.PsiFile;
import com.intellij.psi.SmartPointerManager;
import com.intellij.psi.SmartPsiElementPointer;
import com.intellij.util.containers.ContainerUtil;
import one.util.streamex.EntryStream;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
@@ -95,22 +95,25 @@ public class FindDfaProblemCauseFix implements LocalQuickFix, LowPriorityAction
class CauseWithDepth {
final int myDepth;
final TrackingRunner.CauseItem myCauseItem;
final CauseWithDepth myParent;
CauseWithDepth(int depth, TrackingRunner.CauseItem item) {
myDepth = depth;
CauseWithDepth(CauseWithDepth parent, TrackingRunner.CauseItem item) {
myParent = parent;
myDepth = parent == null ? 0 : parent.myDepth + 1;
myCauseItem = item;
}
@Override
public String toString() {
return StringUtil.repeat(" ", myDepth - 1) + myCauseItem.render(document);
return StringUtil.repeat(" ", myDepth - 1) + myCauseItem.render(document, myParent == null ? null : myParent.myCauseItem);
}
}
List<CauseWithDepth> causes;
if (root == null) {
causes = Collections.emptyList();
} else {
causes = EntryStream.ofTree(root, (depth, c) -> c.children()).skip(1).mapKeyValue((d, i) -> new CauseWithDepth(d, i)).toList();
causes = StreamEx.ofTree(new CauseWithDepth(null, root), cwd -> cwd.myCauseItem.children()
.map(child -> new CauseWithDepth(cwd, child))).skip(1).toList();
}
if (causes.isEmpty()) {
HintManagerImpl hintManager = (HintManagerImpl)HintManager.getInstance();
@@ -2,11 +2,11 @@
Value is always false (a > 5 && b < 0 && b > a)
One of the following happens:
Operand #1 of &&-chain is false (a > 5)
Operand #2 of &&-chain is false (b < 0)
Operand #3 of &&-chain is false (b > a)
or operand #2 of &&-chain is false (b < 0)
or operand #3 of &&-chain is false (b > a)
Left operand is <= -1 (b)
Range is known from line #15 (b < 0)
Right operand is >= 6 (a)
and right operand is >= 6 (a)
Range is known from line #15 (a > 5)
*/
@@ -2,7 +2,7 @@
Value is always false (s.length == list.size())
Left operand is >= 1 (s.length)
Range is known from line #12 (s[0])
Right operand is 0 (list.size())
and right operand is 0 (list.size())
Range is known from line #13 (list.isEmpty())
*/
import java.util.List;
@@ -1,7 +1,7 @@
/*
Value is always false (s == null)
's' was assigned (s1)
s1 != null was checked before (s1 == null)
Condition 's1 != null' was checked before (s1 == null)
*/
class Test {
void test(String s, String s1) {
@@ -3,7 +3,7 @@ Value is always false (x == null)
One of the following happens:
'x' was assigned (new Object())
Expression cannot be null as it's newly created object (new Object())
'x' was assigned ("foo")
or 'x' was assigned ("foo")
Expression cannot be null as it's literal ("foo")
*/
@@ -4,7 +4,7 @@ Value is always false (y == null)
One of the following happens:
'x' was assigned (new Object())
Expression cannot be null as it's newly created object (new Object())
'x' was assigned ("foo")
or 'x' was assigned ("foo")
Expression cannot be null as it's literal ("foo")
*/
@@ -1,6 +1,6 @@
/*
Value is always false (s == null)
s != null was checked before (null == s)
Condition 's != null' was checked before (null == s)
*/
class Test {
void test(String s) {
@@ -1,7 +1,7 @@
/*
Value is always false (s == s1)
's1' was assigned (null)
s != null was checked before (null == s)
and condition 's != null' was checked before (null == s)
*/
class Test {
void test(String s) {
@@ -1,7 +1,7 @@
/*
Value is always false (x * 2 == y * 2 + 1)
Result of '*' is even (x * 2)
Result of '+' is odd (y * 2 + 1)
and result of '+' is odd (y * 2 + 1)
Result of '*' is even (y * 2)
*/
class Test {
@@ -2,17 +2,17 @@
Value is always true (x || (a+b)+(c+d)==10)
One of the following happens:
Operand #1 of ||-chain is true (x)
Operand #2 of ||-chain is true ((a+b)+(c+d)==10)
or operand #2 of ||-chain is true ((a+b)+(c+d)==10)
Result of '+' is 10 ((a+b)+(c+d))
Result of '+' is 3 (a+b)
Left operand is 1 (a)
'a' was assigned (1)
Right operand is 2 (b)
and right operand is 2 (b)
'b' was assigned (2)
Result of '+' is 7 (c+d)
and result of '+' is 7 (c+d)
Left operand is 3 (c)
'c' was assigned (3)
Right operand is 4 (d)
and right operand is 4 (d)
'd' was assigned (4)
*/
@@ -1,6 +1,6 @@
/*
Value is always true (x <= y)
x <= y was checked before (x > y)
Condition 'x <= y' was checked before (x > y)
*/
class Test {
void test(int x, int y) {
@@ -1,6 +1,6 @@
/*
Value is always true (x < y)
x < y was checked before (x > y)
Condition 'x < y' was checked before (x > y)
*/
class Test {
void test(int x, int y) {
@@ -1,9 +1,9 @@
/*
Cast may fail ((Integer)x)
An execution might exist where...
An execution might exist where:
An object type is exactly Double which is not a subtype of Integer (x)
Type of 'x' is known from line #14 (x instanceof Double)
An object type is exactly String which is not a subtype of Integer (x)
or an object type is exactly String which is not a subtype of Integer (x)
Type of 'x' is known from line #12 (x instanceof String)
*/
@@ -1,6 +1,6 @@
/*
Cast may fail ((Integer)x)
An execution might exist where...
An execution might exist where:
An object type is exactly String which is not a subtype of Integer (x)
Type of 'x' is known from line #10 (x instanceof String)
*/