ReplaceStringBuilderWithString: wrap char[] appends in String.valueOf

[martinfrancois]

What's changed?

ReplaceStringBuilderWithString
flattens a chain such as new StringBuilder().append(x).append(y).toString() into the concatenation
x + y. On main that flattening keeps the characters of a char[] argument when the char[] sits
in the first position of the chain, and loses them in every later position. This change makes every
position behave the way the first one already does: the argument of every append call that resolves
to the java.lang.StringBuilder append(char[]) overload is now wrapped in String.valueOf(...).

Input:

String multiple(char[] a, char[] b) {
    return new StringBuilder().append(a).append("-").append(b).toString();
}

Output on current main, which at run time renders the characters of a and not those of b:

return String.valueOf(a) + "-" + b;

Output with this change:

return String.valueOf(a) + "-" + String.valueOf(b);

The two wrappings do not stack. main wraps a non-String first operand in String.valueOf(...) in
a separate step, adjustExpressions; the new wrapping runs earlier, in flatMethodInvocationChain,
and yields an expression whose type is String, so adjustExpressions leaves it alone. A char[]
in the first position comes out as String.valueOf(chars), never as
String.valueOf(String.valueOf(chars)), which charArrayAppendInEveryChainPosition pins.

What's your motivation?

The recipe as it stands on main today produces code that returns a different value at run time than
the code it replaced. StringBuilder.append(char[]) appends the characters of the array. The same
array used as an operand of + goes through string conversion instead, and
JLS 5.1.11 says a
reference value is converted by invoking its toString method, so a non-null array renders as the
JVM type descriptor [C for char[], then @, then the identity hash code in hexadecimal, for
example [C@1b6d3586. A null array renders as the four character text null.

Give both arrays in the input above the value {'o','k'}: the input returns ok-ok and the main
output returns ok-[C@1b6d3586. Now let b be null: the input throws NullPointerException,
because StringBuilder.append(char[]) dereferences the array, while the main output returns
ok-null. The output with this change matches the input in both cases, because
String.valueOf(char[]) copies the characters of a non-null array and throws NullPointerException
for a null one.

The recipe is listed in common-static-analysis.yml, so it runs for everyone who uses
CommonStaticAnalysis. I reproduced this on v2.39.0, v2.40.0 and current main (5785534): the
recipe file is the same git blob (794eb20c) in all three.

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

This change adds 5 tests to ReplaceStringBuilderWithStringTest. Without the code change in this
pull request, 3 of them fail: charArrayAppendInEveryChainPosition,
charArrayAppendKeepsCharacterRendering and charArrayAppendOfAnyExpressionShape. The other two,
doNotChangeObjectAppendHoldingCharArray and doNotChangeCharArrayRangeAppend, pass either way and
pin the two cases the new wrapping deliberately leaves alone, described in the limitations below.

No existing test expectation changed. The test file has 176 added lines and no deleted lines, and the
10 tests that were already in the class are untouched by this diff.

Four limitations are worth your attention:

• A comment inside the argument list is dropped. For
  new StringBuilder().append("a").append(chars /* the array */).toString() this branch produces
  "a" + String.valueOf(chars) without the comment. Not new here: main drops the same comment,
  producing "a" + chars. Comments elsewhere in the chain are still kept, which the existing
  retainComments test covers.
• An append(Object) call is not wrapped, even when the value is a char[] at run time, so
  Object o = chars; followed by append("a").append(o) still becomes "a" + o, which renders the
  [C@... text. Wrapping there would not help: the argument's declared type is Object, so
  String.valueOf(...) would select String.valueOf(Object), which calls toString() and produces
  that same text. doNotChangeObjectAppendHoldingCharArray pins that the argument is left alone
  rather than wrapped in a call that changes nothing. Unchanged from main.
• The three argument overload append(char[], int, int) still stops the whole chain from being
  converted, because the recipe only flattens append calls that take one argument. Wrapping the
  array there would drop the offset and length and change the value, so leaving the chain
  unconverted is the right outcome, and doNotChangeCharArrayRangeAppend pins it. Unchanged from
  main.
• In a Kotlin source file, a CharArray argument does not match the
  java.lang.StringBuilder append(char[]) matcher, so the new wrapping never fires there. Kotlin
  chains are still flattened and they still lose the characters, exactly as on main:
  StringBuilder().append("a").append(chars).toString() becomes "a" + chars both with and without
  this change.

Have you considered any alternatives or workarounds?

One option is to leave the whole chain unchanged whenever an append(char[]) call is present, rather
than wrapping the argument. I did not pick it, for two reasons. First, the String.valueOf(...)
wrapper is not a new shape of output for this recipe: main already produces it for a non-String
first operand, so wrapping the later positions the same way extends output the recipe already
produces. Second, the bail out would take away a conversion that works today, the chain whose
char[] is in the first position and whose rendering main already gets right.

If you prefer the bail out anyway, it is a two line change in flatMethodInvocationChain and I will
push it.

Any additional context

ReplaceStringConcatenationWithStringValueOf
has a char[] problem that is the mirror image of this one, and the fix there goes in the opposite
direction: that source already concatenates, which renders the array like any other Object, and the
recipe replaces the concatenation with String.valueOf(chars), which copies the characters, so the
fix there is to stop introducing String.valueOf(...) rather than to introduce it. I am sending that
change separately, and neither depends on the other.

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

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 used the IntelliJ IDEA auto-formatter on affected files

I ran the formatter with the repository's .editorconfig. It also wanted to re-indent lines that this
change does not touch, so I left those alone and kept the diff limited to this change.