SONARJAVA-7155: Implemented rule S9433 Literals should not be converted to strings using empty string concatenation

[romainbrenguier]

Detects primitive literals converted to strings by direct concatenation with an empty string literal, and recommends an explicit conversion.

Agent workflow

PR created using uv run new_rule_implementation.py S9433 -j SONARJAVA-7155 -a codex -g AGENTS.md .claude/skills/new-rule/SKILL.md

Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document /tmp/action_plan_romain/new-rule-s9433-sonarjava-7155.txt." -a "/tmp/action_plan_romain/new-rule-s9433-sonarjava-7155.txt" -g "claude"

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document /tmp/action_plan_romain/new-rule-s9433-sonarjava-7155.txt." -a "/tmp/action_plan_romain/new-rule-s9433-sonarjava-7155.txt" -g "claude"

Iterated on the PR with uv run ci_loop.py for 2 iterations.
✔️ The PR is now ready for review.

---

Summary by Gitar

• New API:
  • Added Tree.Kind.LITERAL_KINDS listing all literal kinds
• Constant contexts:
  • Reports concatenations in annotation values, constant field initializers, and case labels with a suggested string literal replacement

_{This will update automatically on new commits.}

Addressed review comments in pr_report_6364.md using uv run address_reviews.py pr_report_6364.md

[hashicorp-vault-sonar-prod]

SONARJAVA-7155

[rombirli]

/pvf

[hashicorp-vault-sonar-prod]

⚠️ Performance Validation finished without publishing a dashboard. See run logs.

[rombirli]

LGTM but few questions to answer/address

[rombirli]

why is it compliant? Known FN? I expect the rule to raise on ("" + 1) part (or without parentheses)

[romainbrenguier]

This is intentional, not a known FN. "a" + ("" + 1) is still a compile-time constant expression, so NESTED is a constant field. Replacing the inner part with String.valueOf(1) would make it non-constant, the same issue as for CONSTANT (see the thread on line 8). The constant-context walk only goes through parentheses, + and conditionals, which can all appear in constant expressions. Inside any other expression (method call, array initializer, lambda, switch expression) the issue is still raised.

[romainbrenguier]

Actually, we maybe just need a different message for that case.

[rombirli]

Same for this one

[romainbrenguier]

Same reasoning: true ? "" + 1 : "x" is a compile-time constant expression (JLS 15.29 allows the conditional operator), so CONDITIONAL is a constant field and the explicit conversion would break that.

[rombirli]

why are constants excluded? Are static final fields excluded from this rule and why?

[romainbrenguier]

Only constant fields (static final, or interface fields) whose initializer is a compile-time constant expression are excluded. Other static final fields are still reported, see LIST, FORMATTED, ARRAY and LAMBDA in the sample. The reason: "" + 42 is a compile-time constant (JLS 15.29), but Integer.toString(42) / String.valueOf(42) is not. Applying the suggested fix would stop the field being a constant. Any case CONSTANT: or @Annotation(CONSTANT) using it would no longer compile, and the value would stop being inlined in other classes. javac also folds "" + 42 into "42" at compile time, so the performance argument from the RSPEC doesn't apply there.

[romainbrenguier]

Maybe we should have a rule in such cases to write "42" instead

[rombirli]

This should be extracted to java-frontend/src/main/java/org/sonar/plugins/java/api/tree/Tree.java to prevent future problems like it was done in this ticket for class types.

It also seems to not be exhaustive, it should also contain

Tree.Kind.STRING_LITERAL,
Tree.Kind.NULL_LITERAL

[romainbrenguier]

Done in 8aca0fe: added Tree.Kind.LITERAL_KINDS with all kinds backed by LiteralTree, including STRING_LITERAL, TEXT_BLOCK and NULL_LITERAL. A TreeTest check keeps it exhaustive. The rule uses it but leaves out string, text block and null literals on purpose. "" + "x" (or a text block) is not a conversion, and "" + null has no clean explicit replacement: String.valueOf(null) resolves to the char[] overload and throws an NPE. Both cases are compliant in the sample.

[rombirli]

as explained in the sample i don't understand this heuristic, is it to kill the noise? should the rspec be updated accordingly?

[romainbrenguier]

It's not there to reduce noise. It avoids raising issues where the suggested fix can't be applied. In annotation arguments, annotation element defaults and case labels, a compile-time constant is required, so Integer.toString(1) doesn't compile there. In constant field initializers, the fix silently turns the field into a non-constant, which breaks its use in those same places and changes inlining. Agreed that this should be documented in the RSPEC, as an Exceptions section listing these constant contexts. That needs a change in the rspec repository, not here.

[gitar-bot]

Code Review ✅ Approved 5 closed / 5 findings

🟡 Medium risk · Adding the S9433 analyzer rule could flag existing literal concatenations and change issue counts; Tree.Kind.LITERAL_KINDS also adds a public API constant.

Adds rule S9433 to flag primitive literals converted to strings via empty string concatenation, and adds Tree.Kind.LITERAL_KINDS. Reports in annotation values, constant field initializers, and case labels now carry a string literal replacement. Resolved: annotation element defaults are no longer wrongly flagged, static final initializers are no longer over-suppressed, and char constants now get a correct replacement literal. No open issues found.

✅ 5 closed

✅ Edge Case: Annotation element default values still flagged, though they must be constants

📄 java-checks/src/main/java/org/sonar/java/checks/LiteralToStringByConcatenationCheck.java:57-70
isInConstantContext stops walking as soon as it hits a METHOD node. An annotation element default like @interface A { String value() default "" + 1; } has the MethodTree as the binary expression's direct parent (via defaultValue), so the method returns false and the issue is raised. Following the advice (String.valueOf(1)) would not compile, because default values must be compile-time constants. This is the same kind of false positive the commit means to remove for annotation arguments. Fix: before stopping at METHOD, return true when the parent is a method whose defaultValue() contains the expression.

✅ Edge Case: Any expression inside a static final initializer is suppressed

📄 java-checks/src/main/java/org/sonar/java/checks/LiteralToStringByConcatenationCheck.java:57-71
isInConstantContext walks up through every kind of node until it reaches a VARIABLE, then returns isConstantField. So a concatenation anywhere inside a static final or interface field initializer is skipped, even when it sits inside a method call or another non-constant expression: static final List<String> L = List.of("" + 1);, static final String S = String.format("%s", "" + 42);, or an arrow-switch result case 1 -> "" + 1. None of these are compile-time constant contexts, and the Javadoc only promises to skip "initializers of constant fields", so these are false negatives. Fix: only treat the field as a constant context when the path from the concatenation up to the VARIABLE goes through constant-expression nodes only (parentheses, binary, or conditional expressions).

✅ Bug: char constants resolve to Integer, so the suggested replacement literal is wrong

📄 java-checks/src/main/java/org/sonar/java/checks/LiteralToStringByConcatenationCheck.java:102-115 📄 java-checks/src/main/java/org/sonar/java/checks/LiteralToStringByConcatenationCheck.java:54-57
For identifiers and member selects, resolveConstantValue falls back to expression.asConstant(). That goes through ExpressionUtils.resolveIdentifier → JVariableSymbol.constantValue(), and constantValue() turns Character constants into Integer (JVariableSymbol.java, c = Integer.valueOf(characterValue)). Take static final char C = 'a'; static final String S = C + ("" + 1); or "" + 1 + Character.MAX_VALUE. enclosingConstantConcatenation climbs to the outer + and left.toString() + right builds "971" (or "165535"). Java actually produces "a1" (or "1\uffff"). So the issue tells the user to swap the concatenation for a literal that changes the program's behaviour. The fix is to map the value back to Character when the operand's static type is char, or to return empty for such operands so the message falls back to "a string literal".

Closed: Edge Case: Flags compile-time constant contexts where the suggested fix won't compile

📄 java-checks/src/main/java/org/sonar/java/checks/LiteralToStringByConcatenationCheck.java:38-46 📄 java-checks-test-sources/default/src/main/java/checks/LiteralToStringByConcatenationCheckSample.java:42-53
"" + 123 is a compile-time constant expression (JLS 15.29), while Integer.toString(123) / String.valueOf(123) is not. The check reports every PLUS node with no context check, so it also flags code like @SuppressWarnings("" + 1), case "" + 42: in a String switch, and static final String X = "" + 42; (which the compiler inlines). In the first two cases the recommended explicit conversion doesn't compile, and in the third it changes constant-inlining semantics, so these reports are false positives the user can't fix. Fix: skip the report when the expression sits inside an annotation argument or a case label (and possibly a static final field initializer), and add those cases to the sample as compliant.

Closed: Quality: Test sample has an outdated LGPL license header unlike every other sample

📄 java-checks-test-sources/default/src/main/java/checks/LiteralToStringByConcatenationCheckSample.java:1-15
The new sample file opens with the old Copyright (C) 2012-2026 SonarSource SA GNU LGPL header. No other file under java-checks-test-sources has this header, and the repository is now under the Sonar Source-Available License (the header the new main and test classes use). This mislabels the file's license. Remove the header (other samples have none) or replace it with the current SSAL header.

Verification Rules 1/1 met Repos 3 inspected · PRs 1 inspected

Functional validation No results

Rules 1 passed 2 not applicable

✅ Repository license compliance

Broader impact Repos 3 inspected PRs 1 inspected

Cross-repo 3 repositories inspected 0 open impacts

SonarSource/sonarlint-core Inspected
SonarSource/sonarlint-language-server Inspected
SonarSource/sonarlint-vscode Inspected

Cross-PR 1 PR inspected 0 open impacts

PR #6332: SONARJAVA-7157: Implemented rule S9432 Strings should not be trimmed or stripped just to check if they are blank Inspected

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
98.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[romainbrenguier]

/pvf S9433

[github-actions]

@romainbrenguier ❌ Your /pvf command was not dispatched. See the failed run.

No candidate-version artifact for 677f480; wait for Build to finish