diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java index 2ab6f5ce4710..1d10b6b56f13 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/ControlFlowBuilder.java @@ -20,6 +20,7 @@ import com.intellij.openapi.util.Pair; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ArrayUtil; import com.intellij.util.containers.hash.HashSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -28,6 +29,7 @@ import org.jetbrains.plugins.groovy.lang.psi.GroovyFileBase; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.GroovyRecursiveElementVisitor; import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.GrCondition; +import org.jetbrains.plugins.groovy.lang.psi.api.formatter.GrControlStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.*; import org.jetbrains.plugins.groovy.lang.psi.api.statements.arguments.GrArgumentList; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock; @@ -138,14 +140,45 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { } } + + private static boolean isCertainlyReturnStatement(GrStatement st) { + final PsiElement parent = st.getParent(); + if (parent instanceof GrOpenBlock) { + if (st != ArrayUtil.getLastElement(((GrOpenBlock)parent).getStatements())) return false; + + PsiElement pparent = parent.getParent(); + if (pparent instanceof GrMethod) { + return true; + } + //todo switch + if (pparent instanceof GrBlockStatement || pparent instanceof GrCatchClause || pparent instanceof GrLabeledStatement) pparent = pparent.getParent(); + if (pparent instanceof GrIfStatement || pparent instanceof GrControlStatement || pparent instanceof GrTryCatchStatement) { + return isCertainlyReturnStatement((GrStatement)pparent); + } + } + + else if (parent instanceof GrClosableBlock) { + return st == ArrayUtil.getLastElement(((GrClosableBlock)parent).getStatements()); + } + + else if (parent instanceof GroovyFileBase) { + return st == ArrayUtil.getLastElement(((GroovyFileBase)parent).getStatements()); + } + else if (parent instanceof GrIfStatement || parent instanceof GrControlStatement) return isCertainlyReturnStatement((GrStatement)parent); + + return false; + } + private void handlePossibleReturn(@NotNull GrStatement last) { + if (!isCertainlyReturnStatement(last)) return; + //last statement inside finally clause cannot be possible return statement final GrFinallyClause finallyClause = PsiTreeUtil.getParentOfType(last, GrFinallyClause.class, false, GrClosableBlock.class, GrMember.class); if (finallyClause != null) return; if (!(last instanceof GrExpression && PsiTreeUtil.isAncestor(myLastInScope, last, false))) return; - addNodeAndCheckPending(new MaybeReturnInstruction((GrExpression)last)); + InstructionImpl head = addNodeAndCheckPending(new MaybeReturnInstruction((GrExpression)last)); for (ListIterator> iterator = myPending.listIterator(myPending.size());iterator.hasPrevious(); ) { Pair pair = iterator.previous(); @@ -153,10 +186,13 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { if (scopeWhenToAdd == null) continue; if (!PsiTreeUtil.isAncestor(scopeWhenToAdd, last, false)) break; + interruptFlow(); MaybeReturnInstruction may = addNode(new MaybeReturnInstruction((GrExpression)last)); addEdge(pair.first, may); iterator.set(new Pair(may, scopeWhenToAdd)); } + + myHead = head; } public Instruction[] buildControlFlow(GroovyPsiElement scope) { @@ -652,11 +688,14 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { if (thenBranch != null || elseBranch != null) { final InstructionImpl end = new IfEndInstruction(ifStatement); addNode(end); - if (thenEnd != null) addEdge(thenEnd, end); + if (thenEnd != null) { + addEdge(thenEnd, end); + } + if (elseEnd != null) { addEdge(elseEnd, end); } - else { + else if (elseBranch == null) { addEdge(conditionEnd != null ? conditionEnd : ifInstruction, end); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/IfEndInstruction.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/IfEndInstruction.java index 1458e2fb2384..40e7e2b71f93 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/IfEndInstruction.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/IfEndInstruction.java @@ -29,4 +29,9 @@ public class IfEndInstruction extends InstructionImpl{ public GrIfStatement getElement() { return (GrIfStatement)super.getElement(); } + + @Override + protected String getElementPresentation() { + return "End element: " + myPsiElement; + } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/InstructionImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/InstructionImpl.java index a979fe46c19d..f464607d90e3 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/InstructionImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/controlFlow/impl/InstructionImpl.java @@ -87,6 +87,7 @@ public class InstructionImpl implements Instruction { } protected String getElementPresentation() { + //return "element: " + (myPsiElement != null ? myPsiElement.getText() : null); return "element: " + myPsiElement; } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.groovy index 7ef21b7472f2..bc22222ad6da 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/MissingReturnTest.groovy @@ -1,43 +1,77 @@ -package org.jetbrains.plugins.groovy.lang; - - -import org.jetbrains.plugins.groovy.LightGroovyTestCase -import org.jetbrains.plugins.groovy.codeInspection.noReturnMethod.MissingReturnInspection -import org.jetbrains.plugins.groovy.util.TestUtils - -/** - * @author peter - */ -public class MissingReturnTest extends LightGroovyTestCase { - - @Override - protected String getBasePath() { - return "${TestUtils.testDataPath}highlighting/missingReturn"; - } - - public void testMissingReturnWithLastLoop() throws Throwable { doTest(); } - public void testMissingReturnWithUnknownCall() throws Throwable { doTest(); } - public void testMissingReturnWithIf() throws Throwable { doTest(); } - public void testMissingReturnWithAssertion() throws Throwable { doTest(); } - public void testMissingReturnThrowException() throws Throwable { doTest(); } - public void testMissingReturnTryCatch() throws Throwable { doTest(); } - public void testMissingReturnLastNull() throws Throwable { doTest(); } - public void testMissingReturnImplicitReturns() throws Throwable {doTest();} - public void testMissingReturnOvertReturnType() throws Throwable {doTest();} - public void testMissingReturnFromClosure() throws Throwable {doTest();} - public void testReturnsWithoutValue() throws Throwable {doTest();} - public void testEndlessLoop() throws Throwable {doTest();} - public void testEndlessLoop2() throws Throwable {doTest();} - public void testExceptionWithFinally() throws Throwable {doTest();} - public void testOnlyAssert() throws Throwable {doTest();} - public void testImplicitReturnNull() throws Throwable {doTest();} - public void testMissingReturnInClosure() {doTest();} - public void testFinally() {doTest();} - public void testClosureWithExplicitExpectedType() {doTest()} - - private void doTest() { - myFixture.enableInspections(new MissingReturnInspection()); - myFixture.testHighlighting(true, false, false, getTestName(false) + ".groovy"); - } - -} +package org.jetbrains.plugins.groovy.lang; + + +import org.jetbrains.plugins.groovy.LightGroovyTestCase +import org.jetbrains.plugins.groovy.codeInspection.noReturnMethod.MissingReturnInspection +import org.jetbrains.plugins.groovy.util.TestUtils + +/** + * @author peter + */ +public class MissingReturnTest extends LightGroovyTestCase { + + @Override + protected String getBasePath() { + return "${TestUtils.testDataPath}highlighting/missingReturn"; + } + + public void testMissingReturnWithLastLoop() throws Throwable { doTest(); } + public void testMissingReturnWithUnknownCall() throws Throwable { doTest(); } + public void testMissingReturnWithIf() throws Throwable { doTest(); } + public void testMissingReturnWithAssertion() throws Throwable { doTest(); } + public void testMissingReturnThrowException() throws Throwable { doTest(); } + public void testMissingReturnTryCatch() throws Throwable { doTest(); } + public void testMissingReturnLastNull() throws Throwable { doTest(); } + public void testMissingReturnImplicitReturns() throws Throwable {doTest();} + public void testMissingReturnOvertReturnType() throws Throwable {doTest();} + public void testMissingReturnFromClosure() throws Throwable {doTest();} + public void testReturnsWithoutValue() throws Throwable {doTest();} + public void testEndlessLoop() throws Throwable {doTest();} + public void testEndlessLoop2() throws Throwable {doTest();} + public void testExceptionWithFinally() throws Throwable {doTest();} + public void testOnlyAssert() throws Throwable {doTest();} + public void testImplicitReturnNull() throws Throwable {doTest();} + public void testMissingReturnInClosure() {doTest();} + public void testFinally() {doTest();} + public void testClosureWithExplicitExpectedType() {doTest()} + + + public void testInterruptFlowInElseBranch() { + doTextText('''\ +//correct +public int foo(int bar) { + if (bar < 0) { + return -1 + } + else if (bar > 0) { + return 12 + } + else { + throw new IllegalArgumentException('bar cannot be zero!') + } +} + +//incorrect +public int foo(int bar) { + if (bar < 0) { + return -1 + } + else if (bar > 0) { + return 12 + } +} + +''') + } + + void doTextText(String text) { + myFixture.configureByText('___.groovy', text) + myFixture.testHighlighting(true, false, false) + } + + private void doTest() { + myFixture.enableInspections(new MissingReturnInspection()); + myFixture.testHighlighting(true, false, false, getTestName(false) + ".groovy"); + } + +} diff --git a/plugins/groovy/testdata/groovy/controlFlow/grvy1497.test b/plugins/groovy/testdata/groovy/controlFlow/grvy1497.test index acc55ec03c94..86a68bc5318a 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/grvy1497.test +++ b/plugins/groovy/testdata/groovy/controlFlow/grvy1497.test @@ -8,7 +8,7 @@ println blah 2(3,5) element: IF statement 3(4) READ blah 4(5) WRITE blah -5(6) element: IF statement +5(6) End element: IF statement 6(7) READ blah 7(8) WRITE blah 8(9) READ println diff --git a/plugins/groovy/testdata/groovy/controlFlow/if1.test b/plugins/groovy/testdata/groovy/controlFlow/if1.test index 6432efeca933..fda74f477359 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/if1.test +++ b/plugins/groovy/testdata/groovy/controlFlow/if1.test @@ -10,5 +10,5 @@ if (true) { 3(6) element: Assignment expression MAYBE_RETURN 4(5) WRITE a 5(6) element: Assignment expression MAYBE_RETURN -6(7) element: IF statement +6(7) End element: IF statement 7() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/ifInstanceofElse.test b/plugins/groovy/testdata/groovy/controlFlow/ifInstanceofElse.test index 767599b66614..c9f1e87ce4f8 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/ifInstanceofElse.test +++ b/plugins/groovy/testdata/groovy/controlFlow/ifInstanceofElse.test @@ -23,6 +23,6 @@ else b = 3 18(21) element: Assignment expression MAYBE_RETURN 19(20) WRITE b 20(21) element: Assignment expression MAYBE_RETURN -21(22) element: IF statement -22(23) element: IF statement +21(22) End element: IF statement +22(23) End element: IF statement 23() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/nested.test b/plugins/groovy/testdata/groovy/controlFlow/nested.test index 83c08bb98285..fa781daa8be8 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/nested.test +++ b/plugins/groovy/testdata/groovy/controlFlow/nested.test @@ -15,7 +15,7 @@ for (e in [1,2,3,4]) { 6(7,3) element: For statement 7(8) element: Block statement 8(1,9) element: IF statement -9(10) element: IF statement +9(10) End element: IF statement 10(11) READ print 11(12) READ e 12(6) READ ee diff --git a/plugins/groovy/testdata/groovy/controlFlow/orInReturn.test b/plugins/groovy/testdata/groovy/controlFlow/orInReturn.test index e0b37c0de3cd..7670956c3498 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/orInReturn.test +++ b/plugins/groovy/testdata/groovy/controlFlow/orInReturn.test @@ -5,6 +5,6 @@ OperatingSystem.isWindows() || OperatingSystem.isMacOsX() 2(3) READ OperatingSystem 3(4,6) element: Logical expression 4(5) READ OperatingSystem -5(6) element: Logical expression MAYBE_RETURN +5(7) element: Logical expression MAYBE_RETURN 6(7) element: Logical expression MAYBE_RETURN 7() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/return.test b/plugins/groovy/testdata/groovy/controlFlow/return.test index 6809d545a74e..7a30a9f1afae 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/return.test +++ b/plugins/groovy/testdata/groovy/controlFlow/return.test @@ -1,10 +1,10 @@ if (true) return a else return b ----- 0(1) element: null -1(2,4,6) element: IF statement +1(2,4) element: IF statement 2(3) READ a 3(7) element: RETURN statement 4(5) READ b 5(7) element: RETURN statement -6(7) element: IF statement +6(7) End element: IF statement 7() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/try1.test b/plugins/groovy/testdata/groovy/controlFlow/try1.test index f284f4e9150e..e9a7d08ac973 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/try1.test +++ b/plugins/groovy/testdata/groovy/controlFlow/try1.test @@ -10,7 +10,7 @@ print e 2(3) element: IF statement 3(4,5) READ c 4(7) element: RETURN statement -5(9) element: IF statement +5(9) End element: IF statement 6(11) element: Finally clause 7(6,8) CALL 6 8(17) AFTER CALL 7 diff --git a/plugins/groovy/testdata/groovy/controlFlow/try10.test b/plugins/groovy/testdata/groovy/controlFlow/try10.test index 0123a89ab4d5..7922ab38b3ec 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/try10.test +++ b/plugins/groovy/testdata/groovy/controlFlow/try10.test @@ -15,7 +15,7 @@ cde 6(7) element: Block statement 7(8) element: IF statement 8(9,10) READ abc -9(5) element: IF statement +9(5) End element: IF statement 10(4) RETURN 11(12) READ cde 12(13) element: Reference expression MAYBE_RETURN diff --git a/plugins/groovy/testdata/groovy/controlFlow/try5.test b/plugins/groovy/testdata/groovy/controlFlow/try5.test index a6a14ee5a77b..12a64682a37e 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/try5.test +++ b/plugins/groovy/testdata/groovy/controlFlow/try5.test @@ -15,7 +15,7 @@ fScript.previewTask(taskName) 2(3,5) READ ddd 3(4) READ fXRec 4(16) element: RETURN statement -5(6) element: IF statement +5(6) End element: IF statement 6(7) WRITE fScript 7(9) element: Open block 8(11) element: Finally clause diff --git a/plugins/groovy/testdata/groovy/controlFlow/try6.test b/plugins/groovy/testdata/groovy/controlFlow/try6.test index 173eaa6bc614..00a1103fabaf 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/try6.test +++ b/plugins/groovy/testdata/groovy/controlFlow/try6.test @@ -18,7 +18,7 @@ return url 7(8) READ e 8(9) ARGUMENT element: Reference expression 9(13) THROW. element: THROW statement -10(11) element: IF statement +10(11) End element: IF statement 11(12) READ url 12(13) element: RETURN statement 13() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/varInString.test b/plugins/groovy/testdata/groovy/controlFlow/varInString.test index 86ca16970b78..70ab7dcefbcb 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/varInString.test +++ b/plugins/groovy/testdata/groovy/controlFlow/varInString.test @@ -10,5 +10,5 @@ if (i in String) i = i.substring(2) 7(8) READ i 8(9) WRITE i 9(10) element: Assignment expression MAYBE_RETURN -10(11) element: IF statement +10(11) End element: IF statement 11() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/while1.test b/plugins/groovy/testdata/groovy/controlFlow/while1.test index cd7c29f2b9da..be9844ac6ae1 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/while1.test +++ b/plugins/groovy/testdata/groovy/controlFlow/while1.test @@ -9,7 +9,7 @@ while (true) { 2(3) element: WHILE statement 3(4) element: IF statement 4(5,8) READ i -5(6) element: IF statement +5(6) End element: IF statement 6(7) READ i 7(2) WRITE i 8() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/while2.test b/plugins/groovy/testdata/groovy/controlFlow/while2.test index 15605553beb5..9275a2fdfd16 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/while2.test +++ b/plugins/groovy/testdata/groovy/controlFlow/while2.test @@ -9,7 +9,7 @@ while (true) { 2(3) element: WHILE statement 3(4) element: IF statement 4(2,5) READ i -5(6) element: IF statement +5(6) End element: IF statement 6(7) READ i 7(2) WRITE j 8() element: null \ No newline at end of file diff --git a/plugins/groovy/testdata/groovy/controlFlow/whileNonConstant.test b/plugins/groovy/testdata/groovy/controlFlow/whileNonConstant.test index 043e4f397ac6..183fe9112aa4 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/whileNonConstant.test +++ b/plugins/groovy/testdata/groovy/controlFlow/whileNonConstant.test @@ -10,7 +10,7 @@ while (condition()) { 3(4,9) READ condition 4(5) element: IF statement 5(6,9) READ i -6(7) element: IF statement +6(7) End element: IF statement 7(8) READ i 8(2) WRITE i 9() element: null \ No newline at end of file