From a19ec3f60c93c0f93431b5d48d843e49a607d735 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 8 May 2019 12:28:24 +0700 Subject: [PATCH] IDEA-209947 Simple non-trivial contract support GitOrigin-RevId: a72bfec76185608927774ff5643d30970c3567bf --- .../dataFlow/ContractValue.java | 64 +++++ .../dataFlow/TrackingDfaMemoryState.java | 33 ++- .../dataFlow/TrackingRunner.java | 249 ++++++++++-------- .../dataFlow/tracker/SimpleContract.java | 16 ++ .../dataFlow/tracker/SimpleContract2.java | 15 ++ .../DataFlowInspectionTrackerTest.java | 2 + 6 files changed, 262 insertions(+), 117 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract.java create mode 100644 java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract2.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java index f68af8326297..958c787e542d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractValue.java @@ -88,6 +88,18 @@ public abstract class ContractValue { return OptionalInt.empty(); } + public String getPresentationText(PsiMethod method) { + return toString(); + } + + public PsiExpression findLeftPlace(PsiCallExpression call) { + return null; + } + + public PsiExpression findRightPlace(PsiCallExpression call) { + return null; + } + public static ContractValue qualifier() { return Qualifier.INSTANCE; } @@ -149,6 +161,18 @@ public abstract class ContractValue { return arguments.myArguments[myIndex]; } + @Override + public String getPresentationText(PsiMethod method) { + PsiParameter[] params = method.getParameterList().getParameters(); + if (myIndex == 0 && params.length == 1) { + return "parameter"; + } + if (myIndex < params.length) { + return params[myIndex].getName(); + } + return toString(); + } + @Override public boolean equals(Object obj) { return obj == this || (obj instanceof Argument && myIndex == ((Argument)obj).myIndex); @@ -217,6 +241,11 @@ public abstract class ContractValue { return myQualifier.equals(that.myQualifier) && myField == that.myField; } + @Override + public String getPresentationText(PsiMethod method) { + return myQualifier.getPresentationText(method) + "." + myField + (myField == SpecialField.ARRAY_LENGTH ? "" : "()"); + } + @Override public String toString() { return myQualifier + "." + myField + "()"; @@ -316,6 +345,41 @@ public abstract class ContractValue { return factory.createCondition(left, myRelationType, right); } + @Override + public String getPresentationText(PsiMethod method) { + if (myLeft instanceof IndependentValue) { + return myRight.getPresentationText(method) + " " + myRelationType.getFlipped() + " " + myLeft.getPresentationText(method); + } + return myLeft.getPresentationText(method) + " " + myRelationType + " " + myRight.getPresentationText(method); + } + + @Override + public PsiExpression findLeftPlace(PsiCallExpression call) { + return findPlace(call, myLeft); + } + + @Override + public PsiExpression findRightPlace(PsiCallExpression call) { + return findPlace(call, myRight); + } + + private static PsiExpression findPlace(PsiCallExpression call, ContractValue value) { + while (value instanceof Spec) { + value = ((Spec)value).myQualifier; + } + if (value instanceof Argument) { + PsiExpressionList list = call.getArgumentList(); + if (list != null) { + PsiExpression[] args = list.getExpressions(); + int index = ((Argument)value).myIndex; + if (index < args.length - 1 || (index == args.length - 1 && !MethodCallUtils.isVarArgCall(call))) { + return args[index]; + } + } + } + return null; + } + @Override public String toString() { return myLeft + " " + myRelationType + " " + myRight; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java index f7cc28ea34a0..8cd4808c5013 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java @@ -6,7 +6,6 @@ import com.intellij.codeInspection.dataFlow.instructions.ExpressionPushingInstru import com.intellij.codeInspection.dataFlow.instructions.Instruction; import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType; -import com.intellij.openapi.util.Pair; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiExpression; import com.intellij.psi.util.PsiTreeUtil; @@ -340,30 +339,28 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl { } @NotNull - Pair findFact(DfaValue value, DfaFactType type) { + FactDefinition findFact(DfaValue value, DfaFactType type) { if (value instanceof DfaVariableValue) { for (MemoryStateChange change = this; change != null; change = change.myPrevious) { - Pair factPair = factFromChange(type, change, change.myChanges.get(value)); + FactDefinition 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 new FactDefinition<>(null, ((DfaVariableValue)value).getInherentFacts().get(type)); } - return Pair.create(null, type.fromDfaValue(value)); + return new FactDefinition<>(null, type.fromDfaValue(value)); } @Nullable - private static Pair factFromChange(DfaFactType type, - MemoryStateChange change, - Change varChange) { + private static FactDefinition factFromChange(DfaFactType type, MemoryStateChange change, Change varChange) { if (varChange != null) { T added = varChange.myAddedFacts.get(type); if (added != null) { - return Pair.create(change, added); + return new FactDefinition<>(change, added); } if (varChange.myRemovedFacts.get(type) != null) { - return Pair.create(change, null); + return new FactDefinition<>(change, null); } } return null; @@ -496,4 +493,20 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl { "; Bridge changes: " + EntryStream.of(myBridgeChanges).join(": ", "\n\t", "").joining()); } } + + static class FactDefinition { + final @Nullable MemoryStateChange myChange; + final @Nullable T myFact; + + FactDefinition(@Nullable MemoryStateChange change, @Nullable T fact) { + myChange = change; + myFact = fact; + } + + @Nullable + @Contract("!null -> !null") + T getFact(T defaultFact) { + return myFact == null ? defaultFact : myFact; + } + } } 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 d7c1f9f7a8c5..fdcab93e4e72 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 @@ -3,6 +3,7 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.NullabilityAnnotationInfo; import com.intellij.codeInsight.NullableNotNullManager; +import com.intellij.codeInspection.dataFlow.TrackingDfaMemoryState.FactDefinition; import com.intellij.codeInspection.dataFlow.TrackingDfaMemoryState.MemoryStateChange; import com.intellij.codeInspection.dataFlow.TrackingDfaMemoryState.Relation; import com.intellij.codeInspection.dataFlow.instructions.*; @@ -12,7 +13,6 @@ import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType; import com.intellij.lang.ASTNode; import com.intellij.openapi.editor.Document; import com.intellij.openapi.progress.ProgressManager; -import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.Segment; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; @@ -111,23 +111,49 @@ public class TrackingRunner extends StandardDataFlowRunner { StandardInstructionVisitor visitor = new StandardInstructionVisitor(true); RunnerResult result = runner.analyzeMethodRecursively(body, visitor, ignoreAssertions); if (result != RunnerResult.OK) return Collections.emptyList(); + return ContainerUtil.createMaybeSingletonList(runner.findProblemCause(expression, type)); + } + + /* + TODO: 1. Find causes of other warnings: + Cause for AIOOBE + Cause for "Contract always fails" + Cause for "modifying an immutable collection" + Cause for "Collection is always empty" (separate inspection now) + TODO: 2. Describe causes in more cases: + Warning caused by contract + Warning caused by CustomMethodHandler + Warning caused by polyadic math + Warning caused by unary minus + Warning caused by final field initializer + TODO: 3. Check how it works with: + Inliners (notably: Stream API) + Boxed numbers + TODO: 4. Check for possible performance disasters (likely on some code patterns current algo might blow up) + TODO: 5. Problem when interesting state doesn't reach the current condition, need to do something with this + */ + @Nullable + private CauseItem findProblemCause(PsiExpression expression, DfaProblemType type) { CauseItem cause = null; - for (MemoryStateChange history : runner.myHistoryForContext) { - CauseItem root = findCauseChain(expression, history, type); + for (MemoryStateChange history : myHistoryForContext) { + CauseItem item = new CauseItem(type, expression); + if (history.getExpression() == expression) { + item.addChildren(type.findCauses(this, expression, history)); + } if (cause == null) { - cause = root; + cause = item; } else { - cause = cause.merge(root); - if (cause == null) return Collections.emptyList(); + cause = cause.merge(item); + if (cause == null) return null; } } - return Collections.singletonList(cause); + return cause; } public abstract static class DfaProblemType { public abstract String toString(); - CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { + CauseItem[] findCauses(TrackingRunner runner, PsiExpression expression, MemoryStateChange history) { return new CauseItem[0]; } } @@ -285,12 +311,12 @@ public class TrackingRunner extends StandardDataFlowRunner { public static class CastDfaProblemType extends DfaProblemType { @Override - public CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { + public CauseItem[] findCauses(TrackingRunner runner, PsiExpression expression, MemoryStateChange history) { if (expression instanceof PsiTypeCastExpression) { PsiType expressionType = expression.getType(); MemoryStateChange operandPush = history.findExpressionPush(((PsiTypeCastExpression)expression).getOperand()); if (operandPush != null) { - return new CauseItem[]{findTypeCause(operandPush, expressionType, false)}; + return new CauseItem[]{runner.findTypeCause(operandPush, expressionType, false)}; } } return new CauseItem[0]; @@ -303,10 +329,10 @@ public class TrackingRunner extends StandardDataFlowRunner { public static class NullableDfaProblemType extends DfaProblemType { @Override - public CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { - Pair nullability = history.findFact(history.myTopOfStack, DfaFactType.NULLABILITY); - if (nullability.second == DfaNullability.NULLABLE || nullability.second == DfaNullability.NULL) { - return new CauseItem[]{findNullabilityCause(history, nullability.first, nullability.second)}; + public CauseItem[] findCauses(TrackingRunner runner, PsiExpression expression, MemoryStateChange history) { + FactDefinition nullability = history.findFact(history.myTopOfStack, DfaFactType.NULLABILITY); + if (nullability.myFact == DfaNullability.NULLABLE || nullability.myFact == DfaNullability.NULL) { + return new CauseItem[]{runner.findNullabilityCause(history, nullability.myFact)}; } return new CauseItem[0]; } @@ -334,8 +360,8 @@ public class TrackingRunner extends StandardDataFlowRunner { } @Override - public CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { - return findConstantValueCause(expression, history, myValue); + public CauseItem[] findCauses(TrackingRunner runner, PsiExpression expression, MemoryStateChange history) { + return runner.findConstantValueCause(expression, history, myValue); } @Override @@ -357,34 +383,8 @@ public class TrackingRunner extends StandardDataFlowRunner { } } - /* - TODO: 1. Find causes of other warnings: - Cause for AIOOBE - Cause for "Contract always fails" - Cause for "modifying an immutable collection" - Cause for "Collection is always empty" (separate inspection now) - TODO: 2. Describe causes in more cases: - Warning caused by contract - Warning caused by CustomMethodHandler - Warning caused by polyadic math - Warning caused by unary minus - Warning caused by final field initializer - TODO: 3. Check how it works with: - Inliners (notably: Stream API) - Boxed numbers - TODO: 4. Check for possible performance disasters (likely on some code patterns current algo might blow up) - TODO: 5. Problem when interesting state doesn't reach the current condition, need to do something with this - */ @NotNull - private static CauseItem findCauseChain(PsiExpression expression, MemoryStateChange history, DfaProblemType type) { - CauseItem root = new CauseItem(type, expression); - if (history.getExpression() != expression) return root; - root.addChildren(type.findCauses(expression, history)); - return root; - } - - @NotNull - private static CauseItem[] findConstantValueCause(PsiExpression expression, MemoryStateChange history, Object expectedValue) { + private CauseItem[] findConstantValueCause(PsiExpression expression, MemoryStateChange history, Object expectedValue) { if (expression instanceof PsiLiteralExpression) return new CauseItem[0]; Object constantExpressionValue = ExpressionUtils.computeConstantExpression(expression); DfaValue value = history.myTopOfStack; @@ -449,9 +449,9 @@ public class TrackingRunner extends StandardDataFlowRunner { return new CauseItem("'" + target + "' was assigned" + suffix, anchor); } - private static CauseItem[] findBooleanResultCauses(PsiExpression expression, - MemoryStateChange history, - boolean value) { + private CauseItem[] findBooleanResultCauses(PsiExpression expression, + MemoryStateChange history, + boolean value) { if (BoolUtils.isNegation(expression)) { PsiExpression negated = BoolUtils.getNegated(expression); if (negated != null) { @@ -522,8 +522,8 @@ public class TrackingRunner extends StandardDataFlowRunner { if (operandHistory != null) { DfaValue operandValue = operandHistory.myTopOfStack; if (!value) { - Pair nullability = operandHistory.findFact(operandValue, DfaFactType.NULLABILITY); - if (nullability.second == DfaNullability.NULL) { + FactDefinition nullability = operandHistory.findFact(operandValue, DfaFactType.NULLABILITY); + if (nullability.myFact == DfaNullability.NULL) { CauseItem causeItem = new CauseItem("value '" + operand.getText() + "' is always 'null'", operand); causeItem.addChildren(findConstantValueCause(operand, operandHistory, null)); return new CauseItem[]{causeItem}; @@ -544,20 +544,20 @@ public class TrackingRunner extends StandardDataFlowRunner { } @Nullable - private static CauseItem findTypeCause(MemoryStateChange operandHistory, PsiType type, boolean isInstance) { + private CauseItem findTypeCause(MemoryStateChange operandHistory, PsiType type, boolean isInstance) { PsiExpression operand = Objects.requireNonNull(operandHistory.getExpression()); DfaValue operandValue = operandHistory.myTopOfStack; - DfaPsiType wanted = operandValue.getFactory().createDfaType(type); + DfaPsiType wanted = getFactory().createDfaType(type); - Pair fact = operandHistory.findFact(operandValue, DfaFactType.TYPE_CONSTRAINT); - String explanation = fact.second == null ? null : fact.second.getAssignabilityExplanation(wanted, isInstance); + FactDefinition fact = operandHistory.findFact(operandValue, DfaFactType.TYPE_CONSTRAINT); + String explanation = fact.myFact == null ? null : fact.myFact.getAssignabilityExplanation(wanted, isInstance); while (explanation != null) { - MemoryStateChange causeLocation = fact.first; + MemoryStateChange causeLocation = fact.myChange; 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; + TypeConstraint prevConstraint = fact.getFact(TypeConstraint.empty()); String prevExplanation = prevConstraint.getAssignabilityExplanation(wanted, isInstance); if (prevExplanation == null) { CauseItem causeItem = new CauseItem(explanation, operand); @@ -570,28 +570,32 @@ public class TrackingRunner extends StandardDataFlowRunner { } @NotNull - private static CauseItem[] findRelationCause(RelationType relationType, - MemoryStateChange leftChange, - MemoryStateChange rightChange) { + private CauseItem[] findRelationCause(RelationType relationType, MemoryStateChange leftChange, MemoryStateChange rightChange) { + return findRelationCause(relationType, leftChange, leftChange.myTopOfStack, rightChange, rightChange.myTopOfStack); + } + + @NotNull + private CauseItem[] findRelationCause(RelationType relationType, + MemoryStateChange leftChange, DfaValue leftValue, + MemoryStateChange rightChange, DfaValue rightValue) { ProgressManager.checkCanceled(); - DfaValue leftValue = leftChange.myTopOfStack; - DfaValue rightValue = rightChange.myTopOfStack; - Pair leftNullability = leftChange.findFact(leftValue, DfaFactType.NULLABILITY); - Pair rightNullability = rightChange.findFact(rightValue, DfaFactType.NULLABILITY); - if ((leftNullability.second == DfaNullability.NULL && rightNullability.second == DfaNullability.NOT_NULL) || - (rightNullability.second == DfaNullability.NULL && leftNullability.second == DfaNullability.NOT_NULL)) { - return new CauseItem[]{findNullabilityCause(leftChange, leftNullability.first, leftNullability.second), - findNullabilityCause(rightChange, rightNullability.first, rightNullability.second)}; + FactDefinition leftNullability = leftChange.findFact(leftValue, DfaFactType.NULLABILITY); + FactDefinition rightNullability = rightChange.findFact(rightValue, DfaFactType.NULLABILITY); + if ((leftNullability.myFact == DfaNullability.NULL && rightNullability.myFact == DfaNullability.NOT_NULL) || + (rightNullability.myFact == DfaNullability.NULL && leftNullability.myFact == DfaNullability.NOT_NULL)) { + return new CauseItem[]{ + findNullabilityCause(leftChange, leftNullability.myFact), + findNullabilityCause(rightChange, rightNullability.myFact)}; } - Pair leftRange = leftChange.findFact(leftValue, DfaFactType.RANGE); - Pair rightRange = rightChange.findFact(rightValue, DfaFactType.RANGE); - if (leftRange.second != null && rightRange.second != null) { - LongRangeSet fromRelation = rightRange.second.fromRelation(relationType.getNegated()); - if (fromRelation != null && !fromRelation.intersects(leftRange.second)) { + FactDefinition leftRange = leftChange.findFact(leftValue, DfaFactType.RANGE); + FactDefinition rightRange = rightChange.findFact(rightValue, DfaFactType.RANGE); + if (leftRange.myFact != null && rightRange.myFact != null) { + LongRangeSet fromRelation = rightRange.myFact.fromRelation(relationType.getNegated()); + if (fromRelation != null && !fromRelation.intersects(leftRange.myFact)) { 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.myFact, "left operand is %s"), + findRangeCause(rightChange, rightRange.myFact, "right operand is %s")}; } } if (leftValue instanceof DfaVariableValue) { @@ -626,9 +630,8 @@ public class TrackingRunner extends StandardDataFlowRunner { return new CauseItem[0]; } - private static CauseItem findRelationCause(MemoryStateChange change, - DfaVariableValue value, - Relation relation, MemoryStateChange counterPartChange) { + private CauseItem findRelationCause(MemoryStateChange change, DfaVariableValue value, + Relation relation, MemoryStateChange counterPartChange) { Instruction instruction = change.myInstruction; String condition = value + " " + relation; if (instruction instanceof AssignInstruction) { @@ -667,12 +670,12 @@ public class TrackingRunner extends StandardDataFlowRunner { return null; } - private static CauseItem findNullabilityCause(MemoryStateChange factUse, MemoryStateChange factDef, DfaNullability nullability) { + private CauseItem findNullabilityCause(MemoryStateChange factUse, DfaNullability nullability) { PsiExpression expression = factUse.getExpression(); if (expression instanceof PsiTypeCastExpression) { MemoryStateChange operandPush = factUse.findSubExpressionPush(((PsiTypeCastExpression)expression).getOperand()); if (operandPush != null) { - return findNullabilityCause(operandPush, factDef, nullability); + return findNullabilityCause(operandPush, nullability); } } if (expression instanceof PsiMethodCallExpression) { @@ -701,6 +704,8 @@ public class TrackingRunner extends StandardDataFlowRunner { } } } + FactDefinition info = factUse.findFact(factUse.myTopOfStack, DfaFactType.NULLABILITY); + MemoryStateChange factDef = info.myFact == nullability ? info.myChange : null; if (nullability == DfaNullability.NOT_NULL) { String explanation = getObviouslyNonNullExplanation(expression); if (explanation != null) { @@ -726,8 +731,7 @@ public class TrackingRunner extends StandardDataFlowRunner { MemoryStateChange rValuePush = factDef.findSubExpressionPush(rExpression); if (rValuePush != null) { CauseItem assignmentItem = createAssignmentCause((AssignInstruction)factDef.myInstruction, value); - Pair rValueFact = rValuePush.findFact(rValuePush.myTopOfStack, DfaFactType.NULLABILITY); - assignmentItem.addChildren(findNullabilityCause(rValuePush, rValueFact.first, nullability)); + assignmentItem.addChildren(findNullabilityCause(rValuePush, nullability)); return assignmentItem; } } @@ -741,8 +745,7 @@ public class TrackingRunner extends StandardDataFlowRunner { return null; } - private static CauseItem fromMemberNullability(DfaNullability nullability, - PsiModifierListOwner owner, + private static CauseItem fromMemberNullability(DfaNullability nullability, PsiModifierListOwner owner, String memberName, PsiElement anchor) { if (owner != null) { NullabilityAnnotationInfo info = NullableNotNullManager.getInstance(owner.getProject()).findEffectiveNullabilityInfo(owner); @@ -800,7 +803,7 @@ public class TrackingRunner extends StandardDataFlowRunner { return null; } - private static CauseItem fromCallContract(MemoryStateChange history, PsiCallExpression call, ContractReturnValue contractReturnValue) { + private CauseItem fromCallContract(MemoryStateChange history, PsiCallExpression call, ContractReturnValue contractReturnValue) { PsiMethod method = call.resolveMethod(); if (method == null) return null; List contracts = @@ -808,23 +811,60 @@ public class TrackingRunner extends StandardDataFlowRunner { mc -> contractReturnValue.isSuperValueOf(mc.getReturnValue())); boolean explicit = JavaMethodContractUtil.hasExplicitContractAnnotation(method); if (call instanceof PsiMethodCallExpression) { + String prefix = "according to " + (explicit ? "contract" : "inferred contract"); PsiReferenceExpression methodExpression = ((PsiMethodCallExpression)call).getMethodExpression(); String name = methodExpression.getReferenceName(); for (MethodContract contract : contracts) { if (contract.isTrivial()) { - return new CauseItem("according to " + (explicit ? "contract" : "inferred contract") + + return new CauseItem(prefix + ", method '" + name + "' always returns '" + contract.getReturnValue() + "' value", methodExpression.getReferenceNameElement()); } } + if (contracts.size() == 1) { + List conditions = contracts.get(0).getConditions(); + String conditionsText = StringUtil.join(conditions, c -> c.getPresentationText(method), " and "); + CauseItem causeItem = new CauseItem( + prefix + ", method '" + name + "' returns '" + contracts.get(0).getReturnValue() + "' value when " + conditionsText, + methodExpression.getReferenceNameElement()); + for (ContractValue condition : conditions) { + DfaRelationValue relation = ObjectUtils.tryCast(condition.fromCall(getFactory(), call), DfaRelationValue.class); + PsiExpression leftPlace = condition.findLeftPlace(call); + MemoryStateChange leftPush = history.findExpressionPush(leftPlace); + PsiExpression rightPlace = condition.findRightPlace(call); + MemoryStateChange rightPush = history.findExpressionPush(rightPlace); + if (relation != null) { + DfaValue left = relation.getLeftOperand(); + DfaValue right = relation.getRightOperand(); + RelationType type = relation.getRelation(); + MemoryStateChange leftChange = history; + MemoryStateChange rightChange = history; + if (leftPush != null) { + if (leftPush.myTopOfStack == left) { + leftChange = leftPush; + } + else if (leftPush.myTopOfStack == right) { + rightChange = leftPush; + } + } + if (rightPush != null) { + if (rightPush.myTopOfStack == right) { + rightChange = rightPush; + } + else if (rightPush.myTopOfStack == left) { + leftChange = rightPush; + } + } + causeItem.addChildren(findRelationCause(type, leftChange, left, rightChange, right)); + } + } + return causeItem; + } } return null; } - private static CauseItem findRangeCause(MemoryStateChange factUse, - MemoryStateChange factDef, - LongRangeSet range, - String template) { + private static CauseItem findRangeCause(MemoryStateChange factUse, LongRangeSet range, String template) { DfaValue value = factUse.myTopOfStack; if (value instanceof DfaVariableValue) { VariableDescriptor descriptor = ((DfaVariableValue)value).getDescriptor(); @@ -861,15 +901,15 @@ public class TrackingRunner extends StandardDataFlowRunner { PsiExpression operand = ((PsiTypeCastExpression)expression).getOperand(); MemoryStateChange operandPush = factUse.findExpressionPush(operand); if (operandPush != null) { - Pair operandInfo = operandPush.findFact(operandPush.myTopOfStack, DfaFactType.RANGE); - LongRangeSet operandRange = operandInfo.second == null ? LongRangeSet.fromType(type) : operandInfo.second; + FactDefinition operandInfo = operandPush.findFact(operandPush.myTopOfStack, DfaFactType.RANGE); + LongRangeSet operandRange = operandInfo.myFact == null ? LongRangeSet.fromType(type) : operandInfo.myFact; if (operandRange != null) { LongRangeSet result = operandRange.castTo((PsiPrimitiveType)type); if (range.equals(result)) { CauseItem cause = new CauseItem("result of '(" + type.getCanonicalText() + ")' cast is " + range.getPresentationText(null), expression); if (!operandRange.equals(LongRangeSet.fromType(operand.getType()))) { - cause.addChildren(findRangeCause(operandPush, operandInfo.first, operandRange, "cast operand is %s")); + cause.addChildren(findRangeCause(operandPush, operandRange, "cast operand is %s")); } return cause; } @@ -888,27 +928,21 @@ public class TrackingRunner extends StandardDataFlowRunner { MemoryStateChange leftPush = factUse.findExpressionPush(left); MemoryStateChange rightPush = factUse.findExpressionPush(right); if (leftPush != null && rightPush != null) { - DfaValue leftValue = leftPush.myTopOfStack; - DfaValue rightValue = rightPush.myTopOfStack; - Pair leftSet = leftPush.findFact(leftValue, DfaFactType.RANGE); - Pair rightSet = rightPush.findFact(rightValue, DfaFactType.RANGE); + FactDefinition leftSet = leftPush.findFact(leftPush.myTopOfStack, DfaFactType.RANGE); + FactDefinition rightSet = rightPush.findFact(rightPush.myTopOfStack, DfaFactType.RANGE); LongRangeSet fromType = Objects.requireNonNull(LongRangeSet.fromType(type)); - if (leftSet.second == null) { - leftSet = Pair.create(null, fromType); - } - if (rightSet.second == null) { - rightSet = Pair.create(null, fromType); - } - LongRangeSet result = leftSet.second.binOpFromToken(binOp.getOperationTokenType(), rightSet.second, isLong); + LongRangeSet leftRange = leftSet.getFact(fromType); + LongRangeSet rightRange = rightSet.getFact(fromType); + LongRangeSet result = leftRange.binOpFromToken(binOp.getOperationTokenType(), rightRange, isLong); if (range.equals(result)) { CauseItem cause = new CauseItem("result of '" + binOp.getOperationSign().getText() + "' is " + range.getPresentationText(type), factUse); CauseItem leftCause = null, rightCause = null; - if (!leftSet.second.equals(fromType)) { - leftCause = findRangeCause(leftPush, leftSet.first, leftSet.second, "left operand is %s"); + if (!leftRange.equals(fromType)) { + leftCause = findRangeCause(leftPush, leftRange, "left operand is %s"); } - if (!rightSet.second.equals(fromType)) { - rightCause = findRangeCause(rightPush, rightSet.first, rightSet.second, "right operand is %s"); + if (!rightRange.equals(fromType)) { + rightCause = findRangeCause(rightPush, rightRange, "right operand is %s"); } cause.addChildren(leftCause, rightCause); return cause; @@ -919,6 +953,8 @@ public class TrackingRunner extends StandardDataFlowRunner { } String rangeText = range.getPresentationText(expression != null ? expression.getType() : null); CauseItem item = new CauseItem(String.format(template, rangeText), factUse); + FactDefinition info = factUse.findFact(value, DfaFactType.RANGE); + MemoryStateChange factDef = range.equals(info.myFact) ? info.myChange : null; if (factDef != null) { if (factDef.myInstruction instanceof AssignInstruction && factDef.myTopOfStack == value) { PsiExpression rExpression = ((AssignInstruction)factDef.myInstruction).getRExpression(); @@ -926,8 +962,7 @@ public class TrackingRunner extends StandardDataFlowRunner { MemoryStateChange rValuePush = factDef.findSubExpressionPush(rExpression); if (rValuePush != null) { CauseItem assignmentItem = createAssignmentCause((AssignInstruction)factDef.myInstruction, value); - Pair rValueFact = rValuePush.findFact(rValuePush.myTopOfStack, DfaFactType.RANGE); - assignmentItem.addChildren(findRangeCause(rValuePush, rValueFact.first, range, "Value is %s")); + assignmentItem.addChildren(findRangeCause(rValuePush, range, "Value is %s")); item.addChildren(assignmentItem); return item; } diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract.java b/java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract.java new file mode 100644 index 000000000000..9285724ddaf0 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract.java @@ -0,0 +1,16 @@ +/* +Value is always false (Objects.isNull(s); line#12) + According to inferred contract, method 'isNull' returns 'false' value when parameter != null (isNull; line#12) + 's' was dereferenced (s; line#11) + */ + +import java.util.Objects; + +class Test { + void test(String s) { + System.out.println(s.trim()); + if (Objects.isNull(s)) { + + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract2.java b/java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract2.java new file mode 100644 index 000000000000..23d433d14a2d --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/SimpleContract2.java @@ -0,0 +1,15 @@ +/* +Value is always false (Objects.isNull(s.trim()); line#11) + According to inferred contract, method 'isNull' returns 'false' value when parameter != null (isNull; line#11) + Method 'trim' is externally annotated as 'non-null' (trim; line#11) + */ + +import java.util.Objects; + +class Test { + void test(String s) { + if (Objects.isNull(s.trim())) { + + } + } +} \ 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 851c37f1fcc0..85a0863679ad 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java @@ -161,4 +161,6 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa public void testNumericCast() { doTest(); } public void testNumericCast2() { doTest(); } public void testNumericWidening() { doTest(); } + public void testSimpleContract() { doTest(); } + public void testSimpleContract2() { doTest(); } }