Fix parsing of groovy method/constructor invocations which capture va…

…riables from their enclosing lexical scope.
openrewrite · Feb 10, 2024 · 06a1bc8 · 06a1bc8
1 parent 76f5e5e
commit 06a1bc8
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 34 deletions.
diff --git a/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java b/rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java
@@ -659,7 +659,9 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) {
                 cursor = saveCursor;
             }
 
-            List<org.codehaus.groovy.ast.expr.Expression> unparsedArgs = expression.getExpressions();
+            List<org.codehaus.groovy.ast.expr.Expression> unparsedArgs = expression.getExpressions().stream()
+                    .filter(GroovyParserVisitor::appearsInSource)
+                    .collect(Collectors.toList());
             // If the first parameter to a function is a Map, then groovy allows "named parameters" style invocations, see:
             //     https://docs.groovy-lang.org/latest/html/documentation/#_named_parameters_2
             // When named parameters are in use they may appear before, after, or intermixed with any positional arguments
@@ -695,43 +697,33 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) {
                 }
             }
 
-            for (int i = 0; i < unparsedArgs.size(); i++) {
-                org.codehaus.groovy.ast.expr.Expression rawArg = unparsedArgs.get(i);
-                Expression arg;
-                if (appearsInSource(rawArg)) {
-                    arg = visit(rawArg);
-                } else {
-                    arg = new J.Empty(randomId(), whitespace(), Markers.EMPTY);
-                }
-                if (omitParentheses != null) {
-                    arg = arg.withMarkers(arg.getMarkers().add(omitParentheses));
-                }
-
-                Space after = EMPTY;
-                if (i == unparsedArgs.size() - 1) {
-                    if (omitParentheses == null) {
-                        after = sourceBefore(")");
-                    }
-                } else {
-                    after = whitespace();
-                    if (source.charAt(cursor) == ')') {
-                        // the next argument will have an OmitParentheses marker
-                        omitParentheses = new org.openrewrite.java.marker.OmitParentheses(randomId());
+            if(unparsedArgs.isEmpty()) {
+                args.add(JRightPadded.build((Expression)new J.Empty(randomId(), whitespace(), Markers.EMPTY))
+                        .withAfter(omitParentheses == null ? sourceBefore(")") : EMPTY));
+            } else {
+                for (int i = 0; i < unparsedArgs.size(); i++) {
+                    org.codehaus.groovy.ast.expr.Expression rawArg = unparsedArgs.get(i);
+                    Expression arg = visit(rawArg);
+                    if (omitParentheses != null) {
+                        arg = arg.withMarkers(arg.getMarkers().add(omitParentheses));
                     }
-                    cursor++;
-                }
 
-                args.add(JRightPadded.build(arg).withAfter(after));
-            }
+                    Space after = EMPTY;
+                    if (i == unparsedArgs.size() - 1) {
+                        if (omitParentheses == null) {
+                            after = sourceBefore(")");
+                        }
+                    } else {
+                        after = whitespace();
+                        if (source.charAt(cursor) == ')') {
+                            // the next argument will have an OmitParentheses marker
+                            omitParentheses = new org.openrewrite.java.marker.OmitParentheses(randomId());
+                        }
+                        cursor++;
+                    }
 
-            if (unparsedArgs.isEmpty()) {
-                Expression element = new J.Empty(randomId(),
-                        omitParentheses == null ? sourceBefore(")") : EMPTY, Markers.EMPTY);
-                if (omitParentheses != null) {
-                    element = element.withMarkers(element.getMarkers().add(omitParentheses));
+                    args.add(JRightPadded.build(arg).withAfter(after));
                 }
-
-                args.add(JRightPadded.build(element));
             }
 
             queue.add(JContainer.build(beforeOpenParen, args, Markers.EMPTY));

diff --git a/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ConstructorTest.java b/rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/ConstructorTest.java
@@ -20,6 +20,7 @@
 
 import static org.openrewrite.groovy.Assertions.groovy;
 
+@SuppressWarnings("GroovyResultOfObjectAllocationIgnored")
 class ConstructorTest implements RewriteTest {
 
     @Test
@@ -32,4 +33,20 @@ void inParens() {
           )
         );
     }
+
+    @Test
+    void anonymousClassDeclarationClosedOverVariable() {
+        rewriteRun(
+          groovy(
+            """
+              int i = 1
+              new Object() {
+                  int one() {
+                      return i
+                  }
+              }
+              """
+          )
+        );
+    }
 }