IDEA-89735, IDEA-89650 fix control flow for else branch if control flow is broken in it

This commit is contained in:
Maxim.Medvedev
2012-08-05 20:25:45 +04:00
parent 9ff41550a8
commit 5c610fff60
18 changed files with 141 additions and 62 deletions
@@ -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<Pair<InstructionImpl, GroovyPsiElement>> iterator = myPending.listIterator(myPending.size());iterator.hasPrevious(); ) {
Pair<InstructionImpl, GroovyPsiElement> 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<InstructionImpl, GroovyPsiElement>(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);
}
}
@@ -29,4 +29,9 @@ public class IfEndInstruction extends InstructionImpl{
public GrIfStatement getElement() {
return (GrIfStatement)super.getElement();
}
@Override
protected String getElementPresentation() {
return "End element: " + myPsiElement;
}
}
@@ -87,6 +87,7 @@ public class InstructionImpl implements Instruction {
}
protected String getElementPresentation() {
//return "element: " + (myPsiElement != null ? myPsiElement.getText() : null);
return "element: " + myPsiElement;
}
@@ -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");
}
}
+1 -1
View File
@@ -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
+1 -1
View File
@@ -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
@@ -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
+1 -1
View File
@@ -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
+1 -1
View File
@@ -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
+2 -2
View File
@@ -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
+1 -1
View File
@@ -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
+1 -1
View File
@@ -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
+1 -1
View File
@@ -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
+1 -1
View File
@@ -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
@@ -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
+1 -1
View File
@@ -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
+1 -1
View File
@@ -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
@@ -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