Skip to content

Commit 47041dc

Browse files
paulk-asertpaulk-asert
committed
GROOVY-11857: encapsulate ForStatement without value variable
1 parent 92643ee commit 47041dc

10 files changed

Lines changed: 69 additions & 47 deletions

File tree

‎src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ public Function<Statement, ForStatement> visitOriginalForControl(final OriginalF
520520
.map(e -> (Expression) this.visit(e)).orElse(EmptyExpression.INSTANCE));
521521
closureListExpression.addExpression(this.visitForUpdate(ctx.forUpdate()));
522522

523-
return (body) -> new ForStatement(ForStatement.FOR_LOOP_DUMMY, closureListExpression, body);
523+
return (body) -> new ForStatement(closureListExpression, body);
524524
}
525525

526526
@Override

‎src/main/java/org/codehaus/groovy/ast/ClassCodeExpressionTransformer.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@ public void visitExpressionStatement(final ExpressionStatement stmt) {
137137

138138
@Override
139139
public void visitForLoop(final ForStatement stmt) {
140-
visitAnnotations(stmt.getVariable()); // "for(T x : y)" or "for(x in y)"
140+
if (stmt.getValueVariable() != null) {
141+
visitAnnotations(stmt.getValueVariable()); // "for(T x : y)" or "for(x in y)"
142+
}
141143
stmt.setCollectionExpression(transform(stmt.getCollectionExpression()));
142144
stmt.getLoopBlock().visit(this);
143145
}

‎src/main/java/org/codehaus/groovy/ast/ClassCodeVisitorSupport.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,9 @@ public void visitExpressionStatement(ExpressionStatement statement) {
215215
@Override
216216
public void visitForLoop(ForStatement statement) {
217217
visitStatement(statement);
218-
visitAnnotations(statement.getVariable());
218+
if (statement.getValueVariable() != null) {
219+
visitAnnotations(statement.getValueVariable());
220+
}
219221
super.visitForLoop(statement);
220222
}
221223

‎src/main/java/org/codehaus/groovy/ast/stmt/ForStatement.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@
3232
*/
3333
public class ForStatement extends Statement implements LoopingStatement {
3434

35-
public static final Parameter FOR_LOOP_DUMMY = new Parameter(ClassHelper.OBJECT_TYPE, "forLoopDummyParameter");
35+
private static final Parameter DUMMY_VALUE_VARIABLE = new Parameter(ClassHelper.OBJECT_TYPE, "forLoopDummyParameter");
36+
@Deprecated(since = "6.0.0")
37+
public static final Parameter FOR_LOOP_DUMMY = DUMMY_VALUE_VARIABLE;
3638

3739
private final Parameter indexVariable, valueVariable;
3840
private Expression collectionExpression;
@@ -52,6 +54,13 @@ public ForStatement(final Parameter variable, final Expression collectionExpress
5254
this(null, variable, collectionExpression, loopBlock);
5355
}
5456

57+
/**
58+
* @since 6.0.0
59+
*/
60+
public ForStatement(final Expression collectionExpression, final Statement loopBlock) {
61+
this(DUMMY_VALUE_VARIABLE, collectionExpression, loopBlock);
62+
}
63+
5564
public void setCollectionExpression(final Expression collectionExpression) {
5665
this.collectionExpression = requireNonNull(collectionExpression);
5766
}
@@ -74,7 +83,7 @@ public Parameter getIndexVariable() {
7483
* @since 5.0.0
7584
*/
7685
public Parameter getValueVariable() {
77-
return valueVariable != FOR_LOOP_DUMMY ? valueVariable : null;
86+
return valueVariable != DUMMY_VALUE_VARIABLE ? valueVariable : null;
7887
}
7988

8089
@Deprecated(since = "5.0.0")

‎src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -792,9 +792,15 @@ public void visitCatchStatement(final CatchStatement cs) {
792792

793793
@Override
794794
public void visitForLoop(final ForStatement fs) {
795-
List<String> modifiers = new ArrayList<>();
796-
int mods = fs.getVariable().getModifiers();
795+
if (fs.getValueVariable() != null) {
796+
checkModifiers(fs, fs.getValueVariable());
797+
}
798+
super.visitForLoop(fs);
799+
}
797800

801+
private void checkModifiers(ForStatement fs, Parameter p) {
802+
List<String> modifiers = new ArrayList<>();
803+
int mods = p.getModifiers();
798804
if (isAbstract (mods)) modifiers.add("abstract" );
799805
if (isPrivate (mods)) modifiers.add("private" );
800806
if (isProtected(mods)) modifiers.add("protected");
@@ -803,10 +809,8 @@ public void visitForLoop(final ForStatement fs) {
803809
if (isStrict (mods)) modifiers.add("strictfp" );
804810

805811
for (String modifier : modifiers) {
806-
addError("The variable '" + fs.getVariable().getName() + "' has invalid modifier " + modifier + ".", fs);
812+
addError("The variable '" + p.getName() + "' has invalid modifier " + modifier + ".", fs);
807813
}
808-
809-
super.visitForLoop(fs);
810814
}
811815

812816
@Override

‎src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesStatementWriter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ protected void writeForInLoop(final ForStatement loop) {
9797
ClassNode collectionType = controller.getTypeChooser().resolveType(collectionExpression, controller.getClassNode());
9898

9999
int mark = operandStack.getStackLength();
100-
if (collectionType.isArray() && collectionType.getComponentType().equals(loop.getVariableType())) {
100+
if (collectionType.isArray() && collectionType.getComponentType().equals(loop.getValueVariable().getType())) {
101101
writeOptimizedForEachLoop(loop, collectionExpression, collectionType);
102102
} else if (GeneralUtils.isOrImplements(collectionType, ENUMERATION_CLASSNODE)) {
103103
writeEnumerationBasedForEachLoop(loop, collectionExpression, collectionType);

‎src/main/java/org/codehaus/groovy/control/ResolveVisitor.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1392,7 +1392,9 @@ public void visitCatchStatement(final CatchStatement cs) {
13921392

13931393
@Override
13941394
public void visitForLoop(final ForStatement forLoop) {
1395-
resolveOrFail(forLoop.getVariableType(), forLoop);
1395+
if (forLoop.getValueVariable() != null) {
1396+
resolveOrFail(forLoop.getValueVariable().getType(), forLoop);
1397+
}
13961398
super.visitForLoop(forLoop);
13971399
}
13981400

‎src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2176,7 +2176,7 @@ public void visitForLoop(final ForStatement forLoop) {
21762176
visitStatement(forLoop);
21772177
collectionExpression.visit(this);
21782178
ClassNode collectionType = getType(collectionExpression);
2179-
ClassNode forLoopVariableType = forLoop.getVariableType();
2179+
ClassNode forLoopVariableType = forLoop.getValueVariable().getType();
21802180
ClassNode componentType;
21812181
if (isStringType(collectionType)) {
21822182
if (isWrapperCharacter(getWrapper(forLoopVariableType))) {

‎src/testFixtures/groovy/org/codehaus/groovy/ast/builder/AstAssert.groovy‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,11 @@ class AstAssert {
237237
assertSyntaxTree([expected.arguments], [actual.arguments])
238238
},
239239
ForStatement : { expected, actual ->
240-
assertSyntaxTree([expected.variable], [actual.variable])
240+
assert expected.valueVariable.asBoolean() == actual.valueVariable.asBoolean()
241+
if (expected.valueVariable) {
242+
assertSyntaxTree([expected.valueVariable], [actual.valueVariable])
243+
}
244+
assertSyntaxTree([expected.indexVariable], [actual.indexVariable])
241245
assertSyntaxTree([expected.collectionExpression], [actual.collectionExpression])
242246
assertSyntaxTree([expected.loopBlock], [actual.loopBlock])
243247
},

‎subprojects/groovy-astbuilder/src/test/groovy/org/codehaus/groovy/ast/builder/AstBuilderFromCodeTest.groovy‎

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -386,39 +386,38 @@ class AstBuilderFromCodeTest extends GroovyTestCase {
386386
}
387387

388388
def expected = new BlockStatement([new ForStatement(
389-
new Parameter(ClassHelper.OBJECT_TYPE, "forLoopDummyParameter"),
390-
new ClosureListExpression(
391-
[
392-
new DeclarationExpression(
393-
new VariableExpression("x"),
394-
new Token(Types.EQUALS, "=", -1, -1),
395-
new ConstantExpression(0)
396-
),
397-
new BinaryExpression(
398-
new VariableExpression("x"),
399-
new Token(Types.COMPARE_LESS_THAN, "<", -1, -1),
400-
new ConstantExpression(10)
401-
),
402-
new PostfixExpression(
403-
new VariableExpression("x"),
404-
new Token(Types.PLUS_PLUS, "++", -1, -1)
405-
)
406-
]
407-
),
408-
new BlockStatement(
409-
[
410-
new ExpressionStatement(
411-
new MethodCallExpression(
412-
new VariableExpression("this"),
413-
new ConstantExpression("println"),
414-
new ArgumentListExpression(
415-
new VariableExpression("x"),
416-
)
417-
)
418-
)
419-
],
420-
new VariableScope()
421-
)
389+
new ClosureListExpression(
390+
[
391+
new DeclarationExpression(
392+
new VariableExpression("x"),
393+
new Token(Types.EQUALS, "=", -1, -1),
394+
new ConstantExpression(0)
395+
),
396+
new BinaryExpression(
397+
new VariableExpression("x"),
398+
new Token(Types.COMPARE_LESS_THAN, "<", -1, -1),
399+
new ConstantExpression(10)
400+
),
401+
new PostfixExpression(
402+
new VariableExpression("x"),
403+
new Token(Types.PLUS_PLUS, "++", -1, -1)
404+
)
405+
]
406+
),
407+
new BlockStatement(
408+
[
409+
new ExpressionStatement(
410+
new MethodCallExpression(
411+
new VariableExpression("this"),
412+
new ConstantExpression("println"),
413+
new ArgumentListExpression(
414+
new VariableExpression("x"),
415+
)
416+
)
417+
)
418+
],
419+
new VariableScope()
420+
)
422421
)], new VariableScope())
423422

424423
AstAssert.assertSyntaxTree([expected], result)

0 commit comments

Comments
 (0)