diff --git a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java index 83264004d0..31417e98cb 100644 --- a/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java +++ b/base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java @@ -2496,11 +2496,113 @@ public void testAbstractClass_GRE274_2() { "----------\n"); } + @Test + public void testSwitchCases1() { + //@formatter:off + String[] sources = { + "X.groovy", + "def foo(p) {\n" + + " switch (p) {\n" + + " case 1:\n" + + " 'a'\n" + + " break\n" + + " case 2:\n" + + " if (false) 'b'\n" + + " else 'c'\n" + + " break\n" + + " case 3:\n" + + " 'skip'\n" + + " default:\n" + + " 'd'\n" + + " }\n" + + "}\n" + + "print foo(1)\n" + + "print foo(2)\n" + + "print foo(3)\n" + + "print foo(4)\n", + }; + //@formatter:on + + runConformTest(sources, "acdd"); + } + + @Test // GROOVY-9896 + public void testSwitchCases2() { + //@formatter:off + String[] sources = { + "X.groovy", + "def foo(p) {\n" + + " switch (p) {\n" + + " case 1:\n" + + " 'a'\n" + + " break\n" + + " case 2:\n" + + " 'b'\n" + + " break\n" + + " case 3:\n" + + " 'c'\n" + + " }\n" + + "}\n" + + "print foo(1)\n" + + "print foo(2)\n" + + "print foo(3)\n" + + "print foo(4)\n", + }; + //@formatter:on + + runConformTest(sources, "abcnull"); + } + + @Test // GROOVY-4727 + public void testSwitchCases3() { + //@formatter:off + String[] sources = { + "X.groovy", + "def foo(x,y) {\n" + + " switch (x) {\n" + + " case 'x1':\n" + + " switch (y) {\n" + + " case 'y1':\n" + + " 'r1'\n" + + " break\n" + + " case 'y2':\n" + + " 'r2'\n" + + " break\n" + + " }\n" + + // no break + " }\n" + + "}\n" + + "print foo('x1','y1')\n", + }; + //@formatter:on + + runConformTest(sources, "r1"); + } + + @Test // GROOVY-9880 + public void testBreakAfterIf() { + //@formatter:off + String[] sources = { + "X.groovy", + "switch ('value') {\n" + + " case 'value':\n" + + " print 'foo'\n" + + " if (false) print 'X'\n" + + " break\n" + + " default:\n" + + " print 'bar'\n" + + "}\n", + }; + //@formatter:on + + runConformTest(sources, "foo"); // not "foobar" + } + @Test public void testBreak_GRE290() { //@formatter:off String[] sources = { - "p/X.groovy", + "X.groovy", "words: [].each { final item ->\n" + " break words\n" + "}\n", @@ -2509,7 +2611,7 @@ public void testBreak_GRE290() { runNegativeTest(sources, "----------\n" + - "1. ERROR in p\\X.groovy (at line 2)\n" + + "1. ERROR in X.groovy (at line 2)\n" + "\tbreak words\n" + "\t^^^^^^^^^^^\n" + "Groovy:" + (!isParrotParser() diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/ReturnAdder.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/ReturnAdder.java index f715fd4849..b9162eb53f 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/ReturnAdder.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/ReturnAdder.java @@ -38,6 +38,7 @@ import org.codehaus.groovy.ast.stmt.TryCatchStatement; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; /** @@ -120,13 +121,9 @@ private Statement addReturnsIfNeeded(Statement statement, VariableScope scope) { } if (statement instanceof EmptyStatement) { - /* GRECLIPSE edit -- GROOVY-9373 final ReturnStatement returnStatement = new ReturnStatement(ConstantExpression.NULL); listener.returnStatementAdded(returnStatement); return returnStatement; - */ - return statement; - // GRECLIPSE end } if (statement instanceof ExpressionStatement) { @@ -159,11 +156,23 @@ private Statement addReturnsIfNeeded(Statement statement, VariableScope scope) { if (statement instanceof SwitchStatement) { SwitchStatement swi = (SwitchStatement) statement; + /* GRECLIPSE edit -- GROOVY-4727, GROOVY-9896 for (CaseStatement caseStatement : swi.getCaseStatements()) { final Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, false); if (doAdd) caseStatement.setCode(code); } final Statement defaultStatement = adjustSwitchCaseCode(swi.getDefaultStatement(), scope, true); + */ + Statement defaultStatement = swi.getDefaultStatement(); + List caseStatements = swi.getCaseStatements(); + for (Iterator it = caseStatements.iterator(); it.hasNext();) { + CaseStatement caseStatement = it.next(); + Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, + defaultStatement == EmptyStatement.INSTANCE && !it.hasNext()); + if (doAdd) caseStatement.setCode(code); + } + defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true); + // GRECLIPSE end if (doAdd) swi.setDefaultStatement(defaultStatement); return swi; } diff --git a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/StatementWriter.java b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/StatementWriter.java index e49750aec2..14fd536be5 100644 --- a/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/StatementWriter.java +++ b/base/org.codehaus.groovy25/src/org/codehaus/groovy/classgen/asm/StatementWriter.java @@ -89,31 +89,16 @@ public void writeBlockStatement(BlockStatement block) { statement.visit(controller.getAcg()); } compileStack.pop(); - /* GRECLIPSE edit - // GROOVY-7647, GROOVY-9126 - if (block.getLastLineNumber() > 0 && !isMethodOrConstructorNonEmptyBlock(block)) { + + // GROOVY-7647 + if (block.getLastLineNumber() > 0) { MethodVisitor mv = controller.getMethodVisitor(); Label blockEnd = new Label(); mv.visitLabel(blockEnd); mv.visitLineNumber(block.getLastLineNumber(), blockEnd); } - */ - controller.getOperandStack().popDownTo(mark); - } - - /* GRECLIPSE edit - private boolean isMethodOrConstructorNonEmptyBlock(BlockStatement block) { - MethodNode methodNode = controller.getMethodNode(); - if (null == methodNode) { - methodNode = controller.getConstructorNode(); - } - if (null == methodNode || block != methodNode.getCode()) { // check if the block is method/constructor's code - return false; - } - - return !block.getStatements().isEmpty(); + controller.getOperandStack().popDownTo(mark); } - */ public void writeForStatement(ForStatement loop) { Parameter loopVar = loop.getVariable(); diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/AsmClassGenerator.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/AsmClassGenerator.java index 0f7bf2f4e8..11ebe85857 100644 --- a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -474,16 +474,13 @@ private void visitStdMethod(final MethodNode node, final boolean isConstructor, */ if (code != null) { code.visit(this); - } - if (!checkIfLastStatementIsReturnOrThrow(code)) { - if (code != null) { - // GROOVY-7647, GROOVY-9373 - int line = code.getLastLineNumber(); - if (line > controller.getLineNumber()) { - Label label = new Label(); mv.visitLabel(label); - mv.visitLineNumber(line, label); controller.setLineNumber(line); - } + // GROOVY-7647, GROOVY-9373 + int line = code.getLastLineNumber(); + if (line > controller.getLineNumber()) { + Label label = new Label(); mv.visitLabel(label); + mv.visitLineNumber(line, label); controller.setLineNumber(line); } + } // GRECLIPSE end if (node.isVoidMethod()) { mv.visitInsn(RETURN); @@ -501,27 +498,10 @@ private void visitStdMethod(final MethodNode node, final boolean isConstructor, } } // GRECLIPSE add - } controller.getCompileStack().clear(); // GRECLIPSE end } - private boolean checkIfLastStatementIsReturnOrThrow(Statement code) { - if (code instanceof BlockStatement) { - BlockStatement blockStatement = (BlockStatement) code; - List statementList = blockStatement.getStatements(); - int statementCnt = statementList.size(); - if (statementCnt > 0) { - Statement lastStatement = statementList.get(statementCnt - 1); - if (lastStatement instanceof ReturnStatement || lastStatement instanceof ThrowStatement) { - return true; - } - } - } - - return false; - } - private void visitAnnotationDefaultExpression(final AnnotationVisitor av, final ClassNode type, final Expression exp) { if (exp instanceof ClosureExpression) { ClassNode closureClass = controller.getClosureWriter().getOrAddClosureClass((ClosureExpression) exp, ACC_PUBLIC); diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/ReturnAdder.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/ReturnAdder.java index 5192558d5a..cfff39ddeb 100644 --- a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/ReturnAdder.java +++ b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/ReturnAdder.java @@ -36,6 +36,7 @@ import org.codehaus.groovy.ast.stmt.TryCatchStatement; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Objects; @@ -100,9 +101,6 @@ public void visitMethod(final MethodNode node) { private Statement addReturnsIfNeeded(final Statement statement, final VariableScope scope) { if (statement instanceof ReturnStatement || statement instanceof ThrowStatement - // GRECLIPSE add -- GROOVY-9373 - || statement instanceof EmptyStatement - // GRECLIPSE end || statement instanceof BytecodeSequence) { return statement; } @@ -142,11 +140,23 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc if (statement instanceof SwitchStatement) { SwitchStatement switchStatement = (SwitchStatement) statement; + /* GRECLIPSE edit -- GROOVY-4727, GROOVY-9896 for (CaseStatement caseStatement : switchStatement.getCaseStatements()) { Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, false); if (doAdd) caseStatement.setCode(code); } Statement defaultStatement = adjustSwitchCaseCode(switchStatement.getDefaultStatement(), scope, true); + */ + Statement defaultStatement = switchStatement.getDefaultStatement(); + List caseStatements = switchStatement.getCaseStatements(); + for (Iterator it = caseStatements.iterator(); it.hasNext();) { + CaseStatement caseStatement = it.next(); + Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, + defaultStatement == EmptyStatement.INSTANCE && !it.hasNext()); + if (doAdd) caseStatement.setCode(code); + } + defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true); + // GRECLIPSE end if (doAdd) switchStatement.setDefaultStatement(defaultStatement); return switchStatement; } diff --git a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/StatementWriter.java b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/StatementWriter.java index 52bba8ea13..53fb71c11c 100644 --- a/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/StatementWriter.java +++ b/base/org.codehaus.groovy30/src/org/codehaus/groovy/classgen/asm/StatementWriter.java @@ -93,27 +93,16 @@ public void writeBlockStatement(final BlockStatement block) { statement.visit(controller.getAcg()); } compileStack.pop(); - /* GRECLIPSE edit - // GROOVY-7647, GROOVY-9126 - if (block.getLastLineNumber() > 0 && !isMethodOrConstructorNonEmptyBlock(block)) { + + // GROOVY-7647 + if (block.getLastLineNumber() > 0) { MethodVisitor mv = controller.getMethodVisitor(); - Label blockEnd = new Label(); - mv.visitLabel(blockEnd); + Label blockEnd = new Label(); mv.visitLabel(blockEnd); mv.visitLineNumber(block.getLastLineNumber(), blockEnd); } - */ - controller.getOperandStack().popDownTo(mark); - } - /* GRECLIPSE edit - private boolean isMethodOrConstructorNonEmptyBlock(final BlockStatement block) { - MethodNode methodNode = controller.getMethodNode(); - if (methodNode == null) { - methodNode = controller.getConstructorNode(); - } - return (methodNode != null && methodNode.getCode() == block && !block.isEmpty()); + controller.getOperandStack().popDownTo(mark); } - */ public void writeForStatement(final ForStatement statement) { if (statement.getVariable() == ForStatement.FOR_LOOP_DUMMY) { diff --git a/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/ReturnAdder.java b/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/ReturnAdder.java index 5192558d5a..8f47ad7bd9 100644 --- a/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/ReturnAdder.java +++ b/base/org.codehaus.groovy40/src/org/codehaus/groovy/classgen/ReturnAdder.java @@ -36,10 +36,12 @@ import org.codehaus.groovy.ast.stmt.TryCatchStatement; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Objects; import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX; +import static org.codehaus.groovy.runtime.DefaultGroovyMethods.last; /** * Utility class to add return statements. @@ -142,12 +144,26 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc if (statement instanceof SwitchStatement) { SwitchStatement switchStatement = (SwitchStatement) statement; + /* GRECLIPSE edit -- GROOVY-4727, GROOVY-9880, GROOVY-9896 for (CaseStatement caseStatement : switchStatement.getCaseStatements()) { Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, false); if (doAdd) caseStatement.setCode(code); } Statement defaultStatement = adjustSwitchCaseCode(switchStatement.getDefaultStatement(), scope, true); if (doAdd) switchStatement.setDefaultStatement(defaultStatement); + */ + Statement defaultStatement = switchStatement.getDefaultStatement(); + List caseStatements = switchStatement.getCaseStatements(); + for (Iterator it = caseStatements.iterator(); it.hasNext(); ) { + CaseStatement caseStatement = it.next(); + Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope, + // GROOVY-9896: return if no default and last case lacks break + defaultStatement == EmptyStatement.INSTANCE && !it.hasNext()); + if (doAdd) caseStatement.setCode(code); + } + defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true); + if (doAdd) switchStatement.setDefaultStatement(defaultStatement); + // GRECLIPSE end return switchStatement; } @@ -185,11 +201,13 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc int lastIndex = statements.size() - 1; Statement last = addReturnsIfNeeded(statements.get(lastIndex), blockStatement.getVariableScope()); if (doAdd) statements.set(lastIndex, last); + /* GRECLIPSE edit -- GROOVY-9373 if (!returns(last)) { ReturnStatement returnStatement = new ReturnStatement(nullX()); listener.returnStatementAdded(returnStatement); if (doAdd) statements.add(returnStatement); } + */ return blockStatement; } } @@ -207,6 +225,7 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc } private Statement adjustSwitchCaseCode(final Statement statement, final VariableScope scope, final boolean defaultCase) { + /* GRECLIPSE edit if (statement instanceof BlockStatement) { List statements = ((BlockStatement) statement).getStatements(); if (!statements.isEmpty()) { @@ -228,9 +247,33 @@ private Statement adjustSwitchCaseCode(final Statement statement, final Variable } } } + */ + if (!statement.isEmpty() && statement instanceof BlockStatement) { + BlockStatement block = (BlockStatement) statement; + int breakIndex = block.getStatements().size() - 1; + if (block.getStatements().get(breakIndex) instanceof BreakStatement) { + if (doAdd) { + Statement breakStatement = block.getStatements().remove(breakIndex); + if (breakIndex == 0) block.addStatement(EmptyStatement.INSTANCE); + addReturnsIfNeeded(block, scope); + // GROOVY-9880: some code structures will fall through + Statement lastStatement = last(block.getStatements()); + if (!(lastStatement instanceof ReturnStatement + || lastStatement instanceof ThrowStatement)) { + block.addStatement(breakStatement); + } + } else { + addReturnsIfNeeded(new BlockStatement(block.getStatements().subList(0, breakIndex), null), scope); + } + } else if (defaultCase) { + return addReturnsIfNeeded(statement, scope); + } + } + // GRECLIPSE end return statement; } + /* GRECLIPSE edit private static boolean returns(final Statement statement) { return statement instanceof ReturnStatement || statement instanceof BlockStatement @@ -242,4 +285,5 @@ private static boolean returns(final Statement statement) { || statement instanceof SynchronizedStatement || statement instanceof BytecodeSequence; } + */ }