Preserve Base64 imports and receiver evaluation

[martinfrancois]

Suggested review order: 40 of 52 (Score: 2)
Review first: openrewrite/rewrite-static-analysis#995

What's changed?

Adds 2 known-failing tests to UseJavaUtilBase64Test that reproduce two defects in the UseJavaUtilBase64 recipe: addImportWhenNoCoderVariablePresent (the migrated file is missing the java.util.Base64 import and does not compile) and retainMethodCallReceiver (a method call receiver is silently dropped). No recipe code changes. The tests are marked @ExpectedToFail so the suite stays green; removing the annotation shows the failure.

What's your motivation?

Recipe: org.openrewrite.java.migrate.UseJavaUtilBase64.

Case 1: add the Base64 import

Before

Take a file that uses BASE64Encoder inline, with no coder variable anywhere:

package test.sun.misc;

class Test {
    String test(byte[] bBytes) {
        return new BASE64Encoder().encode(bBytes);
    }
}

Actual after the recipe

Using current main. the call becomes return Base64.getEncoder().encodeToString(bBytes); but no import java.util.Base64; is added. Compiling that exact output with javac fails with cannot find symbol: variable Base64. The recipe's JavaTemplate declares .imports("java.util.Base64") but never calls maybeAddImport; the import only appears as a side effect of ChangeType retyping a BASE64Encoder or BASE64Decoder variable, and this file has none. After the recipe runs, the code should keep the same rewrite and also gain the import.

Expected after the recipe

The rewritten call MUST be retained and import java.util.Base64; MUST be added.

Case 2: preserve a non-identifier receiver

Before

Take a call whose receiver is itself a method call:

BASE64Encoder encoder() {
    return new BASE64Encoder();
}

String test(byte[] bBytes) {
    return encoder().encode(bBytes);
}

Actual after the recipe

Using current main: return Base64.getEncoder().encodeToString(bBytes);.

Expected after the recipe

return encoder().encodeToString(bBytes); The encoder() receiver is gone, so its evaluation and any side effects are lost. In the same output, ChangeType does retype the helper to Base64.Encoder encoder() { return Base64.getEncoder(); }, so the expected result is simply return encoder().encodeToString(bBytes);. The cause is that visitMethodInvocation restores the original receiver only when method.getSelect() instanceof J.Identifier; any other receiver (method call, field access, new-class) is discarded.

Found while preparing #1195, which adds a guard to this recipe. Its description discloses both cases, but its guard fixes neither: both inputs above still pass the guard and are migrated incorrectly. These two tests fail on current main whether or not #1195 is merged.

Affected code in real projects

• shuzheng/zheng AESUtil.java: AES utility that encodes cipher output with an inline new BASE64Encoder().encode(byteAES) and decodes with an inline new BASE64Decoder().decodeBuffer(content) (line 105); the file declares no coder variable, so the recipe from main rewrites both calls to java.util.Base64 factory calls without ever adding import java.util.Base64;, and the class no longer compiles.
• paascloud/paascloud-master HttpAesUtil.java: AES helper with the same inline shape, new BASE64Encoder().encode(bytes) and new BASE64Decoder().decodeBuffer(contentParam) (line 86) and no coder variable; the recipe from main rewrites both calls but adds no java.util.Base64 import, so the migrated file does not compile.
• DTStack/Taier DtStringUtil.java: compresses a string and encodes it with an inline new sun.misc.BASE64Encoder().encodeBuffer(compressed); the file has no coder variable and no java.util.Base64 import, so the rewrite from main to Base64.getEncoder().encodeToString(compressed) leaves the Base64 reference unresolved and the file does not compile.

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

I think both are genuine bugs: the first produces code that does not compile, and the second silently loses a method call and its side effects. 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: #1195. It does not fix these two cases. Before this change the class had 3 tests; with the additions it has 5. A fresh run reports tests=5, failures=0, errors=0, skipped=2; the 2 skipped are the new @ExpectedToFail tests, which fail when the annotation is removed.

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