Do not remove unused variables with constructor calls (#147)

* Do not remove unused variables with constructor calls

* Also visitNewClass for side effects

* Document another uncovered case

Author: timtebeek

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

@@ -80,19 +80,19 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
             private Cursor getCursorToParentScope(Cursor cursor) {
                 return cursor.dropParentUntil(is ->
                         is instanceof J.ClassDeclaration ||
-                                is instanceof J.Block ||
-                                is instanceof J.MethodDeclaration ||
-                                is instanceof J.ForLoop ||
-                                is instanceof J.ForEachLoop ||
-                                is instanceof J.ForLoop.Control ||
-                                is instanceof J.ForEachLoop.Control ||
-                                is instanceof J.Case ||
-                                is instanceof J.Try ||
-                                is instanceof J.Try.Resource ||
-                                is instanceof J.Try.Catch ||
-                                is instanceof J.MultiCatch ||
-                                is instanceof J.Lambda ||
-                                is instanceof JavaSourceFile
+                        is instanceof J.Block ||
+                        is instanceof J.MethodDeclaration ||
+                        is instanceof J.ForLoop ||
+                        is instanceof J.ForEachLoop ||
+                        is instanceof J.ForLoop.Control ||
+                        is instanceof J.ForEachLoop.Control ||
+                        is instanceof J.Case ||
+                        is instanceof J.Try ||
+                        is instanceof J.Try.Resource ||
+                        is instanceof J.Try.Catch ||
+                        is instanceof J.MultiCatch ||
+                        is instanceof J.Lambda ||
+                        is instanceof JavaSourceFile
                 );
             }
 
@@ -106,20 +106,20 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations
                 Cursor parentScope = getCursorToParentScope(getCursor());
                 J parent = parentScope.getValue();
                 if (parentScope.getParent() == null ||
-                        // skip class instance variables. parentScope.getValue() covers java records.
-                        parentScope.getParent().getValue() instanceof J.ClassDeclaration || parentScope.getValue() instanceof J.ClassDeclaration ||
-                        // skip anonymous class instance variables
-                        parentScope.getParent().getValue() instanceof J.NewClass ||
-                        // skip if method declaration parameter
-                        parent instanceof J.MethodDeclaration ||
-                        // skip if defined in an enhanced or standard for loop, since there isn't much we can do about the semantics at that point
-                        parent instanceof J.ForLoop.Control || parent instanceof J.ForEachLoop.Control ||
-                        // skip if defined in a try's catch clause as an Exception variable declaration
-                        parent instanceof J.Try.Resource || parent instanceof J.Try.Catch || parent instanceof J.MultiCatch ||
-                        // skip if defined as a parameter to a lambda expression
-                        parent instanceof J.Lambda ||
-                        // skip if the initializer may have a side effect
-                        initializerMightSideEffect(variable)
+                    // skip class instance variables. parentScope.getValue() covers java records.
+                    parentScope.getParent().getValue() instanceof J.ClassDeclaration || parentScope.getValue() instanceof J.ClassDeclaration ||
+                    // skip anonymous class instance variables
+                    parentScope.getParent().getValue() instanceof J.NewClass ||
+                    // skip if method declaration parameter
+                    parent instanceof J.MethodDeclaration ||
+                    // skip if defined in an enhanced or standard for loop, since there isn't much we can do about the semantics at that point
+                    parent instanceof J.ForLoop.Control || parent instanceof J.ForEachLoop.Control ||
+                    // skip if defined in a try's catch clause as an Exception variable declaration
+                    parent instanceof J.Try.Resource || parent instanceof J.Try.Catch || parent instanceof J.MultiCatch ||
+                    // skip if defined as a parameter to a lambda expression
+                    parent instanceof J.Lambda ||
+                    // skip if the initializer may have a side effect
+                    initializerMightSideEffect(variable)
                 ) {
                     return variable;
                 }
@@ -179,6 +179,12 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocat
                         return methodInvocation;
                     }
 
+                    @Override
+                    public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean result) {
+                        result.set(true);
+                        return newClass;
+                    }
+
                     @Override
                     public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean result) {
                         result.set(true);
@@ -219,9 +225,10 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation m, ExecutionC
     @EqualsAndHashCode(callSuper = true)
     private static class AssignmentToLiteral extends JavaVisitor<ExecutionContext> {
         J.Assignment assignment;
+
         @Override
         public J visitAssignment(J.Assignment a, ExecutionContext executionContext) {
-            if(assignment.isScope(a)) {
+            if (assignment.isScope(a)) {
                 return a.getAssignment().withPrefix(a.getPrefix());
             }
             return a;

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

@@ -15,7 +15,9 @@
  */
 package org.openrewrite.staticanalysis;
 
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.ExpectedToFail;
 import org.openrewrite.DocumentExample;
 import org.openrewrite.Issue;
 import org.openrewrite.test.RecipeSpec;
@@ -1011,4 +1013,64 @@ fun foo() {
           )
         );
     }
+
+    @Test
+    void retainJavaUnusedLocalVariableWithNewClass() {
+        rewriteRun(
+          java(
+            """
+              class A {}
+              class B {
+                void foo() {
+                  A a = new A();
+                }
+              }
+              """
+          )
+        );
+    }
+
+    @Nested
+    class Kotlin {
+
+        @Test
+        void retainUnusedLocalVariableWithNewClass() {
+            rewriteRun(
+              kotlin(
+                """
+                  class A {}
+                  class B {
+                    fun foo() {
+                      val a = A();
+                    }
+                  }
+                  """
+              )
+            );
+        }
+
+        @Test
+        @ExpectedToFail("Not yet implemented")
+        void retainUnusedLocalVariableConst() {
+            rewriteRun(
+              kotlin(
+                """
+                  package constants
+                  const val FOO = "bar"
+                  """
+              ),
+              kotlin(
+                """
+                  package config
+                  import constants.FOO
+                  fun baz() {
+                    val foo = FOO
+                    println(foo)
+                  }
+                  """
+              )
+            );
+        }
+
+    }
 }