Fix RemoveUnusedLocalVariables to preserve assignments with side effects (#741)

* Fix RemoveUnusedLocalVariables to preserve assignments with side effects in try blocks

The recipe was incorrectly removing variable assignments in try blocks even when
those assignments could throw exceptions and change program behavior. This fix
ensures that method invocations within try blocks are preserved since they may
throw exceptions that affect control flow.

Fixes #740

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Simplify

* Avoid making changes when the assignment might have a side effect

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: timtebeek

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

@@ -171,7 +171,7 @@ public J.DeconstructionPattern visitDeconstructionPattern(J.DeconstructionPatter
                     // 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)
+                    mightSideEffect(variable.getInitializer())
                 ) {
                     return variable;
                 }
@@ -181,6 +181,9 @@ public J.DeconstructionPattern visitDeconstructionPattern(J.DeconstructionPatter
                     List<Statement> assignmentReferences = VariableReferences.findLhsReferences(parentScope.getValue(), variable.getName());
                     for (Statement ref : assignmentReferences) {
                         if (ref instanceof J.Assignment) {
+                            if (mightSideEffect(((J.Assignment) ref).getAssignment())) {
+                                return variable;
+                            }
                             doAfterVisit(new PruneAssignmentExpression((J.Assignment) ref));
                         }
                         doAfterVisit(new DeleteStatement<>(ref));
@@ -216,18 +219,14 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m
                 return mv;
             }
 
-            private boolean initializerMightSideEffect(J.VariableDeclarations.NamedVariable variable) {
-                if (variable.getInitializer() == null) {
-                    return false;
-                }
-                AtomicBoolean mightSideEffect = new AtomicBoolean(false);
-                new JavaIsoVisitor<AtomicBoolean>() {
+            private boolean mightSideEffect(Expression initializer) {
+                return initializer != null && new JavaIsoVisitor<AtomicBoolean>() {
                     @Override
                     public J.MethodInvocation visitMethodInvocation(J.MethodInvocation methodInvocation, AtomicBoolean result) {
                         if (SAFE_GETTER_METHODS.matches(methodInvocation)) {
                             return methodInvocation;
                         }
-                        if (withSideEffects == null || Boolean.FALSE.equals(withSideEffects)) {
+                        if (withSideEffects == null || !withSideEffects) {
                             result.set(true);
                         }
                         return methodInvocation;
@@ -244,8 +243,7 @@ public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean resul
                         result.set(true);
                         return assignment;
                     }
-                }.visit(variable.getInitializer(), mightSideEffect);
-                return mightSideEffect.get();
+                }.reduce(initializer, new AtomicBoolean(false)).get();
             }
         });
     }

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

@@ -704,7 +704,7 @@ static void method() {
                       try {
                           used = new Object();
                           assertEquals(used, null);
-                          unused = new Object();
+                          unused = used;
                       } catch (Exception e) {
                           // do nothing
                       }
@@ -1192,6 +1192,29 @@ void method() {
         );
     }
 
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/740")
+    @Test
+    void doNotRemoveVariableAssignmentWithPotentialSideEffects() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                  private String baz() {
+                      String foo;
+                      try {
+                          foo = String.valueOf(1);
+                      } catch (RuntimeException e) {
+                          return "error";
+                      }
+                      return "ok";
+                  }
+              }
+              """
+          )
+        );
+    }
+
     @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/760")
     @Test
     void doNotRemoveUnusedPatternMatchingVariables() {