Preserve argument comments and ranged append behavior in string-builder conversion

[martinfrancois]

Suggested review order: 46 of 52 (Score: 1.5)
Review first: #993

What's changed?

Adds 2 known-failing tests to ReplaceStringBuilderWithStringTest:

• retainCommentInsideAppendArguments reproduces a comment inside an append argument list being dropped.
• convertCharArrayRangeAppend proposes converting append(char[], int, int) instead of skipping the whole chain.

No recipe code changes. The 15 tests already in the class are unchanged, including doNotChangeCharArrayRangeAppend, which pins the current behavior from #977. The new tests are marked @ExpectedToFail so the suite stays green; removing the mark shows each failure.

What's your motivation?

Recipe: org.openrewrite.staticanalysis.ReplaceStringBuilderWithString.

Case 1: preserve an argument comment

Before

return new StringBuilder().append("a").append(chars /* the array */).toString();

Actual after the recipe

Using current main, after the merge of #977.

return "a" + String.valueOf(chars);

Expected after the recipe

return "a" + String.valueOf(chars /* the array */);

The chain itself flattens fine now, with the String.valueOf wrap from #977. But the /* the array */ comment is lost. flatMethodInvocationChain takes the bare expression from the argument list and discards the padding that holds the trailing comment. After the recipe runs, the code should keep it: "a" + String.valueOf(chars /* the array */). Note this is a different spot than #511, which covered comments in the chain prefix before .append; the existing retainComments test already protects those.

Case 2: convert a ranged char[] append

Before

new StringBuilder().append("a").append(chars, 0, 2).toString()

Actual after the recipe

(unchanged)

The recipe rejects every append call with more than one argument. The measured red run reports Recipe was expected to make a change but made no changes.

Expected after the recipe

"a" + String.valueOf(chars, 0, 2)

String.valueOf(char[], int, int) has the same semantics as append(char[], int, int). I checked both forms with a small compiled program: both printed axy for chars of {x, y, z} and compared equal.

Found while preparing #977, which fixed a related defect in this recipe.

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

The judgment call. The dropped comment is a genuine bug: user text is lost from otherwise correct output. The ranged append is only an improvement: the current no-change result is safe, and skipping it may be deliberate conservatism. If a fix lands for the second case, the sibling test doNotChangeCharArrayRangeAppend must be updated at the same time; the @ExpectedToFail reason string on convertCharArrayRangeAppend says so. If you agree these should change, I would gladly prepare the fix. If the ranged-append behavior is intended, feel free to close that part and I know it is settled.

Any additional context

Pre-existing tests changed: None.

Related work on this recipe: #977 (merged, added the String.valueOf wrapping for char[] and the doNotChangeCharArrayRangeAppend pin) and #511 (chain-prefix comments). Neither covers 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