Skip to content

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

Closed
romainbrenguier wants to merge 7 commits into
masterfrom
romain/new-rule-s9432-sonarjava-7157
Closed

romainbrenguier wants to merge 7 commits into
masterfrom
romain/new-rule-s9432-sonarjava-7157

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Detects direct String trim() or strip() call chains used only for blankness checks, including isEmpty(), isBlank(), and zero/one length comparisons.

Agent workflow

PR created using uv run new_rule_implementation.py S9432 -j SONARJAVA-7157 -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 Change the error message, so that in the case of trim().isBlank(), we suggest isBlank but says that it is if we want to treat unicode spaces as blank and control characters as non-blank.." -a "Change the error message, so that in the case of trim().isBlank(), we suggest isBlank but says that it is if we want to treat unicode spaces as blank and control characters as non-blank." -g "claude"

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

SONARJAVA-7157

@datadog-sonarsource

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6334

Comment thread java-checks/src/main/java/org/sonar/java/checks/S9432Check.java
trim() removes characters <= U+0020 while isBlank() relies on
Character.isWhitespace, so replacing a trim()-based check by isBlank()
is only equivalent if Unicode whitespace should count as blank and
control characters as non-blank.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6334

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Ruling Diff Summary

Detected changes in 2 rule files: 0 issues removed, 6 issues added.

S9432 (java) on eclipse-jetty - 0 issues removed, 5 issues added - new ruling file

Added jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java (line 143)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java)

Added jetty-util/src/main/java/org/eclipse/jetty/util/HttpCookieStore.java (line 84)

(source file not found at this revision: jetty-util/src/main/java/org/eclipse/jetty/util/HttpCookieStore.java)

Added jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java (line 1324)

(source file not found at this revision: jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java)

Added jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java (line 1333)

(source file not found at this revision: jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java)

Added jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java (line 1485)

(source file not found at this revision: jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java)
S9432 (java) on eclipse-jetty-similar-to-main - 0 issues removed, 1 issues added - new ruling file

Added jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java (line 143)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java)

github-actions Bot and others added 2 commits October 14, 2026 09:40
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

⚖️ Ruling update ready for review: #6345

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@romainbrenguier
romainbrenguier marked this pull request as ready for review October 14, 2026 13:31
@gitar-bot

gitar-bot Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Code Review ✅ Approved 1 closed / 1 findings

🟡 Medium risk · Adding S9432 findings for trim()/strip() blankness checks could produce false positives or prompt replacements with different whitespace semantics.

Adds rule S9432 for direct trim() or strip() chains used only for blankness checks, and updates the message so trim().isBlank() notes that isBlank() treats unicode spaces as blank and control characters as non-blank. The earlier concern that the suggested fix changes behavior was resolved. No open issues found.

✅ 1 closed
✅ Bug: trim()-based checks are not equivalent to isBlank(); suggested fix changes behavior

📄 java-checks/src/main/java/org/sonar/java/checks/S9432Check.java:37 📄 java-checks/src/main/java/org/sonar/java/checks/S9432Check.java:40-44 📄 java-checks/src/main/java/org/sonar/java/checks/S9432Check.java:75-76 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9432.html:30-31
String.trim() strips every code point <= U+0020, including control characters like \u0000–\u0008. String.isBlank() (like strip()) uses Character.isWhitespace, which leaves those control characters out but counts Unicode spaces such as \u2003 and \u3000. So "\u0000".trim().isEmpty() is true while "\u0000".isBlank() is false, and "\u2003".trim().isEmpty() is false while "\u2003".isBlank() is true. The check matches trim and strip the same way and reports Use "isBlank()" on the original string instead. on trim() chains, so developers who apply the fix (the rule is in Sonar way) can quietly change validation results. Fix: report only strip() chains, or give trim() chains a different message and mention the semantic difference in the rule description. At the very least, the HTML should warn that the fix is not behavior-preserving for trim().

Verification Rules 1/1 met Repos 8 inspected

Functional validation No results

Rules 1 passed 2 not applicable

✅ Repository license compliance

Broader impact Repos 8 inspected
Cross-repo 8 repositories inspected 0 open impacts

SonarSource/sonarlint-intellij Inspected
SonarSource/sonarlint-vscode Inspected
SonarSource/sonarlint-eclipse Inspected
SonarSource/sonar-java-symbolic-execution Inspected
SonarSource/sonar-lits Inspected

3 more repositories

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

Copy link
Copy Markdown
Contributor

@asya-vorobeva

Copy link
Copy Markdown
Contributor

Closed due to decision raise such issues in the existing rule #6359

@asya-vorobeva
asya-vorobeva deleted the romain/new-rule-s9432-sonarjava-7157 branch October 16, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants