Issue #80: Resolve MultiVariableDeclaration Bug

Anmol202005 · Anmol202005 · commit 9374d70eb228 · 2025-09-01T09:58:21.000+05:30
diff --git a/src/main/java/org/checkstyle/autofix/recipe/FinalLocalVariable.java b/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,92 @@ public String getDescription() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new LocalVariableVisitor();
+        return new JavaIsoVisitor<>() {
+            @Override
+            public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu,
+                                                          ExecutionContext ctx) {
+                return new LocalVariableVisitor()
+                        .visitCompilationUnit(new MarkViolationVisitor()
+                                .visitCompilationUnit(cu, ctx), ctx);
+            }
+        };
     }
 
-    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 ctx) {
+            this.sourcePath = cu.getSourcePath().toAbsolutePath();
+            this.currentCompilationUnit = cu;
+            return super.visitCompilationUnit(cu, ctx);
+        }
+
+        @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 +163,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 visited.withStatements(newStatements);
+        }
+
+        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<>();
+
+            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 declarations;
         }
 
-        private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable literal) {
-            final J.CompilationUnit cursor = getCursor().firstEnclosing(J.CompilationUnit.class);
+        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<>()));
 
-            final int line = PositionHelper.computeLinePosition(cursor, literal, getCursor());
-            final int column = PositionHelper.computeColumnPosition(cursor, literal, getCursor());
+            modifiers.addAll(varDecl.getModifiers());
 
-            return violations.stream().anyMatch(violation -> {
-                return violation.getLine() == line
-                        && violation.getColumn() == column
-                        && Path.of(violation.getFileName()).endsWith(sourcePath);
-            });
+            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> M withId(UUID id) {
+            return null;
         }
     }
 }
diff --git a/src/test/java/org/checkstyle/autofix/recipe/FinalLocalVariableTest.java b/src/test/java/org/checkstyle/autofix/recipe/FinalLocalVariableTest.java
@@ -51,4 +51,9 @@ void annotationDeclaration() throws Exception {
         verify("AnnotationDeclaration");
     }
 
+    @Test
+    void multiple() throws Exception {
+        verify("MultipleVariable");
+    }
+
 }
diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/DiffMultipleVariable.diff b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/DiffMultipleVariable.diff
@@ -0,0 +1,24 @@
+--- src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/InputMultipleVariable.java
++++ src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/OutputMultipleVariable.java
+@@ -8,15 +8,17 @@
+ 
+ package org.checkstyle.autofix.recipe.finallocalvariable.multiplevariable;
+ 
+-public class InputMultipleVariable {
++public class OutputMultipleVariable {
+ 
+     public void multipleDeclarations() {
+ 
+-        int x = 10, y = 20, z = 30;           // 3 violations
++        final int x = 10, y = 20, z = 30;
+ 
+-        String name = "John", city = "NYC";   // 2 violations
++        final String name = "John", city = "NYC";
+ 
+-        double price = 19.99, tax = 0.08;     // violation, "should be declared final"
++        double tax = 0.08;
++
++        final double price = 19.99;
+ 
+         tax = 50;
+         System.out.println("Sum: " + (x + y + z));
diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/InputMultipleVariable.java b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/InputMultipleVariable.java
@@ -0,0 +1,26 @@
+/*xml
+<module name="Checker">
+  <module name="TreeWalker">
+    <module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck"/>
+  </module>
+</module>
+*/
+
+package org.checkstyle.autofix.recipe.finallocalvariable.multiplevariable;
+
+public class InputMultipleVariable {
+
+    public void multipleDeclarations() {
+
+        int x = 10, y = 20, z = 30;           // 3 violations
+
+        String name = "John", city = "NYC";   // 2 violations
+
+        double price = 19.99, tax = 0.08;     // violation, "should be declared final"
+
+        tax = 50;
+        System.out.println("Sum: " + (x + y + z));
+        System.out.println(name + " lives in " + city);
+        System.out.println("Total: " + (price + (price * tax)));
+    }
+}
diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/OutputMultipleVariable.java b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/OutputMultipleVariable.java
@@ -0,0 +1,28 @@
+/*xml
+<module name="Checker">
+  <module name="TreeWalker">
+    <module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck"/>
+  </module>
+</module>
+*/
+
+package org.checkstyle.autofix.recipe.finallocalvariable.multiplevariable;
+
+public class OutputMultipleVariable {
+
+    public void multipleDeclarations() {
+
+        final int x = 10, y = 20, z = 30;
+
+        final String name = "John", city = "NYC";
+
+        double tax = 0.08;
+
+        final double price = 19.99;
+
+        tax = 50;
+        System.out.println("Sum: " + (x + y + z));
+        System.out.println(name + " lives in " + city);
+        System.out.println("Total: " + (price + (price * tax)));
+    }
+}
diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/DiffSingleLocalTest.diff b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/DiffSingleLocalTest.diff
@@ -1,6 +1,6 @@
 --- src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java
 +++ src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java
-@@ -15,36 +15,35 @@
+@@ -15,37 +15,37 @@
  import java.util.HashMap;
  import java.util.List;
  
@@ -17,7 +17,7 @@
 -        StringBuilder builder = new StringBuilder();       // violation, "should be declared final"
 -        Date currentDate = new Date();                     // violation, "should be declared final"
 -        File // prefix comment preceding target element
--        tempFile = new File("temp.txt");     // violation, "should be declared final"
+-                tempFile = new File("temp.txt");     // violation, "should be declared final"
 +        final String name = "John Doe";
 +        final int age = 25;
 +        final double salary = 50000.0;
@@ -28,7 +28,7 @@
 +        final StringBuilder builder = new StringBuilder();
 +        final Date currentDate = new Date();
 +        final File // prefix comment preceding target element
-+        tempFile = new File("temp.txt");
++                tempFile = new File("temp.txt");
      }
  
      public void methodWithCalculations() {
@@ -53,9 +53,12 @@
 -        String trimmed = input.trim();                     // violation, "should be declared final"
 -        String upperCase = trimmed.toUpperCase();          // violation, "should be declared final"
 -        String formatted = "[" + upperCase + "]";          // violation, "should be declared final"
+-        int a,b;                                           // violation, "should be declared final"
 +        final String trimmed = input.trim();
 +        final String upperCase = trimmed.toUpperCase();
 +        final String formatted = "[" + upperCase + "]";
-         return formatted;
-     }
- 
++        int a;
++        final int b;
+         a = upperCase.indexOf(formatted);
+         b = upperCase.indexOf(formatted);
+         a = a + formatted.length();
diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java
@@ -27,7 +27,7 @@ public void basicLocalVariables() {
         StringBuilder builder = new StringBuilder();       // violation, "should be declared final"
         Date currentDate = new Date();                     // violation, "should be declared final"
         File // prefix comment preceding target element
-        tempFile = new File("temp.txt");     // violation, "should be declared final"
+                tempFile = new File("temp.txt");     // violation, "should be declared final"
     }
 
     public void methodWithCalculations() {
@@ -45,6 +45,10 @@ public String processResult(String input) {
         String trimmed = input.trim();                     // violation, "should be declared final"
         String upperCase = trimmed.toUpperCase();          // violation, "should be declared final"
         String formatted = "[" + upperCase + "]";          // violation, "should be declared final"
+        int a,b;                                           // violation, "should be declared final"
+        a = upperCase.indexOf(formatted);
+        b = upperCase.indexOf(formatted);
+        a = a + formatted.length();
         return formatted;
     }
 
diff --git a/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java b/src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java

Original file line number	Diff line number	Diff line change
`@@ -51,4 +51,9 @@ void annotationDeclaration() throws Exception {`
`51`	`51`	`verify("AnnotationDeclaration");`
`52`	`52`	`}`
`53`	`53`
	`54`	`+ @Test`
	`55`	`+ void multiple() throws Exception {`
	`56`	`+ verify("MultipleVariable");`
	`57`	`+ }`
	`58`	`+`
`54`	`59`	`}`