Preserve receiver effects when retargeting methods to static calls

[martinfrancois]

Suggested review order: 38 of 52 (Score: 2)
Review first: openrewrite/rewrite-testing-frameworks#1083

What's changed?

Adds 2 known-failing tests to ChangeMethodTargetToStaticTest that reproduce two cases where ChangeMethodTargetToStatic deletes a receiver expression whose evaluation has observable effects. No recipe code changes. The tests are marked @ExpectedToFail (junit-pioneer) so the suite stays green; removing the annotation shows the failures.

What's your motivation?

Recipe: org.openrewrite.java.ChangeMethodTargetToStatic.

When the recipe rewrites a call to a static call, it drops the old receiver. Two shapes of receiver still carry behavior that gets lost.

Case 1: statically imported field receiver

Before

Test staticallyImportedFieldReceiverNotChanged uses a bare statically imported field receiver. With the recipe ChangeMethodTargetToStatic("a.A stat()", "b.B", null, null, false) and this input:

import static holder.Holder.INSTANCE;

class C {
   public void test() {
       INSTANCE.stat();
   }
}

Actual after the recipe

Using current main.

import b.B;

import static holder.Holder.INSTANCE;

class C {
   public void test() {
       B.stat();
   }
}

Expected after the recipe

(unchanged)

The read of INSTANCE is gone. Before the recipe runs, that read triggers class initialization of Holder (JLS 12.4.1). I checked this with a compiled scratch program: INSTANCE.stat() printed "Holder initialized" then "A.stat called", while the rewritten B.stat() printed only "B.stat called", so Holder's static initializer never runs. The bare name is a J.Identifier, so the recipe treats it as a droppable variable read; a qualified Holder.INSTANCE receiver is a J.FieldAccess and already stops the rewrite. The test expects no change to the file.

Case 2: nested matched call with an argument

Before

With ChangeMethodTargetToStatic("a.A *(..)", "b.B", null, null, false), the input is legacy.combine(argument()).value().

Actual after the recipe

B.value()

Expected after the recipe

(unchanged)

The intermediate matched call combine(argument()) is deleted together with its argument. Verified the same way: the original chain evaluates argument(), the rewritten form never calls it. A pattern that matches only value() does not show this. The test expects no change here as well.

Both tests fail on main with exact-diff assertion errors. All 11 pre-existing passing tests in the class still pass; the class has 12 tests on main (11 pass, 1 skipped by a pre-existing @Disabled), and 14 with these additions.

I found this while preparing #8444, which narrows receiver dropping in this recipe to forms without observable effects. That PR deliberately leaves both of these shapes unchanged and lists them as remaining defects; the tests here still fail on main and on top of #8444.

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

I think both are genuine bugs: the rewritten code silently loses evaluation that ran before, so program behavior changes even though the output compiles. 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.

Any additional context

Pre-existing tests changed: None.

Related open PR of mine touching the same recipe: #8444. It does not fix these two cases.

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