From 48c4b185cd4307d4c4565bd9934c688d18ec6192 Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 26 Jul 2010 20:29:53 +0100 Subject: [PATCH] then- and else branches of groovy if may return a value from function: support this in control flow (IDEA-56628) --- .../GroovyAssignabilityCheckInspection.java | 9 ++++--- .../MissingReturnInspection.java | 7 +++-- .../utils/ControlFlowUtils.java | 13 ++++++--- .../controlFlow/impl/ControlFlowBuilder.java | 17 +++++++----- .../testdata/groovy/controlFlow/if1.test | 5 ++-- .../groovy/controlFlow/ifInstanceofElse.test | 27 ++++++++++--------- 6 files changed, 48 insertions(+), 30 deletions(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java index 44b23e2aae38..f2be2a783d20 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/assignment/GroovyAssignabilityCheckInspection.java @@ -21,6 +21,7 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.TypeConversionUtil; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.GroovyBundle; import org.jetbrains.plugins.groovy.codeInspection.BaseInspection; import org.jetbrains.plugins.groovy.codeInspection.BaseInspectionVisitor; @@ -94,10 +95,10 @@ public class GroovyAssignabilityCheckInspection extends BaseInspection { if (expectedType == null || PsiType.VOID.equals(expectedType)) return; ControlFlowUtils.visitAllExitPoints(block, new ControlFlowUtils.ExitPointVisitor() { - public boolean visit(Instruction instruction) { - final PsiElement psiElement = instruction.getElement(); - if (psiElement instanceof GrExpression) { - checkAssignability(expectedType, (GrExpression)psiElement, (GrExpression)psiElement); + @Override + public boolean visitExitPoint(Instruction instruction, @Nullable GrExpression returnValue) { + if (returnValue != null && !(returnValue.getParent() instanceof GrReturnStatement)) { + checkAssignability(expectedType, returnValue, returnValue); } return true; } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java index 684dbfe780ac..a39ba6cc3fee 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/noReturnMethod/MissingReturnInspection.java @@ -24,6 +24,7 @@ import com.intellij.psi.PsiType; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.codeInspection.GroovyInspectionBundle; import org.jetbrains.plugins.groovy.codeInspection.GroovySuppressableInspectionTool; import org.jetbrains.plugins.groovy.codeInspection.utils.ControlFlowUtils; @@ -35,6 +36,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrOpenBlock; import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrAssertStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrReturnStatement; import org.jetbrains.plugins.groovy.lang.psi.api.statements.branch.GrThrowStatement; +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod; import org.jetbrains.plugins.groovy.lang.psi.controlFlow.Instruction; import org.jetbrains.plugins.groovy.lang.psi.controlFlow.impl.MaybeReturnInstruction; @@ -86,7 +88,8 @@ public class MissingReturnInspection extends GroovySuppressableInspectionTool { final Ref hasExplicitReturn = new Ref(false); final Ref sometimes = new Ref(false); ControlFlowUtils.visitAllExitPoints(block, new ControlFlowUtils.ExitPointVisitor() { - public boolean visit(Instruction instruction) { + @Override + public boolean visitExitPoint(Instruction instruction, @Nullable GrExpression returnValue) { if (instruction instanceof MaybeReturnInstruction) { if (((MaybeReturnInstruction)instruction).mayReturnValue()) { sometimes.set(true); @@ -99,7 +102,7 @@ public class MissingReturnInspection extends GroovySuppressableInspectionTool { final PsiElement element = instruction.getElement(); if (element instanceof GrReturnStatement) { sometimes.set(true); - if (((GrReturnStatement)element).getReturnValue() != null) { + if (returnValue != null) { hasExplicitReturn.set(true); } } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java index 86aa6c0503e4..995b0302494d 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/codeInspection/utils/ControlFlowUtils.java @@ -543,7 +543,7 @@ public class ControlFlowUtils { public interface ExitPointVisitor { - boolean visit(Instruction instruction); + boolean visitExitPoint(Instruction instruction, @Nullable GrExpression returnValue); } public static void visitAllExitPoints(@Nullable GrCodeBlock block, ExitPointVisitor visitor) { @@ -556,12 +556,16 @@ public class ControlFlowUtils { private static boolean visitAllExitPointsInner(Instruction last, Instruction first, boolean[] visited, ExitPointVisitor visitor) { if (first == last) return true; if (last instanceof MaybeReturnInstruction) { - return visitor.visit(last); + return visitor.visitExitPoint(last, (GrExpression)last.getElement()); } - final PsiElement element = last.getElement(); + PsiElement element = last.getElement(); if (element != null) { - return visitor.visit(last); + if (element instanceof GrReturnStatement) { + element = ((GrReturnStatement)element).getReturnValue(); + } + + return visitor.visitExitPoint(last, element instanceof GrExpression ? (GrExpression)element : null); } visited[last.num()] = true; for (Instruction pred : last.allPred()) { @@ -571,4 +575,5 @@ public class ControlFlowUtils { } return true; } + } 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 ea85bc7a40b1..174ac80aa94e 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 @@ -107,16 +107,19 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { if (!(block.getParent() instanceof GrBlockStatement && block.getParent().getParent() instanceof GrLoopStatement)) { final GrStatement[] statements = block.getStatements(); if (statements.length > 0) { - final GrStatement last = statements[statements.length - 1]; - if (last instanceof GrExpression) { - final MaybeReturnInstruction instruction = new MaybeReturnInstruction((GrExpression)last, myInstructionNumber++); - checkPending(instruction); - addNode(instruction); - } + handlePossibleReturn(statements[statements.length - 1]); } } } + private void handlePossibleReturn(GrStatement last) { + if (last instanceof GrExpression) { + final MaybeReturnInstruction instruction = new MaybeReturnInstruction((GrExpression)last, myInstructionNumber++); + checkPending(instruction); + addNode(instruction); + } + } + public Instruction[] buildControlFlow(GroovyPsiElement scope, GroovyPsiElement startInScope, GroovyPsiElement endInScope) { myInstructions = new ArrayList(); myProcessingStack = new Stack(); @@ -386,6 +389,7 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { condition.accept(this); } thenBranch.accept(this); + handlePossibleReturn(thenBranch); addPendingEdge(ifStatement, myHead); } @@ -402,6 +406,7 @@ public class ControlFlowBuilder extends GroovyRecursiveElementVisitor { final GrStatement elseBranch = ifStatement.getElseBranch(); if (elseBranch != null) { elseBranch.accept(this); + handlePossibleReturn(elseBranch); addPendingEdge(ifStatement, myHead); } diff --git a/plugins/groovy/testdata/groovy/controlFlow/if1.test b/plugins/groovy/testdata/groovy/controlFlow/if1.test index a88e8969eb44..f4dc6626de6c 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/if1.test +++ b/plugins/groovy/testdata/groovy/controlFlow/if1.test @@ -7,6 +7,7 @@ if (true) { 0(1) element: null 1(2,4) element: IF statement 2(3) WRITE a -3(5) element: Assignment expression MAYBE_RETURN +3(6) element: Assignment expression MAYBE_RETURN 4(5) WRITE a -5() element: null \ No newline at end of file +5(6) element: Assignment expression MAYBE_RETURN +6() 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 7851696d58a6..e1f89d70302f 100644 --- a/plugins/groovy/testdata/groovy/controlFlow/ifInstanceofElse.test +++ b/plugins/groovy/testdata/groovy/controlFlow/ifInstanceofElse.test @@ -3,17 +3,20 @@ else if (!(o instanceof Integer)) b = 2 else b = 3 ----- 0(1) element: null -1(2,5) element: IF statement +1(2,6) element: IF statement 2(3) READ o 3(4) assertion: o instanceof String -4(14) WRITE b -5(6) READ o -6(7) assertion: ! o instanceof String -7(8,11) element: IF statement -8(9) READ o -9(10) assertion: ! o instanceof Integer -10(14) WRITE b -11(12) READ o -12(13) assertion: o instanceof Integer -13(14) WRITE b -14() element: null \ No newline at end of file +4(5) WRITE b +5(17) element: Assignment expression MAYBE_RETURN +6(7) READ o +7(8) assertion: ! o instanceof String +8(9,13) element: IF statement +9(10) READ o +10(11) assertion: ! o instanceof Integer +11(12) WRITE b +12(17) element: Assignment expression MAYBE_RETURN +13(14) READ o +14(15) assertion: o instanceof Integer +15(16) WRITE b +16(17) element: Assignment expression MAYBE_RETURN +17() element: null \ No newline at end of file