This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-7463 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 3b81f2bd222b5bbea96964c5eef056c9bddfe575 Author: Eric Milles <[email protected]> AuthorDate: Mon Jan 19 13:30:18 2026 -0600 GROOVY-7463: `break` out of named `if` statement --- .../apache/groovy/parser/antlr4/AstBuilder.java | 15 -- .../codehaus/groovy/classgen/asm/CompileStack.java | 206 +++++++++++---------- .../groovy/classgen/asm/StatementWriter.java | 38 ++-- .../org/codehaus/groovy/control/LabelVerifier.java | 177 +++++++++--------- src/spec/doc/core-semantics.adoc | 18 +- src/spec/test/semantics/LabelsTest.groovy | 23 ++- .../groovy/groovy/BreakContinueLabelTest.groovy | 109 +++++++++-- .../groovy/parser/antlr4/SyntaxErrorTest.groovy | 21 --- 8 files changed, 337 insertions(+), 270 deletions(-) diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java index 72f04a3810..a2b5c6f981 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -458,12 +458,10 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { @Override public Statement visitLoopStmtAlt(final LoopStmtAltContext ctx) { switchExpressionRuleContextStack.push(ctx); - visitingLoopStatementCount += 1; try { return configureAST((Statement) this.visit(ctx.loopStatement()), ctx); } finally { switchExpressionRuleContextStack.pop(); - visitingLoopStatementCount -= 1; } } @@ -723,7 +721,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { @Override public SwitchStatement visitSwitchStatement(final SwitchStatementContext ctx) { switchExpressionRuleContextStack.push(ctx); - visitingSwitchStatementCount += 1; try { List<Statement> statementList = ctx.switchBlockStatementGroup().stream() @@ -762,7 +759,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { ctx); } finally { switchExpressionRuleContextStack.pop(); - visitingSwitchStatementCount -= 1; } } @@ -847,10 +843,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { @Override public BreakStatement visitBreakStatement(final BreakStatementContext ctx) { - if (visitingLoopStatementCount == 0 && visitingSwitchStatementCount == 0) { - throw createParsingFailedException("break statement is only allowed inside loops or switches", ctx); - } - if (switchExpressionRuleContextStack.peek() instanceof SwitchExpressionContext) { throw createParsingFailedException("switch expression does not support `break`", ctx); } @@ -876,10 +868,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { @Override public ContinueStatement visitContinueStatement(final ContinueStatementContext ctx) { - if (visitingLoopStatementCount == 0) { - throw createParsingFailedException("continue statement is only allowed inside loops", ctx); - } - if (switchExpressionRuleContextStack.peek() instanceof SwitchExpressionContext) { throw createParsingFailedException("switch expression does not support `continue`", ctx); } @@ -889,7 +877,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { : null; return configureAST(new ContinueStatement(label), ctx); - } @Override @@ -4760,8 +4747,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { private Tuple2<GroovyParserRuleContext, Exception> numberFormatError; private int visitingClosureCount; - private int visitingLoopStatementCount; - private int visitingSwitchStatementCount; private int visitingAssertStatementCount; private int visitingArrayInitializerCount; diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java index dc461eacfd..a02729a168 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java @@ -33,9 +33,9 @@ import org.objectweb.asm.TypePath; import org.objectweb.asm.TypeReference; import java.util.Collection; +import java.util.Collections; import java.util.Deque; import java.util.HashMap; -import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -471,108 +471,140 @@ public class CompileStack { /** * Should be called when descending into a loop that defines * also a scope. Calls pushVariableScope and prepares labels - * for a loop structure. Creates a element for the state stack - * so pop has to be called later, TODO: @Deprecate + * for a loop structure. Creates an element for the state stack + * so pop has to be called later */ - public void pushLoop(final VariableScope scope, final String labelName) { + public void pushLoop(final VariableScope scope, final List<String> labelNames) { pushVariableScope(scope); continueLabel = new Label(); breakLabel = new Label(); - if (labelName != null) { - initLoopLabels(labelName); + if (labelNames != null) { + for (String labelName: labelNames) { + if (labelName == null) continue; + namedLoopBreakLabel.put(labelName, breakLabel); + namedLoopContinueLabel.put(labelName, continueLabel); + } } } /** * Should be called when descending into a loop that defines * also a scope. Calls pushVariableScope and prepares labels - * for a loop structure. Creates an element for the state stack - * so pop has to be called later + * for a loop structure. Creates a element for the state stack + * so pop has to be called later. */ - public void pushLoop(final VariableScope el, final List<String> labelNames) { - pushVariableScope(el); + @Deprecated(since = "6.0.0") + public void pushLoop(final VariableScope scope, final String labelName) { + pushLoop(scope, Collections.singletonList(labelName)); + } + + /** + * Should be called when descending into a loop that does not define a scope + * Creates a element for the state stack so pop has to be called later. + */ + public void pushLoop(final List<String> labelNames) { + pushState(); continueLabel = new Label(); breakLabel = new Label(); if (labelNames != null) { - for (String labelName : labelNames) { - initLoopLabels(labelName); + for (String labelName: labelNames) { + if (labelName == null) continue; + namedLoopBreakLabel.put(labelName, breakLabel); + namedLoopContinueLabel.put(labelName, continueLabel); } } } - private void initLoopLabels(final String labelName) { - namedLoopBreakLabel.put(labelName, breakLabel); - namedLoopContinueLabel.put(labelName, continueLabel); + /** + * Should be called when descending into a loop that does not define a scope. + * Creates a element for the state stack so pop has to be called later. + */ + @Deprecated(since = "6.0.0") + public void pushLoop(final String labelName) { + pushLoop(Collections.singletonList(labelName)); } /** - * Should be called when descending into a loop that does - * not define a scope. Creates a element for the state stack - * so pop has to be called later, TODO: @Deprecate + * Creates a new break label and an element for the state stack so pop has + * to be called later. + * + * @return the break label */ - public void pushLoop(final String labelName) { + public Label pushSwitch() { pushState(); - continueLabel = new Label(); breakLabel = new Label(); - initLoopLabels(labelName); + return breakLabel; } /** - * Should be called when descending into a loop that does - * not define a scope. Creates an element for the state stack - * so pop has to be called later + * Creates a new break label and an element for the state stack so pop has + * to be called later. + * + * @return the break label */ - public void pushLoop(final List<String> labelNames) { + public Label pushBreakable(final List<String> labelNames) { pushState(); - continueLabel = new Label(); - breakLabel = new Label(); + Label namedBreakLabel = new Label(); if (labelNames != null) { - for (String labelName : labelNames) { - initLoopLabels(labelName); + for (String labelName: labelNames) { + if (labelName == null) continue; + namedLoopBreakLabel.put(labelName, namedBreakLabel); } } + return namedBreakLabel; } /** - * Used for <code>break foo</code> inside a loop to end the - * execution of the marked loop. This method will return the - * break label of the loop if there is one found for the name. - * If not, the current break label is returned. + * @return the {@code break} label for name */ public Label getNamedBreakLabel(final String name) { - Label label = getBreakLabel(); - Label endLabel = null; - if (name != null) - endLabel = namedLoopBreakLabel.get(name); - if (endLabel != null) - label = endLabel; + Label label; + if (name == null) { + label = getBreakLabel(); + if (label == null) throw new GroovyBugError("cannot break"); + } else { + label = namedLoopBreakLabel.get(name); + if (label == null) throw new GroovyBugError("undefined break label '" + name + "'"); + } return label; } /** - * Used for <code>continue foo</code> inside a loop to continue - * the execution of the marked loop. This method will return - * the break label of the loop if there is one found for the - * name. If not, getLabel is used. + * @return the {@code continue} label for name */ public Label getNamedContinueLabel(final String name) { - Label label = getLabel(name); - Label endLabel = null; - if (name != null) - endLabel = namedLoopContinueLabel.get(name); - if (endLabel != null) - label = endLabel; + Label label; + if (name == null) { + label = getContinueLabel(); + if (label == null) throw new GroovyBugError("cannot continue"); + } else { + label = namedLoopContinueLabel.get(name); + if (label == null) { + label = superBlockNamedLabels.get(name); + if (label == null) throw new GroovyBugError("undefined continue label '" + name + "'"); + } + } return label; } /** - * Creates a new break label and an element for the state stack - * so pop has to be called later + * Returns the label for the given name. */ - public Label pushSwitch() { - pushState(); - breakLabel = new Label(); - return breakLabel; + public Label getLabel(final String name) { + if (name == null) return null; + var label = superBlockNamedLabels.get(name); + if (label == null) { + label = createLocalLabel(name); + } + return label; + } + + /** + * Creates or returns the label for the given name. + */ + public Label createLocalLabel(final String name) { + if (name == null) return null; + return currentBlockNamedLabels.computeIfAbsent(name, k -> new Label()); } /** @@ -743,50 +775,38 @@ public class CompileStack { nextVariableIndex += 1; } - /** - * Returns the label for the given name - */ - public Label getLabel(final String name) { - if (name == null) return null; - Label l = superBlockNamedLabels.get(name); - if (l == null) { - l = createLocalLabel(name); - } - return l; - } - - /** - * creates a new named label - */ - public Label createLocalLabel(final String name) { - Label l = currentBlockNamedLabels.computeIfAbsent(name, k -> new Label()); - return l; - } - public void applyFinallyBlocks(final Label label, final boolean isBreakLabel) { - // first find the state defining the label. That is the state - // directly after the state not knowing this label. If no state - // in the list knows that label, then the defining state is the - // current state. - StateStackElement result = null; - for (Iterator<StateStackElement> iter = stateStack.descendingIterator(); iter.hasNext(); ) { - StateStackElement element = iter.next(); - - if (!element.currentBlockNamedLabels.containsValue(label)) { - if (isBreakLabel && element.breakLabel != label) { - result = element; - break; + StateStackElement before = null; + search: { + StateStackElement sse = null; + for (StateStackElement element : stateStack) { + if ((isBreakLabel ? element.breakLabel : element.continueLabel) == label + || element.currentBlockNamedLabels.containsValue(label)) { + before = sse; // the state before label was declared + break search; } - if (!isBreakLabel && element.continueLabel != label) { - result = element; - break; + sse = element; + } + if (isBreakLabel) { + String name = null; // try to recover the break label's name + for (Map.Entry<String,Label> e : namedLoopBreakLabel.entrySet()) { + if (e.getValue() == label) { + name = e.getKey(); + break; + } + } + for (var it = stateStack.descendingIterator(); it.hasNext(); ) { + if (it.next().currentBlockNamedLabels.containsKey(name)) { + before = it.hasNext() ? it.next() : null; + break; + } } } } - Collection<BlockRecorder> blockRecorders = new LinkedList<>(finallyBlocks); - if (result != null) { - blockRecorders.removeAll(result.finallyBlocks); + var blockRecorders = new LinkedList<>(finallyBlocks); + if (before != null) { + blockRecorders.removeAll(before.finallyBlocks); } applyBlockRecorder(blockRecorders); } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java index f821cd6757..1a46fc2175 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java @@ -74,11 +74,14 @@ public class StatementWriter { } protected void writeStatementLabel(final Statement statement) { - Optional.ofNullable(statement.getStatementLabels()).ifPresent(labels -> { - labels.stream().map(controller.getCompileStack()::createLocalLabel).forEach(label -> { - controller.getMethodVisitor().visitLabel(label); - }); - }); + List<String> labels = statement.getStatementLabels(); + if (labels != null) { + CompileStack cs = controller.getCompileStack (); + MethodVisitor mv = controller.getMethodVisitor(); + for (String label : labels) { + mv.visitLabel(cs.createLocalLabel(label)); + } + } } public void writeBlockStatement(final BlockStatement block) { @@ -323,24 +326,21 @@ public class StatementWriter { controller.getAcg().onLineNumber(statement, "visitIfElse"); writeStatementLabel(statement); - CompileStack compileStack = controller.getCompileStack(); - compileStack.pushState(); - + Label exitPath = controller.getCompileStack().pushBreakable(statement.getStatementLabels()); // GROOVY-7463 statement.getBooleanExpression().visit(controller.getAcg()); Label elsePath = controller.getOperandStack().jump(IFEQ); statement.getIfBlock().visit(controller.getAcg()); - compileStack.pop(); + controller.getCompileStack().pop(); MethodVisitor mv = controller.getMethodVisitor(); if (statement.getElseBlock().isEmpty()) { mv.visitLabel(elsePath); } else { - Label exitPath = new Label(); mv.visitJumpInsn(GOTO, exitPath); mv.visitLabel(elsePath); statement.getElseBlock().visit(controller.getAcg()); - mv.visitLabel(exitPath); } + mv.visitLabel(exitPath); } public void writeTryCatchFinally(final TryCatchStatement statement) { @@ -521,22 +521,18 @@ public class StatementWriter { controller.getAcg().onLineNumber(statement, "visitBreakStatement"); writeStatementLabel(statement); - String name = statement.getLabel(); - Label breakLabel = controller.getCompileStack().getNamedBreakLabel(name); - controller.getCompileStack().applyFinallyBlocks(breakLabel, true); - - controller.getMethodVisitor().visitJumpInsn(GOTO, breakLabel); + Label label = controller.getCompileStack().getNamedBreakLabel(statement.getLabel()); + controller.getCompileStack().applyFinallyBlocks(label, true); + controller.getMethodVisitor().visitJumpInsn(GOTO, label); } public void writeContinue(final ContinueStatement statement) { controller.getAcg().onLineNumber(statement, "visitContinueStatement"); writeStatementLabel(statement); - String name = statement.getLabel(); - Label continueLabel = controller.getCompileStack().getContinueLabel(); - if (name != null) continueLabel = controller.getCompileStack().getNamedContinueLabel(name); - controller.getCompileStack().applyFinallyBlocks(continueLabel, false); - controller.getMethodVisitor().visitJumpInsn(GOTO, continueLabel); + Label label = controller.getCompileStack().getNamedContinueLabel(statement.getLabel()); + controller.getCompileStack().applyFinallyBlocks(label, false); + controller.getMethodVisitor().visitJumpInsn(GOTO, label); } public void writeSynchronized(final SynchronizedStatement statement) { diff --git a/src/main/java/org/codehaus/groovy/control/LabelVerifier.java b/src/main/java/org/codehaus/groovy/control/LabelVerifier.java index 5c413f7c4e..9f53340114 100644 --- a/src/main/java/org/codehaus/groovy/control/LabelVerifier.java +++ b/src/main/java/org/codehaus/groovy/control/LabelVerifier.java @@ -23,24 +23,26 @@ import org.codehaus.groovy.ast.stmt.BreakStatement; import org.codehaus.groovy.ast.stmt.ContinueStatement; import org.codehaus.groovy.ast.stmt.DoWhileStatement; import org.codehaus.groovy.ast.stmt.ForStatement; +import org.codehaus.groovy.ast.stmt.IfStatement; import org.codehaus.groovy.ast.stmt.Statement; import org.codehaus.groovy.ast.stmt.SwitchStatement; import org.codehaus.groovy.ast.stmt.WhileStatement; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Set; /** * This class checks the handling of labels in the AST */ public class LabelVerifier extends ClassCodeVisitorSupport { + boolean inIf, inLoop, inSwitch; private final SourceUnit source; - private LinkedList<String> visitedLabels; - private LinkedList<ContinueStatement> continueLabels; - private LinkedList<BreakStatement> breakLabels; - boolean inLoop = false; - boolean inSwitch = false; + private Set<String> visitedLabels; + private List<BreakStatement> breakLabels; + private List<ContinueStatement> continueLabels; public LabelVerifier(SourceUnit src) { source = src; @@ -51,126 +53,127 @@ public class LabelVerifier extends ClassCodeVisitorSupport { return source; } - private void init() { - visitedLabels = new LinkedList<String>(); - continueLabels = new LinkedList<ContinueStatement>(); - breakLabels = new LinkedList<BreakStatement>(); - inLoop = false; - inSwitch = false; + protected void assertNoLabelsMissed() { + // TODO: Report multiple missing labels of the same name only once? + for (ContinueStatement element : continueLabels) { + addError("continue to missing label", element); + } + for (BreakStatement element : breakLabels) { + addError("break to missing label", element); + } + } + + @Override + public void visitStatement(Statement s) { + List<String> labels = s.getStatementLabels(); + if (labels != null && visitedLabels != null) { + for (String label : labels) { + visitedLabels.add(label); + + breakLabels.removeIf(breakStatement -> breakStatement.getLabel().equals(label)); + continueLabels.removeIf(continueStatement -> continueStatement.getLabel().equals(label)); + } + } } @Override protected void visitClassCodeContainer(Statement code) { - init(); + inIf = false; + inLoop = false; + inSwitch = false; + + visitedLabels = new HashSet<>(); + breakLabels = new LinkedList<>(); + continueLabels = new LinkedList<>(); + super.visitClassCodeContainer(code); + assertNoLabelsMissed(); } - @Override - public void visitStatement(Statement statement) { - List<String> labels = statement.getStatementLabels(); - - if (labels != null) { - for (String label : labels) { - if (breakLabels != null) { - breakLabels.removeIf(breakStatement -> breakStatement.getLabel().equals(label)); - } - if (continueLabels != null) { - continueLabels.removeIf(continueStatement -> continueStatement.getLabel().equals(label)); - } - if (visitedLabels != null) { - visitedLabels.add(label); - } - } - } + //-------------------------------------------------------------------------- - super.visitStatement(statement); + @Override + public void visitIfElse(IfStatement cond) { + visitStatement(cond); + cond.getBooleanExpression().visit(this); + boolean oldInIf = inIf; + inIf = true; + cond.getIfBlock().visit(this); + cond.getElseBlock().visit(this); + inIf = oldInIf; } @Override - public void visitForLoop(ForStatement forLoop) { + public void visitForLoop(ForStatement loop) { + visitStatement(loop); + loop.getCollectionExpression().visit(this); boolean oldInLoop = inLoop; inLoop = true; - super.visitForLoop(forLoop); + loop.getLoopBlock().visit(this); inLoop = oldInLoop; } @Override - public void visitDoWhileLoop(DoWhileStatement loop) { + public void visitWhileLoop(WhileStatement loop) { + visitStatement(loop); + loop.getBooleanExpression().visit(this); boolean oldInLoop = inLoop; inLoop = true; - super.visitDoWhileLoop(loop); + loop.getLoopBlock().visit(this); inLoop = oldInLoop; } @Override - public void visitWhileLoop(WhileStatement loop) { + public void visitDoWhileLoop(DoWhileStatement loop) { + visitStatement(loop); boolean oldInLoop = inLoop; inLoop = true; - super.visitWhileLoop(loop); + loop.getLoopBlock().visit(this); inLoop = oldInLoop; + loop.getBooleanExpression().visit(this); } @Override - public void visitBreakStatement(BreakStatement statement) { - String label = statement.getLabel(); - boolean hasNamedLabel = label != null; - if (!hasNamedLabel && !inLoop && !inSwitch) { - addError("the break statement is only allowed inside loops or switches", statement); - } else if (hasNamedLabel && !inLoop) { - addError("the break statement with named label is only allowed inside loops", statement); - } - if (label != null) { - boolean found = false; - for (String element : visitedLabels) { - if (element.equals(label)) { - found = true; - break; - } - } - if (!found) breakLabels.add(statement); - } - - super.visitBreakStatement(statement); + public void visitSwitch(SwitchStatement switchStatement) { + visitStatement(switchStatement); + switchStatement.getExpression().visit(this); + boolean oldInSwitch = inSwitch; + inSwitch = true; + switchStatement.getCaseStatements().forEach(this::visit); + switchStatement.getDefaultStatement().visit(this); + inSwitch = oldInSwitch; } @Override - public void visitContinueStatement(ContinueStatement statement) { - String label = statement.getLabel(); - boolean hasNamedLabel = label != null; - if (!hasNamedLabel && !inLoop) { - addError("the continue statement is only allowed inside loops", statement); - } - if (label != null) { - boolean found = false; - for (String element : visitedLabels) { - if (element.equals(label)) { - found = true; - break; - } + public void visitBreakStatement(BreakStatement breakStatement) { + String label = breakStatement.getLabel(); + if (label == null) { + if (!inLoop && !inSwitch) { + addError("the break statement is only allowed inside loops or switches", breakStatement); + } + } else { + if (!inLoop && !inIf) { // GROOVY-7463 + addError("the break statement with named label is only allowed inside control statements", breakStatement); + } + if (!visitedLabels.contains(label)) { + breakLabels.add(breakStatement); } - if (!found) continueLabels.add(statement); } - - super.visitContinueStatement(statement); + super.visitBreakStatement(breakStatement); } - protected void assertNoLabelsMissed() { - //TODO: report multiple missing labels of the same name only once? - for (ContinueStatement element : continueLabels) { - addError("continue to missing label", element); + @Override + public void visitContinueStatement(ContinueStatement continueStatement) { + if (!inLoop) { // GROOVY-3908 + addError("the continue statement is only allowed inside loops", continueStatement); } - for (BreakStatement element : breakLabels) { - addError("break to missing label", element); + String label = continueStatement.getLabel(); + if (label != null) { + if (!visitedLabels.contains(label)) { + continueLabels.add(continueStatement); + } } + super.visitContinueStatement(continueStatement); } - - @Override - public void visitSwitch(SwitchStatement statement) { - boolean oldInSwitch = inSwitch; - inSwitch = true; - super.visitSwitch(statement); - inSwitch = oldInSwitch; - } - } diff --git a/src/spec/doc/core-semantics.adoc b/src/spec/doc/core-semantics.adoc index e528523b74..bd7b7ca1ad 100644 --- a/src/spec/doc/core-semantics.adoc +++ b/src/spec/doc/core-semantics.adoc @@ -455,23 +455,21 @@ the code easier to read like in the following example: [source,groovy] ---- -include::../test/semantics/LabelsTest.groovy[tags=test_labels,indent=0] +include::../test/semantics/LabelsTest.groovy[tags=labels,indent=0] ---- -Despite not changing the semantics of the labelled statement, it is possible to use labels in the `break` instruction -as a target for jump, as in the next example. However, even if this is allowed, this coding style is in general considered -a bad practice: +It is important to understand that by default labels have no impact on the semantics of the code, however they belong to the abstract +syntax tree (AST) so it is possible for an AST transformation to use that information to perform transformations over the code, hence +leading to different semantics. This is in particular what the http://spockframework.github.io/spock/docs/current/index.html[Spock Framework] +does to make testing easier. + +It is possible to use labels in the `break` instruction as a target for jump, as in the next example: [source,groovy] ---- -include::../test/semantics/LabelsTest.groovy[tags=label_bad_practice,indent=0] +include::../test/semantics/LabelsTest.groovy[tags=break_label,indent=0] ---- -It is important to understand that by default labels have no impact on the semantics of the code, however they belong to the abstract -syntax tree (AST) so it is possible for an AST transformation to use that information to perform transformations over -the code, hence leading to different semantics. This is in particular what the http://spockframework.github.io/spock/docs/current/index.html[Spock Framework] -does to make testing easier. - == Expressions Expressions are the building blocks of Groovy programs that are used to reference diff --git a/src/spec/test/semantics/LabelsTest.groovy b/src/spec/test/semantics/LabelsTest.groovy index cf5f37995b..e448dac048 100644 --- a/src/spec/test/semantics/LabelsTest.groovy +++ b/src/spec/test/semantics/LabelsTest.groovy @@ -18,11 +18,13 @@ */ package semantics -import groovy.test.GroovyTestCase +import org.junit.jupiter.api.Test -class LabelsTest extends GroovyTestCase { +final class LabelsTest { + + @Test void testLabels() { - // tag::test_labels[] + // tag::labels[] given: def x = 1 def y = 2 @@ -30,20 +32,21 @@ class LabelsTest extends GroovyTestCase { def z = x+y then: assert z == 3 - // end::test_labels[] + // end::labels[] } + @Test void testUseOfLabel() { - // tag::label_bad_practice[] - for (int i=0;i<10;i++) { - for (int j=0;j<i;j++) { + // tag::break_label[] + out:for (int i=0; i<9; i++) { + for (int j=0; j<i; j++) { println "j=$j" if (j == 5) { - break exit + break out } } - exit: println "i=$i" + println "i=$i" } - // end::label_bad_practice[] + // end::break_label[] } } diff --git a/src/test/groovy/groovy/BreakContinueLabelTest.groovy b/src/test/groovy/groovy/BreakContinueLabelTest.groovy index 703fb7ffdb..5788aaba65 100644 --- a/src/test/groovy/groovy/BreakContinueLabelTest.groovy +++ b/src/test/groovy/groovy/BreakContinueLabelTest.groovy @@ -20,21 +20,64 @@ package groovy import org.junit.jupiter.api.Test +import static groovy.test.GroovyAssert.shouldFail import static org.junit.jupiter.api.Assertions.fail final class BreakContinueLabelTest { @Test - void testDeclareSimpleLabel() { - label_1: assert true + void testDeclareSimpleLabels() { + label_1: print('foo') label_2: - assert true + print('bar') + } + + // GROOVY-7463 + @Test + void testBreakLabelInIfStatement() { + boolean flag = true + label: + if (flag) { + print('foo') + if (flag) { + break label + fail() + } + fail() + } else { + fail() + } + print('bar') + } + + // GROOVY-7463 + @Test + void testBreakLabelInIfStatement2() { + int i = 0 + boolean flag = true + label: + if (flag) { + print('foo') + try { + if (flag) { + print('bar') + break label + fail() + } + fail() + } finally { + i += 1 + } + fail() + } + print('baz') + assert i == 1 } @Test void testBreakLabelInSimpleForLoop() { - label_1: for (i in [1]) { - break label_1 + label: for (i in [1]) { + break label; assert false } } @@ -42,8 +85,8 @@ final class BreakContinueLabelTest { @Test void testBreakLabelInNestedForLoop() { label: for (i in [1]) { - for (j in [1]){ - break label + for (j in [1]) { + break label; assert false : 'did not break inner loop' } assert false : 'did not break outer loop' @@ -51,7 +94,28 @@ final class BreakContinueLabelTest { } @Test - void testUnlabelledBreakInNestedForLoop() { + void testBreakLabelInForLoopTryFinally() { + int i = 0 + out: + for (j in 1..2) { + try { + try { + break out + assert false + } finally { + i += 10 + } + assert false + } finally { + i += 1 + } + assert false + } + assert i == 11 + } + + @Test + void testBreakInNestedForLoop() { def reached = false for (i in [1]) { for (j in [1]) { @@ -75,7 +139,7 @@ final class BreakContinueLabelTest { void testBreakLabelInNestedWhileLoop() { def count = 0 label: while (count < 1) { - count++ + count += 1 while (true) { break label assert false : 'did not break inner loop' @@ -88,7 +152,7 @@ final class BreakContinueLabelTest { void testBreakLabelInNestedMixedForAndWhileLoop() { def count = 0 label_1: while (count < 1) { - count++ + count += 1 for (i in [1]) { break label_1 assert false : 'did not break inner loop' @@ -106,8 +170,8 @@ final class BreakContinueLabelTest { // GROOVY-11739 @Test - void testUnlabelledContinueWithinDoWhileLoop() { - int i = 0; + void testContinueWithinDoWhileLoop() { + int i = 0 do { i += 1 if (i > 1000) break // prevent infinite loop @@ -118,7 +182,7 @@ final class BreakContinueLabelTest { } @Test - void testUnlabelledContinueInNestedForLoop() { + void testContinueInNestedForLoop() { def log = '' for (i in [1,2]) { log += i @@ -144,6 +208,25 @@ final class BreakContinueLabelTest { assert log == '1323' } + // GROOVY-3908 + @Test + void testContinueOutsideOfLoop() { + shouldFail ''' + continue + ''' + shouldFail ''' + if (true) continue + ''' + shouldFail ''' + xx: if (true) continue xx + ''' + shouldFail ''' + switch (value) { + case "foobar": continue + } + ''' + } + @Test void testBreakToLastLabelSucceeds() { one: diff --git a/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy b/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy index 748db1fd8c..9d5b789773 100644 --- a/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy +++ b/src/test/groovy/org/apache/groovy/parser/antlr4/SyntaxErrorTest.groovy @@ -395,27 +395,6 @@ final class SyntaxErrorTest { |'''.stripMargin() } - @Test // GROOVY-3908: groovyc should enforce correct usage of "continue" - void 'groovy core - Continue 1'() { - expectParseError '''\ - |class UseContinueAsGoto { - | static main(args) { - | continue label1 - | return - | - | label1: - | println "Groovy supports goto!" - | } - |} - |'''.stripMargin(), '''\ - |continue statement is only allowed inside loops @ line 3, column 6. - | continue label1 - | ^ - | - |1 error - |'''.stripMargin() - } - @Test void 'groovy core - void'() { TestUtils.doRunAndShouldFail('fail/Void_01x.groovy')
