Issue #80: Resolve MultiVariableDeclaration Bug

Anmol202005 · Anmol202005 · commit 92617e9a41ce · 2025-08-29T10:01:39.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,7 +19,9 @@
 
 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;
@@ -30,6 +32,8 @@
 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 +60,89 @@ public String getDescription() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new LocalVariableVisitor();
+        return new JavaIsoVisitor<>() {
+            @Override
+            public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu,
+                                                          ExecutionContext ctx) {
+                J.CompilationUnit compilationUnit = cu;
+                compilationUnit = new MarkViolationVisitor()
+                        .visitCompilationUnit(compilationUnit, ctx);
+
+                compilationUnit = new LocalVariableVisitor()
+                        .visitCompilationUnit(compilationUnit, ctx);
+
+                return compilationUnit;
+            }
+        };
     }
 
-    private final class LocalVariableVisitor extends JavaIsoVisitor<ExecutionContext> {
+    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(UUID.randomUUID()))));
+                    }
+                    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());
+            });
+        }
+    }
+
+    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,7 +159,7 @@ public J.VariableDeclarations visitVariableDeclarations(
                     && !declarations.hasModifier(J.Modifier.Type.Final)) {
                 final J.VariableDeclarations.NamedVariable variable = declarations
                         .getVariables().get(0);
-                if (isAtViolationLocation(variable)) {
+                if (variable.getMarkers().findFirst(FinalLocalVariableMarker.class).isPresent()) {
                     final List<J.Modifier> modifiers = new ArrayList<>();
 
                     final Space finalPrefix = declarations.getTypeExpression().getPrefix();
@@ -99,17 +175,119 @@ public J.VariableDeclarations visitVariableDeclarations(
             return declarations;
         }
 
-        private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable literal) {
-            final J.CompilationUnit cursor = getCursor().firstEnclosing(J.CompilationUnit.class);
+        @Override
+        public J.Block visitBlock(J.Block block, ExecutionContext executionContext) {
+            J.Block visited = super.visitBlock(block, executionContext);
 
-            final int line = PositionHelper.computeLinePosition(cursor, literal, getCursor());
-            final int column = PositionHelper.computeColumnPosition(cursor, literal, getCursor());
+            final List<Statement> newStatements = new ArrayList<>();
+            boolean changed = false;
 
-            return violations.stream().anyMatch(violation -> {
-                return violation.getLine() == line
-                        && violation.getColumn() == column
-                        && Path.of(violation.getFileName()).endsWith(sourcePath);
-            });
+            for (Statement stmt : visited.getStatements()) {
+                if (!(stmt instanceof J.VariableDeclarations)) {
+                    newStatements.add(stmt);
+                    continue;
+                }
+
+                final J.VariableDeclarations varDecl = (J.VariableDeclarations) stmt;
+
+                if (varDecl.hasModifier(J.Modifier.Type.Final) || getCursor().getParentTreeCursor()
+                        .getValue() instanceof J.ClassDeclaration) {
+                    newStatements.add(stmt);
+                    continue;
+                }
+
+                if (varDecl.getVariables().size() > 1) {
+                    changed |= handleMultiVariableDeclaration(varDecl, newStatements);
+                }
+                else {
+                    newStatements.add(stmt);
+                }
+            }
+
+            if (changed) {
+                visited = visited.withStatements(newStatements);
+            }
+
+            return visited;
+        }
+
+        private boolean 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));
+                }
+            }
+
+            boolean changed = false;
+
+            if (violationsList.isEmpty()) {
+                newStatements.add(varDecl);
+            }
+            else if (nonViolations.isEmpty()) {
+                newStatements.add(addFinalModifier(varDecl));
+                changed = true;
+            }
+            else {
+                newStatements.add(varDecl.withVariables(nonViolations));
+                for (J.VariableDeclarations.NamedVariable variable : violationsList) {
+                    newStatements.add(addFinalModifier(varDecl
+                            .withVariables(Collections.singletonList(variable))));
+                }
+                changed = true;
+            }
+
+            return changed;
+        }
+
+        private J.VariableDeclarations addFinalModifier(J.VariableDeclarations varDecl) {
+            final List<J.Modifier> modifiers = new ArrayList<>();
+            final Space finalPrefix;
+            if (varDecl.getTypeExpression() != null) {
+                finalPrefix = varDecl.getTypeExpression().getPrefix();
+            }
+            else {
+                finalPrefix = Space.EMPTY;
+            }
+
+            modifiers.add(new J.Modifier(Tree.randomId(), finalPrefix,
+                    Markers.EMPTY, null, J.Modifier.Type.Final, new ArrayList<>()));
+            modifiers.addAll(varDecl.getModifiers());
+
+            J.VariableDeclarations result = varDecl.withModifiers(modifiers);
+
+            if (result.getTypeExpression() != null) {
+                result = result.withTypeExpression(
+                        result.getTypeExpression().withPrefix(Space.SINGLE_SPACE));
+            }
+
+            return result;
+        }
+    }
+
+    public static class FinalLocalVariableMarker implements Marker {
+        private final UUID id;
+
+        public FinalLocalVariableMarker(UUID uuid) {
+            this.id = uuid;
+        }
+
+        @Override
+        public UUID getId() {
+            return id;
         }
+
+        @Override
+        public <M extends Marker> M withId(UUID uuid) {
+            return (M) new FinalLocalVariableMarker(uuid);
+        }
+
     }
+
 }
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
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`	`}`