Issue #80: Resolve MultiVariableDeclaration Bug

Author: Anmol202005

config/import-control.xml

@@ -12,4 +12,5 @@
     <allow pkg="org.checkstyle"/>
     <allow pkg="java.util"/>
     <allow pkg="com.puppycrawl.tools.checkstyle"/>
+    <allow pkg="org.jspecify.annotations"/>
 </import-control>

src/main/java/org/checkstyle/autofix/recipe/FinalLocalVariable.java

@@ -19,17 +19,22 @@
 
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.UUID;
 
 import org.checkstyle.autofix.PositionHelper;
 import org.checkstyle.autofix.parser.CheckstyleViolation;
+import org.jspecify.annotations.Nullable;
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Recipe;
 import org.openrewrite.Tree;
 import org.openrewrite.TreeVisitor;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.Space;
+import org.openrewrite.java.tree.Statement;
+import org.openrewrite.marker.Marker;
 import org.openrewrite.marker.Markers;
 
 /**
@@ -56,17 +61,93 @@ public String getDescription() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new LocalVariableVisitor();
+        return new JavaIsoVisitor<>() {
+            @Override
+            public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu,
+                                                          ExecutionContext executionContext) {
+                return new LocalVariableVisitor()
+                        .visitCompilationUnit(new MarkViolationVisitor()
+                                .visitCompilationUnit(cu, executionContext), executionContext);
+            }
+        };
     }
 
-    private final class LocalVariableVisitor extends JavaIsoVisitor<ExecutionContext> {
+    /**
+     * Visitor that identifies and marks variable declarations at violation locations.
+     * This visitor traverses the AST and adds markers to variables that match
+     * the checkstyle violation locations, preparing them for the final modifier addition.
+     */
+    private final class MarkViolationVisitor extends JavaIsoVisitor<ExecutionContext> {
 
         private Path sourcePath;
+        private J.CompilationUnit currentCompilationUnit;
+
+        @Override
+        public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu,
+                                                      ExecutionContext executionContext) {
+            this.sourcePath = cu.getSourcePath().toAbsolutePath();
+            this.currentCompilationUnit = cu;
+            return super.visitCompilationUnit(cu, executionContext);
+        }
+
+        @Override
+        public J.VariableDeclarations visitVariableDeclarations(
+                J.VariableDeclarations multiVariable, ExecutionContext executionContext) {
+
+            final J.VariableDeclarations variableDeclarations;
+
+            final J.VariableDeclarations declarations = super
+                    .visitVariableDeclarations(multiVariable, executionContext);
+
+            if (!(getCursor().getParentTreeCursor().getValue() instanceof J.ClassDeclaration)
+                    && !declarations.hasModifier(J.Modifier.Type.Final)) {
+
+                final List<J.VariableDeclarations.NamedVariable> variables = declarations
+                        .getVariables();
+                final List<J.VariableDeclarations.NamedVariable> marked = new ArrayList<>();
+                for (J.VariableDeclarations.NamedVariable variable : variables) {
+                    if (isAtViolationLocation(variable)) {
+                        marked.add(variable.withMarkers(
+                                variable.getMarkers().add(new FinalLocalVariableMarker())));
+                    }
+                    else {
+                        marked.add(variable);
+                    }
+                }
+                variableDeclarations = declarations.withVariables(marked);
+            }
+            else {
+                variableDeclarations = declarations;
+            }
+            return variableDeclarations;
+        }
+
+        private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable variable) {
+
+            final int line = PositionHelper
+                    .computeLinePosition(currentCompilationUnit, variable, getCursor());
+            final int column = PositionHelper
+                    .computeColumnPosition(currentCompilationUnit, variable, getCursor());
+
+            return violations.removeIf(violation -> {
+                final Path absolutePath = Path.of(violation.getFileName()).toAbsolutePath();
+                return violation.getLine() == line
+                        && violation.getColumn() == column
+                        && absolutePath.endsWith(sourcePath)
+                        && violation.getMessage().contains(variable.getSimpleName());
+            });
+        }
+    }
+
+    /**
+     * Visitor that processes marked variable declarations and applies the final modifier.
+     * This visitor handles both single and multi-variable declarations.
+     */
+    private final class LocalVariableVisitor extends JavaIsoVisitor<ExecutionContext> {
 
         @Override
         public J.CompilationUnit visitCompilationUnit(
                 J.CompilationUnit cu, ExecutionContext executionContext) {
-            this.sourcePath = cu.getSourcePath();
             return super.visitCompilationUnit(cu, executionContext);
         }
 
@@ -83,33 +164,91 @@ public J.VariableDeclarations visitVariableDeclarations(
                     && !declarations.hasModifier(J.Modifier.Type.Final)) {
                 final J.VariableDeclarations.NamedVariable variable = declarations
                         .getVariables().get(0);
-                if (isAtViolationLocation(variable)) {
-                    final List<J.Modifier> modifiers = new ArrayList<>();
+                if (variable.getMarkers().findFirst(FinalLocalVariableMarker.class).isPresent()) {
+                    declarations = addFinalModifier(declarations);
+                }
+            }
+            return declarations;
+        }
+
+        @Override
+        public J.Block visitBlock(J.Block block, ExecutionContext executionContext) {
+            final J.Block visited = super.visitBlock(block, executionContext);
 
-                    final Space finalPrefix = declarations.getTypeExpression().getPrefix();
+            final List<Statement> newStatements = new ArrayList<>();
 
-                    modifiers.add(new J.Modifier(Tree.randomId(), finalPrefix,
-                            Markers.EMPTY, null, J.Modifier.Type.Final, new ArrayList<>()));
-                    modifiers.addAll(declarations.getModifiers());
-                    declarations = declarations.withModifiers(modifiers)
-                            .withTypeExpression(declarations.getTypeExpression()
-                                    .withPrefix(Space.SINGLE_SPACE));
+            for (Statement stmt : visited.getStatements()) {
+                if (isVariableDeclaration(stmt)) {
+                    handleMultiVariableDeclaration((J.VariableDeclarations) stmt, newStatements);
+                }
+                else {
+                    newStatements.add(stmt);
                 }
             }
-            return declarations;
+
+            return visited.withStatements(newStatements);
         }
 
-        private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable literal) {
-            final J.CompilationUnit cursor = getCursor().firstEnclosing(J.CompilationUnit.class);
+        private void handleMultiVariableDeclaration(J.VariableDeclarations varDecl,
+                                                       List<Statement> newStatements) {
+            final List<J.VariableDeclarations.NamedVariable> violationsList = new ArrayList<>();
+            final List<J.VariableDeclarations.NamedVariable> nonViolations = new ArrayList<>();
 
-            final int line = PositionHelper.computeLinePosition(cursor, literal, getCursor());
-            final int column = PositionHelper.computeColumnPosition(cursor, literal, getCursor());
+            for (J.VariableDeclarations.NamedVariable variable : varDecl.getVariables()) {
+                if (variable.getMarkers().findFirst(FinalLocalVariableMarker.class).isPresent()) {
+                    violationsList.add(variable.withPrefix(Space.SINGLE_SPACE));
+                }
+                else {
+                    nonViolations.add(variable.withPrefix(Space.SINGLE_SPACE));
+                }
+            }
+            if (violationsList.isEmpty()) {
+                newStatements.add(varDecl);
+            }
+            else if (nonViolations.isEmpty()) {
+                newStatements.add(addFinalModifier(varDecl));
+            }
+            else {
+                newStatements.add(varDecl.withVariables(nonViolations));
+                for (J.VariableDeclarations.NamedVariable variable : violationsList) {
+                    newStatements.add(addFinalModifier(varDecl
+                            .withVariables(Collections.singletonList(variable))));
+                }
+            }
+        }
 
-            return violations.stream().anyMatch(violation -> {
-                return violation.getLine() == line
-                        && violation.getColumn() == column
-                        && Path.of(violation.getFileName()).endsWith(sourcePath);
-            });
+        private J.VariableDeclarations addFinalModifier(J.VariableDeclarations varDecl) {
+            final List<J.Modifier> modifiers = new ArrayList<>(varDecl.getModifiers());
+            final Space finalPrefix = varDecl.getTypeExpression().getPrefix();
+            modifiers.add(new J.Modifier(Tree.randomId(), finalPrefix,
+                    Markers.EMPTY, null, J.Modifier.Type.Final, new ArrayList<>()));
+
+            modifiers.addAll(varDecl.getModifiers());
+
+            return varDecl.withModifiers(modifiers)
+                    .withTypeExpression(varDecl.getTypeExpression().withPrefix(Space.SINGLE_SPACE));
+        }
+
+        private boolean isVariableDeclaration(Statement stmt) {
+            return stmt instanceof J.VariableDeclarations varDecl
+                    && varDecl.getVariables().size() > 1
+                    && !varDecl.hasModifier(J.Modifier.Type.Final)
+                    && varDecl.getTypeExpression() != null
+                    && !(getCursor().getParentTreeCursor()
+                    .getValue() instanceof J.ClassDeclaration);
+        }
+    }
+
+    public static class FinalLocalVariableMarker implements Marker {
+
+        @Override
+        public @Nullable UUID getId() {
+            return null;
+        }
+
+        @Override
+        public <M extends Marker> @Nullable M withId(UUID id) {
+            return null;
         }
     }
 }

src/test/java/org/checkstyle/autofix/recipe/FinalLocalVariableTest.java

@@ -51,4 +51,69 @@ void annotationDeclaration() throws Exception {
         verify("AnnotationDeclaration");
     }
 
+    @Test
+    void multiple() throws Exception {
+        verify("MultipleVariable");
+    }
+
+    @Test
+    void localVariable() throws Exception {
+        verify("LocalVariableOne");
+    }
+
+    @Test
+    void localVariableTwo() throws Exception {
+        verify("LocalVariableTwo");
+    }
+
+    @Test
+    void localVariableThree() throws Exception {
+        verify("LocalVariableThree");
+    }
+
+    @Test
+    void localVariableFour() throws Exception {
+        verify("LocalVariableFour");
+    }
+
+    @Test
+    void localVariableFive() throws Exception {
+        verify("LocalVariableFive");
+    }
+
+    @Test
+    void localVariableCheckRecord() throws Exception {
+        verify("LocalVariableCheckRecord");
+    }
+
+    @Test
+    void finalLocalVariable2One() throws Exception {
+        verify("FinalLocalVariable2One");
+    }
+
+    @Test
+    void finalLocalForLoop() throws Exception {
+        verify("VariableEnhancedForLoopVariable");
+    }
+
+    @Test
+    void finalLocalForLoop2() throws Exception {
+        verify("VariableEnhancedForLoopVariable2");
+    }
+
+    @Test
+    void localVariableAssignedMultipleTimes() throws Exception {
+        verify("LocalVariableAssignedMultipleTimes");
+    }
+
+    @Test
+    void localVariableCheckSwitchExpressions() throws Exception {
+        verify("LocalVariableCheckSwitchExpressions");
+    }
+
+    @Test
+    void localVariableCheckSwitchAssignment() throws Exception {
+        verify("LocalVariableCheckSwitchAssignment");
+    }
+
 }

src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/finallocalvariable2one/DiffFinalLocalVariable2One.diff

@@ -0,0 +1,22 @@
+--- src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/finallocalvariable2one/InputFinalLocalVariable2One.java
++++ src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/finallocalvariable2one/OutputFinalLocalVariable2One.java
+@@ -10,7 +10,7 @@
+ */
+ package org.checkstyle.autofix.recipe.finallocalvariable.finallocalvariable2one;
+ 
+-public class InputFinalLocalVariable2One {
++public class OutputFinalLocalVariable2One {
+     private int m_ClassVariable = 0;
+     //static block
+     static
+@@ -49,9 +49,7 @@
+             }
+         };
+     }
+-
+-    // violation below "Variable 'aArg' should be declared final"
+-    public void method(int aArg, final int aFinal, int aArg2)
++    public void method(final int aArg, final int aFinal, int aArg2)
+     {
+         int z = 0;
+