Do not delete expressions that may have side effects

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

@@ -21,6 +21,7 @@
 import org.openrewrite.TreeVisitor;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.search.SemanticallyEqual;
+import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
 import org.openrewrite.java.tree.Statement;
 
@@ -30,6 +31,7 @@
 import java.util.Set;
 
 import static java.util.Collections.singleton;
+import static org.openrewrite.staticanalysis.SideEffects.mayHaveSideEffects;
 
 @Getter
 public class AllBranchesIdentical extends Recipe {
@@ -56,10 +58,12 @@ public J visitIf(J.If if_, ExecutionContext ctx) {
                 }
 
                 List<Statement> bodies = new ArrayList<>();
+                List<Expression> conditions = new ArrayList<>();
                 J.If current = if__;
 
                 while (current != null) {
                     bodies.add(current.getThenPart());
+                    conditions.add(current.getIfCondition().getTree());
                     if (current.getElsePart() == null) {
                         return if__;
                     }
@@ -79,6 +83,13 @@ public J visitIf(J.If if_, ExecutionContext ctx) {
                     }
                 }
 
+                // Collapsing the chain stops evaluating the conditions, which is only safe when they are pure
+                for (Expression condition : conditions) {
+                    if (mayHaveSideEffects(condition)) {
+                        return if__;
+                    }
+                }
+
                 doAfterVisit(new RemoveUnneededBlock().getVisitor());
                 return first.withPrefix(if__.getPrefix());
             }

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

@@ -59,7 +59,8 @@ public Expression visitExpression(Expression expression, ExecutionContext ctx) {
             @Override
             public J visitUnary(J.Unary unary, ExecutionContext ctx) {
                 J.Unary un = (J.Unary) super.visitUnary(unary, ctx);
-                if (J.Unary.Type.Not == un.getOperator() && TypeUtils.isOfClassType(un.getExpression().getType(), "java.lang.Boolean")) {
+                if (J.Unary.Type.Not == un.getOperator() && TypeUtils.isOfClassType(un.getExpression().getType(), "java.lang.Boolean") &&
+                        isControlExpression(unary)) {
                     return JavaTemplate.apply("Boolean.FALSE.equals(#{any(java.lang.Boolean)})",
                             updateCursor(un), un.getCoordinates().replace(), un.getExpression());
                 }

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

@@ -31,6 +31,7 @@
 import java.util.Set;
 
 import static java.util.Collections.singleton;
+import static org.openrewrite.staticanalysis.SideEffects.mayHaveSideEffects;
 
 @Getter
 public class RemoveDuplicateConditions extends Recipe {
@@ -76,6 +77,14 @@ public J visitIf(J.If if_, ExecutionContext ctx) {
                     current = elseBody instanceof J.If ? (J.If) elseBody : null;
                 }
 
+                // A later condition is only unreachable if every condition up to it evaluates the same way each
+                // time; a side effect anywhere in the chain can change that, so require them all to be pure
+                for (Expression condition : conditions) {
+                    if (mayHaveSideEffects(condition)) {
+                        return if__;
+                    }
+                }
+
                 // Find and remove branches with duplicate conditions
                 J.If result = if__;
                 boolean changed = false;

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

@@ -32,6 +32,7 @@
 import java.util.Set;
 
 import static java.util.Collections.singleton;
+import static org.openrewrite.staticanalysis.SideEffects.mayHaveSideEffects;
 
 @Getter
 public class RemoveUnconditionalValueOverwrite extends Recipe {
@@ -78,14 +79,37 @@ public J.Block visitBlock(J.Block block, ExecutionContext ctx) {
                     }
 
                     if (SemanticallyEqual.areEqual(key, nextKey) &&
-                        SemanticallyEqual.areEqual(receiver, nextReceiver)) {
+                        SemanticallyEqual.areEqual(receiver, nextReceiver) &&
+                        !discardsSideEffects(stmt)) {
                         //noinspection DataFlowIssue
                         return null;
                     }
                     return stmt;
                 }));
             }
 
+            /**
+             * The overwritten call is dead, but the expressions it evaluates on the way are not: dropping the
+             * statement also drops the receiver, the key and the value it would have computed.
+             */
+            private boolean discardsSideEffects(Statement stmt) {
+                if (stmt instanceof J.Assignment) {
+                    J.Assignment assignment = (J.Assignment) stmt;
+                    return mayHaveSideEffects(assignment.getVariable()) ||
+                           mayHaveSideEffects(assignment.getAssignment());
+                }
+                J.MethodInvocation method = (J.MethodInvocation) stmt;
+                if (mayHaveSideEffects(method.getSelect())) {
+                    return true;
+                }
+                for (Expression argument : method.getArguments()) {
+                    if (mayHaveSideEffects(argument)) {
+                        return true;
+                    }
+                }
+                return false;
+            }
+
             private Expression extractMapPutKey(Statement stmt) {
                 // Python dict subscript assignment: `d[key] = value`
                 if (stmt instanceof J.Assignment && ((J.Assignment) stmt).getVariable() instanceof J.ArrayAccess) {

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

@@ -0,0 +1,91 @@
+/*
+ * Copyright 2026 the original author or authors.
+ * <p>
+ * Licensed under the Moderne Source Available License (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://docs.moderne.io/licensing/moderne-source-available-license
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.staticanalysis;
+
+import org.jspecify.annotations.Nullable;
+import org.openrewrite.Tree;
+import org.openrewrite.java.JavaIsoVisitor;
+import org.openrewrite.java.tree.J;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * Whether evaluating an expression might do something observable beyond producing its value. Recipes that
+ * delete an expression, or that stop evaluating one, are only correct when the answer is {@code false}.
+ * <p>
+ * Deliberately conservative: any method invocation, constructor call, assignment or increment counts, since
+ * whether those are pure cannot be decided from the LST alone. {@link org.openrewrite.java.tree.Expression#getSideEffects()}
+ * is not used here because it reports only the side effects of the expression's own node type, and so misses
+ * those nested inside a ternary or a lambda.
+ */
+final class SideEffects {
+
+    private SideEffects() {
+    }
+
+    static boolean mayHaveSideEffects(@Nullable J tree) {
+        if (tree == null) {
+            return false;
+        }
+        return new JavaIsoVisitor<AtomicBoolean>() {
+            @Override
+            public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean result) {
+                result.set(true);
+                return method;
+            }
+
+            @Override
+            public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean result) {
+                result.set(true);
+                return assignment;
+            }
+
+            @Override
+            public J.AssignmentOperation visitAssignmentOperation(J.AssignmentOperation assignOp, AtomicBoolean result) {
+                result.set(true);
+                return assignOp;
+            }
+
+            @Override
+            public J.Unary visitUnary(J.Unary unary, AtomicBoolean result) {
+                switch (unary.getOperator()) {
+                    case PreIncrement:
+                    case PreDecrement:
+                    case PostIncrement:
+                    case PostDecrement:
+                        result.set(true);
+                        return unary;
+                    default:
+                        return super.visitUnary(unary, result);
+                }
+            }
+
+            @Override
+            public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean result) {
+                result.set(true);
+                return newClass;
+            }
+
+            @Override
+            public @Nullable J visit(@Nullable Tree t, AtomicBoolean result) {
+                if (result.get()) {
+                    return (J) t;
+                }
+                return super.visit(t, result);
+            }
+        }.reduce(tree, new AtomicBoolean(false)).get();
+    }
+}

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

@@ -16,21 +16,18 @@
 package org.openrewrite.staticanalysis;
 
 import lombok.Getter;
-import org.jspecify.annotations.Nullable;
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Recipe;
-import org.openrewrite.Tree;
 import org.openrewrite.TreeVisitor;
-import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.JavaVisitor;
 import org.openrewrite.java.search.SemanticallyEqual;
 import org.openrewrite.java.tree.J;
 
 import java.time.Duration;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 import static java.util.Collections.singleton;
+import static org.openrewrite.staticanalysis.SideEffects.mayHaveSideEffects;
 
 @Getter
 public class SimplifyRedundantLogicalExpression extends Recipe {
@@ -59,7 +56,7 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
                     case BitAnd:
                     case BitOr:
                         if (SemanticallyEqual.areEqual(b.getLeft(), b.getRight()) &&
-                                !hasSideEffects(b.getLeft())) {
+                                !mayHaveSideEffects(b.getLeft())) {
                             return b.getLeft().unwrap().withPrefix(b.getPrefix());
                         }
                         break;
@@ -68,56 +65,6 @@ public J visitBinary(J.Binary binary, ExecutionContext ctx) {
                 }
                 return b;
             }
-
-            private boolean hasSideEffects(J tree) {
-                return new JavaIsoVisitor<AtomicBoolean>() {
-                    @Override
-                    public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean result) {
-                        result.set(true);
-                        return method;
-                    }
-
-                    @Override
-                    public J.Assignment visitAssignment(J.Assignment assignment, AtomicBoolean result) {
-                        result.set(true);
-                        return assignment;
-                    }
-
-                    @Override
-                    public J.AssignmentOperation visitAssignmentOperation(J.AssignmentOperation assignOp, AtomicBoolean result) {
-                        result.set(true);
-                        return assignOp;
-                    }
-
-                    @Override
-                    public J.Unary visitUnary(J.Unary unary, AtomicBoolean result) {
-                        switch (unary.getOperator()) {
-                            case PreIncrement:
-                            case PreDecrement:
-                            case PostIncrement:
-                            case PostDecrement:
-                                result.set(true);
-                                return unary;
-                            default:
-                                return super.visitUnary(unary, result);
-                        }
-                    }
-
-                    @Override
-                    public J.NewClass visitNewClass(J.NewClass newClass, AtomicBoolean result) {
-                        result.set(true);
-                        return newClass;
-                    }
-
-                    @Override
-                    public @Nullable J visit(@Nullable Tree t, AtomicBoolean result) {
-                        if (result.get()) {
-                            return (J) t;
-                        }
-                        return super.visit(t, result);
-                    }
-                }.reduce(tree, new AtomicBoolean(false)).get();
-            }
         };
     }
 }

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

@@ -17,6 +17,7 @@
 
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
+import org.openrewrite.Issue;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
@@ -288,4 +289,30 @@ def test(a):
           )
         );
     }
+
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/953")
+    @Test
+    void doNotChangeWhenConditionHasSideEffects() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              import java.util.Iterator;
+
+              class Test {
+                  void p(String s) {
+                  }
+
+                  void test(Iterator<String> it) {
+                      if (it.next() != null) {
+                          p("x");
+                      } else {
+                          p("x");
+                      }
+                  }
+              }
+              """
+          )
+        );
+    }
 }

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

@@ -17,6 +17,7 @@
 
 import org.junit.jupiter.api.Test;
 import org.openrewrite.DocumentExample;
+import org.openrewrite.Issue;
 import org.openrewrite.test.RecipeSpec;
 import org.openrewrite.test.RewriteTest;
 
@@ -147,4 +148,22 @@ String whatToGet(Boolean forThing1) {
           )
         );
     }
+
+    @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/953")
+    @Test
+    void doNotChangeUnaryOutsideControlExpression() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class Test {
+                  boolean test(Boolean b) {
+                      boolean x = !b;
+                      return x;
+                  }
+              }
+              """
+          )
+        );
+    }
 }