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 <tim@moderne.io> Co-authored-by: Tim te Beek <timtebeek@gmail.com>
openrewrite · Oct 2, 2023 · ad96b86 · ad96b86
1 parent c41a24a
commit ad96b86
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 65 deletions.
diff --git 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<?, 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
@@ -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
@@ -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;
+                  }
+                };
+              }
+              """
+          )
         );
     }
 }