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
The following commit(s) were added to refs/heads/GROOVY-7463 by this push:
new 41ab74b61b GROOVY-7617: limit label scope
41ab74b61b is described below
commit 41ab74b61b8a9070cace3bcafaa39187580f87e1
Author: Eric Milles <[email protected]>
AuthorDate: Tue Jan 20 09:28:01 2026 -0600
GROOVY-7617: limit label scope
---
.../codehaus/groovy/ast/stmt/ThrowStatement.java | 10 +-
.../org/codehaus/groovy/control/LabelVerifier.java | 179 +++++++++--
.../groovy/groovy/BreakContinueLabelTest.groovy | 336 ++++++++++++---------
3 files changed, 352 insertions(+), 173 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/stmt/ThrowStatement.java
b/src/main/java/org/codehaus/groovy/ast/stmt/ThrowStatement.java
index ca9632f337..316ba87131 100644
--- a/src/main/java/org/codehaus/groovy/ast/stmt/ThrowStatement.java
+++ b/src/main/java/org/codehaus/groovy/ast/stmt/ThrowStatement.java
@@ -23,7 +23,7 @@ import org.codehaus.groovy.ast.expr.Expression;
/**
- * Represents a throw statement
+ * Represents a throw statement.
*/
public class ThrowStatement extends Statement {
@@ -37,10 +37,6 @@ public class ThrowStatement extends Statement {
return expression;
}
- @Override
- public void visit(GroovyCodeVisitor visitor) {
- visitor.visitThrowStatement(this);
- }
public void setExpression(Expression expression) {
this.expression = expression;
}
@@ -50,4 +46,8 @@ public class ThrowStatement extends Statement {
return "throw " + expression.getText();
}
+ @Override
+ public void visit(GroovyCodeVisitor visitor) {
+ visitor.visitThrowStatement(this);
+ }
}
diff --git a/src/main/java/org/codehaus/groovy/control/LabelVerifier.java
b/src/main/java/org/codehaus/groovy/control/LabelVerifier.java
index 9f53340114..5aa081fe7d 100644
--- a/src/main/java/org/codehaus/groovy/control/LabelVerifier.java
+++ b/src/main/java/org/codehaus/groovy/control/LabelVerifier.java
@@ -19,30 +19,40 @@
package org.codehaus.groovy.control;
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
+import org.codehaus.groovy.ast.stmt.AssertStatement;
+import org.codehaus.groovy.ast.stmt.BlockStatement;
import org.codehaus.groovy.ast.stmt.BreakStatement;
+import org.codehaus.groovy.ast.stmt.CaseStatement;
+import org.codehaus.groovy.ast.stmt.CatchStatement;
import org.codehaus.groovy.ast.stmt.ContinueStatement;
import org.codehaus.groovy.ast.stmt.DoWhileStatement;
+import org.codehaus.groovy.ast.stmt.EmptyStatement;
+import org.codehaus.groovy.ast.stmt.ExpressionStatement;
import org.codehaus.groovy.ast.stmt.ForStatement;
import org.codehaus.groovy.ast.stmt.IfStatement;
+import org.codehaus.groovy.ast.stmt.ReturnStatement;
import org.codehaus.groovy.ast.stmt.Statement;
import org.codehaus.groovy.ast.stmt.SwitchStatement;
+import org.codehaus.groovy.ast.stmt.SynchronizedStatement;
+import org.codehaus.groovy.ast.stmt.ThrowStatement;
+import org.codehaus.groovy.ast.stmt.TryCatchStatement;
import org.codehaus.groovy.ast.stmt.WhileStatement;
+import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
import java.util.Set;
/**
* This class checks the handling of labels in the AST
*/
-public class LabelVerifier extends ClassCodeVisitorSupport {
+public class LabelVerifier extends PrePostStatementVisitor {
boolean inIf, inLoop, inSwitch;
private final SourceUnit source;
- private Set<String> visitedLabels;
- private List<BreakStatement> breakLabels;
- private List<ContinueStatement> continueLabels;
+ private Set<String> availableLabels;
+ private Map<String, Statement> undefinedLabels;
public LabelVerifier(SourceUnit src) {
source = src;
@@ -54,25 +64,25 @@ public class LabelVerifier extends ClassCodeVisitorSupport {
}
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);
+ for (Map.Entry<String, Statement> entry : undefinedLabels.entrySet()) {
+ String kind = (entry.getValue() instanceof BreakStatement ?
"break" : "continue");
+ addError(String.format("cannot %s to label '%s' from here", kind,
entry.getKey()), entry.getValue());
}
}
@Override
- public void visitStatement(Statement s) {
+ protected void visitStatement(Statement s) {
List<String> labels = s.getStatementLabels();
- if (labels != null && visitedLabels != null) {
- for (String label : labels) {
- visitedLabels.add(label);
+ if (labels != null) {
+ availableLabels.addAll(labels);
+ }
+ }
- breakLabels.removeIf(breakStatement ->
breakStatement.getLabel().equals(label));
- continueLabels.removeIf(continueStatement ->
continueStatement.getLabel().equals(label));
- }
+ @Override
+ protected void postVisitStatement(Statement s) {
+ List<String> labels = s.getStatementLabels();
+ if (labels != null) {
+ availableLabels.removeAll(labels); // GROOVY-7617
}
}
@@ -82,9 +92,8 @@ public class LabelVerifier extends ClassCodeVisitorSupport {
inLoop = false;
inSwitch = false;
- visitedLabels = new HashSet<>();
- breakLabels = new LinkedList<>();
- continueLabels = new LinkedList<>();
+ availableLabels = new HashSet<>();
+ undefinedLabels = new HashMap<>();
super.visitClassCodeContainer(code);
@@ -102,6 +111,7 @@ public class LabelVerifier extends ClassCodeVisitorSupport {
cond.getIfBlock().visit(this);
cond.getElseBlock().visit(this);
inIf = oldInIf;
+ postVisitStatement(cond);
}
@Override
@@ -112,6 +122,7 @@ public class LabelVerifier extends ClassCodeVisitorSupport {
inLoop = true;
loop.getLoopBlock().visit(this);
inLoop = oldInLoop;
+ postVisitStatement(loop);
}
@Override
@@ -122,6 +133,7 @@ public class LabelVerifier extends ClassCodeVisitorSupport {
inLoop = true;
loop.getLoopBlock().visit(this);
inLoop = oldInLoop;
+ postVisitStatement(loop);
}
@Override
@@ -132,6 +144,7 @@ public class LabelVerifier extends ClassCodeVisitorSupport {
loop.getLoopBlock().visit(this);
inLoop = oldInLoop;
loop.getBooleanExpression().visit(this);
+ postVisitStatement(loop);
}
@Override
@@ -143,6 +156,7 @@ public class LabelVerifier extends ClassCodeVisitorSupport {
switchStatement.getCaseStatements().forEach(this::visit);
switchStatement.getDefaultStatement().visit(this);
inSwitch = oldInSwitch;
+ postVisitStatement(switchStatement);
}
@Override
@@ -156,11 +170,10 @@ public class LabelVerifier extends
ClassCodeVisitorSupport {
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 (!availableLabels.contains(label)) {
+ undefinedLabels.put(label, breakStatement);
}
}
- super.visitBreakStatement(breakStatement);
}
@Override
@@ -170,10 +183,122 @@ public class LabelVerifier extends
ClassCodeVisitorSupport {
}
String label = continueStatement.getLabel();
if (label != null) {
- if (!visitedLabels.contains(label)) {
- continueLabels.add(continueStatement);
+ if (!availableLabels.contains(label)) {
+ undefinedLabels.put(label, continueStatement);
}
}
- super.visitContinueStatement(continueStatement);
+ }
+}
+
+//------------------------------------------------------------------------------
+
+abstract class PrePostStatementVisitor extends ClassCodeVisitorSupport {
+
+ @Override
+ protected abstract void visitStatement(Statement statement);
+
+ protected abstract void postVisitStatement(Statement statement);
+
+ @Override
+ public void visitAssertStatement(AssertStatement statement) {
+ super.visitAssertStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitBlockStatement(BlockStatement statement) {
+ super.visitBlockStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitBreakStatement(BreakStatement statement) {
+ super.visitBreakStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitCaseStatement(CaseStatement statement) {
+ super.visitCaseStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitCatchStatement(CatchStatement statement) {
+ super.visitCatchStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitContinueStatement(ContinueStatement statement) {
+ super.visitContinueStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitDoWhileLoop(DoWhileStatement statement) {
+ super.visitDoWhileLoop(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitEmptyStatement(EmptyStatement statement) {
+ visitStatement(statement); // TODO: Move to super?
+ super.visitEmptyStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitExpressionStatement(ExpressionStatement statement) {
+ super.visitExpressionStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitForLoop(ForStatement statement) {
+ super.visitForLoop(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitIfElse(IfStatement statement) {
+ super.visitIfElse(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitReturnStatement(ReturnStatement statement) {
+ super.visitReturnStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitSwitch(SwitchStatement statement) {
+ super.visitSwitch(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitSynchronizedStatement(SynchronizedStatement statement) {
+ super.visitSynchronizedStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitThrowStatement(ThrowStatement statement) {
+ super.visitThrowStatement(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitTryCatchFinally(TryCatchStatement statement) {
+ super.visitTryCatchFinally(statement);
+ postVisitStatement(statement);
+ }
+
+ @Override
+ public void visitWhileLoop(WhileStatement statement) {
+ super.visitWhileLoop(statement);
+ postVisitStatement(statement);
}
}
diff --git a/src/test/groovy/groovy/BreakContinueLabelTest.groovy
b/src/test/groovy/groovy/BreakContinueLabelTest.groovy
index 5788aaba65..a865e009ea 100644
--- a/src/test/groovy/groovy/BreakContinueLabelTest.groovy
+++ b/src/test/groovy/groovy/BreakContinueLabelTest.groovy
@@ -20,192 +20,240 @@ package groovy
import org.junit.jupiter.api.Test
+import static groovy.test.GroovyAssert.assertScript
import static groovy.test.GroovyAssert.shouldFail
-import static org.junit.jupiter.api.Assertions.fail
final class BreakContinueLabelTest {
@Test
void testDeclareSimpleLabels() {
- label_1: print('foo')
- label_2:
- print('bar')
+ assertScript '''
+ label_1: print('foo')
+ label_2:
+ print('bar')
+ '''
}
// GROOVY-7463
@Test
void testBreakLabelInIfStatement() {
- boolean flag = true
- label:
- if (flag) {
- print('foo')
+ assertScript '''
+ boolean flag = true
+ label:
if (flag) {
- break label
- fail()
+ print('foo')
+ if (flag) {
+ break label
+ assert false
+ }
+ assert false
+ } else {
+ assert false
}
- fail()
- } else {
- fail()
- }
- print('bar')
+ 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()
+ assertScript '''
+ int i = 0
+ boolean flag = true
+ label:
+ if (flag) {
+ print('foo')
+ try {
+ if (flag) {
+ print('bar')
+ break label;
+ assert false
+ }
+ assert false
+ } finally {
+ i += 1
}
- fail()
- } finally {
- i += 1
+ assert false
}
- fail()
- }
- print('baz')
- assert i == 1
+ print('baz')
+ assert i == 1
+ '''
}
@Test
void testBreakLabelInSimpleForLoop() {
- label: for (i in [1]) {
- break label;
- assert false
- }
+ assertScript '''
+ label: for (i in [1]) {
+ break label;
+ assert false
+ }
+ '''
+ }
+
+ // GROOVY-7617
+ @Test
+ void testBreakLabelInSecondForLoop() {
+ def err = shouldFail '''
+ @groovy.transform.TimedInterrupt(1)
+ void test() {
+ first:
+ for (i in [1]) {
+ break first // okay
+ assert false
+ }
+ for (j in [2]) {
+ break first // fail
+ assert false
+ }
+ }
+
+ test()
+ '''
+ assert err =~ /cannot break to label 'first' from here/
}
@Test
void testBreakLabelInNestedForLoop() {
- label: for (i in [1]) {
- for (j in [1]) {
- break label;
- assert false : 'did not break inner loop'
+ assertScript '''
+ label: for (i in [1]) {
+ for (j in [1]) {
+ break label;
+ assert false : 'did not break inner loop'
+ }
+ assert false : 'did not break outer loop'
}
- assert false : 'did not break outer loop'
- }
+ '''
}
@Test
void testBreakLabelInForLoopTryFinally() {
- int i = 0
- out:
- for (j in 1..2) {
- try {
+ assertScript '''
+ int i = 0
+ out:
+ for (j in 1..2) {
try {
- break out
+ try {
+ break out
+ assert false
+ } finally {
+ i += 10
+ }
assert false
} finally {
- i += 10
+ i += 1
}
assert false
- } finally {
- i += 1
}
- assert false
- }
- assert i == 11
+ assert i == 11
+ '''
}
@Test
void testBreakInNestedForLoop() {
- def reached = false
- for (i in [1]) {
- for (j in [1]) {
- break
- assert false : 'did not break inner loop'
+ assertScript '''
+ def reached = false
+ for (i in [1]) {
+ for (j in [1]) {
+ break
+ assert false : 'did not break inner loop'
+ }
+ reached = true
}
- reached = true
- }
- assert reached : 'must not break outer loop'
+ assert reached : 'must not break outer loop'
+ '''
}
@Test
void testBreakLabelInSimpleWhileLoop() {
- label_1: while (true) {
- break label_1
- assert false
- }
+ assertScript '''
+ label: while (true) {
+ break label;
+ assert false
+ }
+ '''
}
@Test
void testBreakLabelInNestedWhileLoop() {
- def count = 0
- label: while (count < 1) {
- count += 1
- while (true) {
- break label
- assert false : 'did not break inner loop'
+ assertScript '''
+ def count = 0
+ label: while (count < 1) {
+ count += 1
+ while (true) {
+ break label
+ assert false : 'did not break inner loop'
+ }
+ assert false : 'did not break outer loop'
}
- assert false : 'did not break outer loop'
- }
+ '''
}
@Test
void testBreakLabelInNestedMixedForAndWhileLoop() {
- def count = 0
- label_1: while (count < 1) {
- count += 1
- for (i in [1]) {
- break label_1
- assert false : 'did not break inner loop'
+ assertScript '''
+ def count = 0
+ label_1: while (count < 1) {
+ count += 1
+ for (i in [1]) {
+ break label_1
+ assert false : 'did not break inner loop'
+ }
+ assert false : 'did not break outer loop'
}
- assert false : 'did not break outer loop'
- }
- label_2: for (i in [1]) {
- while (true) {
- break label_2
- assert false : 'did not break inner loop'
+ label_2: for (i in [1]) {
+ while (true) {
+ break label_2
+ assert false : 'did not break inner loop'
+ }
+ assert false : 'did not break outer loop'
}
- assert false : 'did not break outer loop'
- }
+ '''
}
// GROOVY-11739
@Test
void testContinueWithinDoWhileLoop() {
- int i = 0
- do {
- i += 1
- if (i > 1000) break // prevent infinite loop
- continue // control should pass to condition
- } while (i < 100)
+ assertScript '''
+ int i = 0
+ do {
+ i += 1
+ if (i > 1000) break // prevent infinite loop
+ continue // control should pass to condition
+ } while (i < 100)
- assert i == 100
+ assert i == 100
+ '''
}
@Test
void testContinueInNestedForLoop() {
- def log = ''
- for (i in [1,2]) {
- log += i
- for (j in [3,4]) {
- if (j==3) continue
- log += j
+ assertScript '''
+ def log = ''
+ for (i in [1,2]) {
+ log += i
+ for (j in [3,4]) {
+ if (j==3) continue
+ log += j
+ }
}
- }
- assert log == '1424'
+ assert log == '1424'
+ '''
}
@Test
void testContinueLabelInNestedForLoop() {
- def log = ''
- label: for (i in [1,2]) {
- log += i
- for (j in [3,4]) {
- if (j==4) continue label
- log += j
+ assertScript '''
+ def log = ''
+ label: for (i in [1,2]) {
+ log += i
+ for (j in [3,4]) {
+ if (j==4) continue label
+ log += j
+ }
+ log += 'never reached'
}
- log += 'never reached'
- }
- assert log == '1323'
+ assert log == '1323'
+ '''
}
// GROOVY-3908
@@ -229,44 +277,50 @@ final class BreakContinueLabelTest {
@Test
void testBreakToLastLabelSucceeds() {
- one:
- two:
- three:
- for (i in 1..2) {
- break three
- fail()
- }
+ assertScript '''
+ one:
+ two:
+ three:
+ for (i in 1..2) {
+ break three
+ assert false
+ }
+ '''
}
@Test
void testMultipleLabelSupport() {
- def visited = []
- label1:
- label2:
- label3:
- for (int i = 0; i < 9; i++) {
- visited << i
- if (i == 1) continue label1
- visited << 10 + i
- if (i == 3) continue label2
- visited << 100 + i
- if (i == 5) break label3
- }
- assert visited == [0, 10, 100, 1, 2, 12, 102, 3, 13, 4, 14, 104, 5,
15, 105]
+ assertScript '''
+ def visited = []
+ label1:
+ label2:
+ label3:
+ for (int i = 0; i < 9; i++) {
+ visited << i
+ if (i == 1) continue label1
+ visited << 10 + i
+ if (i == 3) continue label2
+ visited << 100 + i
+ if (i == 5) break label3
+ }
+ assert visited == [0, 10, 100, 1, 2, 12, 102, 3, 13, 4, 14, 104,
5, 15, 105]
+ '''
}
// this is in accordance with Java; Spock Framework relies on this
@Test
void testLabelCanOccurMultipleTimesInSameScope() {
- one:
- for (i in 1..2) {
- break one
- fail()
- }
- one:
- for (i in 1..2) {
- break one
- fail()
- }
+ assertScript '''
+ one:
+ for (i in 1..2) {
+ break one
+ assert false
+ }
+ one:
+ for (i in 1..2) {
+ break one
+ assert false
+ }
+ '''
}
}
