Issue #80: Resolve MultiVariableDeclaration Bug

Author: Anmol202005

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,84 @@ 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> {
+    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,33 +154,96 @@ 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<>();
+            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 {
+        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);
         }
     }
 }

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");
+    }
+
 }

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));

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)));
+    }
+}

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)));
+    }
+}

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();

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;
     }
 

src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java

@@ -27,7 +27,7 @@ public void basicLocalVariables() {
         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() {
@@ -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;
     }