Skip to content

Commit 03a10eb

Browse files
Anmol202005Anmol202005
committed
Issue #80: Resolve MultiVariableDeclaration Bug
1 parent 85c5edb commit 03a10eb

File tree

8 files changed

+262
-30
lines changed

8 files changed

+262
-30
lines changed

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

Lines changed: 159 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919

2020
import java.nio.file.Path;
2121
import java.util.ArrayList;
22+
import java.util.Collections;
2223
import java.util.List;
24+
import java.util.UUID;
2325

2426
import org.checkstyle.autofix.PositionHelper;
2527
import org.checkstyle.autofix.parser.CheckstyleViolation;
@@ -30,6 +32,8 @@
3032
import org.openrewrite.java.JavaIsoVisitor;
3133
import org.openrewrite.java.tree.J;
3234
import org.openrewrite.java.tree.Space;
35+
import org.openrewrite.java.tree.Statement;
36+
import org.openrewrite.marker.Marker;
3337
import org.openrewrite.marker.Markers;
3438

3539
/**
@@ -56,17 +60,92 @@ public String getDescription() {
5660

5761
@Override
5862
public TreeVisitor<?, ExecutionContext> getVisitor() {
59-
return new LocalVariableVisitor();
63+
return new JavaIsoVisitor<>() {
64+
@Override
65+
public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu,
66+
ExecutionContext ctx) {
67+
return new LocalVariableVisitor()
68+
.visitCompilationUnit(new MarkViolationVisitor()
69+
.visitCompilationUnit(cu, ctx), ctx);
70+
}
71+
};
6072
}
6173

62-
private final class LocalVariableVisitor extends JavaIsoVisitor<ExecutionContext> {
74+
/**
75+
* Visitor that identifies and marks variable declarations at violation locations.
76+
* This visitor traverses the AST and adds markers to variables that match
77+
* the checkstyle violation locations, preparing them for the final modifier addition.
78+
*/
79+
private final class MarkViolationVisitor extends JavaIsoVisitor<ExecutionContext> {
6380

6481
private Path sourcePath;
82+
private J.CompilationUnit currentCompilationUnit;
83+
84+
@Override
85+
public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) {
86+
this.sourcePath = cu.getSourcePath().toAbsolutePath();
87+
this.currentCompilationUnit = cu;
88+
return super.visitCompilationUnit(cu, ctx);
89+
}
90+
91+
@Override
92+
public J.VariableDeclarations visitVariableDeclarations(
93+
J.VariableDeclarations multiVariable, ExecutionContext executionContext) {
94+
95+
final J.VariableDeclarations variableDeclarations;
96+
97+
final J.VariableDeclarations declarations = super
98+
.visitVariableDeclarations(multiVariable, executionContext);
99+
100+
if (!(getCursor().getParentTreeCursor().getValue() instanceof J.ClassDeclaration)
101+
&& !declarations.hasModifier(J.Modifier.Type.Final)) {
102+
103+
final List<J.VariableDeclarations.NamedVariable> variables = declarations
104+
.getVariables();
105+
final List<J.VariableDeclarations.NamedVariable> marked = new ArrayList<>();
106+
for (J.VariableDeclarations.NamedVariable variable : variables) {
107+
if (isAtViolationLocation(variable)) {
108+
marked.add(variable.withMarkers(
109+
variable.getMarkers().add(new FinalLocalVariableMarker())));
110+
}
111+
else {
112+
marked.add(variable);
113+
}
114+
}
115+
variableDeclarations = declarations.withVariables(marked);
116+
}
117+
else {
118+
variableDeclarations = declarations;
119+
}
120+
return variableDeclarations;
121+
}
122+
123+
private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable variable) {
124+
125+
final int line = PositionHelper
126+
.computeLinePosition(currentCompilationUnit, variable, getCursor());
127+
final int column = PositionHelper
128+
.computeColumnPosition(currentCompilationUnit, variable, getCursor());
129+
130+
return violations.removeIf(violation -> {
131+
final Path absolutePath = Path.of(violation.getFileName()).toAbsolutePath();
132+
return violation.getLine() == line
133+
&& violation.getColumn() == column
134+
&& absolutePath.endsWith(sourcePath)
135+
&& violation.getMessage().contains(variable.getSimpleName());
136+
});
137+
}
138+
}
139+
140+
/**
141+
* Visitor that processes marked variable declarations and applies the final modifier.
142+
* This visitor handles both single and multi-variable declarations.
143+
*/
144+
private final class LocalVariableVisitor extends JavaIsoVisitor<ExecutionContext> {
65145

66146
@Override
67147
public J.CompilationUnit visitCompilationUnit(
68148
J.CompilationUnit cu, ExecutionContext executionContext) {
69-
this.sourcePath = cu.getSourcePath();
70149
return super.visitCompilationUnit(cu, executionContext);
71150
}
72151

@@ -83,33 +162,91 @@ public J.VariableDeclarations visitVariableDeclarations(
83162
&& !declarations.hasModifier(J.Modifier.Type.Final)) {
84163
final J.VariableDeclarations.NamedVariable variable = declarations
85164
.getVariables().get(0);
86-
if (isAtViolationLocation(variable)) {
87-
final List<J.Modifier> modifiers = new ArrayList<>();
165+
if (variable.getMarkers().findFirst(FinalLocalVariableMarker.class).isPresent()) {
166+
declarations = addFinalModifier(declarations);
167+
}
168+
}
169+
return declarations;
170+
}
171+
172+
@Override
173+
public J.Block visitBlock(J.Block block, ExecutionContext executionContext) {
174+
final J.Block visited = super.visitBlock(block, executionContext);
88175

89-
final Space finalPrefix = declarations.getTypeExpression().getPrefix();
176+
final List<Statement> newStatements = new ArrayList<>();
90177

91-
modifiers.add(new J.Modifier(Tree.randomId(), finalPrefix,
92-
Markers.EMPTY, null, J.Modifier.Type.Final, new ArrayList<>()));
93-
modifiers.addAll(declarations.getModifiers());
94-
declarations = declarations.withModifiers(modifiers)
95-
.withTypeExpression(declarations.getTypeExpression()
96-
.withPrefix(Space.SINGLE_SPACE));
178+
for (Statement stmt : visited.getStatements()) {
179+
if (isVariableDeclaration(stmt)) {
180+
handleMultiVariableDeclaration((J.VariableDeclarations) stmt, newStatements);
181+
}
182+
else {
183+
newStatements.add(stmt);
184+
}
185+
}
186+
187+
return visited.withStatements(newStatements);
188+
}
189+
190+
private void handleMultiVariableDeclaration(J.VariableDeclarations varDecl,
191+
List<Statement> newStatements) {
192+
final List<J.VariableDeclarations.NamedVariable> violationsList = new ArrayList<>();
193+
final List<J.VariableDeclarations.NamedVariable> nonViolations = new ArrayList<>();
194+
195+
for (J.VariableDeclarations.NamedVariable variable : varDecl.getVariables()) {
196+
if (variable.getMarkers().findFirst(FinalLocalVariableMarker.class).isPresent()) {
197+
violationsList.add(variable.withPrefix(Space.SINGLE_SPACE));
198+
}
199+
else {
200+
nonViolations.add(variable.withPrefix(Space.SINGLE_SPACE));
201+
}
202+
}
203+
if (violationsList.isEmpty()) {
204+
newStatements.add(varDecl);
205+
}
206+
else if (nonViolations.isEmpty()) {
207+
newStatements.add(addFinalModifier(varDecl));
208+
}
209+
else {
210+
newStatements.add(varDecl.withVariables(nonViolations));
211+
for (J.VariableDeclarations.NamedVariable variable : violationsList) {
212+
newStatements.add(addFinalModifier(varDecl
213+
.withVariables(Collections.singletonList(variable))));
97214
}
98215
}
99-
return declarations;
100216
}
101217

102-
private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable literal) {
103-
final J.CompilationUnit cursor = getCursor().firstEnclosing(J.CompilationUnit.class);
218+
private J.VariableDeclarations addFinalModifier(J.VariableDeclarations varDecl) {
219+
final List<J.Modifier> modifiers = new ArrayList<>(varDecl.getModifiers());
220+
final Space finalPrefix = varDecl.getTypeExpression().getPrefix();
221+
modifiers.add(new J.Modifier(Tree.randomId(), finalPrefix,
222+
Markers.EMPTY, null, J.Modifier.Type.Final, new ArrayList<>()));
104223

105-
final int line = PositionHelper.computeLinePosition(cursor, literal, getCursor());
106-
final int column = PositionHelper.computeColumnPosition(cursor, literal, getCursor());
224+
modifiers.addAll(varDecl.getModifiers());
107225

108-
return violations.stream().anyMatch(violation -> {
109-
return violation.getLine() == line
110-
&& violation.getColumn() == column
111-
&& Path.of(violation.getFileName()).endsWith(sourcePath);
112-
});
226+
return varDecl.withModifiers(modifiers)
227+
.withTypeExpression(varDecl.getTypeExpression().withPrefix(Space.SINGLE_SPACE));
228+
}
229+
230+
private boolean isVariableDeclaration(Statement stmt) {
231+
return stmt instanceof J.VariableDeclarations varDecl
232+
&& varDecl.getVariables().size() > 1
233+
&& !varDecl.hasModifier(J.Modifier.Type.Final)
234+
&& varDecl.getTypeExpression() != null
235+
&& !(getCursor().getParentTreeCursor()
236+
.getValue() instanceof J.ClassDeclaration);
237+
}
238+
}
239+
240+
public static class FinalLocalVariableMarker implements Marker {
241+
242+
@Override
243+
public UUID getId() {
244+
return null;
245+
}
246+
247+
@Override
248+
public <M extends Marker> M withId(UUID id) {
249+
return null;
113250
}
114251
}
115252
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,9 @@ void annotationDeclaration() throws Exception {
5151
verify("AnnotationDeclaration");
5252
}
5353

54+
@Test
55+
void multiple() throws Exception {
56+
verify("MultipleVariable");
57+
}
58+
5459
}

‎src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/DiffMultipleVariable.diff‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--- src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/InputMultipleVariable.java
2+
+++ src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/OutputMultipleVariable.java
3+
@@ -8,15 +8,17 @@
4+
5+
package org.checkstyle.autofix.recipe.finallocalvariable.multiplevariable;
6+
7+
-public class InputMultipleVariable {
8+
+public class OutputMultipleVariable {
9+
10+
public void multipleDeclarations() {
11+
12+
- int x = 10, y = 20, z = 30; // 3 violations
13+
+ final int x = 10, y = 20, z = 30;
14+
15+
- String name = "John", city = "NYC"; // 2 violations
16+
+ final String name = "John", city = "NYC";
17+
18+
- double price = 19.99, tax = 0.08; // violation, "should be declared final"
19+
+ double tax = 0.08;
20+
+
21+
+ final double price = 19.99;
22+
23+
tax = 50;
24+
System.out.println("Sum: " + (x + y + z));

‎src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/InputMultipleVariable.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*xml
2+
<module name="Checker">
3+
<module name="TreeWalker">
4+
<module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck"/>
5+
</module>
6+
</module>
7+
*/
8+
9+
package org.checkstyle.autofix.recipe.finallocalvariable.multiplevariable;
10+
11+
public class InputMultipleVariable {
12+
13+
public void multipleDeclarations() {
14+
15+
int x = 10, y = 20, z = 30; // 3 violations
16+
17+
String name = "John", city = "NYC"; // 2 violations
18+
19+
double price = 19.99, tax = 0.08; // violation, "should be declared final"
20+
21+
tax = 50;
22+
System.out.println("Sum: " + (x + y + z));
23+
System.out.println(name + " lives in " + city);
24+
System.out.println("Total: " + (price + (price * tax)));
25+
}
26+
}

‎src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/multiplevariable/OutputMultipleVariable.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*xml
2+
<module name="Checker">
3+
<module name="TreeWalker">
4+
<module name="com.puppycrawl.tools.checkstyle.checks.coding.FinalLocalVariableCheck"/>
5+
</module>
6+
</module>
7+
*/
8+
9+
package org.checkstyle.autofix.recipe.finallocalvariable.multiplevariable;
10+
11+
public class OutputMultipleVariable {
12+
13+
public void multipleDeclarations() {
14+
15+
final int x = 10, y = 20, z = 30;
16+
17+
final String name = "John", city = "NYC";
18+
19+
double tax = 0.08;
20+
21+
final double price = 19.99;
22+
23+
tax = 50;
24+
System.out.println("Sum: " + (x + y + z));
25+
System.out.println(name + " lives in " + city);
26+
System.out.println("Total: " + (price + (price * tax)));
27+
}
28+
}

‎src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/DiffSingleLocalTest.diff‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
--- src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/InputSingleLocalTest.java
22
+++ src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/singlelocaltest/OutputSingleLocalTest.java
3-
@@ -15,36 +15,35 @@
3+
@@ -15,37 +15,37 @@
44
import java.util.HashMap;
55
import java.util.List;
66

@@ -17,7 +17,7 @@
1717
- StringBuilder builder = new StringBuilder(); // violation, "should be declared final"
1818
- Date currentDate = new Date(); // violation, "should be declared final"
1919
- File // prefix comment preceding target element
20-
- tempFile = new File("temp.txt"); // violation, "should be declared final"
20+
- tempFile = new File("temp.txt"); // violation, "should be declared final"
2121
+ final String name = "John Doe";
2222
+ final int age = 25;
2323
+ final double salary = 50000.0;
@@ -28,7 +28,7 @@
2828
+ final StringBuilder builder = new StringBuilder();
2929
+ final Date currentDate = new Date();
3030
+ final File // prefix comment preceding target element
31-
+ tempFile = new File("temp.txt");
31+
+ tempFile = new File("temp.txt");
3232
}
3333

3434
public void methodWithCalculations() {
@@ -53,9 +53,12 @@
5353
- String trimmed = input.trim(); // violation, "should be declared final"
5454
- String upperCase = trimmed.toUpperCase(); // violation, "should be declared final"
5555
- String formatted = "[" + upperCase + "]"; // violation, "should be declared final"
56+
- int a,b; // violation, "should be declared final"
5657
+ final String trimmed = input.trim();
5758
+ final String upperCase = trimmed.toUpperCase();
5859
+ final String formatted = "[" + upperCase + "]";
59-
return formatted;
60-
}
61-
60+
+ int a;
61+
+ final int b;
62+
a = upperCase.indexOf(formatted);
63+
b = upperCase.indexOf(formatted);
64+
a = a + formatted.length();

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public void basicLocalVariables() {
2727
StringBuilder builder = new StringBuilder(); // violation, "should be declared final"
2828
Date currentDate = new Date(); // violation, "should be declared final"
2929
File // prefix comment preceding target element
30-
tempFile = new File("temp.txt"); // violation, "should be declared final"
30+
tempFile = new File("temp.txt"); // violation, "should be declared final"
3131
}
3232

3333
public void methodWithCalculations() {
@@ -45,6 +45,10 @@ public String processResult(String input) {
4545
String trimmed = input.trim(); // violation, "should be declared final"
4646
String upperCase = trimmed.toUpperCase(); // violation, "should be declared final"
4747
String formatted = "[" + upperCase + "]"; // violation, "should be declared final"
48+
int a,b; // violation, "should be declared final"
49+
a = upperCase.indexOf(formatted);
50+
b = upperCase.indexOf(formatted);
51+
a = a + formatted.length();
4852
return formatted;
4953
}
5054

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public void basicLocalVariables() {
2727
final StringBuilder builder = new StringBuilder();
2828
final Date currentDate = new Date();
2929
final File // prefix comment preceding target element
30-
tempFile = new File("temp.txt");
30+
tempFile = new File("temp.txt");
3131
}
3232

3333
public void methodWithCalculations() {
@@ -44,6 +44,11 @@ public String processResult(String input) {
4444
final String trimmed = input.trim();
4545
final String upperCase = trimmed.toUpperCase();
4646
final String formatted = "[" + upperCase + "]";
47+
int a;
48+
final int b;
49+
a = upperCase.indexOf(formatted);
50+
b = upperCase.indexOf(formatted);
51+
a = a + formatted.length();
4752
return formatted;
4853
}
4954

0 commit comments

Comments
 (0)