Skip to content

Commit ad96b86

Browse files
fix: skip finalizing anonymous class fields in FinalizeLocalVariables (#182)
* fix: skip finalizing anonymous class fields * chore: add issue to test * chore: mark contributed code * chore: update anonymous class identifying logic * Apply formatter * chore: remove javafx types from test * Polish * Apply suggestions from code review --------- Co-authored-by: Tim te Beek <[email protected]> Co-authored-by: Tim te Beek <[email protected]>
1 parent c41a24a commit ad96b86

File tree

2 files changed

+99
-65
lines changed

2 files changed

+99
-65
lines changed

src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ public String getDescription() {
4444
public TreeVisitor<?, ExecutionContext> getVisitor() {
4545
return new JavaIsoVisitor<ExecutionContext>() {
4646

47-
@Override
48-
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext p) {
47+
@Override
48+
public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext p) {
4949
J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, p);
5050

5151
// if this already has "final", we don't need to bother going any further; we're done
@@ -67,6 +67,11 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
6767
return mv;
6868
}
6969

70+
// ignores anonymous class fields, contributed code for issue #181
71+
if (this.getCursorToParentScope(this.getCursor()).getValue() instanceof J.NewClass) {
72+
return mv;
73+
}
74+
7075
if (mv.getVariables().stream()
7176
.noneMatch(v -> {
7277
Cursor declaringCursor = v.getDeclaringScope(getCursor());
@@ -80,6 +85,10 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
8085

8186
return mv;
8287
}
88+
89+
private Cursor getCursorToParentScope(final Cursor cursor) {
90+
return cursor.dropParentUntil(is -> is instanceof J.NewClass || is instanceof J.ClassDeclaration);
91+
}
8392
};
8493
}
8594

src/test/java/org/openrewrite/staticanalysis/FinalizeLocalVariablesTest.java

Lines changed: 88 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -225,14 +225,14 @@ class Test {
225225
}
226226
""",
227227
"""
228-
class Test {
229-
static {
230-
final int n = 1;
231-
for(int i = 0; i < n; i++) {
232-
}
233-
}
234-
}
235-
"""
228+
class Test {
229+
static {
230+
final int n = 1;
231+
for(int i = 0; i < n; i++) {
232+
}
233+
}
234+
}
235+
"""
236236
)
237237
);
238238
}
@@ -282,7 +282,7 @@ void forLoopVariablesIgnored() {
282282
java(
283283
"""
284284
import java.util.concurrent.FutureTask;
285-
285+
286286
class A {
287287
void f() {
288288
for(FutureTask<?> future; (future = new FutureTask<>(() -> "hello world")) != null;) { }
@@ -296,34 +296,36 @@ void f() {
296296
@Test
297297
void shouldNotFinalizeForCounterWhichIsReassignedWithinForHeader() {
298298
rewriteRun(
299-
//language=java
300-
java("""
301-
class A {
302-
static {
303-
for (int i = 0; i < 10; i++) {
304-
// no-op
305-
}
306-
}
307-
}
308-
"""
309-
)
299+
//language=java
300+
java(
301+
"""
302+
class A {
303+
static {
304+
for (int i = 0; i < 10; i++) {
305+
// no-op
306+
}
307+
}
308+
}
309+
"""
310+
)
310311
);
311312
}
312313

313314
@Test
314315
void shouldNotFinalizeForCounterWhichIsReassignedWithinForBody() {
315316
rewriteRun(
316-
//language=java
317-
java("""
318-
class A {
319-
static {
320-
for (int i = 0; i < 10;) {
321-
i = 11;
322-
}
323-
}
324-
}
325-
"""
326-
)
317+
//language=java
318+
java(
319+
"""
320+
class A {
321+
static {
322+
for (int i = 0; i < 10;) {
323+
i = 11;
324+
}
325+
}
326+
}
327+
"""
328+
)
327329
);
328330
}
329331

@@ -394,48 +396,71 @@ class Person {
394396
void recordShouldNotIntroduceExtraClosingParenthesis() {
395397
rewriteRun(
396398
version(
397-
//language=java
398-
java(
399-
"""
400-
public class Main {
401-
public static void test() {
402-
var myVar = "";
403-
}
404-
public record EmptyRecord() {
405-
}
406-
}
407-
""",
408-
"""
409-
public class Main {
410-
public static void test() {
411-
final var myVar = "";
412-
}
413-
public record EmptyRecord() {
399+
//language=java
400+
java(
401+
"""
402+
public class Main {
403+
public static void test() {
404+
var myVar = "";
405+
}
406+
public record EmptyRecord() {
407+
}
414408
}
415-
}
409+
""",
416410
"""
417-
), 17)
411+
public class Main {
412+
public static void test() {
413+
final var myVar = "";
414+
}
415+
public record EmptyRecord() {
416+
}
417+
}
418+
"""
419+
), 17)
418420
);
419421
}
420422

421423
@Test
422424
void shouldNotFinalizeVariableWhichIsReassignedInAnotherSwitchBranch() {
423425
rewriteRun(
424-
//language=java
425-
java("""
426-
class A {
427-
static int variable = 0;
428-
static {
429-
switch (variable) {
430-
case 0:
431-
int notFinalized = 0;
432-
default:
433-
notFinalized = 1;
434-
}
426+
//language=java
427+
java(
428+
"""
429+
class A {
430+
static int variable = 0;
431+
static {
432+
switch (variable) {
433+
case 0:
434+
int notFinalized = 0;
435+
default:
436+
notFinalized = 1;
435437
}
436438
}
437-
"""
438-
)
439+
}
440+
"""
441+
)
442+
);
443+
}
444+
445+
@Test
446+
@Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/181")
447+
void shouldNotFinalizeVariablesWhichAreAssignedInAnonymousClasses() {
448+
this.rewriteRun(
449+
// language=java
450+
java(
451+
"""
452+
class Test {
453+
private final Object objectWithInnerField = new Object() {
454+
private int count = 0;
455+
@Override
456+
public String toString() {
457+
this.count++;
458+
return super.toString() + this.count;
459+
}
460+
};
461+
}
462+
"""
463+
)
439464
);
440465
}
441466
}

0 commit comments

Comments
 (0)