MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-209947 Report obviously not-null values; better propagation through assignment

Browse Source 
GitOrigin-RevId: 2f6cd06b9750e8f080ab543af40b522f4c2bfefa
This commit is contained in:
Tagir Valeev
2019-04-28 16:47:05 +03:00
committed by intellij-monorepo-bot
parent efcbb20422
commit e12a1970e4
7 changed files with 95 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingDfaMemoryState.java
+5 -5
View File
@@ -241,14 +241,14 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
@Nullable
MemoryStateChange findExpressionPush(@Nullable PsiExpression expression) {
if (expression == null) return null;
return findChange(change -> change.getExpression() == expression);
return findChange(change -> change.getExpression() == expression, false);
}
MemoryStateChange findRelation(DfaVariableValue value, @NotNull Predicate<Relation> relationPredicate) {
MemoryStateChange findRelation(DfaVariableValue value, @NotNull Predicate<Relation> relationPredicate, boolean startFromSelf) {
return findChange(change -> {
Change varChange = change.myChanges.get(value);
return varChange != null && varChange.myAddedRelations.stream().anyMatch(relationPredicate);
});
}, startFromSelf);
}
@NotNull
@@ -272,8 +272,8 @@ public class TrackingDfaMemoryState extends DfaMemoryStateImpl {
}
@Nullable
private MemoryStateChange findChange(@NotNull Predicate<MemoryStateChange> predicate) {
for (MemoryStateChange change = myPrevious; change != null; change = change.myPrevious) {
private MemoryStateChange findChange(@NotNull Predicate<MemoryStateChange> predicate, boolean startFromSelf) {
for (MemoryStateChange change = startFromSelf ? this : myPrevious; change != null; change = change.myPrevious) {
if (predicate.test(change)) {
return change;
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java
+41 -9
View File
@@ -10,11 +10,13 @@ import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet;
import com.intellij.codeInspection.dataFlow.value.*;
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType;
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;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
@@ -219,7 +221,6 @@ public class TrackingRunner extends StandardDataFlowRunner {
TODO: 2. Describe causes in more cases:
Warning caused by contract
Warning caused by CustomMethodHandler
Warning caused by mismatched type constraints
TODO: 3. Check how it works with
Inliners (notably: Stream API)
Ternary operators
@@ -254,7 +255,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (value instanceof DfaVariableValue) {
MemoryStateChange change = history.findRelation(
(DfaVariableValue)value, rel -> rel.myRelationType == RelationType.EQ && rel.myCounterpart instanceof DfaConstValue &&
Objects.equals(expectedValue, ((DfaConstValue)rel.myCounterpart).getValue()));
Objects.equals(expectedValue, ((DfaConstValue)rel.myCounterpart).getValue()), false);
if (change != null) {
PsiExpression varSourceExpression = change.getExpression();
Instruction instruction = change.myInstruction;
@@ -297,8 +298,8 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (!value) {
relationType = relationType.getNegated();
}
PsiExpression leftOperand = binOp.getLOperand();
PsiExpression rightOperand = binOp.getROperand();
PsiExpression leftOperand = PsiUtil.skipParenthesizedExprDown(binOp.getLOperand());
PsiExpression rightOperand = PsiUtil.skipParenthesizedExprDown(binOp.getROperand());
MemoryStateChange leftChange = history.findExpressionPush(leftOperand);
MemoryStateChange rightChange = history.findExpressionPush(rightOperand);
if (leftChange != null && rightChange != null) {
@@ -372,6 +373,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
private static CauseItem[] findRelationCause(RelationType relationType,
MemoryStateChange leftChange,
MemoryStateChange rightChange) {
ProgressManager.checkCanceled();
DfaValue leftValue = leftChange.myTopOfStack;
DfaValue rightValue = rightChange.myTopOfStack;
if (leftValue instanceof DfaVariableValue) {
@@ -443,7 +445,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
}
if (instruction instanceof AssignInstruction) {
DfaValue target = change.myTopOfStack;
PsiExpression rValue = ((AssignInstruction)instruction).getRExpression();
PsiExpression rValue = PsiUtil.skipParenthesizedExprDown(((AssignInstruction)instruction).getRExpression());
if (target == value) {
CauseItem item = new CauseItem("'" + target + "' was assigned", rValue);
MemoryStateChange rValuePush = change.findExpressionPush(rValue);
@@ -482,9 +484,7 @@ public class TrackingRunner extends StandardDataFlowRunner {
if (factDef != null && expression != null) {
PsiExpression defExpression = factDef.getExpression();
if (defExpression != null) {
return new CauseItem(
new CustomDfaProblemType(expression.getText() + " is known to be '" + nullability.getPresentationName() + "' from #ref"),
defExpression);
return new CauseItem(expression.getText() + " is known to be '" + nullability.getPresentationName() + "' from #ref", defExpression);
}
}
if (expression instanceof PsiMethodCallExpression) {
@@ -504,6 +504,12 @@ public class TrackingRunner extends StandardDataFlowRunner {
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 null;
}
@@ -630,6 +636,19 @@ public class TrackingRunner extends StandardDataFlowRunner {
String rangeText = range.getPresentationText(expression != null ? expression.getType() : null);
CauseItem item = new CauseItem(String.format(template, rangeText), factUse);
if (factDef != null) {
if (factDef.myInstruction instanceof AssignInstruction && factDef.myTopOfStack == value) {
PsiExpression rExpression = PsiUtil.skipParenthesizedExprDown(((AssignInstruction)factDef.myInstruction).getRExpression());
if (rExpression != null) {
MemoryStateChange rValuePush = factDef.findExpressionPush(rExpression);
if (rValuePush != null) {
CauseItem assignmentItem = new CauseItem("'" + value + "' was assigned", rExpression);
Pair<MemoryStateChange, LongRangeSet> rValueFact = rValuePush.findFact(rValuePush.myTopOfStack, DfaFactType.RANGE);
assignmentItem.addChildren(findRangeCause(rValuePush, rValueFact.first, range, "Value is %s"));
item.addChildren(assignmentItem);
return item;
}
}
}
PsiExpression defExpression = factDef.getExpression();
if (defExpression != null) {
item.addChildren(new CauseItem("Range is known from #ref", defExpression));
@@ -638,6 +657,19 @@ public class TrackingRunner extends StandardDataFlowRunner {
return item;
}
@Nullable
public static String getObviouslyNonNullExplanation(PsiExpression arg) {
if (arg == null || ExpressionUtils.isNullLiteral(arg)) return null;
if (arg instanceof PsiNewExpression) return "newly created object";
if (arg instanceof PsiLiteralExpression) return "literal";
if (arg.getType() instanceof PsiPrimitiveType) return "a value of primitive type '" + arg.getType().getCanonicalText() + "'";
if (arg instanceof PsiPolyadicExpression && ((PsiPolyadicExpression)arg).getOperationTokenType() == JavaTokenType.PLUS) {
return "concatenation";
}
if (arg instanceof PsiThisExpression) return "'this' object";
return null;
}
private static MemoryStateChange findRelationAddedChange(MemoryStateChange history, DfaVariableValue var, Relation relation) {
List<Relation> subRelations;
switch (relation.myRelationType) {
@@ -656,6 +688,6 @@ public class TrackingRunner extends StandardDataFlowRunner {
default:
subRelations = Collections.singletonList(relation);
}
return history.findRelation(var, subRelations::contains);
return history.findRelation(var, subRelations::contains, true);
}
}
java/java-impl/src/com/intellij/codeInspection/ObviousNullCheckInspection.java
+3 -19
View File
@@ -2,11 +2,8 @@
package com.intellij.codeInspection;
import com.intellij.codeInsight.BlockUtils;
import com.intellij.codeInspection.dataFlow.ContractReturnValue;
import com.intellij.codeInspection.dataFlow.*;
import com.intellij.codeInspection.dataFlow.ContractReturnValue.ParameterReturnValue;
import com.intellij.codeInspection.dataFlow.ContractValue;
import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil;
import com.intellij.codeInspection.dataFlow.MethodContract;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
@@ -34,13 +31,13 @@ public class ObviousNullCheckInspection extends AbstractBaseJavaLocalInspectionT
PsiExpression[] args = call.getArgumentList().getExpressions();
// Avoid method resolve if no argument is a candidate for obvious non-null warning
// (checking this is easier than resolving and calls without arguments are excluded at all)
if (!ContainerUtil.exists(args, arg -> getObviouslyNonNullExplanation(PsiUtil.skipParenthesizedExprDown(arg)) != null)) return;
if (!ContainerUtil.exists(args, arg -> TrackingRunner.getObviouslyNonNullExplanation(PsiUtil.skipParenthesizedExprDown(arg)) != null)) return;
NullCheckParameter nullCheckParameter = NullCheckParameter.fromCall(call);
if (nullCheckParameter == null) return;
if (!ExpressionUtils.isVoidContext(call) && !nullCheckParameter.myReturnsParameter) return;
if (args.length <= nullCheckParameter.myIndex) return;
PsiExpression nullArg = PsiUtil.skipParenthesizedExprDown(args[nullCheckParameter.myIndex]);
String explanation = getObviouslyNonNullExplanation(nullArg);
String explanation = TrackingRunner.getObviouslyNonNullExplanation(nullArg);
if (explanation == null) return;
if(nullCheckParameter.myNull) {
holder.registerProblem(nullArg, InspectionsBundle.message("inspection.redundant.null.check.always.fail.message", explanation));
@@ -53,19 +50,6 @@ public class ObviousNullCheckInspection extends AbstractBaseJavaLocalInspectionT
};
}
@Nullable
private static String getObviouslyNonNullExplanation(PsiExpression arg) {
if (arg == null || ExpressionUtils.isNullLiteral(arg)) return null;
if (arg instanceof PsiNewExpression) return "newly created object";
if (arg instanceof PsiLiteralExpression) return "literal";
if (arg.getType() instanceof PsiPrimitiveType) return "a value of primitive type '" + arg.getType().getCanonicalText() + "'";
if (arg instanceof PsiPolyadicExpression && ((PsiPolyadicExpression)arg).getOperationTokenType() == JavaTokenType.PLUS) {
return "concatenation";
}
if (arg instanceof PsiThisExpression) return "'this' object";
return null;
}
static class NullCheckParameter {
int myIndex;
boolean myNull;
java/java-tests/testData/inspection/dataFlow/tracker/IndexOfPlusOne.java
+19
View File
@@ -0,0 +1,19 @@
/*
Value is always true (j >= 0)
Left operand is >= 0 (j)
'j' was assigned (i + 1)
Result of '+' is >= 0 (i + 1)
Left operand is in {-1..Integer.MAX_VALUE-1} (i)
'i' was assigned (s.indexOf(' '))
Value is in {-1..Integer.MAX_VALUE-1} (s.indexOf(' '))
*/
class Test {
private static void dosmth(String s) {
int i = s.indexOf(' ');
int j = i + 1;
if (<selection>j >= 0</selection>) {
}
}
}
java/java-tests/testData/inspection/dataFlow/tracker/NotNullAssignmentInside.java
+12
View File
@@ -0,0 +1,12 @@
/*
Value is always true ((s = new Object()) != null)
's' was assigned (new Object())
Expression cannot be null as it's newly created object (new Object())
*/
class Test {
void test() {
Object s;
if (<selection>(s = new Object()) != null</selection>) {}
}
}
java/java-tests/testData/inspection/dataFlow/tracker/NotNullObvious.java
+12
View File
@@ -0,0 +1,12 @@
/*
Value is always true (s != null)
's' was assigned (new Object())
Expression cannot be null as it's newly created object (new Object())
*/
class Test {
void test() {
Object s = (new Object());
if (<selection>s != null</selection>) {}
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java
+3
View File
@@ -121,4 +121,7 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa
public void testInstanceOfChain2() { doTest(); }
public void testNotInstanceOf() { doTest(); }
public void testInstanceOfPreviousCast() { doTest(); }
public void testNotNullObvious() { doTest(); }
public void testNotNullAssignmentInside() { doTest(); }
public void testIndexOfPlusOne() { doTest(); }
}