This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new a670c08693 GROOVY-11857: encapsulate ForStatement without value
variable
a670c08693 is described below
commit a670c08693d8ad4882866a506b0588f357fc9bec
Author: Paul King <[email protected]>
AuthorDate: Wed Feb 11 13:13:10 2026 +1000
GROOVY-11857: encapsulate ForStatement without value variable
---
.../apache/groovy/parser/antlr4/AstBuilder.java | 2 +-
.../groovy/ast/ClassCodeExpressionTransformer.java | 5 +-
.../groovy/ast/ClassCodeVisitorSupport.java | 4 +-
.../org/codehaus/groovy/ast/stmt/ForStatement.java | 55 ++++++++++++++++--
.../groovy/classgen/ClassCompletionVerifier.java | 14 +++--
.../asm/sc/StaticTypesStatementWriter.java | 2 +-
.../codehaus/groovy/control/ResolveVisitor.java | 4 +-
.../transform/stc/StaticTypeCheckingVisitor.java | 2 +-
.../codehaus/groovy/ast/builder/AstAssert.groovy | 9 ++-
.../ast/builder/AstBuilderFromCodeTest.groovy | 65 +++++++++++-----------
10 files changed, 112 insertions(+), 50 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 2bd997c6a2..ff07bc7fb1 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -521,7 +521,7 @@ public class AstBuilder extends
GroovyParserBaseVisitor<Object> {
.map(e -> (Expression)
this.visit(e)).orElse(EmptyExpression.INSTANCE));
closureListExpression.addExpression(this.visitForUpdate(ctx.forUpdate()));
- return (body) -> new ForStatement(ForStatement.FOR_LOOP_DUMMY,
closureListExpression, body);
+ return (body) -> new ForStatement(closureListExpression, body);
}
@Override
diff --git
a/src/main/java/org/codehaus/groovy/ast/ClassCodeExpressionTransformer.java
b/src/main/java/org/codehaus/groovy/ast/ClassCodeExpressionTransformer.java
index b2841a6efe..860f962084 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassCodeExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassCodeExpressionTransformer.java
@@ -20,6 +20,7 @@ package org.codehaus.groovy.ast;
import org.codehaus.groovy.ast.expr.BooleanExpression;
import org.codehaus.groovy.ast.expr.ClosureExpression;
+import org.codehaus.groovy.ast.expr.ClosureListExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ExpressionTransformer;
import org.codehaus.groovy.ast.expr.PropertyExpression;
@@ -137,7 +138,9 @@ public abstract class ClassCodeExpressionTransformer
extends ClassCodeVisitorSup
@Override
public void visitForLoop(final ForStatement stmt) {
- visitAnnotations(stmt.getVariable()); // "for(T x : y)" or "for(x in
y)"
+ if (!(stmt.getCollectionExpression() instanceof
ClosureListExpression)) {
+ visitAnnotations(stmt.getValueVariable()); // "for(T x : y)" or
"for(x in y)"
+ }
stmt.setCollectionExpression(transform(stmt.getCollectionExpression()));
stmt.getLoopBlock().visit(this);
}
diff --git a/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java
b/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java
index 083bae6389..845fe0bcb7 100644
--- a/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java
+++ b/src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java
@@ -215,7 +215,9 @@ public abstract class ClassCodeVisitorSupport extends
CodeVisitorSupport impleme
@Override
public void visitForLoop(ForStatement statement) {
visitStatement(statement);
- visitAnnotations(statement.getVariable());
+ if (statement.getValueVariable() != null) {
+ visitAnnotations(statement.getValueVariable());
+ }
super.visitForLoop(statement);
}
diff --git a/src/main/java/org/codehaus/groovy/ast/stmt/ForStatement.java
b/src/main/java/org/codehaus/groovy/ast/stmt/ForStatement.java
index 40d3e5dd13..442557f3c6 100644
--- a/src/main/java/org/codehaus/groovy/ast/stmt/ForStatement.java
+++ b/src/main/java/org/codehaus/groovy/ast/stmt/ForStatement.java
@@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.GroovyCodeVisitor;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.VariableScope;
+import org.codehaus.groovy.ast.expr.ClosureListExpression;
import org.codehaus.groovy.ast.expr.Expression;
import static java.util.Objects.requireNonNull;
@@ -32,24 +33,68 @@ import static java.util.Objects.requireNonNull;
*/
public class ForStatement extends Statement implements LoopingStatement {
- public static final Parameter FOR_LOOP_DUMMY = new
Parameter(ClassHelper.OBJECT_TYPE, "forLoopDummyParameter");
+ private static final Parameter DUMMY_VALUE_VARIABLE = new
Parameter(ClassHelper.OBJECT_TYPE, "forLoopDummyParameter");
+ @Deprecated(since = "6.0.0")
+ public static final Parameter FOR_LOOP_DUMMY = DUMMY_VALUE_VARIABLE;
private final Parameter indexVariable, valueVariable;
private Expression collectionExpression;
private Statement loopBlock;
/**
+ * A constructor of for-in loops (possibly with an optional index
variable).
+ * An example with an index variable:
+ * <pre>
+ * for (int idx, int i in 10..12) {
+ * println "$idx $i"
+ * }
+ * </pre>
+ *
* @since 5.0.0
*/
public ForStatement(final Parameter indexVariable, final Parameter
valueVariable, final Expression collectionExpression, final Statement
loopBlock) {
- this.indexVariable = indexVariable; // optional component
+ this.indexVariable = indexVariable; // null implies no index variable
this.valueVariable = requireNonNull(valueVariable);
setCollectionExpression(collectionExpression);
setLoopBlock(loopBlock);
}
- public ForStatement(final Parameter variable, final Expression
collectionExpression, final Statement loopBlock) {
- this(null, variable, collectionExpression, loopBlock);
+ /**
+ * A constructor of for-in loops without an index variable.
+ * Variants using Java-style ":" or the Groovy reserved word "in" are
equivalent:
+ * <pre>
+ * for (int i : 0..2) {
+ * println i
+ * }
+ * for (int j in 5..7) {
+ * println j
+ * }
+ * </pre>
+ *
+ * @since 5.0.0
+ */
+ public ForStatement(final Parameter valueVariable, final Expression
collectionExpression, final Statement loopBlock) {
+ this(null, valueVariable, collectionExpression, loopBlock);
+ }
+
+ /**
+ * A constructor of classic (C-style) for loops.
+ * <pre>
+ * for (int i = 20; i < 23; i++) {
+ * println i
+ * }
+ * </pre>
+ * Also handles multi-assignment for loops.
+ * <pre>
+ * for (def (String i, int j) = ['a', 30]; j < 33; i++, j++) {
+ * println "$i $j"
+ * }
+ * </pre>
+ *
+ * @since 6.0.0
+ */
+ public ForStatement(final ClosureListExpression closureListExpression,
final Statement loopBlock) {
+ this(DUMMY_VALUE_VARIABLE, closureListExpression, loopBlock);
}
public void setCollectionExpression(final Expression collectionExpression)
{
@@ -74,7 +119,7 @@ public class ForStatement extends Statement implements
LoopingStatement {
* @since 5.0.0
*/
public Parameter getValueVariable() {
- return valueVariable != FOR_LOOP_DUMMY ? valueVariable : null;
+ return valueVariable != DUMMY_VALUE_VARIABLE ? valueVariable : null;
}
@Deprecated(since = "5.0.0")
diff --git
a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index e37faad013..48f6425b83 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -792,9 +792,15 @@ out: for (ClassNode sc : superTypes) {
@Override
public void visitForLoop(final ForStatement fs) {
- List<String> modifiers = new ArrayList<>();
- int mods = fs.getVariable().getModifiers();
+ if (fs.getValueVariable() != null) {
+ checkModifiers(fs, fs.getValueVariable());
+ }
+ super.visitForLoop(fs);
+ }
+ private void checkModifiers(ForStatement fs, Parameter p) {
+ List<String> modifiers = new ArrayList<>();
+ int mods = p.getModifiers();
if (isAbstract (mods)) modifiers.add("abstract" );
if (isPrivate (mods)) modifiers.add("private" );
if (isProtected(mods)) modifiers.add("protected");
@@ -803,10 +809,8 @@ out: for (ClassNode sc : superTypes) {
if (isStrict (mods)) modifiers.add("strictfp" );
for (String modifier : modifiers) {
- addError("The variable '" + fs.getVariable().getName() + "' has
invalid modifier " + modifier + ".", fs);
+ addError("The variable '" + p.getName() + "' has invalid modifier
" + modifier + ".", fs);
}
-
- super.visitForLoop(fs);
}
@Override
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
index d9fdd428ae..76c0b50455 100644
---
a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
+++
b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java
@@ -97,7 +97,7 @@ public class StaticTypesStatementWriter extends
StatementWriter {
ClassNode collectionType =
controller.getTypeChooser().resolveType(collectionExpression,
controller.getClassNode());
int mark = operandStack.getStackLength();
- if (collectionType.isArray() &&
collectionType.getComponentType().equals(loop.getVariableType())) {
+ if (collectionType.isArray() &&
collectionType.getComponentType().equals(loop.getValueVariable().getType())) {
writeOptimizedForEachLoop(loop, collectionExpression,
collectionType);
} else if (GeneralUtils.isOrImplements(collectionType,
ENUMERATION_CLASSNODE)) {
writeEnumerationBasedForEachLoop(loop, collectionExpression,
collectionType);
diff --git a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
index f223cb6521..10d6670d35 100644
--- a/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
+++ b/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java
@@ -1392,7 +1392,9 @@ public class ResolveVisitor extends
ClassCodeExpressionTransformer {
@Override
public void visitForLoop(final ForStatement forLoop) {
- resolveOrFail(forLoop.getVariableType(), forLoop);
+ if (forLoop.getValueVariable() != null) {
+ resolveOrFail(forLoop.getValueVariable().getType(), forLoop);
+ }
super.visitForLoop(forLoop);
}
diff --git
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 6f1a101fb1..2c783c99f7 100644
---
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -2176,7 +2176,7 @@ out: if ((samParameterTypes.length == 1 &&
isOrImplements(samParameterTypes[0
visitStatement(forLoop);
collectionExpression.visit(this);
ClassNode collectionType = getType(collectionExpression);
- ClassNode forLoopVariableType = forLoop.getVariableType();
+ ClassNode forLoopVariableType =
forLoop.getValueVariable().getType();
ClassNode componentType;
if (isStringType(collectionType)) {
if (isWrapperCharacter(getWrapper(forLoopVariableType))) {
diff --git
a/src/testFixtures/groovy/org/codehaus/groovy/ast/builder/AstAssert.groovy
b/src/testFixtures/groovy/org/codehaus/groovy/ast/builder/AstAssert.groovy
index cfbdedc8ed..6a6f95faae 100644
--- a/src/testFixtures/groovy/org/codehaus/groovy/ast/builder/AstAssert.groovy
+++ b/src/testFixtures/groovy/org/codehaus/groovy/ast/builder/AstAssert.groovy
@@ -237,7 +237,14 @@ class AstAssert {
assertSyntaxTree([expected.arguments], [actual.arguments])
},
ForStatement : { expected, actual ->
- assertSyntaxTree([expected.variable], [actual.variable])
+ assert expected.valueVariable.asBoolean() ==
actual.valueVariable.asBoolean()
+ if (expected.valueVariable) {
+ assertSyntaxTree([expected.valueVariable],
[actual.valueVariable])
+ assert expected.indexVariable.asBoolean() ==
actual.indexVariable.asBoolean()
+ if (expected.indexVariable) {
+ assertSyntaxTree([expected.indexVariable],
[actual.indexVariable])
+ }
+ }
assertSyntaxTree([expected.collectionExpression],
[actual.collectionExpression])
assertSyntaxTree([expected.loopBlock], [actual.loopBlock])
},
diff --git
a/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/ast/builder/AstBuilderFromCodeTest.groovy
b/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/ast/builder/AstBuilderFromCodeTest.groovy
index 682aac8db9..547766e8a2 100644
---
a/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/ast/builder/AstBuilderFromCodeTest.groovy
+++
b/subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/ast/builder/AstBuilderFromCodeTest.groovy
@@ -413,39 +413,38 @@ class AstBuilderFromCodeTest {
}
def expected = new BlockStatement([new ForStatement(
- new Parameter(ClassHelper.OBJECT_TYPE,
"forLoopDummyParameter"),
- new ClosureListExpression(
- [
- new DeclarationExpression(
- new VariableExpression("x"),
- new Token(Types.EQUALS, "=", -1, -1),
- new ConstantExpression(0)
- ),
- new BinaryExpression(
- new VariableExpression("x"),
- new Token(Types.COMPARE_LESS_THAN,
"<", -1, -1),
- new ConstantExpression(10)
- ),
- new PostfixExpression(
- new VariableExpression("x"),
- new Token(Types.PLUS_PLUS, "++", -1,
-1)
- )
- ]
- ),
- new BlockStatement(
- [
- new ExpressionStatement(
- new MethodCallExpression(
- new VariableExpression("this"),
- new
ConstantExpression("println"),
- new ArgumentListExpression(
- new
VariableExpression("x"),
- )
- )
- )
- ],
- new VariableScope()
- )
+ new ClosureListExpression(
+ [
+ new DeclarationExpression(
+ new VariableExpression("x"),
+ new Token(Types.EQUALS, "=", -1, -1),
+ new ConstantExpression(0)
+ ),
+ new BinaryExpression(
+ new VariableExpression("x"),
+ new Token(Types.COMPARE_LESS_THAN, "<", -1, -1),
+ new ConstantExpression(10)
+ ),
+ new PostfixExpression(
+ new VariableExpression("x"),
+ new Token(Types.PLUS_PLUS, "++", -1, -1)
+ )
+ ]
+ ),
+ new BlockStatement(
+ [
+ new ExpressionStatement(
+ new MethodCallExpression(
+ new VariableExpression("this"),
+ new ConstantExpression("println"),
+ new ArgumentListExpression(
+ new VariableExpression("x"),
+ )
+ )
+ )
+ ],
+ new VariableScope()
+ )
)], new VariableScope())
AstAssert.assertSyntaxTree([expected], result)
