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 36b633068b87..bd225a84bb91 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 @@ -26,6 +26,7 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.*; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.FactoryMap; import com.intellij.util.containers.Stack; import com.siyeh.ig.numeric.UnnecessaryExplicitNumericCastInspection; import org.jetbrains.annotations.Contract; @@ -54,9 +55,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { private Stack myElementStack = new Stack(); /** - * A mock variable for try-related control transfers. Contains exceptions or an (Throwable-inconvertible) string to indicate return inside finally + * Variables for try-related control transfers. Contain exceptions or an (Throwable-inconvertible) string to indicate return inside finally */ - private DfaVariableValue myExceptionHolder; + private FactoryMap myExceptionHolders; ControlFlowAnalyzer(final DfaValueFactory valueFactory) { myFactory = valueFactory; @@ -64,7 +65,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { public ControlFlow buildControlFlow(@NotNull PsiElement codeFragment, boolean ignoreAssertions) { myIgnoreAssertions = ignoreAssertions; - PsiManager manager = codeFragment.getManager(); + final PsiManager manager = codeFragment.getManager(); GlobalSearchScope scope = codeFragment.getResolveScope(); myRuntimeException = myFactory.createTypeValue(createClassType(manager, scope, JAVA_LANG_RUNTIME_EXCEPTION), Nullness.NOT_NULL); myError = myFactory.createTypeValue(createClassType(manager, scope, JAVA_LANG_ERROR), Nullness.NOT_NULL); @@ -72,9 +73,16 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { myAssertionError = createClassType(manager, scope, JAVA_LANG_ASSERTION_ERROR); myString = myFactory.createTypeValue(createClassType(manager, scope, JAVA_LANG_STRING), Nullness.NOT_NULL); - PsiParameter mockVar = JavaPsiFacade.getElementFactory(manager.getProject()).createParameterFromText("java.lang.Object $exception$", null); - myExceptionHolder = myFactory.getVarFactory().createVariableValue(mockVar, false); - + myExceptionHolders = new FactoryMap() { + @Nullable + @Override + protected DfaVariableValue create(PsiTryStatement key) { + String text = "java.lang.Object $exception" + myExceptionHolders.size() + "$"; + PsiParameter mockVar = JavaPsiFacade.getElementFactory(manager.getProject()).createParameterFromText(text, null); + return myFactory.getVarFactory().createVariableValue(mockVar, false); + } + }; + myCatchStack = new Stack(); myCurrentFlow = new ControlFlow(myFactory); @@ -232,9 +240,10 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (description != null) { description.accept(this); } - - initException(myAssertionError); - addThrowCode(false, statement); + + CatchDescriptor cd = findNextCatch(false); + initException(myAssertionError, cd); + addThrowCode(cd, statement); } finishElement(statement); } @@ -569,14 +578,14 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } private void returnCheckingFinally(boolean viaException, @NotNull PsiElement anchor) { - ControlFlow.ControlFlowOffset finallyOffset = getFinallyOffset(); - if (finallyOffset != null) { - addInstruction(new PushInstruction(myExceptionHolder, null)); + CatchDescriptor finallyDescriptor = findFinally(); + if (finallyDescriptor != null) { + addInstruction(new PushInstruction(getExceptionHolder(finallyDescriptor), null)); addInstruction(new PushInstruction(myString, null)); addInstruction(new AssignInstruction(null)); addInstruction(new PopInstruction()); - addInstruction(new GotoInstruction(finallyOffset)); + addInstruction(new GotoInstruction(finallyDescriptor.getJumpOffset(this))); } else { addInstruction(new ReturnInstruction(viaException, anchor)); } @@ -694,7 +703,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (exception != null) { exception.accept(this); - if (myCatchStack.isEmpty()) { + CatchDescriptor cd = findNextCatch(false); + if (cd == null) { addInstruction(new ReturnInstruction(true, statement)); finishElement(statement); return; @@ -708,22 +718,23 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(gotoInstruction); addInstruction(new PopInstruction()); - initException(myNpe); - addThrowCode(false, statement); + initException(myNpe, cd); + addThrowCode(cd, statement); gotoInstruction.setOffset(myCurrentFlow.getInstructionCount()); - addInstruction(new PushInstruction(myExceptionHolder, null)); + addInstruction(new PushInstruction(getExceptionHolder(cd), null)); addInstruction(new SwapInstruction()); addInstruction(new AssignInstruction(null)); addInstruction(new PopInstruction()); - addThrowCode(false, statement); + addThrowCode(cd, statement); } finishElement(statement); } private void addConditionalRuntimeThrow() { - if (myCatchStack.isEmpty()) { + CatchDescriptor cd = findNextCatch(false); + if (cd == null) { return; } @@ -731,7 +742,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { final ConditionalGotoInstruction ifNoException = addInstruction(new ConditionalGotoInstruction(null, false, null)); addInstruction(new EmptyStackInstruction()); - addInstruction(new PushInstruction(myExceptionHolder, null)); + addInstruction(new PushInstruction(getExceptionHolder(cd), null)); pushUnknown(); final ConditionalGotoInstruction ifError = addInstruction(new ConditionalGotoInstruction(null, false, null)); @@ -744,7 +755,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new AssignInstruction(null)); addInstruction(new PopInstruction()); - addThrowCode(false, null); + addThrowCode(cd, null); ifNoException.setOffset(myCurrentFlow.getInstructionCount()); } @@ -762,14 +773,24 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } // the exception object should be in $exception$ variable - private void addThrowCode(boolean catchRethrow, @Nullable PsiElement explicitThrower) { - if (myCatchStack.isEmpty()) { + private void addThrowCode(@Nullable CatchDescriptor cd, @Nullable PsiElement explicitThrower) { + if (cd == null) { addInstruction(new ReturnInstruction(true, explicitThrower)); return; } + flushVariablesOnControlTransfer(cd.getBlock()); + addInstruction(new GotoInstruction(cd.getJumpOffset(this))); + } + + @Nullable + private CatchDescriptor findNextCatch(boolean catchRethrow) { + if (myCatchStack.isEmpty()) { + return null; + } + PsiElement currentElement = myElementStack.peek(); - + CatchDescriptor cd = myCatchStack.get(myCatchStack.size() - 1); if (!cd.isFinally() && PsiTreeUtil.isAncestor(cd.getBlock().getParent(), currentElement, false)) { int i = myCatchStack.size() - 2; @@ -777,21 +798,20 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { i--; } if (i < 0) { - addInstruction(new ReturnInstruction(true, explicitThrower)); - return; + return null; } cd = myCatchStack.get(i); } - flushVariablesOnControlTransfer(cd.getBlock()); - addInstruction(new GotoInstruction(cd.getJumpOffset(this))); + return cd; } + @Nullable - private ControlFlow.ControlFlowOffset getFinallyOffset() { + private CatchDescriptor findFinally() { for (int i = myCatchStack.size() - 1; i >= 0; i--) { CatchDescriptor cd = myCatchStack.get(i); - if (cd.isFinally()) return cd.getJumpOffset(this); + if (cd.isFinally()) return cd; } return null; @@ -909,17 +929,17 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } if (finallyBlock != null) { - myCatchStack.pop(); + CatchDescriptor finallyDescriptor = myCatchStack.pop(); finallyBlock.accept(this); //if $exception$==null => continue normal execution - addInstruction(new PushInstruction(myExceptionHolder, null)); + addInstruction(new PushInstruction(getExceptionHolder(finallyDescriptor), null)); addInstruction(new PushInstruction(myFactory.getConstFactory().getNull(), null)); addInstruction(new BinopInstruction(JavaTokenType.EQEQ, null, statement.getProject())); addInstruction(new ConditionalGotoInstruction(getEndOffset(statement), false, null)); // else throw $exception$ - addThrowCode(false, null); + rethrowException(finallyDescriptor, false); } finishElement(statement); @@ -930,35 +950,49 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { startElement(section); PsiCodeBlock catchBlock = section.getCatchBlock(); if (catchBlock != null) { - // exception is in myExceptionHolder mock variable + CatchDescriptor currentDescriptor = new CatchDescriptor(section.getParameter(), catchBlock); + DfaVariableValue exceptionHolder = getExceptionHolder(currentDescriptor); + + // exception is in exceptionHolder mock variable // check if it's assignable to catch parameter type PsiType declaredType = section.getCatchType(); - List flattened = declaredType instanceof PsiDisjunctionType ? - ((PsiDisjunctionType)declaredType).getDisjunctions() : + List flattened = declaredType instanceof PsiDisjunctionType ? + ((PsiDisjunctionType)declaredType).getDisjunctions() : ContainerUtil.createMaybeSingletonList(declaredType); for (PsiType catchType : flattened) { - addInstruction(new PushInstruction(myExceptionHolder, null)); + addInstruction(new PushInstruction(exceptionHolder, null)); addInstruction(new PushInstruction(myFactory.createTypeValue(catchType, Nullness.UNKNOWN), null)); addInstruction(new BinopInstruction(JavaTokenType.INSTANCEOF_KEYWORD, null, section.getProject())); addInstruction(new ConditionalGotoInstruction(ControlFlow.deltaOffset(getStartOffset(catchBlock), -5), false, null)); } // not assignable => rethrow - addThrowCode(true, null); + rethrowException(currentDescriptor, true); // e = $exception$ addInstruction(new PushInstruction(myFactory.getVarFactory().createVariableValue(section.getParameter(), false), null)); - addInstruction(new PushInstruction(myExceptionHolder, null)); + addInstruction(new PushInstruction(exceptionHolder, null)); addInstruction(new AssignInstruction(null)); addInstruction(new PopInstruction()); - addInstruction(new FlushVariableInstruction(myExceptionHolder)); + addInstruction(new FlushVariableInstruction(exceptionHolder)); catchBlock.accept(this); } finishElement(section); } + private void rethrowException(CatchDescriptor currentDescriptor, boolean catchRethrow) { + CatchDescriptor nextCatch = findNextCatch(catchRethrow); + if (nextCatch != null) { + addInstruction(new PushInstruction(getExceptionHolder(nextCatch), null, false)); + addInstruction(new PushInstruction(getExceptionHolder(currentDescriptor), null, true)); + addInstruction(new AssignInstruction(null)); + addInstruction(new PopInstruction()); + } + addThrowCode(nextCatch, null); + } + @Override public void visitResourceList(PsiResourceList resourceList) { for (PsiResourceVariable variable : resourceList.getResourceVariables()) { @@ -1335,6 +1369,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } private void addMethodThrows(PsiMethod method, @Nullable PsiElement explicitCall) { + CatchDescriptor cd = findNextCatch(false); if (method != null) { PsiClassType[] refs = method.getThrowsList().getReferencedTypes(); for (PsiClassType ref : refs) { @@ -1342,20 +1377,25 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { ConditionalGotoInstruction cond = new ConditionalGotoInstruction(null, false, null); addInstruction(cond); addInstruction(new EmptyStackInstruction()); - initException(ref); - addThrowCode(false, explicitCall); + initException(ref, cd); + addThrowCode(cd, explicitCall); cond.setOffset(myCurrentFlow.getInstructionCount()); } } } - private void initException(PsiType ref) { - addInstruction(new PushInstruction(myExceptionHolder, null)); + private void initException(PsiType ref, @Nullable CatchDescriptor cd) { + if (cd == null) return; + addInstruction(new PushInstruction(getExceptionHolder(cd), null)); addInstruction(new PushInstruction(myFactory.createTypeValue(ref, Nullness.NOT_NULL), null)); addInstruction(new AssignInstruction(null)); addInstruction(new PopInstruction()); } + private DfaVariableValue getExceptionHolder(CatchDescriptor cd) { + return myExceptionHolders.get(cd.getTryStatement()); + } + @Override public void visitMethodCallExpression(PsiMethodCallExpression expression) { startElement(expression); diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java b/java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java new file mode 100644 index 000000000000..b87492380f6d --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/TryFinallyInsideFinally.java @@ -0,0 +1,23 @@ +class Test { + private String foo; + + public void test() { + boolean rangeMarkersDisposed = false; + try { + if (foo == "dd") { + throw new RuntimeException(); + } + rangeMarkersDisposed = true; + } + finally { + try { + } + finally { + } + + if (!rangeMarkersDisposed) { + foo = "dd"; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index 405478625937..754a6dcd31f3 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -67,6 +67,7 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase { public void testExceptionFromFinally() throws Throwable { doTest(); } public void testExceptionFromFinallyNesting() throws Throwable { doTest(); } public void testNestedFinally() { doTest(); } + public void testTryFinallyInsideFinally() { doTest(); } public void testFieldChangedBetweenSynchronizedBlocks() throws Throwable { doTest(); } public void testGeneratedEquals() throws Throwable { doTest(); }