Skip to content

Commit fe50883

Browse files
Anmol202005romani
Anmol202005
authored and
romani
committed
Issue #80: Resolve MultiVariableDeclaration Bug
1 parent f042cc9 commit fe50883

File tree

44 files changed

+3804
-30
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+3804
-30
lines changed

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

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

66148
@Override
67149
public J.CompilationUnit visitCompilationUnit(
68150
J.CompilationUnit cu, ExecutionContext executionContext) {
69-
this.sourcePath = cu.getSourcePath();
70151
return super.visitCompilationUnit(cu, executionContext);
71152
}
72153

@@ -83,33 +164,97 @@ public J.VariableDeclarations visitVariableDeclarations(
83164
&& !declarations.hasModifier(J.Modifier.Type.Final)) {
84165
final J.VariableDeclarations.NamedVariable variable = declarations
85166
.getVariables().get(0);
86-
if (isAtViolationLocation(variable)) {
87-
final List<J.Modifier> modifiers = new ArrayList<>();
167+
if (variable.getMarkers().findFirst(FinalLocalVariableMarker.class).isPresent()) {
168+
declarations = addFinalModifier(declarations);
169+
}
170+
}
171+
return declarations;
172+
}
173+
174+
@Override
175+
public J.Block visitBlock(J.Block block, ExecutionContext executionContext) {
176+
final J.Block visited = super.visitBlock(block, executionContext);
88177

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

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));
180+
for (Statement stmt : visited.getStatements()) {
181+
if (isVariableDeclaration(stmt)) {
182+
handleMultiVariableDeclaration((J.VariableDeclarations) stmt, newStatements);
183+
}
184+
else {
185+
newStatements.add(stmt);
97186
}
98187
}
99-
return declarations;
188+
189+
return visited.withStatements(newStatements);
100190
}
101191

102-
private boolean isAtViolationLocation(J.VariableDeclarations.NamedVariable literal) {
103-
final J.CompilationUnit cursor = getCursor().firstEnclosing(J.CompilationUnit.class);
192+
private void handleMultiVariableDeclaration(J.VariableDeclarations varDecl,
193+
List<Statement> newStatements) {
194+
final List<J.VariableDeclarations.NamedVariable> violationsList = new ArrayList<>();
195+
final List<J.VariableDeclarations.NamedVariable> nonViolations = new ArrayList<>();
104196

105-
final int line = PositionHelper.computeLinePosition(cursor, literal, getCursor());
106-
final int column = PositionHelper.computeColumnPosition(cursor, literal, getCursor());
197+
for (J.VariableDeclarations.NamedVariable variable : varDecl.getVariables()) {
198+
if (variable.getMarkers().findFirst(FinalLocalVariableMarker.class).isPresent()) {
199+
violationsList.add(variable.withPrefix(Space.SINGLE_SPACE));
200+
}
201+
else {
202+
nonViolations.add(variable.withPrefix(Space.SINGLE_SPACE));
203+
}
204+
}
205+
if (violationsList.isEmpty()) {
206+
newStatements.add(varDecl);
207+
}
208+
else if (nonViolations.isEmpty()) {
209+
newStatements.add(addFinalModifier(varDecl));
210+
}
211+
else {
212+
newStatements.add(varDecl.withVariables(nonViolations));
213+
for (J.VariableDeclarations.NamedVariable variable : violationsList) {
214+
newStatements.add(addFinalModifier(varDecl
215+
.withVariables(Collections.singletonList(variable))));
216+
}
217+
}
218+
}
107219

108-
return violations.stream().anyMatch(violation -> {
109-
return violation.getLine() == line
110-
&& violation.getColumn() == column
111-
&& Path.of(violation.getFileName()).endsWith(sourcePath);
112-
});
220+
private J.VariableDeclarations addFinalModifier(J.VariableDeclarations varDecl) {
221+
final List<J.Modifier> modifiers = new ArrayList<>();
222+
final Space finalPrefix = varDecl.getTypeExpression().getPrefix();
223+
modifiers.add(new J.Modifier(Tree.randomId(), finalPrefix,
224+
Markers.EMPTY, null, J.Modifier.Type.Final, new ArrayList<>()));
225+
226+
modifiers.addAll(varDecl.getModifiers());
227+
228+
return varDecl.withModifiers(modifiers)
229+
.withTypeExpression(varDecl.getTypeExpression().withPrefix(Space.SINGLE_SPACE));
230+
}
231+
232+
private boolean isVariableDeclaration(Statement stmt) {
233+
return stmt instanceof J.VariableDeclarations varDecl
234+
&& varDecl.getVariables().size() > 1
235+
&& !varDecl.hasModifier(J.Modifier.Type.Final)
236+
&& varDecl.getTypeExpression() != null
237+
&& !(getCursor().getParentTreeCursor()
238+
.getValue() instanceof J.ClassDeclaration);
239+
}
240+
}
241+
242+
private static final class FinalLocalVariableMarker implements Marker {
243+
244+
private final UUID id;
245+
246+
private FinalLocalVariableMarker(UUID uuid) {
247+
this.id = uuid;
248+
}
249+
250+
@Override
251+
public UUID getId() {
252+
return id;
253+
}
254+
255+
@Override
256+
public <M extends Marker> M withId(UUID uuid) {
257+
return (M) new FinalLocalVariableMarker(uuid);
113258
}
114259
}
115260
}

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

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

54+
@Test
55+
void multiple() throws Exception {
56+
verify("MultipleVariable");
57+
}
58+
59+
@Test
60+
void localVariable() throws Exception {
61+
verify("LocalVariableOne");
62+
}
63+
64+
@Test
65+
void localVariableTwo() throws Exception {
66+
verify("LocalVariableTwo");
67+
}
68+
69+
@Test
70+
void localVariableThree() throws Exception {
71+
verify("LocalVariableThree");
72+
}
73+
74+
@Test
75+
void localVariableFour() throws Exception {
76+
verify("LocalVariableFour");
77+
}
78+
79+
@Test
80+
void localVariableFive() throws Exception {
81+
verify("LocalVariableFive");
82+
}
83+
84+
@Test
85+
void localVariableCheckRecord() throws Exception {
86+
verify("LocalVariableCheckRecord");
87+
}
88+
89+
@Test
90+
void finalLocalVariable2One() throws Exception {
91+
verify("FinalLocalVariable2One");
92+
}
93+
94+
@Test
95+
void finalLocalForLoop() throws Exception {
96+
verify("VariableEnhancedForLoopVariable");
97+
}
98+
99+
@Test
100+
void finalLocalForLoop2() throws Exception {
101+
verify("VariableEnhancedForLoopVariable2");
102+
}
103+
104+
@Test
105+
void localVariableAssignedMultipleTimes() throws Exception {
106+
verify("LocalVariableAssignedMultipleTimes");
107+
}
108+
109+
@Test
110+
void localVariableCheckSwitchExpressions() throws Exception {
111+
verify("LocalVariableCheckSwitchExpressions");
112+
}
113+
114+
@Test
115+
void localVariableCheckSwitchAssignment() throws Exception {
116+
verify("LocalVariableCheckSwitchAssignment");
117+
}
118+
54119
}

‎src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/finallocalvariable2one/DiffFinalLocalVariable2One.diff‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--- src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/finallocalvariable2one/InputFinalLocalVariable2One.java
2+
+++ src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/finallocalvariable2one/OutputFinalLocalVariable2One.java
3+
@@ -10,7 +10,7 @@
4+
*/
5+
package org.checkstyle.autofix.recipe.finallocalvariable.finallocalvariable2one;
6+
7+
-public class InputFinalLocalVariable2One {
8+
+public class OutputFinalLocalVariable2One {
9+
private int m_ClassVariable = 0;
10+
//static block
11+
static
12+
@@ -49,9 +49,7 @@
13+
}
14+
};
15+
}
16+
-
17+
- // violation below "Variable 'aArg' should be declared final"
18+
- public void method(int aArg, final int aFinal, int aArg2)
19+
+ public void method(final int aArg, final int aFinal, int aArg2)
20+
{
21+
int z = 0;
22+

0 commit comments

Comments
 (0)