From e2a4d9b41bfc1ff15a05c8aae756260eb4ceb03f Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 22 Nov 2017 10:56:45 +0700 Subject: [PATCH] myPossibleVariableValues pushed down to DataFlowInspectionVisitor Division by zero handling removed from DataFlowInspection as DivideByZero inspection is smart enough now --- .../dataFlow/DataFlowInspectionBase.java | 38 +++++++++++++- .../dataFlow/StandardInstructionVisitor.java | 50 ------------------- .../dataFlow/fixture/DivisionByZero.java | 19 ------- .../numeric/divide_by_zero/DivideByZero.java | 31 +++++++++--- .../numeric/divide_by_zero/expected.xml | 44 ---------------- .../numeric/DivideByZeroInspectionTest.java | 15 ++++-- 6 files changed, 73 insertions(+), 124 deletions(-) delete mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/expected.xml diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index b82857cd0a08..90491bc92dc0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -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 reportedAnchors) { + private void reportConstantReferenceValues(ProblemsHolder holder, DataFlowInstructionVisitor visitor, Set reportedAnchors) { for (Pair 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, StateInfo> myStateInfos = new LinkedHashMap<>(); private final Set myCCEInstructions = ContainerUtil.newHashSet(); private final Map myFailingCalls = new HashMap<>(); @@ -859,6 +862,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool private final Map myMethodReferenceResults = new HashMap<>(); private final Map myOutOfBoundsArrayAccesses = new HashMap<>(); private final List myOptionalQualifiers = new ArrayList<>(); + private final MultiMap 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> getConstantReferenceValues() { + List> result = ContainerUtil.newArrayList(); + for (PushInstruction instruction : myPossibleVariableValues.keySet()) { + Collection 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 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; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index 2202c7adfd8d..b2513c2648e2 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -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 KNOWN_METHOD_RANGES = new CallMapper() .register(CallMatcher.instanceCall("java.time.LocalDateTime", "getHour"), LongRangeSet.range(0, 23)) @@ -58,7 +55,6 @@ public class StandardInstructionVisitor extends InstructionVisitor { private final Set myReachable = new THashSet<>(); private final Set myCanBeNullInInstanceof = new THashSet<>(); - private final MultiMap myPossibleVariableValues = MultiMap.createSet(); private final Set 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> getConstantReferenceValues() { - List> result = ContainerUtil.newArrayList(); - for (PushInstruction instruction : myPossibleVariableValues.keySet()) { - Collection 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(); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/DivisionByZero.java b/java/java-tests/testData/inspection/dataFlow/fixture/DivisionByZero.java index bf6b1070a2f2..8a99379c74b0 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/DivisionByZero.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/DivisionByZero.java @@ -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 / d; - } 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); - } - } - } - } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/DivideByZero.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/DivideByZero.java index 4375676fe438..84effcecfe4a 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/DivideByZero.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/DivideByZero.java @@ -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 / 0; } int rest(int num) { - return num % 0 % 1; + return num % 0 % 1; } void assignment(int i, double d) { - i /= 1-1; - d %= 0; + i /= 1-1; + d %= 0; i /= d; } // IDEABKL-7552 Report inspection with the highest severity void test(int size) { if (size == 0) { - int x = 42 / size; + int x = 42 / size; } } @@ -32,7 +32,26 @@ public class DivideByZero { System.out.println(41 / size); return; } - System.out.println(42 / size); + System.out.println(42 / size); } + + public static void main(String[] args, int d) { + String is = null; + if (d != 0) return; + + try { + if (Math.random() > 0.5) { + double k = 1 / d; + } 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); + } + } + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/expected.xml deleted file mode 100644 index 917b167db29b..000000000000 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/numeric/divide_by_zero/expected.xml +++ /dev/null @@ -1,44 +0,0 @@ - - - - DivideByZero.java - 6 - Division by zero - Division by zero #loc - - - - DivideByZero.java - 10 - Division by zero - Division by zero #loc - - - - DivideByZero.java - 14 - Division by zero - Division by zero #loc - - - - DivideByZero.java - 15 - Division by zero - Division by zero #loc - - - - DivideByZero.java - 22 - Division by zero - Division by zero #loc - - - - DivideByZero.java - 35 - Division by zero - Division by zero #loc - - \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/DivideByZeroInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/DivideByZeroInspectionTest.java index 74fd95077959..c88444ad16ba 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/DivideByZeroInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/numeric/DivideByZeroInspectionTest.java @@ -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(); } } \ No newline at end of file