Skip to content

SONARJAVA-7188 SONARJAVA-4606 Improvements for the rule S7158 - #6359

Open
asya-vorobeva wants to merge 3 commits into
masterfrom
asya/improve-s7158
Open

asya-vorobeva wants to merge 3 commits into
masterfrom
asya/improve-s7158

Conversation

@asya-vorobeva

@asya-vorobeva asya-vorobeva commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor
  • Detect zero literals written in any base
  • Suggest "isBlank()" for "strip().length()" and "trim().length()" comparisons

asya-vorobeva and others added 2 commits October 14, 2026 15:31
LiteralUtils.isZero compared the literal's decimal token text, so
length() == 0 was flagged but the equivalent length() == 0x0, 00 or
0b0 was not. Compare the parsed literal value instead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…risons

Comparing "s.trim().length()" to zero is a blankness check, so report it
with an "isBlank()" message and quick fix instead of "isEmpty()". Gated
on Java 11, where "String.isBlank()" was introduced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@hashicorp-vault-sonar-prod

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

Copy link
Copy Markdown
Contributor

Comment thread java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java Outdated
… comparisons

"strip()" removes exactly the characters "isBlank()" treats as white space,
so the rewrite is equivalent. "trim()" removes every character up to U+0020
instead, so it disagrees on control characters below U+0020 and on Unicode
white space above it; those cases keep the suggestion but the issue message
and the quick-fix label state what the replacement changes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Oct 15, 2026 •

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

🟡 Medium risk · Expanding zero-literal detection and suggesting isBlank() quick fixes changes analyzer findings; trim() replacements could alter behavior for control characters and Unicode whitespace.

Improves rule S7158 to detect zero literals written in any base and to suggest isBlank() for strip().length() and trim().length() comparisons. No open issues found.

✅ 1 closed
Closed: Cross-PR: S7158 and S9432 both flag trim().length() with conflicting advice

📄 java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java:132-143 📄 java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java:188-196 📄 java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java:9-23 📄 java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java:59-63 📄 java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java:9-11 📄 java-checks-test-sources/default/src/main/java/checks/StringIsBlankCheckSample.java:72-75 📄 java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java:143-157 📄 java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java:205-213
Breaks when this PR and #6332 are both merged, and both S7158 and S9432 are enabled on code analyzed as Java 11 or later that contains s.trim().length() == 0 (or a related form).

How

  1. This PR's StringIsEmptyCheck reports trim().length() compared with 0 or 1, and its quick fix rewrites the comparison to isBlank() as a plain replacement (java-checks/src/main/java/org/sonar/java/checks/StringIsEmptyCheck.java:133-143).
  2. In SONARJAVA-7157: Implemented rule S9432 Strings should not be trimmed or stripped just to check if they are blank #6332, S9432Check.visitNode reports the same expressions at the trim() call (java-checks/src/main/java/org/sonar/java/checks/S9432Check.java:77). Its message says to use isBlank() only if Unicode whitespace should count as blank and control characters should not.
  3. Each such line therefore gets two issues, placed at different locations and with contradicting messages. The S7158 quick fix silently changes behavior in exactly the case S9432 warns about.
  4. For s.strip().length() == 0 and (s.trim()).length() == 0, S7158 suggests isEmpty() while S9432 suggests removing the strip/trim, so the two rules give opposite advice.

Fix

Compared b049767 with #6332 at 6a682bc

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-vscode Inspected
SonarSource/sonarlint-language-server Inspected
SonarSource/sonarlint-intellij 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

Copy link
Copy Markdown
Contributor

Quality Gate failed Quality Gate failed

Failed conditions
1 New issue

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE SonarQube for IDE

This branch has not been deployed

No deployments
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.

1 participant