myPossibleVariableValues pushed down to DataFlowInspectionVisitor

Division by zero handling removed from DataFlowInspection as DivideByZero inspection is smart enough now
This commit is contained in:
Tagir Valeev
2017-11-22 10:56:45 +07:00
parent 952156db3d
commit e2a4d9b41b
6 changed files with 73 additions and 124 deletions
@@ -18,6 +18,7 @@ import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
import com.intellij.codeInspection.dataFlow.value.DfaUnknownValue;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.dataFlow.value.DfaVariableValue;
import com.intellij.codeInspection.nullable.NullableStuffInspectionBase;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
@@ -30,6 +31,7 @@ import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.util.*;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.MultiMap;
import com.siyeh.ig.psiutils.*;
import one.util.streamex.StreamEx;
import org.jdom.Element;
@@ -472,7 +474,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
});
}
private void reportConstantReferenceValues(ProblemsHolder holder, StandardInstructionVisitor visitor, Set<PsiElement> reportedAnchors) {
private void reportConstantReferenceValues(ProblemsHolder holder, DataFlowInstructionVisitor visitor, Set<PsiElement> reportedAnchors) {
for (Pair<PsiReferenceExpression, DfaConstValue> pair : visitor.getConstantReferenceValues()) {
PsiReferenceExpression ref = pair.first;
if (ref.getParent() instanceof PsiReferenceExpression || !reportedAnchors.add(ref)) {
@@ -849,6 +851,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
}
private static class DataFlowInstructionVisitor extends StandardInstructionVisitor {
private static final Object ANY_VALUE = new Object();
private final Map<NullabilityProblem<?>, StateInfo> myStateInfos = new LinkedHashMap<>();
private final Set<Instruction> myCCEInstructions = ContainerUtil.newHashSet();
private final Map<MethodCallInstruction, Boolean> myFailingCalls = new HashMap<>();
@@ -859,6 +862,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
private final Map<PsiMethodReferenceExpression, DfaValue> myMethodReferenceResults = new HashMap<>();
private final Map<PsiArrayAccessExpression, ThreeState> myOutOfBoundsArrayAccesses = new HashMap<>();
private final List<PsiExpression> myOptionalQualifiers = new ArrayList<>();
private final MultiMap<PushInstruction, Object> myPossibleVariableValues = MultiMap.createSet();
private boolean myAlwaysReturnsNotNull = true;
@Override
@@ -992,6 +996,34 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
}
}
@Override
public DfaInstructionState[] visitPush(PushInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
PsiExpression place = instruction.getPlace();
if (!instruction.isReferenceWrite() && place instanceof PsiReferenceExpression) {
DfaValue dfaValue = instruction.getValue();
if (dfaValue instanceof DfaVariableValue) {
DfaConstValue constValue = memState.getConstantValue((DfaVariableValue)dfaValue);
boolean report = constValue != null && shouldReportConstValue(constValue.getValue(), place);
myPossibleVariableValues.putValue(instruction, report ? constValue : ANY_VALUE);
}
}
return super.visitPush(instruction, runner, memState);
}
public List<Pair<PsiReferenceExpression, DfaConstValue>> getConstantReferenceValues() {
List<Pair<PsiReferenceExpression, DfaConstValue>> result = ContainerUtil.newArrayList();
for (PushInstruction instruction : myPossibleVariableValues.keySet()) {
Collection<Object> values = myPossibleVariableValues.get(instruction);
if (values.size() == 1) {
Object singleValue = values.iterator().next();
if (singleValue != ANY_VALUE) {
result.add(Pair.create((PsiReferenceExpression)instruction.getPlace(), (DfaConstValue)singleValue));
}
}
}
return result;
}
private static boolean hasNonTrivialFailingContracts(MethodCallInstruction instruction) {
List<MethodContract> contracts = instruction.getContracts();
return !contracts.isEmpty() && contracts.stream().anyMatch(
@@ -1025,6 +1057,10 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
return ok;
}
private static boolean shouldReportConstValue(Object value, PsiElement place) {
return value == null || value instanceof Boolean;
}
private static class StateInfo {
boolean ephemeralNpe;
boolean normalNpe;
@@ -20,7 +20,6 @@ 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.diagnostic.Logger;
import com.intellij.openapi.util.Pair;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
@@ -28,7 +27,6 @@ import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.MultiMap;
import com.siyeh.ig.callMatcher.CallMapper;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.MethodUtils;
@@ -45,7 +43,6 @@ import java.util.stream.Stream;
*/
public class StandardInstructionVisitor extends InstructionVisitor {
private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.StandardInstructionVisitor");
private static final Object ANY_VALUE = new Object();
private static final CallMapper<LongRangeSet> KNOWN_METHOD_RANGES = new CallMapper<LongRangeSet>()
.register(CallMatcher.instanceCall("java.time.LocalDateTime", "getHour"), LongRangeSet.range(0, 23))
@@ -58,7 +55,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
private final Set<BinopInstruction> myReachable = new THashSet<>();
private final Set<BinopInstruction> myCanBeNullInInstanceof = new THashSet<>();
private final MultiMap<PushInstruction, Object> myPossibleVariableValues = MultiMap.createSet();
private final Set<InstanceofInstruction> myUsefulInstanceofs = new THashSet<>();
@Override
@@ -260,52 +256,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
DfaValue res) {
}
@Override
public DfaInstructionState[] visitPush(PushInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
PsiExpression place = instruction.getPlace();
if (!instruction.isReferenceWrite() && place instanceof PsiReferenceExpression) {
DfaValue dfaValue = instruction.getValue();
if (dfaValue instanceof DfaVariableValue) {
DfaConstValue constValue = memState.getConstantValue((DfaVariableValue)dfaValue);
boolean report = constValue != null && shouldReportConstValue(constValue.getValue(), place);
myPossibleVariableValues.putValue(instruction, report ? constValue : ANY_VALUE);
}
}
return super.visitPush(instruction, runner, memState);
}
private static boolean shouldReportConstValue(Object value, PsiElement place) {
return value == null || value instanceof Boolean ||
value.equals(new Long(0)) && isDivider(PsiUtil.skipParenthesizedExprUp(place));
}
private static boolean isDivider(PsiElement expr) {
PsiElement parent = expr.getParent();
if (parent instanceof PsiBinaryExpression) {
return ControlFlowAnalyzer.isBinaryDivision(((PsiBinaryExpression)parent).getOperationTokenType()) &&
((PsiBinaryExpression)parent).getROperand() == expr;
}
if (parent instanceof PsiAssignmentExpression) {
return ControlFlowAnalyzer.isAssignmentDivision(((PsiAssignmentExpression)parent).getOperationTokenType()) &&
((PsiAssignmentExpression)parent).getRExpression() == expr;
}
return false;
}
public List<Pair<PsiReferenceExpression, DfaConstValue>> getConstantReferenceValues() {
List<Pair<PsiReferenceExpression, DfaConstValue>> result = ContainerUtil.newArrayList();
for (PushInstruction instruction : myPossibleVariableValues.keySet()) {
Collection<Object> values = myPossibleVariableValues.get(instruction);
if (values.size() == 1) {
Object singleValue = values.iterator().next();
if (singleValue != ANY_VALUE) {
result.add(Pair.create((PsiReferenceExpression)instruction.getPlace(), (DfaConstValue)singleValue));
}
}
}
return result;
}
@Override
public DfaInstructionState[] visitTypeCast(TypeCastInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
PsiType type = instruction.getCastTo();
@@ -11,23 +11,4 @@ class Util {
}
}
public static void main(String[] args, int d) {
String is = null;
if (d != 0) return;
try {
if (Math.random() > 0.5) {
double k = 1 / <warning descr="Value 'd' is always '0'">d</warning>;
} else {
is = "This is printed half of the time";
double k = 1 / 0;
}
} catch (Exception ex) {
ex.printStackTrace();
if (is != null) {
System.out.println(is);
}
}
}
}
@@ -3,23 +3,23 @@ package com.siyeh.igtest.numeric.divide_by_zero;
public class DivideByZero {
int divide(int num) {
return num / 3 / 0;
return num / 3 / <warning descr="Division by zero">0</warning>;
}
int rest(int num) {
return num % 0 % 1;
return num % <warning descr="Division by zero">0</warning> % 1;
}
void assignment(int i, double d) {
i /= 1-1;
d %= 0;
<warning descr="Division by zero">i /= 1-1</warning>;
<warning descr="Division by zero">d %= 0</warning>;
i /= d;
}
// IDEABKL-7552 Report inspection with the highest severity
void test(int size) {
if (size == 0) {
int x = 42 / size;
int x = 42 / <warning descr="Division by zero">size</warning>;
}
}
@@ -32,7 +32,26 @@ public class DivideByZero {
System.out.println(41 / size);
return;
}
System.out.println(42 / size);
System.out.println(42 / <warning descr="Division by zero">size</warning>);
}
public static void main(String[] args, int d) {
String is = null;
if (d != 0) return;
try {
if (Math.random() > 0.5) {
double k = 1 / <warning descr="Division by zero">d</warning>;
} else {
is = "This is printed half of the time";
double k = 1 / <warning descr="Division by zero">0</warning>;
}
} catch (Exception ex) {
ex.printStackTrace();
if (is != null) {
System.out.println(is);
}
}
}
}
@@ -1,44 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>DivideByZero.java</file>
<line>6</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Division by zero</problem_class>
<description>Division by zero #loc</description>
</problem>
<problem>
<file>DivideByZero.java</file>
<line>10</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Division by zero</problem_class>
<description>Division by zero #loc</description>
</problem>
<problem>
<file>DivideByZero.java</file>
<line>14</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Division by zero</problem_class>
<description>Division by zero #loc</description>
</problem>
<problem>
<file>DivideByZero.java</file>
<line>15</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Division by zero</problem_class>
<description>Division by zero #loc</description>
</problem>
<problem>
<file>DivideByZero.java</file>
<line>22</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Division by zero</problem_class>
<description>Division by zero #loc</description>
</problem>
<problem>
<file>DivideByZero.java</file>
<line>35</line>
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Division by zero</problem_class>
<description>Division by zero #loc</description>
</problem>
</problems>
@@ -1,10 +1,17 @@
package com.siyeh.ig.numeric;
import com.siyeh.ig.IGInspectionTestCase;
import com.intellij.codeInspection.InspectionProfileEntry;
import com.siyeh.ig.LightInspectionTestCase;
import org.jetbrains.annotations.Nullable;
public class DivideByZeroInspectionTest extends IGInspectionTestCase {
public class DivideByZeroInspectionTest extends LightInspectionTestCase {
public void testDivideByZero() {
doTest();
}
public void test() {
doTest("com/siyeh/igtest/numeric/divide_by_zero", new DivideByZeroInspection());
@Nullable
@Override
protected InspectionProfileEntry getInspection() {
return new DivideByZeroInspection();
}
}