This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-10683 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 8261b961f651bbb0a02e949482b72fd9dd5c64f2 Author: Eric Milles <[email protected]> AuthorDate: Sun Mar 30 10:08:48 2025 -0500 `for` loop parser refactor --- src/antlr/GroovyLexer.g4 | 15 +-- src/antlr/GroovyParser.g4 | 4 +- .../apache/groovy/parser/antlr4/AstBuilder.java | 56 ++++++----- .../org/codehaus/groovy/ast/stmt/ForStatement.java | 55 +++++----- src/spec/test/SemanticsTest.groovy | 111 ++++++++++++--------- 5 files changed, 131 insertions(+), 110 deletions(-) diff --git a/src/antlr/GroovyLexer.g4 b/src/antlr/GroovyLexer.g4 index 3c8f2074a3..79ddf78dbd 100644 --- a/src/antlr/GroovyLexer.g4 +++ b/src/antlr/GroovyLexer.g4 @@ -393,9 +393,6 @@ IN : 'in'; TRAIT : 'trait'; THREADSAFE : 'threadsafe'; // reserved keyword -// the reserved type name of Java10 -VAR : 'var'; - // §3.9 Keywords BuiltInPrimitiveType : BOOLEAN @@ -415,7 +412,6 @@ fragment BOOLEAN : 'boolean'; BREAK : 'break'; -YIELD : 'yield'; fragment BYTE : 'byte'; @@ -444,41 +440,35 @@ FINALLY : 'finally'; fragment FLOAT : 'float'; - FOR : 'for'; IF : 'if'; GOTO : 'goto'; IMPLEMENTS : 'implements'; IMPORT : 'import'; INSTANCEOF : 'instanceof'; +INTERFACE : 'interface'; fragment INT : 'int'; -INTERFACE : 'interface'; - fragment LONG : 'long'; NATIVE : 'native'; NEW : 'new'; NON_SEALED : 'non-sealed'; - PACKAGE : 'package'; PERMITS : 'permits'; PRIVATE : 'private'; PROTECTED : 'protected'; PUBLIC : 'public'; - RECORD : 'record'; RETURN : 'return'; - SEALED : 'sealed'; fragment SHORT : 'short'; - STATIC : 'static'; STRICTFP : 'strictfp'; SUPER : 'super'; @@ -489,10 +479,11 @@ THROW : 'throw'; THROWS : 'throws'; TRANSIENT : 'transient'; TRY : 'try'; +VAR : 'var'; VOID : 'void'; VOLATILE : 'volatile'; WHILE : 'while'; - +YIELD : 'yield'; // §3.10.1 Integer Literals diff --git a/src/antlr/GroovyParser.g4 b/src/antlr/GroovyParser.g4 index a73c13fd4a..ea3eaa437e 100644 --- a/src/antlr/GroovyParser.g4 +++ b/src/antlr/GroovyParser.g4 @@ -687,14 +687,14 @@ switchLabel forControl : enhancedForControl - | classicalForControl + | originalForControl ; enhancedForControl : variableModifiersOpt type? variableDeclaratorId (COLON | IN) expression ; -classicalForControl +originalForControl : forInit? SEMI expression? SEMI forUpdate? ; 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 ba7112a9a7..e6af61f53b 100644 --- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java +++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java @@ -469,26 +469,50 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { @Override public ForStatement visitForStmtAlt(final ForStmtAltContext ctx) { - Tuple2<Parameter, Expression> controlTuple = this.visitForControl(ctx.forControl()); + var controlElements = this.visitForControl(ctx.forControl()); Statement loopBlock = this.unpackStatement((Statement) this.visit(ctx.statement())); - return configureAST(new ForStatement(controlTuple.getV1(), controlTuple.getV2(), loopBlock), ctx); + return configureAST(new ForStatement(controlElements.getV1(), controlElements.getV2(), loopBlock), ctx); } @Override public Tuple2<Parameter, Expression> visitForControl(final ForControlContext ctx) { - if (asBoolean(ctx.enhancedForControl())) { // e.g. for(int i in 0..<10) {} - return this.visitEnhancedForControl(ctx.enhancedForControl()); + // e.g. `for (var e : x) { }` and `for (e in x) { }` + EnhancedForControlContext enhancedCtx = ctx.enhancedForControl(); + if (asBoolean(enhancedCtx)) { + return this.visitEnhancedForControl(enhancedCtx); } - if (asBoolean(ctx.classicalForControl())) { // e.g. for(int i = 0; i < 10; i++) {} - return this.visitClassicalForControl(ctx.classicalForControl()); + // e.g. `for (int i = 0, n = 10; i < n; i += 1) { }` + OriginalForControlContext originalCtx = ctx.originalForControl(); + if (asBoolean(originalCtx)) { + return this.visitOriginalForControl(originalCtx); } throw createParsingFailedException("Unsupported for control: " + ctx.getText(), ctx); } + @Override + public Tuple2<Parameter, Expression> visitEnhancedForControl(final EnhancedForControlContext ctx) { + var parameter = new Parameter(this.visitType(ctx.type()), this.visitIdentifier(ctx.variableDeclaratorId().identifier())); + new ModifierManager(this, this.visitVariableModifiersOpt(ctx.variableModifiersOpt())).processParameter(parameter); + configureAST(parameter, ctx.variableDeclaratorId()); + + return tuple(parameter, (Expression) this.visit(ctx.expression())); + } + + @Override + public Tuple2<Parameter, Expression> visitOriginalForControl(final OriginalForControlContext ctx) { + ClosureListExpression closureListExpression = new ClosureListExpression(); + closureListExpression.addExpression(this.visitForInit(ctx.forInit())); + closureListExpression.addExpression(Optional.ofNullable(ctx.expression()) + .map(e -> (Expression) this.visit(e)).orElse(EmptyExpression.INSTANCE)); + closureListExpression.addExpression(this.visitForUpdate(ctx.forUpdate())); + + return tuple(ForStatement.FOR_LOOP_DUMMY, closureListExpression); + } + @Override public Expression visitForInit(final ForInitContext ctx) { if (!asBoolean(ctx)) { @@ -532,26 +556,6 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> { } } - @Override - public Tuple2<Parameter, Expression> visitEnhancedForControl(final EnhancedForControlContext ctx) { - Parameter parameter = new Parameter(this.visitType(ctx.type()), this.visitVariableDeclaratorId(ctx.variableDeclaratorId()).getName()); - ModifierManager modifierManager = new ModifierManager(this, this.visitVariableModifiersOpt(ctx.variableModifiersOpt())); - modifierManager.processParameter(parameter); - configureAST(parameter, ctx.variableDeclaratorId()); - return tuple(parameter, (Expression) this.visit(ctx.expression())); - } - - @Override - public Tuple2<Parameter, Expression> visitClassicalForControl(final ClassicalForControlContext ctx) { - ClosureListExpression closureListExpression = new ClosureListExpression(); - - closureListExpression.addExpression(this.visitForInit(ctx.forInit())); - closureListExpression.addExpression(asBoolean(ctx.expression()) ? (Expression) this.visit(ctx.expression()) : EmptyExpression.INSTANCE); - closureListExpression.addExpression(this.visitForUpdate(ctx.forUpdate())); - - return tuple(ForStatement.FOR_LOOP_DUMMY, closureListExpression); - } - @Override public WhileStatement visitWhileStmtAlt(final WhileStmtAltContext ctx) { Tuple2<BooleanExpression, Statement> conditionAndBlock = createLoopConditionExpressionAndBlock(ctx.expressionInPar(), ctx.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 6b29156333..3cfdd9d455 100644 --- a/src/main/java/org/codehaus/groovy/ast/stmt/ForStatement.java +++ b/src/main/java/org/codehaus/groovy/ast/stmt/ForStatement.java @@ -26,37 +26,33 @@ import org.codehaus.groovy.ast.VariableScope; import org.codehaus.groovy.ast.expr.Expression; /** - * Represents a standard for loop in Groovy + * Represents a for loop in Groovy. */ public class ForStatement extends Statement implements LoopingStatement { - public static final Parameter FOR_LOOP_DUMMY = new Parameter(ClassHelper.OBJECT_TYPE,"forLoopDummyParameter"); - private Parameter variable; + public static final Parameter FOR_LOOP_DUMMY = new Parameter(ClassHelper.OBJECT_TYPE, "forLoopDummyParameter"); + + private final Parameter variable; private Expression collectionExpression; private Statement loopBlock; - private VariableScope scope; - - public ForStatement(Parameter variable, Expression collectionExpression, Statement loopBlock) { + public ForStatement(final Parameter variable, final Expression collectionExpression, final Statement loopBlock) { this.variable = variable; - this.collectionExpression = collectionExpression; - this.loopBlock = loopBlock; + setCollectionExpression(collectionExpression); + setLoopBlock(loopBlock); } - @Override - public void visit(GroovyCodeVisitor visitor) { - visitor.visitForLoop(this); - } - - public Expression getCollectionExpression() { - return collectionExpression; + public void setCollectionExpression(final Expression collectionExpression) { + this.collectionExpression = collectionExpression; } @Override - public Statement getLoopBlock() { - return loopBlock; + public void setLoopBlock(final Statement loopBlock) { + this.loopBlock = loopBlock; } + //-------------------------------------------------------------------------- + public Parameter getVariable() { return variable; } @@ -65,20 +61,31 @@ public class ForStatement extends Statement implements LoopingStatement { return variable.getType(); } - public void setCollectionExpression(Expression collectionExpression) { - this.collectionExpression = collectionExpression; + public Expression getCollectionExpression() { + return collectionExpression; } - public void setVariableScope(VariableScope variableScope) { - scope = variableScope; + @Override + public Statement getLoopBlock() { + return loopBlock; } + //-------------------------------------------------------------------------- + + private VariableScope scope; + public VariableScope getVariableScope() { - return scope; + return this.scope; + } + + public void setVariableScope(final VariableScope scope) { + this.scope = scope; } + //-------------------------------------------------------------------------- + @Override - public void setLoopBlock(Statement loopBlock) { - this.loopBlock = loopBlock; + public void visit(final GroovyCodeVisitor visitor) { + visitor.visitForLoop(this); } } diff --git a/src/spec/test/SemanticsTest.groovy b/src/spec/test/SemanticsTest.groovy index 52e587c74b..da6a4ca13f 100644 --- a/src/spec/test/SemanticsTest.groovy +++ b/src/spec/test/SemanticsTest.groovy @@ -16,11 +16,15 @@ * specific language governing permissions and limitations * under the License. */ -import gls.CompilableTestSupport + import groovy.transform.Immutable +import org.junit.jupiter.api.Test + +import static groovy.test.GroovyAssert.assertScript -class SemanticsTest extends CompilableTestSupport { +final class SemanticsTest { + @Test void testVariableDefinition() { // tag::variable_definition_example1[] String x @@ -31,6 +35,7 @@ class SemanticsTest extends CompilableTestSupport { // end::variable_definition_example2[] } + @Test void testVariableAssignment() { assertScript ''' // tag::variable_assignment_example[] @@ -52,6 +57,7 @@ class SemanticsTest extends CompilableTestSupport { ''' } + @Test void testMultipleAssignment() { // tag::multiple_assignment_example[] def (a, b, c) = [10, 20, 'foo'] @@ -59,6 +65,7 @@ class SemanticsTest extends CompilableTestSupport { // end::multiple_assignment_example[] } + @Test void testMultipleAssignmentWithTypes() { // tag::multiple_assignment_with_types[] def (int i, String j) = [10, 'foo'] @@ -66,6 +73,7 @@ class SemanticsTest extends CompilableTestSupport { // end::multiple_assignment_with_types[] } + @Test void testMultipleAssignmentWithExistingVariables() { // tag::multiple_assignment_with_existing_variables[] def nums = [1, 3, 5] @@ -75,6 +83,7 @@ class SemanticsTest extends CompilableTestSupport { // end::multiple_assignment_with_existing_variables[] } + @Test void testMultipleAssignmentWithArraysAndLists() { // tag::multiple_assignment_with_arrays_and_lists[] def (_, month, year) = "18th June 2009".split() @@ -82,6 +91,7 @@ class SemanticsTest extends CompilableTestSupport { // end::multiple_assignment_with_arrays_and_lists[] } + @Test void testMultipleAssignmentOverflow() { // tag::multiple_assignment_overflow[] def (a, b, c) = [1, 2] @@ -89,6 +99,7 @@ class SemanticsTest extends CompilableTestSupport { // end::multiple_assignment_overflow[] } + @Test void testMultipleAssignmentUnderflow() { // tag::multiple_assignment_underflow[] def (a, b) = [1, 2, 3] @@ -96,6 +107,7 @@ class SemanticsTest extends CompilableTestSupport { // end::multiple_assignment_underflow[] } + @Test void testIfElse() { // tag::if_else_example[] def x = false @@ -117,51 +129,53 @@ class SemanticsTest extends CompilableTestSupport { // end::if_else_example[] } + @Test void testSwitchCase() { // tag::switch_case_example[] def x = 1.23 def result = "" switch (x) { - case "foo": - result = "found foo" - // lets fall through + case "foo": + result = "found foo" + // lets fall through - case "bar": - result += "bar" + case "bar": + result += "bar" - case [4, 5, 6, 'inList']: - result = "list" - break + case [4, 5, 6, 'inList']: + result = "list" + break - case 12..30: - result = "range" - break + case 12..30: + result = "range" + break - case Integer: - result = "integer" - break + case Integer: + result = "integer" + break - case Number: - result = "number" - break + case Number: + result = "number" + break - case ~/fo*/: // toString() representation of x matches the pattern? - result = "foo regex" - break + case ~/fo*/: // toString() representation of x matches the pattern? + result = "foo regex" + break - case { it < 0 }: // or { x < 0 } - result = "negative" - break + case { it < 0 }: // or { x < 0 } + result = "negative" + break - default: - result = "default" + default: + result = "default" } assert result == "number" // end::switch_case_example[] } + @Test void testSwitchExpression() { def person = 'Romeo' // tag::switch_expression[] @@ -175,16 +189,18 @@ class SemanticsTest extends CompilableTestSupport { assert partner == 'Juliet' } + @Test void testClassicForLoop() { // tag::classic_for_loop_example[] String message = '' - for (int i = 0; i < 5; i++) { + for (int i = 0; i < 5; i += 1) { message += 'Hi ' } assert message == 'Hi Hi Hi Hi Hi ' // end::classic_for_loop_example[] } + @Test void testGroovyForLoop() { // tag::groovy_for_loop_example[] // iterate over a range @@ -202,15 +218,14 @@ class SemanticsTest extends CompilableTestSupport { assert x == 10 // iterate over an array - def array = (0..4).toArray() x = 0 - for ( i in array ) { + for ( i in new int[]{0, 1, 2, 3, 4} ) { x += i } assert x == 10 // iterate over a map - def map = ['abc':1, 'def':2, 'xyz':3] + def map = [a:1, b:2, c:3] x = 0 for ( e in map ) { x += e.value @@ -225,15 +240,15 @@ class SemanticsTest extends CompilableTestSupport { assert x == 6 // iterate over the characters in a string - def text = "abc" def list = [] - for (c in text) { + for ( c in 'abc' ) { list.add(c) } - assert list == ["a", "b", "c"] + assert list == ['a', 'b', 'c'] // end::groovy_for_loop_example[] } + @Test void testWhileLoop() { // tag::while_loop_example[] def x = 0 @@ -247,6 +262,7 @@ class SemanticsTest extends CompilableTestSupport { // end::while_loop_example[] } + @Test void testTryCatch() { // tag::try_catch_example[] try { @@ -258,6 +274,7 @@ class SemanticsTest extends CompilableTestSupport { // end::try_catch_example[] } + @Test void testTryCatchFinally() { // tag::try_catch_finally_example[] def z @@ -276,6 +293,7 @@ class SemanticsTest extends CompilableTestSupport { // end::try_catch_finally_example[] } + @Test void testDestructuringMultipleAssignment() { // tag::destructuring[] def coordinates = new Coordinates(latitude: 43.23, longitude: 3.67) // <1> @@ -286,18 +304,19 @@ class SemanticsTest extends CompilableTestSupport { assert lo == 3.67 // end::destructuring[] } -} -// tag::coordinates-class[] -@Immutable -class Coordinates { - double latitude - double longitude - - double getAt(int idx) { - if (idx == 0) latitude - else if (idx == 1) longitude - else throw new Exception("Wrong coordinate index, use 0 or 1") + static + // tag::coordinates-class[] + @Immutable + class Coordinates { + double latitude + double longitude + + double getAt(int idx) { + if (idx == 0) latitude + else if (idx == 1) longitude + else throw new Exception('Wrong coordinate index, use 0 or 1') + } } + // end::coordinates-class[] } -// end::coordinates-class[]
