Repository navigation
SONARJAVA-7070: Implemented rule S9413 Redundant "String.format" should be removed when native formatting is available - #6246
Conversation
…ld be removed when native formatting is available Reports String.format calls passed directly as the message argument of APIs with native formatting: SLF4J/Log4j 2 loggers, PrintStream/PrintWriter print/println, StringBuilder/StringBuffer append, and Throwable constructors. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
🤖 Generated with GitHub Actions
|
❌ Ruling needs updating. A fix PR has been created: #6248 Please review and merge it into your branch. |
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…-rule-s9413-sonarjava-7070' into romain/new-rule-s9413-sonarjava-7070
Ruling Diff SummaryDetected changes in 2 rule files: 0 issues removed, 12 issues added. S9413 (
|
🤖 Generated with GitHub Actions
|
❌ Ruling needs updating. A fix PR has been created: #6249 Please review and merge it into your branch. |
String concatenation instead of String.format is already covered by S9397. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
❌ Ruling needs updating. A fix PR has been created: #6269 Please review and merge it into your branch. |
|
❌ Ruling needs updating. A fix PR has been created: #6273 Please review and merge it into your branch. |
nathsou
left a comment
nathsou
left a comment
There was a problem hiding this comment.
Approved, with the inline suggestions below.
… review
- Stop reporting String.format passed to StringBuilder/StringBuffer.append.
- Skip logging cases when a format argument implements java.util.Formattable,
since %s calls formatTo while "{}" placeholders call toString.
- Add a quick fix turning print/println(String.format(...)) into printf(...),
appending %n for println when the format is a string literal.
- Remove stale ruling expectations for dropped cases.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Code Review ✅ Approved 2 closed / 2 findings🟡 Medium risk · Adds a new Java analysis rule with reporting and output-rewriting quick fixes Implements rule S9413 to detect and flag redundant ✅ 2 closed✅ Quality: StringBuilder code examples in S9413.html are invalid Java
✅ Quality: Stale S9413 ruling expectations left in eclipse-jetty-similar-to-main
Review coverage🧪 Functional validation 1 of 1 objectives covered 📋 Rules No rules evaluated Cross-repo coverage 5 repositories selected 🤖 Auto-approval Not enabled · Set up Implementation Status ✅ 1 of 1 objectives covered✅ SONARJAVA-7070 - 1 of 1 objectives coveredThis PR implements rule S9413 to remove redundant String.format when native formatting is available. ✅ 1 covered here
OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
2 participants
SONARJAVA-7070
Summary
RedundantStringFormatCheck(S9413). It reportsString.format(...)when its result is passed directly as the message argument of an API that already formats:{}placeholdersPrintStream/PrintWriterprint/println→ useprintfStringparameter of the target. SoLOG.info("v: {}", String.format("%.2f", x))is not reported.%s,%dand%%. Width/precision/flags, other conversions, non-literal formats,Localeoverloads andjava.util.Formattablearguments are ignored, because{}placeholders can't reproduce them (%scallsformatToonFormattablevalues). Any format is reported forprint/println, becauseprintfaccepts the same syntax.print(String.format(...))→printf(...), andprintln(String.format("...", ...))→printf("...%n", ...)when the format is a string literal. TheLocaleand argument order are kept.String.formatin exception constructors is covered by S9397.StringBuilder/StringBufferappendis not reported.Testing notes
RedundantStringFormatCheckSample,RedundantStringFormatCheckLoggingSample). Both run with and without semantic. Without semantic, the logging sample reports no issues because the method symbols are unknown.Agent workflow
PR created using
uv run new_rule_implementation.py S9413 -j SONARJAVA-7070 -a claude -g AGENTS.md .claude/skills/new-rule/SKILL.mdTool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR created using
uv run create_with_claude.py /tmp/action_plan_romain/new-rule-s9413-sonarjava-7070.txtIterated on the PR with
uv run ci_loop.pyfor 1 iterations.✔️ The PR is now ready for review.
PR updated using
uv run update_with_claude.py --prompt "Implement the action plan described in the document Drop from the current implementation of the rule the case about using string concatenation instead of String.format. That case is already handled by rule S9397.." -a "Drop from the current implementation of the rule the case about using string concatenation instead of String.format. That case is already handled by rule S9397." -g "claude"PR updated using
uv run update_with_claude.py --prompt "Implement the action plan described in the document Modify the rule so that it no longer applies to the StringBuilder.append case. Update the PR description and the html description accordingly. Then address the comments by nathsou on the PR..."🤖 Generated with Claude Code
PR updated using
uv run update_with_claude.py --prompt "Implement the action plan described in the document Modify the rule so that it no longer applies to the StringBuilder.append case. Update the PR description and the html description accordingly. Then address the comments by nathsou on the PR.." -a "Modify the rule so that it no longer applies to the StringBuilder.append case. Update the PR description and the html description accordingly. Then address the comments by nathsou on the PR." -g "claude"