Fix groovy multivariable declaration

rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyParserVisitor.java

@@ -39,9 +39,9 @@
 import org.openrewrite.java.marker.OmitParentheses;
 import org.openrewrite.java.marker.Semicolon;
 import org.openrewrite.java.marker.TrailingComma;
-import org.openrewrite.java.tree.*;
 import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.Statement;
+import org.openrewrite.java.tree.*;
 import org.openrewrite.marker.Markers;
 
 import java.math.BigDecimal;
@@ -113,16 +113,16 @@ private static boolean isOlderThanGroovy3() {
     }
 
     /**
-     *  Groovy methods can be declared with "def" AND a return type
-     *  In these cases the "def" is semantically meaningless but needs to be preserved for source code accuracy
-     *  If there is both a def and a return type, this method returns a RedundantDef object and advances the cursor
-     *  position past the "def" keyword, leaving the return type to be parsed as normal.
-     *  In any other situation an empty Optional is returned and the cursor is not advanced.
+     * Groovy methods can be declared with "def" AND a return type
+     * In these cases the "def" is semantically meaningless but needs to be preserved for source code accuracy
+     * If there is both a def and a return type, this method returns a RedundantDef object and advances the cursor
+     * position past the "def" keyword, leaving the return type to be parsed as normal.
+     * In any other situation an empty Optional is returned and the cursor is not advanced.
      */
     private Optional<RedundantDef> maybeRedundantDef(ClassNode type, String name) {
         int saveCursor = cursor;
         Space defPrefix = whitespace();
-        if(source.startsWith("def", cursor)) {
+        if (source.startsWith("def", cursor)) {
             skip("def");
             // The def is redundant only when it is followed by the method's return type
             // I hope no one puts an annotation between "def" and the return type
@@ -182,8 +182,8 @@ public G.CompilationUnit visit(SourceUnit unit, ModuleNode ast) throws GroovyPar
         for (ClassNode aClass : ast.getClasses()) {
             if (aClass.getSuperClass() == null ||
                 !("groovy.lang.Script".equals(aClass.getSuperClass().getName()) ||
-                     "RewriteGradleProject".equals(aClass.getSuperClass().getName()) ||
-                     "RewriteSettings".equals(aClass.getSuperClass().getName()))) {
+                  "RewriteGradleProject".equals(aClass.getSuperClass().getName()) ||
+                  "RewriteSettings".equals(aClass.getSuperClass().getName()))) {
                 sortedByPosition.computeIfAbsent(pos(aClass), i -> new ArrayList<>()).add(aClass);
             }
         }
@@ -822,8 +822,8 @@ public void visitArgumentlistExpression(ArgumentListExpression expression) {
             // When named parameters are in use they may appear before, after, or intermixed with any positional arguments
             if (unparsedArgs.size() > 1 && unparsedArgs.get(0) instanceof MapExpression &&
                 (unparsedArgs.get(0).getLastLineNumber() > unparsedArgs.get(1).getLastLineNumber() ||
-                    (unparsedArgs.get(0).getLastLineNumber() == unparsedArgs.get(1).getLastLineNumber() &&
-                        unparsedArgs.get(0).getLastColumnNumber() > unparsedArgs.get(1).getLastColumnNumber()))) {
+                 (unparsedArgs.get(0).getLastLineNumber() == unparsedArgs.get(1).getLastLineNumber() &&
+                  unparsedArgs.get(0).getLastColumnNumber() > unparsedArgs.get(1).getLastColumnNumber()))) {
 
                 // Figure out the source-code ordering of the expressions
                 MapExpression namedArgExpressions = (MapExpression) unparsedArgs.get(0);
@@ -1099,7 +1099,7 @@ public void visitBlockStatement(BlockStatement block) {
                 J expr = visit(statement);
                 if (i == blockStatements.size() - 1 && (expr instanceof Expression)) {
                     if (parent instanceof ClosureExpression || (parent instanceof MethodNode &&
-                            !JavaType.Primitive.Void.equals(typeMapping.type(((MethodNode) parent).getReturnType())))) {
+                                                                !JavaType.Primitive.Void.equals(typeMapping.type(((MethodNode) parent).getReturnType())))) {
                         expr = new J.Return(randomId(), expr.getPrefix(), Markers.EMPTY,
                                 expr.withPrefix(EMPTY));
                         expr = expr.withMarkers(expr.getMarkers().add(new ImplicitReturn(randomId())));
@@ -1439,6 +1439,8 @@ public void visitNotExpression(NotExpression expression) {
         @Override
         public void visitDeclarationExpression(DeclarationExpression expression) {
             Optional<RedundantDef> redundantDef = maybeRedundantDef(expression.getVariableExpression().getType(), expression.getVariableExpression().getName());
+            Optional<MultiVariable> multiVariable = maybeMultiVariable();
+            List<J.Modifier> modifiers = getModifiers();
             TypeTree typeExpr = visitVariableExpressionType(expression.getVariableExpression());
 
             J.VariableDeclarations.NamedVariable namedVariable;
@@ -1467,18 +1469,59 @@ public void visitDeclarationExpression(DeclarationExpression expression) {
             J.VariableDeclarations variableDeclarations = new J.VariableDeclarations(
                     randomId(),
                     EMPTY,
-                    redundantDef.map(Markers.EMPTY::add).orElse(Markers.EMPTY),
-                    emptyList(),
+                    Markers.EMPTY,
                     emptyList(),
+                    modifiers,
                     typeExpr,
                     null,
                     emptyList(),
                     singletonList(JRightPadded.build(namedVariable))
             );
+            if (redundantDef.isPresent()) {
+                variableDeclarations = variableDeclarations.withMarkers(variableDeclarations.getMarkers().add(redundantDef.get()));
+            }
+            if (multiVariable.isPresent()) {
+                variableDeclarations = variableDeclarations.withMarkers(variableDeclarations.getMarkers().add(multiVariable.get()));
+            }
 
             queue.add(variableDeclarations);
         }
 
+        private Optional<MultiVariable> maybeMultiVariable() {
+            int saveCursor = cursor;
+            Space commaPrefix = whitespace();
+            if (source.startsWith(",", cursor)) {
+                skip(",");
+                return Optional.of(new MultiVariable(randomId(), commaPrefix));
+            }
+            cursor = saveCursor;
+            return Optional.empty();
+        }
+
+        private List<J.Modifier> getModifiers() {
+            List<J.Modifier> modifiers = new ArrayList<>();
+            int saveCursor = cursor;
+            Space prefix = whitespace();
+            while (source.startsWith("def", cursor) || source.startsWith("var", cursor) || source.startsWith("final", cursor)) {
+                if (source.regionMatches(cursor, "var", 0, 3)) {
+                    modifiers.add(new J.Modifier(randomId(), prefix, Markers.EMPTY, "var", J.Modifier.Type.LanguageExtension, emptyList()));
+                    cursor += 3;
+                } else if (source.regionMatches(cursor, "def", 0, 3)) {
+                    modifiers.add(new J.Modifier(randomId(), prefix, Markers.EMPTY, "def", J.Modifier.Type.LanguageExtension, emptyList()));
+                    cursor += 3;
+                } else if (source.regionMatches(cursor, "final", 0, 5)) {
+                    modifiers.add(new J.Modifier(randomId(), prefix, Markers.EMPTY, "final", J.Modifier.Type.LanguageExtension, emptyList()));
+                    cursor += 5;
+                } else {
+                    break;
+                }
+                saveCursor = cursor;
+                prefix = whitespace();
+            }
+            cursor = saveCursor;
+            return modifiers;
+        }
+
         @Override
         public void visitEmptyExpression(EmptyExpression expression) {
             queue.add(new J.Empty(randomId(), EMPTY, Markers.EMPTY));
@@ -1766,7 +1809,7 @@ public void visitMethodCallExpression(MethodCallExpression call) {
                 MethodNode methodNode = (MethodNode) call.getNodeMetaData().get(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET);
                 JavaType.Method methodType = null;
                 if (methodNode == null && call.getObjectExpression() instanceof VariableExpression &&
-                        ((VariableExpression) call.getObjectExpression()).getAccessedVariable() != null) {
+                    ((VariableExpression) call.getObjectExpression()).getAccessedVariable() != null) {
                     // Groovy doesn't know what kind of object this method is being invoked on
                     // But if this invocation is inside a Closure we may have already enriched its parameters with types from the static type checker
                     // Use any such type information to attempt to find a matching method
@@ -2097,29 +2140,18 @@ public void visitPrefixExpression(PrefixExpression unary) {
 
         public TypeTree visitVariableExpressionType(VariableExpression expression) {
             JavaType type = typeMapping.type(staticType(((org.codehaus.groovy.ast.expr.Expression) expression)));
+            Space prefix = whitespace();
+            String typeName = "";
 
-            if (expression.isDynamicTyped()) {
-                Space prefix = whitespace();
-                String keyword;
-                if (source.substring(cursor).startsWith("final")) {
-                    keyword = "final";
-                } else {
-                    keyword = source.substring(cursor, cursor + 3);
-                }
-                cursor += keyword.length();
-                return new J.Identifier(randomId(),
-                        prefix,
-                        Markers.EMPTY,
-                        emptyList(),
-                        keyword,
-                        type, null);
+            if (!expression.isDynamicTyped() && source.startsWith(expression.getOriginType().getUnresolvedName(), cursor)) {
+                typeName = expression.getOriginType().getUnresolvedName();
+                cursor += typeName.length();
             }
-            Space prefix = sourceBefore(expression.getOriginType().getUnresolvedName());
             J.Identifier ident = new J.Identifier(randomId(),
                     EMPTY,
                     Markers.EMPTY,
                     emptyList(),
-                    expression.getOriginType().getUnresolvedName(),
+                    typeName,
                     type, null);
             if (expression.getOriginType().getGenericsTypes() != null) {
                 return new J.ParameterizedType(randomId(), prefix, Markers.EMPTY, ident, visitTypeParameterizations(
@@ -2671,7 +2703,7 @@ private String name() {
        Can contain a J.FieldAccess, as in a variable declaration with fully qualified type parameterization:
            List<java.lang.String>
      */
-    private JContainer<Expression> visitTypeParameterizations(GenericsType @Nullable[] genericsTypes) {
+    private JContainer<Expression> visitTypeParameterizations(GenericsType @Nullable [] genericsTypes) {
         Space prefix = sourceBefore("<");
         List<JRightPadded<Expression>> parameters;
 

rewrite-groovy/src/main/java/org/openrewrite/groovy/GroovyPrinter.java

@@ -265,13 +265,17 @@ public J visitVariableDeclarations(J.VariableDeclarations multiVariable, PrintOu
             beforeSyntax(multiVariable, Space.Location.VARIABLE_DECLARATIONS_PREFIX, p);
             visitSpace(Space.EMPTY, Space.Location.ANNOTATIONS, p);
             visit(multiVariable.getLeadingAnnotations(), p);
-            for (J.Modifier m : multiVariable.getModifiers()) {
-                visitModifier(m, p);
-            }
             multiVariable.getMarkers().findFirst(RedundantDef.class).ifPresent(def -> {
                 visitSpace(def.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p);
                 p.append("def");
             });
+            for (J.Modifier m : multiVariable.getModifiers()) {
+                visitModifier(m, p);
+            }
+            multiVariable.getMarkers().findFirst(MultiVariable.class).ifPresent(multiVar -> {
+                visitSpace(multiVar.getPrefix(), Space.Location.NAMED_VARIABLE_SUFFIX, p);
+                p.append(",");
+            });
             visit(multiVariable.getTypeExpression(), p);
             // For backwards compatibility.
             for (JLeftPadded<Space> dim : multiVariable.getDimensionsBeforeName()) {

rewrite-groovy/src/main/java/org/openrewrite/groovy/marker/MultiVariable.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2024 the original author or authors.
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.groovy.marker;
+
+import lombok.Value;
+import lombok.With;
+import org.openrewrite.java.tree.Space;
+import org.openrewrite.marker.Marker;
+
+import java.util.UUID;
+
+@Value
+@With
+public class MultiVariable implements Marker {
+    UUID id;
+    Space prefix;
+}

rewrite-groovy/src/test/java/org/openrewrite/groovy/tree/AssignmentTest.java

@@ -24,6 +24,41 @@
 @SuppressWarnings({"GroovyUnusedAssignment", "GrUnnecessarySemicolon"})
 class AssignmentTest implements RewriteTest {
 
+    @Test
+    void noKeyword() {
+        rewriteRun(
+          groovy(
+            """
+              x = "s"
+              """
+          )
+        );
+    }
+
+    @Test
+    void simple() {
+        rewriteRun(
+          groovy(
+            """
+              def x = "s"
+              """
+          )
+        );
+    }
+
+    @Test
+    void simpleWithFinal() {
+        rewriteRun(
+          groovy(
+            """
+              final def x = "x"
+              def final y = "y"
+              final z = "z"
+              """
+          )
+        );
+    }
+
     @Test
     void concat() {
         rewriteRun(
@@ -91,4 +126,52 @@ void baseNConversions() {
           )
         );
     }
+
+    @Test
+    void multipleAssignmentsAtOneLine() {
+        rewriteRun(
+          groovy(
+            """
+              def startItem = '|  ', endItem = '  |'
+              def repeatLength = startItem.length() + output.length() + endItem.length()
+              println("\\n" + ("-" * repeatLength) + "\\n" + startItem + output + endItem + "\\n" + ("-" * repeatLength))
+              """
+          )
+        );
+    }
+
+    @Test
+    void multipleAssignmentsAtOneLineSimple() {
+        rewriteRun(
+          groovy(
+            """
+              def a = '1', b = '2'
+              """
+          )
+        );
+    }
+
+    @Test
+    void multipleAssignmentsAtMultipleLineDynamicType() {
+        rewriteRun(
+          groovy(
+            """
+               def a = '1'    ,  
+                  b = '2'
+              """
+          )
+        );
+    }
+
+    @Test
+    void multipleAssignmentsAtMultipleLineStaticType() {
+        rewriteRun(
+          groovy(
+            """
+               String a = '1'    ,  
+                  b = '2'
+              """
+          )
+        );
+    }
 }