Issue #33: FinalLocalVariable recipe created #40
Conversation
|
the recipe currently has partial coverage: When multiple variables are declared together like |
|
@rdiachenko kindly review. |
| import org.junit.jupiter.api.Test; | ||
| import org.openrewrite.Recipe; | ||
|
|
||
| public class FinalLocalVariableTest extends AbstractRecipeTest { |
There was a problem hiding this comment.
I see that https://checkstyle.sourceforge.io/checks/coding/finallocalvariable.html#FinalLocalVariable has different params
Can you please add tests covering the following cases to make sure it works correctly
for (String item : items) {
for (int i = 0; i < 10; i++)
public void processData(String data, int count)
final int alreadyFinal
try (FileReader fr = new FileReader(filename)) {
int a, b, c;
items.forEach(item -> System.out.println(item));
If it requires too much rework, let me know
There was a problem hiding this comment.
Will apply fixes on the bases of violation report.
None of the param will be helpful.
There was a problem hiding this comment.
Sorry, let me clarify
Not asking to support FinalLocalVariable's params
What I meant is that Checkstyle's FinalLocalVariable might fail for different positions based on the configurations
For example, validateEnhancedForLoopVariable means that the FinalLocalVariable check might fail for for (String item : items) { - will your recipe fix it?
There was a problem hiding this comment.
Any questions or blockers here?
src/test/resources/org/checkstyle/autofix/recipe/finallocalvariable/report.xml
Outdated
Show resolved
Hide resolved
|
@timurt kindly review. Added new tests. |
|
was thinking to make this PR small by raising PR to separate Logic for line:column calculation like: #47 But idk if @rdiachenko prefer this :( |
timurt
left a comment
timurt
left a comment
There was a problem hiding this comment.
minor comments, but overall LGTM
| { | ||
| final int[] squares = {0, 1, 4, 9, 16, 25}; | ||
| int x; | ||
| for (final int i : squares) { |
There was a problem hiding this comment.
You probably want to remove the final here, to test that it is being added by the recipe
| { | ||
| final java.util.List<Object> list = new java.util.ArrayList<>(); | ||
|
|
||
| for(final Object a : list){ |
@Anmol202005 is this still an issue? If yes, let's try to fix it in a new PR |
3 participants
Fixes: #33
Implemented FinalLocalVariable recipe.