From c1b01aed5c7a035582eea5d22084949c8f293c3e Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 20 Sep 2017 17:47:39 +0700 Subject: [PATCH] IDEA-179287 Array initializer/dimension should be taken into account to track array length --- .../dataFlow/ContractValue.java | 3 +- .../dataFlow/ControlFlowAnalyzer.java | 71 +++++++++++++++---- .../dataFlow/DfaMemoryStateImpl.java | 2 +- .../codeInspection/dataFlow/SpecialField.java | 2 +- .../dataFlow/StandardInstructionVisitor.java | 3 +- .../inliner/CollectionFactoryInliner.java | 3 +- .../dataFlow/inliner/StreamChainInliner.java | 4 +- .../dataFlow/value/DfaValueFactory.java | 5 ++ .../dataFlow/value/DfaVariableValue.java | 1 + .../fixture/ArrayInitializerLength.java | 55 ++++++++++++++ .../DataFlowInspectionTest.java | 1 + 11 files changed, 128 insertions(+), 22 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ArrayInitializerLength.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 040d969f8913..e97b381ab59f 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 @@ -109,8 +109,7 @@ public abstract class ContractValue { new IndependentValue(factory -> factory.getOptionalFactory().getOptional(true), "present"); static final IndependentValue OPTIONAL_ABSENT = new IndependentValue(factory -> factory.getOptionalFactory().getOptional(false), "empty"); - static final IndependentValue ZERO = - new IndependentValue(factory -> factory.getConstFactory().createFromValue(0, PsiType.INT, null), "0"); + static final IndependentValue ZERO = new IndependentValue(factory -> factory.getInt(0), "0"); private final Function mySupplier; private final String myPresentation; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index d4f01fe07472..e8276fc69f3d 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -492,7 +492,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } if (length != null) { addInstruction(new PushInstruction(length.createValue(myFactory, qualifier), null)); - addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(0, PsiType.INT, null), null)); + addInstruction(new PushInstruction(myFactory.getInt(0), null)); addInstruction(new BinopInstruction(JavaTokenType.EQEQ, iteratedValue, myProject)); addInstruction(new ConditionalGotoInstruction(loopEndOffset, false, null)); hasSizeCheck = true; @@ -1044,13 +1044,46 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } + @Nullable + private DfaVariableValue getTargetVariable(PsiExpression expression) { + PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); + if (parent instanceof PsiVariable) { + // initialization + return getFactory().getVarFactory().createVariableValue((PsiVariable)expression.getParent(), false); + } + if (parent instanceof PsiAssignmentExpression) { + PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)parent; + if (assignmentExpression.getOperationTokenType().equals(JavaTokenType.EQ) && + PsiTreeUtil.isAncestor(assignmentExpression.getRExpression(), expression, false)) { + DfaValue value = getFactory().createValue(assignmentExpression.getLExpression()); + if (value instanceof DfaVariableValue) { + return (DfaVariableValue)value; + } + } + } + return null; + } + @Override public void visitArrayInitializerExpression(PsiArrayInitializerExpression expression) { startElement(expression); PsiType type = expression.getType(); PsiType componentType = type instanceof PsiArrayType ? ((PsiArrayType)type).getComponentType() : null; + DfaVariableValue var = getTargetVariable(expression); processArrayInitializers(expression, componentType, DfaPsiUtil.getTypeNullability(componentType)); - pushUnknown(); + if (var != null) { + // Declaration: write array length + addInstruction(new PushInstruction(var, null, true)); + addInstruction(new PushInstruction(getFactory().createTypeValue(type, Nullness.NOT_NULL), expression)); + addInstruction(new AssignInstruction(expression, var)); + addInstruction(new PushInstruction(SpecialField.ARRAY_LENGTH.createValue(getFactory(), var), null, true)); + addInstruction(new PushInstruction(getFactory().getInt(expression.getInitializers().length), null)); + addInstruction(new AssignInstruction(null, null)); + addInstruction(new PopInstruction()); + } + else { + pushUnknown(); + } finishElement(expression); } @@ -1567,15 +1600,28 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new PopInstruction()); } - pushUnknown(); - PsiType type = expression.getType(); if (type instanceof PsiArrayType) { - final PsiExpression[] dimensions = expression.getArrayDimensions(); - for (final PsiExpression dimension : dimensions) { - dimension.accept(this); + DfaVariableValue var = getTargetVariable(expression); + if (var == null) { + var = getFactory().getVarFactory().createVariableValue(createTempVariable(type), false); } - for (PsiExpression ignored : dimensions) { + addInstruction(new PushInstruction(var, null, true)); + addInstruction(new PushInstruction(getFactory().createTypeValue(type, Nullness.NOT_NULL), expression)); + addInstruction(new AssignInstruction(expression, var)); // var remains on stack as an instruction result + final PsiExpression[] dimensions = expression.getArrayDimensions(); + boolean sizeAssigned = false; + DfaValue length = SpecialField.ARRAY_LENGTH.createValue(getFactory(), var); + for (final PsiExpression dimension : dimensions) { + if (!sizeAssigned) { + addInstruction(new PushInstruction(length, null, true)); + dimension.accept(this); + addInstruction(new AssignInstruction(dimension, null)); + sizeAssigned = true; + } + else { + dimension.accept(this); + } addInstruction(new PopInstruction()); } final PsiArrayInitializerExpression arrayInitializer = expression.getArrayInitializer(); @@ -1588,11 +1634,14 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } } processArrayInitializers(arrayInitializer, ((PsiArrayType)type).getComponentType(), nullability); + addInstruction(new PushInstruction(length, null, true)); + addInstruction(new PushInstruction(getFactory().getInt(arrayInitializer.getInitializers().length), null)); + addInstruction(new AssignInstruction(null, null)); + addInstruction(new PopInstruction()); } - addConditionalRuntimeThrow(); - addInstruction(new MethodCallInstruction(expression, null, Collections.emptyList())); } else { + pushUnknown(); // qualifier PsiMethod constructor = pushConstructorArguments(expression); addConditionalRuntimeThrow(); @@ -1779,8 +1828,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { startElement(block); try { block.accept(this); - // return value for void or incomplete block - pushUnknown(); } finally { finishElement(block); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 9994d84954f5..fde2e0031703 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -965,7 +965,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (dfaRight instanceof DfaConstValue) { Object constVal = ((DfaConstValue)dfaRight).getValue(); if (constVal instanceof Boolean) { - DfaConstValue negVal = myFactory.getConstFactory().createFromValue(!((Boolean)constVal).booleanValue(), PsiType.BOOLEAN, null); + DfaConstValue negVal = myFactory.getBoolean(!((Boolean)constVal).booleanValue()); if (!applyRelation(dfaLeft, negVal, !negated)) { return false; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/SpecialField.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/SpecialField.java index 47720457365a..058a85f0e09c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/SpecialField.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/SpecialField.java @@ -54,7 +54,7 @@ public enum SpecialField { STRING_LENGTH(CommonClassNames.JAVA_LANG_STRING, "length", true, LongRangeSet.indexRange()) { @Override public DfaValue createFromConstant(DfaValueFactory factory, @NotNull Object obj) { - return obj instanceof String ? factory.getConstFactory().createFromValue(((String)obj).length(), PsiType.INT, null) : null; + return obj instanceof String ? factory.getInt(((String)obj).length()) : null; } }, COLLECTION_SIZE(CommonClassNames.JAVA_UTIL_COLLECTION, "size", false, LongRangeSet.indexRange()), 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 57863181ce9d..12c02b78ebb6 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 @@ -120,8 +120,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { boolean alwaysOutOfBounds = false; if (index != DfaUnknownValue.getInstance()) { DfaValueFactory factory = runner.getFactory(); - DfaValue indexNonNegative = - factory.createCondition(index, RelationType.GE, factory.getConstFactory().createFromValue(0, PsiType.INT, null)); + DfaValue indexNonNegative = factory.createCondition(index, RelationType.GE, factory.getInt(0)); if (!memState.applyCondition(indexNonNegative)) { alwaysOutOfBounds = true; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java index 5644994dd80d..a4dd3ca9ffab 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java @@ -22,7 +22,6 @@ import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiMethodCallExpression; -import com.intellij.psi.PsiType; import com.intellij.psi.PsiVariable; import com.siyeh.ig.callMatcher.CallMapper; import org.jetbrains.annotations.NotNull; @@ -64,7 +63,7 @@ public class CollectionFactoryInliner implements CallInliner { .push(factory.createTypeValue(call.getType(), Nullness.NOT_NULL)) .assign() // leave tmpVar on stack: it's result of method call .push(factoryInfo.mySizeField.createValue(factory, variableValue)) // tmpVar.size = - .push(factory.getConstFactory().createFromValue(factoryInfo.mySize, PsiType.INT, null)) + .push(factory.getInt(factoryInfo.mySize)) .assign() .pop(); return true; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java index 80fd027d5100..dc2077d2e452 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/StreamChainInliner.java @@ -552,7 +552,7 @@ public class StreamChainInliner implements CallInliner { .checkNotNull(qualifierExpression, NullabilityProblem.passingNullableToNotNullParameter) .pop() .push(SpecialField.ARRAY_LENGTH.createValue(builder.getFactory(), qualifierValue)) - .push(builder.getFactory().getConstFactory().createFromValue(0, PsiType.INT, null)) + .push(builder.getFactory().getInt(0)) .ifCondition(JavaTokenType.GT) .chain(b -> makeMainLoop(b, firstStep, inType)) .endIf(); @@ -568,7 +568,7 @@ public class StreamChainInliner implements CallInliner { .checkNotNull(sourceCall, NullabilityProblem.callNPE) .pop() .push(SpecialField.COLLECTION_SIZE.createValue(builder.getFactory(), qualifierValue)) - .push(builder.getFactory().getConstFactory().createFromValue(0, PsiType.INT, null)) + .push(builder.getFactory().getInt(0)) .ifCondition(JavaTokenType.GT) .chain(b -> makeMainLoop(b, firstStep, inType)) .endIf(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java index 216cf1152c57..dd78813a1f92 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java @@ -107,6 +107,11 @@ public class DfaValueFactory { return myExpressionFactory.getExpressionDfaValue(psiExpression); } + @NotNull + public DfaConstValue getInt(int value) { + return getConstFactory().createFromValue(value, PsiType.INT, null); + } + @Nullable public DfaValue createLiteralValue(PsiLiteralExpression literal) { return getConstFactory().create(literal); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java index 343e439b4d40..ca71f89edfa8 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java @@ -40,6 +40,7 @@ public class DfaVariableValue extends DfaValue { myFactory = factory; } + @NotNull public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated) { PsiType varType = myVariable.getType(); if (varType instanceof PsiEllipsisType) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ArrayInitializerLength.java b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayInitializerLength.java new file mode 100644 index 000000000000..6db67b580ea1 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayInitializerLength.java @@ -0,0 +1,55 @@ +import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.NotNull; + +class ArrayInitializerLength { + void testDeclaration() { + String[] arr = {"foo"}; + if(arr.length == 2) { + System.out.println("oops"); + } + } + + void testNewExpression() { + String[] arr = new String[]{"foo"}; + if(arr.length == 2) { + System.out.println("oops"); + } + } + + void testDimension() { + int[] arr = new int[3]; + if(arr.length == 1) { + System.out.println("oops"); + } + } + + void testIterate() { + int[] arr = new int[0]; + for (int i : arr) { + System.out.println("never"); + } + } + + void testConditional() { + int[] arr = Math.random() > 0.5 ? new int[2] : new int[4]; + if(arr.length == 3) { + System.out.println("never"); + } + if(arr.length == 2) { + System.out.println("possible"); + } + } + + void testMultiDimensional() { + int[][][] arr = new int[1][2][3]; + if(arr.length == 1) { + System.out.println("ok"); + } + if(arr.length == 2) { + System.out.println("not ok"); + } + if(arr.length == 3) { + System.out.println("not ok"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 092dce8af4ed..28c7e8a05545 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -541,4 +541,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testIteratePositiveCheck() { doTest(); } public void testInnerClass() { doTest(); } public void testCovariantReturn() { doTest(); } + public void testArrayInitializerLength() { doTest(); } }