GROOVY-4727, GROOVY-9373, GROOVY-9880, GROOVY-9896

GROOVY-9126 rollback for Groovy 2.5/3.0

Author: eric-milles

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()

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<CaseStatement> caseStatements = swi.getCaseStatements();
+            for (Iterator<CaseStatement> 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;
         }

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();

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<Statement> 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);

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<CaseStatement> caseStatements = switchStatement.getCaseStatements();
+            for (Iterator<CaseStatement> 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;
         }

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) {