Issue #80: Resolved MultiVariable declaration Bug

Anmol202005 · Anmol202005 · commit 479580f45a01 · 2025-08-20T23:56:26.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,6 +19,7 @@
 
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.checkstyle.autofix.PositionHelper;
@@ -30,6 +31,7 @@
 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.Markers;
 
 /**
@@ -56,51 +58,140 @@ public String getDescription() {
 
     @Override
     public TreeVisitor<?, ExecutionContext> getVisitor() {
-        return new LocalVariableVisitor();
+        return new FinalLocalVariableVisitor();
     }
 
-    private final class LocalVariableVisitor extends JavaIsoVisitor<ExecutionContext> {
+    private final class FinalLocalVariableVisitor extends JavaIsoVisitor<ExecutionContext> {
 
         private Path sourcePath;
 
         @Override
-        public J.CompilationUnit visitCompilationUnit(
-                J.CompilationUnit cu, ExecutionContext executionContext) {
-            this.sourcePath = cu.getSourcePath();
-            return super.visitCompilationUnit(cu, executionContext);
+        public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) {
+            this.sourcePath = cu.getSourcePath().toAbsolutePath();
+            return super.visitCompilationUnit(cu, ctx);
         }
 
         @Override
         public J.VariableDeclarations visitVariableDeclarations(
                 J.VariableDeclarations multiVariable, ExecutionContext executionContext) {
 
-            J.VariableDeclarations declarations = super.visitVariableDeclarations(multiVariable,
-                    executionContext);
+            final J.VariableDeclarations declarations =
+                    super.visitVariableDeclarations(multiVariable, executionContext);
+
+            J.VariableDeclarations result = declarations;
 
             if (!(getCursor().getParentTreeCursor().getValue() instanceof J.ClassDeclaration)
-                    && declarations.getVariables().size() == 1
-                    && declarations.getTypeExpression() != null
                     && !declarations.hasModifier(J.Modifier.Type.Final)) {
-                final J.VariableDeclarations.NamedVariable variable = declarations
-                        .getVariables().get(0);
+
+                boolean hasViolation = false;
+                for (J.VariableDeclarations.NamedVariable variable : declarations.getVariables()) {
+                    if (isAtViolationLocation(variable)) {
+                        hasViolation = true;
+                        break;
+                    }
+                }
+
+                if (hasViolation && declarations.getVariables().size() == 1
+                        && declarations.getTypeExpression() != null) {
+                    result = addFinalModifier(declarations);
+                }
+            }
+
+            return result;
+        }
+
+        @Override
+        public J.Block visitBlock(J.Block block, ExecutionContext executionContext) {
+            J.Block visited = super.visitBlock(block, executionContext);
+
+            final List<Statement> newStatements = new ArrayList<>();
+            boolean changed = false;
+
+            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 (isAtViolationLocation(variable)) {
-                    final List<J.Modifier> modifiers = new ArrayList<>();
-                    modifiers.add(new J.Modifier(Tree.randomId(), Space.EMPTY,
-                            Markers.EMPTY, null, J.Modifier.Type.Final, new ArrayList<>()));
-                    modifiers.addAll(declarations.getModifiers());
-                    declarations = declarations.withModifiers(modifiers)
-                            .withTypeExpression(declarations.getTypeExpression()
-                                    .withPrefix(Space.SINGLE_SPACE));
+                    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<>();
+            modifiers.add(new J.Modifier(Tree.randomId(), Space.EMPTY,
+                    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 declarations;
+
+            return result;
         }
 
-        private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable literal) {
-            final J.CompilationUnit cursor = getCursor().firstEnclosing(J.CompilationUnit.class);
+        private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable variable) {
+            final J.CompilationUnit cu = getCursor().firstEnclosing(J.CompilationUnit.class);
 
-            final int line = PositionHelper.computeLinePosition(cursor, literal, getCursor());
-            final int column = PositionHelper.computeColumnPosition(cursor, literal, getCursor());
+            final int line = PositionHelper.computeLinePosition(cu, variable, getCursor());
+            final int column = PositionHelper.computeColumnPosition(cu, variable, getCursor());
 
             return violations.stream().anyMatch(violation -> {
                 return violation.getLine() == line
diff --git a/src/test/java/org/checkstyle/autofix/recipe/FinalLocalVariableTest.java b/src/test/java/org/checkstyle/autofix/recipe/FinalLocalVariableTest.java
@@ -46,4 +46,9 @@ void enhancedForLoop() throws Exception {
         verify("EnhancedForLoop");
     }
 
+    @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,22 @@
+--- 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,15 @@
+ 
+ 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;     // 2 violations
++        final double price = 19.99, tax = 0.08;
+ 
+         System.out.println("Sum: " + (x + y + z));
+         System.out.println(name + " lives in " + city);
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,25 @@
+/*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;     // 2 violations
+
+        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,25 @@
+/*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";
+
+        final double price = 19.99, tax = 0.08;
+
+        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;
  
@@ -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
@@ -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
@@ -44,6 +44,11 @@ public String processResult(String input) {
         final String trimmed = input.trim();
         final String upperCase = trimmed.toUpperCase();
         final String formatted = "[" + upperCase + "]";
+        int a;
+        final int b;
+        a = upperCase.indexOf(formatted);
+        b = upperCase.indexOf(formatted);
+        a = a + formatted.length();
         return formatted;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -46,4 +46,9 @@ void enhancedForLoop() throws Exception {`
`46`	`46`	`verify("EnhancedForLoop");`
`47`	`47`	`}`
`48`	`48`
	`49`	`+ @Test`
	`50`	`+ void multiple() throws Exception {`
	`51`	`+ verify("MultipleVariable");`
	`52`	`+ }`
	`53`	`+`
`49`	`54`	`}`