From ad96b86d41e3beae3f38ffda20de0bdc092a106d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Bertolo=20Fafi=C3=A1n?= <60966552+AlejandroBertolo@users.noreply.github.com> Date: Mon, 2 Oct 2023 13:44:49 +0200 Subject: [PATCH] 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 Co-authored-by: Tim te Beek --- .../FinalizeLocalVariables.java | 13 +- .../FinalizeLocalVariablesTest.java | 151 ++++++++++-------- 2 files changed, 99 insertions(+), 65 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java b/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java index 1eb96a7c0..aca7846ba 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java @@ -44,8 +44,8 @@ public String getDescription() { public TreeVisitor getVisitor() { return new JavaIsoVisitor() { - @Override - public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext p) { + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext p) { J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, p); // 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 return mv; } + // ignores anonymous class fields, contributed code for issue #181 + if (this.getCursorToParentScope(this.getCursor()).getValue() instanceof J.NewClass) { + return mv; + } + if (mv.getVariables().stream() .noneMatch(v -> { Cursor declaringCursor = v.getDeclaringScope(getCursor()); @@ -80,6 +85,10 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m return mv; } + + private Cursor getCursorToParentScope(final Cursor cursor) { + return cursor.dropParentUntil(is -> is instanceof J.NewClass || is instanceof J.ClassDeclaration); + } }; } diff --git a/src/test/java/org/openrewrite/staticanalysis/FinalizeLocalVariablesTest.java b/src/test/java/org/openrewrite/staticanalysis/FinalizeLocalVariablesTest.java index 876e27cd5..fc396d2c3 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FinalizeLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FinalizeLocalVariablesTest.java @@ -225,14 +225,14 @@ class Test { } """, """ - class Test { - static { - final int n = 1; - for(int i = 0; i < n; i++) { - } - } - } - """ + class Test { + static { + final int n = 1; + for(int i = 0; i < n; i++) { + } + } + } + """ ) ); } @@ -282,7 +282,7 @@ void forLoopVariablesIgnored() { java( """ import java.util.concurrent.FutureTask; - + class A { void f() { for(FutureTask future; (future = new FutureTask<>(() -> "hello world")) != null;) { } @@ -296,34 +296,36 @@ void f() { @Test void shouldNotFinalizeForCounterWhichIsReassignedWithinForHeader() { rewriteRun( - //language=java - java(""" - class A { - static { - for (int i = 0; i < 10; i++) { - // no-op - } - } - } - """ - ) + //language=java + java( + """ + class A { + static { + for (int i = 0; i < 10; i++) { + // no-op + } + } + } + """ + ) ); } @Test void shouldNotFinalizeForCounterWhichIsReassignedWithinForBody() { rewriteRun( - //language=java - java(""" - class A { - static { - for (int i = 0; i < 10;) { - i = 11; - } - } - } - """ - ) + //language=java + java( + """ + class A { + static { + for (int i = 0; i < 10;) { + i = 11; + } + } + } + """ + ) ); } @@ -394,48 +396,71 @@ class Person { void recordShouldNotIntroduceExtraClosingParenthesis() { rewriteRun( version( - //language=java - java( - """ - public class Main { - public static void test() { - var myVar = ""; - } - public record EmptyRecord() { - } - } - """, - """ - public class Main { - public static void test() { - final var myVar = ""; - } - public record EmptyRecord() { + //language=java + java( + """ + public class Main { + public static void test() { + var myVar = ""; + } + public record EmptyRecord() { + } } - } + """, """ - ), 17) + public class Main { + public static void test() { + final var myVar = ""; + } + public record EmptyRecord() { + } + } + """ + ), 17) ); } @Test void shouldNotFinalizeVariableWhichIsReassignedInAnotherSwitchBranch() { rewriteRun( - //language=java - java(""" - class A { - static int variable = 0; - static { - switch (variable) { - case 0: - int notFinalized = 0; - default: - notFinalized = 1; - } + //language=java + java( + """ + class A { + static int variable = 0; + static { + switch (variable) { + case 0: + int notFinalized = 0; + default: + notFinalized = 1; } } - """ - ) + } + """ + ) + ); + } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/181") + void shouldNotFinalizeVariablesWhichAreAssignedInAnonymousClasses() { + this.rewriteRun( + // language=java + java( + """ + class Test { + private final Object objectWithInnerField = new Object() { + private int count = 0; + @Override + public String toString() { + this.count++; + return super.toString() + this.count; + } + }; + } + """ + ) ); } }