Preserve observable array accesses and casts in identical branches

[martinfrancois]

Suggested review order: 39 of 52 (Score: 2)
Review first: openrewrite/rewrite#8467

What's changed?

Adds 2 known-failing tests to AllBranchesIdenticalTest that reproduce a case where the recipe deletes an expression that can throw at runtime: an array access in the condition, and a cast in the condition. No recipe code changes. The tests are marked @ExpectedToFail so the suite stays green; removing the mark shows the failure.

What's your motivation?

Recipe: the identical-branch simplification in SimplifyBooleanExpressionVisitor.

The shared helper SideEffects.mayHaveSideEffects (added in #959) only flags method invocations, assignments, assignment operations, increments and decrements, and new-class expressions. J.ArrayAccess and J.TypeCast fall through as effect-free. So the purity guard in AllBranchesIdentical (and the same guard in RemoveDuplicateConditions, RemoveUnconditionalValueOverwrite, and SimplifyRedundantLogicalExpression) lets the recipe delete an evaluation that can throw.

Before

class Test {
    void test(boolean[] flags) {
        if (flags[5]) {
            System.out.println("hello");
        } else {
            System.out.println("hello");
        }
    }
}

Actual after the recipe

Using current main.

class Test {
    void test(boolean[] flags) {
        System.out.println("hello");
    }
}

The same happens with if ((Boolean) o) over an Object o parameter: the whole condition is deleted.

Expected after the recipe

(unchanged)

Both inputs MUST remain unchanged because removing either condition also removes an exception-producing evaluation. This output is wrong because it changes behavior. I compiled and ran both versions: with new boolean[2], the original method throws java.lang.ArrayIndexOutOfBoundsException: Index 5 out of bounds for length 2, and with a String argument the cast version throws java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Boolean. The cast version also unboxes, so a null argument would throw a NullPointerException. The collapsed methods throw nothing. After the recipe runs, the code should be unchanged, because deleting these evaluations removes exceptions the program relied on.

Found while preparing #972 and #973, which fix related defects around this helper.

Anything in particular you'd like reviewers to focus on?

I think this is a genuine bug: the transformed code silently loses ArrayIndexOutOfBoundsException, ClassCastException, and unboxing NullPointerException behavior. If you agree this should change, I would gladly prepare the fix. If this behavior is intended, feel free to close this and I know it is settled.

Note that #973 has tests (removeRedundantNullCheckWithArrayAccess, removeRedundantNullCheckWithCast) that intentionally keep array access and cast removal allowed in the null-check context. That is safe there, because one evaluation of the expression remains. This reproduction uses AllBranchesIdentical on purpose, because it deletes the expression entirely.

Any additional context

Pre-existing tests changed: None.

Related open PRs of mine touching the same helper: #972 guards RemoveRedundantNullCheckBeforeInstanceof through SideEffects, and #973 adds a volatile field read check to SideEffects. Neither covers array access or casts, which is why the volatile side of this area is not reproduced here; it is resolved once those two merge.

Test counts for the touched class: 12 tests on main, 14 with this change; the 2 new tests are skipped as expected failures; 0 new failures or errors are introduced.

This reproduction was prepared with AI assistance (Claude Code). I reviewed the tests and this description.

The added reproduction tests and the existing suite together cover changed and unchanged behavior. The known-failing tests remain disabled until implementation. The formatter run was calibrated per file; untouched lines were not reformatted.

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've run ./gradlew build locally, and committed any resulting changes to recipes.csv
• I've formatted the lines I changed, without reformatting code I didn't touch