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 a8cf28d2fb77..4e8580390677 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 @@ -19,10 +19,18 @@ import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.ExceptionUtil; import com.intellij.codeInsight.daemon.ImplicitUsageProvider; import com.intellij.codeInsight.daemon.impl.UnusedSymbolUtil; +import com.intellij.codeInspection.dataFlow.ControlFlow.ControlFlowOffset; +import com.intellij.codeInspection.dataFlow.MethodContract.ValueConstraint; +import com.intellij.codeInspection.dataFlow.Trap.InsideFinally; +import com.intellij.codeInspection.dataFlow.Trap.TryCatch; +import com.intellij.codeInspection.dataFlow.Trap.TryFinally; +import com.intellij.codeInspection.dataFlow.Trap.TwrFinally; import com.intellij.codeInspection.dataFlow.inliner.*; import com.intellij.codeInspection.dataFlow.instructions.*; +import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction.MethodType; 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.extensions.Extensions; import com.intellij.openapi.project.Project; @@ -30,12 +38,14 @@ import com.intellij.openapi.util.registry.Registry; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.CachedValueProvider.Result; import com.intellij.psi.util.*; import com.intellij.psi.util.InheritanceUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.ObjectUtils; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.FList; +import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.numeric.UnnecessaryExplicitNumericCastInspection; import com.siyeh.ig.psiutils.*; import one.util.streamex.StreamEx; @@ -51,6 +61,9 @@ import static com.intellij.psi.CommonClassNames.*; public class ControlFlowAnalyzer extends JavaElementVisitor { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer"); public static final String ORG_JETBRAINS_ANNOTATIONS_CONTRACT = Contract.class.getName(); + private static final CallMatcher LIST_INITIALIZER = CallMatcher.anyOf( + CallMatcher.staticCall(JAVA_UTIL_ARRAYS, "asList"), + CallMatcher.staticCall(JAVA_UTIL_LIST, "of")); static final int MAX_UNROLL_SIZE = 3; private final PsiElement myCodeFragment; private final boolean myIgnoreAssertions; @@ -164,11 +177,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { return myCurrentFlow.getInstructionCount(); } - private ControlFlow.ControlFlowOffset getEndOffset(PsiElement element) { + private ControlFlowOffset getEndOffset(PsiElement element) { return myCurrentFlow.getEndOffset(element); } - private ControlFlow.ControlFlowOffset getStartOffset(PsiElement element) { + private ControlFlowOffset getStartOffset(PsiElement element) { return myCurrentFlow.getStartOffset(element); } @@ -300,10 +313,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { else if (!field.hasModifierProperty(PsiModifier.FINAL) && !UnusedSymbolUtil.isImplicitWrite(field)) { // initialize with default value DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(field); - addInstruction(new PushInstruction(dfaVariable, null, true)); - addInstruction(new PushInstruction(myFactory.getConstFactory().createDefault(field.getType()), null)); - addInstruction(new AssignInstruction(null, dfaVariable)); - addInstruction(new PopInstruction()); + DfaConstValue value = myFactory.getConstFactory().createDefault(field.getType()); + new CFGBuilder(this).assignAndPop(dfaVariable, value); } } @@ -476,12 +487,32 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(statement); } + private DfaValue getIteratedElement(PsiExpression iteratedValue) { + PsiExpression[] expressions = null; + if (iteratedValue instanceof PsiNewExpression) { + PsiArrayInitializerExpression initializer = ((PsiNewExpression)iteratedValue).getArrayInitializer(); + if (initializer != null) { + expressions = initializer.getInitializers(); + } + } + else if (iteratedValue instanceof PsiReferenceExpression) { + PsiElement arrayVar = ((PsiReferenceExpression)iteratedValue).resolve(); + if (arrayVar instanceof PsiVariable) { + expressions = ExpressionUtils.getConstantArrayElements((PsiVariable)arrayVar); + } + } + if (iteratedValue instanceof PsiMethodCallExpression && LIST_INITIALIZER.test((PsiMethodCallExpression)iteratedValue)) { + expressions = ((PsiMethodCallExpression)iteratedValue).getArgumentList().getExpressions(); + } + return expressions == null ? DfaUnknownValue.getInstance() : getFactory().createCommonValue(expressions); + } + @Override public void visitForeachStatement(PsiForeachStatement statement) { startElement(statement); final PsiParameter parameter = statement.getIterationParameter(); - final PsiExpression iteratedValue = statement.getIteratedValue(); + final PsiExpression iteratedValue = PsiUtil.skipParenthesizedExprDown(statement.getIteratedValue()); - ControlFlow.ControlFlowOffset loopEndOffset = getEndOffset(statement); + ControlFlowOffset loopEndOffset = getEndOffset(statement); boolean hasSizeCheck = false; if (iteratedValue != null) { @@ -508,9 +539,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } } - ControlFlow.ControlFlowOffset offset = myCurrentFlow.getNextOffset(); + ControlFlowOffset offset = myCurrentFlow.getNextOffset(); DfaVariableValue dfaVariable = myFactory.getVarFactory().createVariableValue(parameter); - addInstruction(new FlushVariableInstruction(dfaVariable)); + new CFGBuilder(this).assignAndPop(dfaVariable, getIteratedElement(iteratedValue)); if (!hasSizeCheck) { pushUnknown(); @@ -580,9 +611,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } } - ControlFlow.ControlFlowOffset offset = initialization != null - ? getEndOffset(initialization) - : getStartOffset(statement); + ControlFlowOffset offset = initialization != null ? getEndOffset(initialization) : getStartOffset(statement); addInstruction(new GotoInstruction(offset)); finishElement(statement); @@ -640,9 +669,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (origin == null) return false; long diff = start == null || end == null ? -1 : end - start; DfaVariableValue loopVar = myFactory.getVarFactory().createVariableValue(counter); - addInstruction(new PushInstruction(loopVar, null, true)); if(diff >= 0 && diff <= MAX_UNROLL_SIZE) { // Unroll small loops + addInstruction(new PushInstruction(loopVar, null, true)); addInstruction(new PushInstruction(loopVar, null)); addInstruction(new PushInstruction(myFactory.getConstFactory().createFromValue(1, PsiType.INT, null), null)); addInstruction(new BinopInstruction(JavaTokenType.PLUS, null, loopVar.getVariableType())); @@ -661,15 +690,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new GotoInstruction(getEndOffset(statement))); } else { - addInstruction(new PushInstruction(myFactory.getFactValue(DfaFactType.RANGE, LongRangeSet.range(start + 1L, maxValue)), null)); - addInstruction(new AssignInstruction(null, null)); - addInstruction(new PopInstruction()); + DfaValue range = myFactory.getFactValue(DfaFactType.RANGE, LongRangeSet.range(start + 1L, maxValue)); + new CFGBuilder(this).assignAndPop(loopVar, range); } } else { - pushUnknown(); - addInstruction(new AssignInstruction(null, null)); - addInstruction(new PushInstruction(origin, null)); - addInstruction(new BinopInstruction(JavaTokenType.LE, null, PsiType.BOOLEAN)); + new CFGBuilder(this).assign(loopVar, DfaUnknownValue.getInstance()) + .push(origin) + .compare(JavaTokenType.LE); addInstruction(new ConditionalGotoInstruction(getEndOffset(statement), false, null)); } return true; @@ -683,9 +710,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { PsiStatement thenStatement = statement.getThenBranch(); PsiStatement elseStatement = statement.getElseBranch(); - ControlFlow.ControlFlowOffset offset = elseStatement != null - ? getStartOffset(elseStatement) - : getEndOffset(statement); + ControlFlowOffset offset = elseStatement != null ? getStartOffset(elseStatement) : getEndOffset(statement); if (condition != null) { condition.accept(this); @@ -859,7 +884,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } else { try { - ControlFlow.ControlFlowOffset offset = getStartOffset(statement); + ControlFlowOffset offset = getStartOffset(statement); PsiExpression caseValue = psiLabelStatement.getCaseValue(); if (enumValues != null && caseValue instanceof PsiReferenceExpression) { @@ -894,7 +919,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } if (enumValues == null || !enumValues.isEmpty()) { - ControlFlow.ControlFlowOffset offset = defaultLabel != null ? getStartOffset(defaultLabel) : getEndOffset(body); + ControlFlowOffset offset = defaultLabel != null ? getStartOffset(defaultLabel) : getEndOffset(body); addInstruction(new GotoInstruction(offset)); } @@ -978,21 +1003,21 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { PsiCodeBlock tryBlock = statement.getTryBlock(); PsiCodeBlock finallyBlock = statement.getFinallyBlock(); - Trap.TryFinally finallyDescriptor = finallyBlock != null ? new Trap.TryFinally(finallyBlock, getStartOffset(finallyBlock)) : null; + TryFinally finallyDescriptor = finallyBlock != null ? new TryFinally(finallyBlock, getStartOffset(finallyBlock)) : null; if (finallyDescriptor != null) { myTrapStack = myTrapStack.prepend(finallyDescriptor); } PsiCatchSection[] sections = statement.getCatchSections(); if (sections.length > 0) { - LinkedHashMap clauses = new LinkedHashMap<>(); + LinkedHashMap clauses = new LinkedHashMap<>(); for (PsiCatchSection section : sections) { PsiCodeBlock catchBlock = section.getCatchBlock(); if (catchBlock != null) { clauses.put(section, getStartOffset(catchBlock)); } } - myTrapStack = myTrapStack.prepend(new Trap.TryCatch(statement, clauses)); + myTrapStack = myTrapStack.prepend(new TryCatch(statement, clauses)); } processTryWithResources(resourceList, tryBlock); @@ -1002,7 +1027,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { controlTransfer(gotoEnd, singleFinally); if (sections.length > 0) { - assert myTrapStack.getHead() instanceof Trap.TryCatch; + assert myTrapStack.getHead() instanceof TryCatch; myTrapStack = myTrapStack.getTail(); } @@ -1015,13 +1040,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } if (finallyBlock != null) { - assert myTrapStack.getHead() instanceof Trap.TryFinally; - myTrapStack = myTrapStack.getTail().prepend(new Trap.InsideFinally(finallyBlock)); + assert myTrapStack.getHead() instanceof TryFinally; + myTrapStack = myTrapStack.getTail().prepend(new InsideFinally(finallyBlock)); finallyBlock.accept(this); addInstruction(new ControlTransferInstruction(null)); // DfaControlTransferValue is on stack - assert myTrapStack.getHead() instanceof Trap.InsideFinally; + assert myTrapStack.getHead() instanceof InsideFinally; myTrapStack = myTrapStack.getTail(); } @@ -1030,13 +1055,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { private void processTryWithResources(@Nullable PsiResourceList resourceList, @Nullable PsiCodeBlock tryBlock) { Set closerExceptions = Collections.emptySet(); - Trap.TwrFinally twrFinallyDescriptor = null; + TwrFinally twrFinallyDescriptor = null; if (resourceList != null) { resourceList.accept(this); closerExceptions = StreamEx.of(resourceList.iterator()).flatCollection(ExceptionUtil::getCloserExceptions).toSet(); if (!closerExceptions.isEmpty()) { - twrFinallyDescriptor = new Trap.TwrFinally(resourceList, getStartOffset(resourceList)); + twrFinallyDescriptor = new TwrFinally(resourceList, getStartOffset(resourceList)); myTrapStack = myTrapStack.prepend(twrFinallyDescriptor); } } @@ -1046,15 +1071,15 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } if (twrFinallyDescriptor != null) { - assert myTrapStack.getHead() instanceof Trap.TwrFinally; + assert myTrapStack.getHead() instanceof TwrFinally; InstructionTransfer gotoEnd = new InstructionTransfer(getEndOffset(resourceList), getVariablesInside(tryBlock)); controlTransfer(gotoEnd, FList.createFromReversed(ContainerUtil.createMaybeSingletonList(twrFinallyDescriptor))); - myTrapStack = myTrapStack.getTail().prepend(new Trap.InsideFinally(resourceList)); + myTrapStack = myTrapStack.getTail().prepend(new InsideFinally(resourceList)); startElement(resourceList); addThrows(null, closerExceptions.toArray(PsiClassType.EMPTY_ARRAY)); addInstruction(new ControlTransferInstruction(null)); // DfaControlTransferValue is on stack finishElement(resourceList); - assert myTrapStack.getHead() instanceof Trap.InsideFinally; + assert myTrapStack.getHead() instanceof InsideFinally; myTrapStack = myTrapStack.getTail(); } } @@ -1232,10 +1257,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new AssignInstruction(originalExpression, var)); } // Declaration: write array length - 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()); + DfaConstValue lengthValue = getFactory().getInt(expression.getInitializers().length); + new CFGBuilder(this).assignAndPop(SpecialField.ARRAY_LENGTH.createValue(getFactory(), var), lengthValue); } @Override @@ -1346,7 +1369,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { parent instanceof PsiArrayAccessExpression) { return true; } - if (parent instanceof PsiBinaryExpression && DfaRelationValue.RelationType.fromElementType(((PsiBinaryExpression)parent).getOperationTokenType()) != null) { + if (parent instanceof PsiBinaryExpression && RelationType.fromElementType(((PsiBinaryExpression)parent).getOperationTokenType()) != null) { return true; } if (parent instanceof PsiLoopStatement) return false; @@ -1389,18 +1412,18 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (PsiType.VOID.equals(expectedType)) return; if (TypeConversionUtil.isPrimitiveAndNotNull(expectedType) && TypeConversionUtil.isPrimitiveWrapper(actualType)) { - addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.UNBOXING, expectedType)); + addInstruction(new MethodCallInstruction(context, MethodType.UNBOXING, expectedType)); } else if (TypeConversionUtil.isPrimitiveAndNotNull(actualType) && TypeConversionUtil.isAssignableFromPrimitiveWrapper(expectedType)) { addConditionalRuntimeThrow(); - addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.BOXING, expectedType)); + addInstruction(new MethodCallInstruction(context, MethodType.BOXING, expectedType)); } else if (actualType != expectedType && TypeConversionUtil.isPrimitiveAndNotNull(actualType) && TypeConversionUtil.isPrimitiveAndNotNull(expectedType) && TypeConversionUtil.isNumericType(actualType) && TypeConversionUtil.isNumericType(expectedType)) { - addInstruction(new MethodCallInstruction(context, MethodCallInstruction.MethodType.CAST, expectedType) { + addInstruction(new MethodCallInstruction(context, MethodType.CAST, expectedType) { @Override public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) { return visitor.visitCast(this, runner, stateBefore); @@ -1525,7 +1548,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { PsiExpression thenExpression = expression.getThenExpression(); PsiExpression elseExpression = expression.getElseExpression(); - final ControlFlow.ControlFlowOffset elseOffset = elseExpression == null ? ControlFlow.deltaOffset(getEndOffset(expression), -1) : getStartOffset(elseExpression); + final ControlFlowOffset elseOffset = elseExpression == null ? ControlFlow.deltaOffset(getEndOffset(expression), -1) : getStartOffset(elseExpression); if (thenExpression != null) { condition.accept(this); generateBoxingUnboxingInstructionFor(condition, PsiType.BOOLEAN); @@ -1663,7 +1686,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { anchor = expression; } addInstruction(instruction); - if (contracts.stream().anyMatch(c -> c.getReturnValue() == MethodContract.ValueConstraint.THROW_EXCEPTION)) { + if (contracts.stream().anyMatch(c -> c.getReturnValue() == ValueConstraint.THROW_EXCEPTION)) { // if a contract resulted in 'fail', handle it addInstruction(new DupInstruction()); addInstruction(new PushInstruction(myFactory.getConstFactory().getContractFail(), null)); @@ -1698,14 +1721,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { final int paramCount = method.getParameterList().getParametersCount(); List applicable = ContainerUtil.filter(StandardMethodContract.parseContract(text), contract -> contract.arguments.length == paramCount); - return CachedValueProvider.Result.create(applicable, contractAnno, method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT); + return Result.create(applicable, contractAnno, method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT); } catch (Exception ignored) { } } } - return CachedValueProvider.Result - .create(Collections.emptyList(), method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT); + return Result.create(Collections.emptyList(), method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT); }); } @@ -1804,17 +1826,13 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { private void setEmptyCollectionSize(PsiNewExpression expression) { DfaVariableValue var = getTargetVariable(expression); if (var != null && ConstructionUtils.isEmptyCollectionInitializer(expression)) { - addInstruction(new PopInstruction()); - addInstruction(new PushInstruction(var, null, true)); - addInstruction(new PushInstruction(myFactory.withFact( - myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), DfaFactType.LOCALITY, true), null)); - addInstruction(new AssignInstruction(null, null)); + DfaValue collectionValue = + myFactory.withFact(myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), DfaFactType.LOCALITY, true); SpecialField sizeField = InheritanceUtil.isInheritor(expression.getType(), JAVA_UTIL_MAP) ? SpecialField.MAP_SIZE : SpecialField.COLLECTION_SIZE; - addInstruction(new PushInstruction(sizeField.createValue(myFactory, var), null, true)); - addInstruction(new PushInstruction(myFactory.getInt(0), null)); - addInstruction(new AssignInstruction(null, null)); - addInstruction(new PopInstruction()); + new CFGBuilder(this).pop() + .assign(var, collectionValue) + .assignAndPop(sizeField.createValue(myFactory, var), myFactory.getInt(0)); } } @@ -1933,8 +1951,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { final PsiExpression qualifierExpression = expression.getQualifierExpression(); if (qualifierExpression != null) { - qualifierExpression.accept(this); - addInstruction(expression.resolve() instanceof PsiField ? new DereferenceInstruction(qualifierExpression) : new PopInstruction()); + PsiElement target = expression.resolve(); + if (!(target instanceof PsiMember) || !((PsiMember)target).hasModifierProperty(PsiModifier.STATIC)) { + qualifierExpression.accept(this); + addInstruction(target instanceof PsiField ? new DereferenceInstruction(qualifierExpression) : new PopInstruction()); + } } // complex assignments (e.g. "|=") are both reading and writing diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ForeachCollectionElement.java b/java/java-tests/testData/inspection/dataFlow/fixture/ForeachCollectionElement.java new file mode 100644 index 000000000000..cbbca3596151 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ForeachCollectionElement.java @@ -0,0 +1,29 @@ +import java.util.*; +import org.jetbrains.annotations.*; + +class ForeachCollectionElement { + void test() { + int[] arr = new int [] {10,20,30,40,50,60,70,80}; + for(int i : arr) { + if(i == 75) { + System.out.println("Impossible"); + } + } + } + + void test2() { + for(int i : new int [] {10,20,30,40,50,60,70,80}) { + if(i > 71 && i < 79) { + System.out.println("Impossible"); + } + } + } + + void test3() { + for(String s : Arrays.asList("foo", "bar", "baz")) { + if(s == null) { + System.out.println("impossible"); + } + } + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java index 3ea03492c309..1549d4b24c31 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -226,4 +226,5 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { public void testEscapeAnalysis() { doTest(); } public void testThisAsVariable() { doTest(); } public void testQueuePeek() { doTest(); } + public void testForeachCollectionElement() { doTest(); } } \ No newline at end of file