You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2025/03/30 21:37:53 UTC

(groovy) branch GROOVY-10683 updated (214b372de8 -> d92cb475a4)

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a change to branch GROOVY-10683
in repository https://gitbox.apache.org/repos/asf/groovy.git


 discard 214b372de8 `for` loop parser refactor
     new d92cb475a4 `for` loop parser refactor

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (214b372de8)
            \
             N -- N -- N   refs/heads/GROOVY-10683 (d92cb475a4)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/test/groovy/ForLoopTest.groovy | 146 ++++++++++++++++++-------------------
 1 file changed, 72 insertions(+), 74 deletions(-)


(groovy) 01/01: `for` loop parser refactor

Posted by em...@apache.org.
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 d92cb475a461dd55f1d8c355d9513edb1f15cc41
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Mar 30 10:08:48 2025 -0500

    `for` loop parser refactor
---
 src/antlr/GroovyLexer.g4                           |  15 +--
 src/antlr/GroovyParser.g4                          |  30 ++---
 .../apache/groovy/parser/antlr4/AstBuilder.java    |  55 ++++----
 .../org/codehaus/groovy/ast/stmt/ForStatement.java |  55 ++++----
 src/spec/test/SemanticsTest.groovy                 | 111 +++++++++-------
 src/test/groovy/ForLoopTest.groovy                 | 146 ++++++++++-----------
 6 files changed, 213 insertions(+), 199 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..edf75c4cde 100644
--- a/src/antlr/GroovyParser.g4
+++ b/src/antlr/GroovyParser.g4
@@ -383,7 +383,7 @@ qualifiedClassNameList
     ;
 
 formalParameters
-    :   LPAREN formalParameterList? rparen
+    :   LPAREN formalParameterList? RPAREN
     ;
 
 formalParameterList
@@ -510,7 +510,7 @@ annotationsOpt
     ;
 
 annotation
-    :   AT annotationName (nls LPAREN elementValues? rparen)?
+    :   AT annotationName (nls LPAREN elementValues? RPAREN)?
     ;
 
 elementValues
@@ -573,7 +573,7 @@ variableDeclaration[int t]
     ;
 
 typeNamePairs
-    :   LPAREN typeNamePair (COMMA typeNamePair)* rparen
+    :   LPAREN typeNamePair (COMMA typeNamePair)* RPAREN
     ;
 
 typeNamePair
@@ -581,7 +581,7 @@ typeNamePair
     ;
 
 variableNames
-    :   LPAREN variableDeclaratorId (COMMA variableDeclaratorId)+ rparen
+    :   LPAREN variableDeclaratorId (COMMA variableDeclaratorId)+ RPAREN
     ;
 
 conditionalStatement
@@ -598,7 +598,7 @@ switchStatement
     ;
 
 loopStatement
-    :   FOR LPAREN forControl rparen nls statement                                                            #forStmtAlt
+    :   FOR LPAREN forControl RPAREN nls statement                                                            #forStmtAlt
     |   WHILE expressionInPar nls statement                                                                   #whileStmtAlt
     |   DO nls statement nls WHILE expressionInPar                                                            #doWhileStmtAlt
     ;
@@ -648,7 +648,7 @@ statement
     ;
 
 catchClause
-    :   CATCH LPAREN variableModifiersOpt catchType? identifier rparen nls block
+    :   CATCH LPAREN variableModifiersOpt catchType? identifier RPAREN nls block
     ;
 
 catchType
@@ -660,7 +660,7 @@ finallyBlock
     ;
 
 resources
-    :   LPAREN nls resourceList sep? rparen
+    :   LPAREN nls resourceList sep? RPAREN
     ;
 
 resourceList
@@ -687,14 +687,14 @@ switchLabel
 
 forControl
     :   enhancedForControl
-    |   classicalForControl
+    |   originalForControl
     ;
 
 enhancedForControl
-    :   variableModifiersOpt type? variableDeclaratorId (COLON | IN) expression
+    :   variableModifiersOpt type? identifier (COLON | IN) expression
     ;
 
-classicalForControl
+originalForControl
     :   forInit? SEMI expression? SEMI forUpdate?
     ;
 
@@ -711,7 +711,7 @@ forUpdate
 // EXPRESSIONS
 
 castParExpression
-    :   LPAREN type rparen
+    :   LPAREN type RPAREN
     ;
 
 parExpression
@@ -719,7 +719,7 @@ parExpression
     ;
 
 expressionInPar
-    :   LPAREN enhancedStatementExpression rparen
+    :   LPAREN enhancedStatementExpression RPAREN
     ;
 
 expressionList[boolean canSpread]
@@ -1174,7 +1174,7 @@ typeArgumentsOrDiamond
     ;
 
 arguments
-    :   LPAREN enhancedArgumentListInPar? COMMA? rparen
+    :   LPAREN enhancedArgumentListInPar? COMMA? RPAREN
     ;
 
 argumentList
@@ -1298,10 +1298,6 @@ keywords
     |   PRIVATE
     ;
 
-rparen
-    :   RPAREN
-    ;
-
 nls
     :   NL*
     ;
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..e6903c80d4 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,49 @@ 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 = configureAST(new Parameter(this.visitType(ctx.type()), this.visitIdentifier(ctx.identifier())), ctx.identifier());
+        new ModifierManager(this, this.visitVariableModifiersOpt(ctx.variableModifiersOpt())).processParameter(parameter);
+
+        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 +555,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[]
diff --git a/src/test/groovy/ForLoopTest.groovy b/src/test/groovy/ForLoopTest.groovy
index e4294b699a..69f81a24e7 100644
--- a/src/test/groovy/ForLoopTest.groovy
+++ b/src/test/groovy/ForLoopTest.groovy
@@ -18,112 +18,103 @@
  */
 package groovy
 
-import groovy.bugs.TestSupport
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
 
-class ForLoopTest extends gls.CompilableTestSupport {
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
 
-    def x
+final class ForLoopTest {
 
-    void testFinalParameterInForLoopIsAllowed() {
-        // only 'final' should be allowed: other modifiers like 'synchronized' should be forbidden
-        shouldNotCompile """
+    private int x
+
+    @ParameterizedTest
+    @ValueSource(strings=[
+        /*'public','private','protected','abstract','static',*/'native',/*'strictfp',*/'synchronized'
+    ])
+    void testFinalParameterInForLoopIsAllowed(String modifier) {
+        // only 'final' should be allowed; other modifiers should be forbidden
+        shouldFail """
             def collection = ["a", "b", "c", "d", "e"]
-            for (synchronized String letter in collection) { }
+            for ($modifier String letter in collection) { }
         """
 
-        // only 'final' allowed, and no additional modifier
-        shouldNotCompile """
+        shouldFail """
             def collection = ["a", "b", "c", "d", "e"]
-            for (final synchronized String letter in collection) { }
+            for (final $modifier String letter in collection) { }
         """
 
-        shouldCompile """
+        assertScript '''
             def collection = ["a", "b", "c", "d", "e"]
             for (final String letter in collection) { }
             for (final String letter : collection) { }
             for (final letter in collection) { }
             for (final letter : collection) { }
-        """
+        '''
     }
 
-    void testRange() {
-        x = 0
-
+    @Test
+    void testForEachInRange() {
         for (i in 0..9) {
             x = x + i
         }
-
         assert x == 45
     }
 
-    void testRangeWithType() {
-        x = 0
-
+    @Test
+    void testForEachInRangeWithType() {
         for (Integer i in 0..9) {
             assert i.getClass() == Integer
             x = x + i
         }
-
         assert x == 45
     }
 
-    void testRangeWithJdk15StyleAndType() {
-        x = 0
-
+    @Test
+    void testForEachInRangeWithJdk15StyleAndType() {
         for (Integer i: 0..9) {
             assert i.getClass() == Integer
             x = x + i
         }
-
         assert x == 45
     }
 
-    void testList() {
-        x = 0
-
+    @Test
+    void testForEachInList() {
         for (i in [0, 1, 2, 3, 4]) {
             x = x + i
         }
-
         assert x == 10
     }
 
-    void testArray() {
-        def array = (0..4).toArray()
-
-        x = 0
-
-        for (i in array) {
+    @Test
+    void testForEachInArray() {
+        for (i in new Integer[]{0, 1, 2, 3, 4}) {
             x = x + i
         }
-
         assert x == 10
     }
 
-    void testIntArray() {
-        def array = TestSupport.getIntArray()
-
-        x = 0
-
-        for (i in array) {
+    @Test
+    void testForEachInIntArray() {
+        for (i in new int[]{1, 2, 3, 4, 5}) {
             x = x + i
         }
-
         assert x == 15
     }
 
-    void testString() {
-        def text = "abc"
-
+    @Test
+    void testForEachInString() {
         def list = []
-        for (c in text) {
+        for (c in 'abc') {
             list.add(c)
         }
-
-        assert list == ["a", "b", "c"]
+        assert list == ['a', 'b', 'c']
     }
 
-    void testVector() {
+    @Test
+    void testForEachInVector() {
         def vector = new Vector()
         vector.addAll([1, 2, 3])
 
@@ -134,74 +125,81 @@ class ForLoopTest extends gls.CompilableTestSupport {
         assert answer == [1, 2, 3]
     }
 
+    @Test
     void testClassicFor() {
-        def sum = 0
         for (int i = 0; i < 10; i++) {
-            sum++
+            x++
         }
-        assert sum == 10
+        assert x == 10
 
         def list = [1, 2]
-        sum = 0
+        x = 0
         for (Iterator i = list.iterator(); i.hasNext();) {
-            sum += i.next()
+            x += i.next()
         }
-        assert sum == 3
+        assert x == 3
     }
 
+    @Test
     void testClassicForNested() {
-        def sum = 0
         for (int i = 0; i < 10; i++) {
             for (int j = 0; j < 10; j++) {
-                sum++
+                x++
             }
         }
-        assert sum == 100
+        assert x == 100
     }
 
+    @Test
     void testClassicForWithContinue() {
-        def sum1 = 0
         for (int i = 0; i < 10; i++) {
             if (i % 2 == 0) continue
-            sum1 += i
+            x += i
         }
-        assert sum1 == 25
+        assert x == 25
 
         // same as before, but with label
-        def sum2 = 0
+        x = 0
         test:
         for (int i = 0; i < 10; i++) {
             if (i % 2 == 0) continue test
-            sum2 += i
+            x += i
         }
-        assert sum2 == 25
+        assert x == 25
+    }
 
+    @Test
+    void testClassicForWithMultiVariable() {
+        def result = ''
+        for (def (int i, char c) = [0, 'x']; i < 3; ++i, c++) {
+            result += c
+        }
+        assert result == 'xyz'
     }
 
+    @Test
     void testClassicForWithEmptyInitializer() {
         def i = 0
-        def sum1 = 0
         for (; i < 10; i++) {
             if (i % 2 == 0) continue
-            sum1 += i
+            x += i
         }
-        assert sum1 == 25
+        assert x == 25
     }
 
-    void testClassicForWithEmptyBody() {
-        int i
-        for (i = 0; i < 5; i++);
-        assert i == 5
+    @Test
+    void testClassicForWithEmptyStatementBody() {
+        for (; x < 5; ++x) ;
+        assert x == 5
     }
 
+    @Test
     void testClassicForWithEverythingInitCondNextExpressionsEmpty() {
         int counter = 0
         for (;;) {
-            counter++
+            counter += 1
             if (counter == 10) break
         }
-
         assert counter == 10, "The body of the for loop wasn't executed, it should have looped 10 times."
     }
-
 }