ReplaceStringConcatenationWithStringValueOf: add failing tests for out-of-scope paren unwrap and untyped operand

[martinfrancois]

Review together with: #975
This companion PR is folded into the same review position rather than receiving a separate rank.

What's changed?

Adds 2 known-failing tests to the NoChange nested class of ReplaceStringConcatenationWithStringValueOfTest. One reproduces the recipe stripping parentheses from a pre-existing String.valueOf(..) call in a file that has no string concatenation at all. The other reproduces the recipe rewriting "" + x even when the type of x is unknown.

No recipe code changes. Both tests are marked @ExpectedToFail, so the suite stays green; removing the annotation shows the failure. On current main the class has 14 tests (9 in the outer class, 5 in NoChange) with 0 failures; with these additions it has 16 tests (7 in NoChange), still 0 failures and the 2 new ones reported as skipped.

What's your motivation?

First defect: the recipe edits files it should not touch. Before the recipe runs:

class Test {
    String method(Object o) {
        return (String.valueOf(o));
    }
}

There is no concatenation here, so the recipe should make no change. Output from current main:

class Test {
    String method(Object o) {
        return String.valueOf(o);
    }
}

The hand-written parentheses are stripped. visitParentheses in ReplaceStringConcatenationWithStringValueOf unwraps any parenthesized String.valueOf(..) call it visits, not only the ones the recipe just introduced.

Second defect: doNotChangeWhenOperandTypeIsMissing parses this input with TypeValidation.none(), so holder.chars() carries no type attribution:

class Test {
    String method(Unresolved holder) {
        return "" + holder.chars();
    }
}

Output from current main:

class Test {
    String method(Unresolved holder) {
        return String.valueOf(holder.chars());
    }
}

This rewrite is not safe when the operand type is unknown, because it is not value-preserving if the operand is really a char[]. We verified that with a small Java program using char[] chars = {'h','i'}: "" + chars printed [C@2b2fa4f7 while String.valueOf(chars) printed hi. The guard in visitBinary only checks !TypeUtils.isString(right.getType()), which is also true for a missing type, so the rewrite proceeds. The test asserts the recipe skips the expression when the type is unknown.

Found while preparing #975, which fixes a related defect in this recipe and lists both of these limitations as out of scope. These tests do not depend on #975; both defects reproduce on today's main.

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

We think both are genuine bugs: the first changes files the recipe has no business editing, and the second can change runtime behavior when types are missing. Note the char[] hazard also exists with full type attribution, since the recipe has no char[] guard at all; #975 adds a typed guard for that, while the test here pins only the missing-type case. If you agree this should change, I would gladly prepare the fix. If this behavior is intended, feel free to close this and we know it is settled.

Any additional context

Related open PR of ours touching the same recipe: #975. It discloses both limitations as out of scope and does not fix them.

This reproduction was prepared with AI assistance (Claude Code). I reviewed 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

This draft adds a reproduction test only, so the first box stays unticked on purpose; I will complete it together with the fix if you want one. The formatter run was calibrated per file; I declined reindentation of untouched lines.

[martinfrancois]

Closing after review: the remaining parentheses-only case does not meet the bug threshold. The missing-type safety case overlaps with #975 and will be handled there.