Skip to content

fix: skip finalizing anonymous class fields in FinalizeLocalVariables #182

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timtebeek merged 9 commits into from
Oct 2, 2023
Merged

fix: skip finalizing anonymous class fields in FinalizeLocalVariables #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4915f02
fix: skip finalizing anonymous class fields
AlejandroBertolo Oct 2, 2023
77d1958
chore: add issue to test
AlejandroBertolo Oct 2, 2023
93566e5
chore: mark contributed code
AlejandroBertolo Oct 2, 2023
c6656f0
chore: update anonymous class identifying logic
AlejandroBertolo Oct 2, 2023
e14cb5d
Apply formatter
timtebeek Oct 2, 2023
29dbcaa
chore: remove javafx types from test
AlejandroBertolo Oct 2, 2023
8ad4628
Merge branch 'main' into bugfix/GH-181-finalizeLocalVariables-skips-a…
timtebeek Oct 2, 2023
91bd13b
Polish
timtebeek Oct 2, 2023
41ce137
Apply suggestions from code review
timtebeek Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/main/java/org/openrewrite/staticanalysis/FinalizeLocalVariables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ public String getDescription() {
public TreeVisitor<?, ExecutionContext> getVisitor() {
return new JavaIsoVisitor<ExecutionContext>() {

@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
Expand All @@ -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());
Expand All @@ -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);
}
};
}

Expand Down
151 changes: 88 additions & 63 deletions src/test/java/org/openrewrite/staticanalysis/FinalizeLocalVariablesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
}
}
}
"""
)
);
}
Expand Down Expand Up @@ -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;) { }
Expand All @@ -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;
}
}
}
"""
)
);
}

Expand Down Expand Up @@ -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;
}
};
}
"""
)
);
}
}