Skip to content

SONARJAVA-6511 Decrease number of FPs for the rule S125#5698

Merged
asya-vorobeva merged 1 commit into
masterfrom
asya/improve-s125
Jun 24, 2026
Merged

SONARJAVA-6511 Decrease number of FPs for the rule S125#5698
asya-vorobeva merged 1 commit into
masterfrom
asya/improve-s125

Conversation

@asya-vorobeva

@asya-vorobeva asya-vorobeva commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Semicolons in the end of comment's lines can be punctuation marks and not code indicators. We have a lot of user feedback pointing on it.

This PR removes FPs for comments where the only sign of code is that line ends with ';'. Comments that additionally have some indicators like CamelCase or keywords are still recognized as code.
This change unavoidably adds some amount of FNs. To reduce them the pattern is tuned to react on typical Java commented messages like System.out(err).print.

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6511

@asya-vorobeva asya-vorobeva force-pushed the asya/improve-s125 branch 2 times, most recently from 12c405d to 1976bdb Compare June 24, 2026 12:32
Comment thread java-checks/src/main/java/org/sonar/java/checks/JavaFootprint.java Outdated
@sonarqube-next

Copy link
Copy Markdown

@asya-vorobeva asya-vorobeva enabled auto-merge (squash) June 24, 2026 17:50
@asya-vorobeva asya-vorobeva merged commit 3a5ee6a into master Jun 24, 2026
15 checks passed
@asya-vorobeva asya-vorobeva deleted the asya/improve-s125 branch June 24, 2026 17:59
@gitar-bot

gitar-bot Bot commented Jun 24, 2026

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

Refined the S125 comment analysis to distinguish trailing semicolons from code indicators, resolving the issue where semicolon probability was incorrectly coupled to the detection threshold.

✅ 1 resolved
Quality: ';' detector probability silently coupled to THRESHOLD

📄 java-checks/src/main/java/org/sonar/java/checks/JavaFootprint.java:34
The new EndWithDetector(0.9, ';') is given a probability (0.9) that is exactly equal to CommentedOutCodeLineCheck.THRESHOLD (0.9). The FP-removal behaviour relies entirely on CodeRecognizer.isLineOfCode using a strict greater-than comparison (recognition(line) - threshold > 0): a line whose only code signal is a trailing ';' yields recognition == 0.9, which is NOT > 0.9, so it is correctly ignored. This is the intended fix, but the coupling is implicit and brittle:

  • If THRESHOLD is ever changed (e.g. to 0.85 or 0.95), or if the upstream commons library changes the comparison to >=, the rule will either start re-emitting all the semicolon FPs again or stop detecting other code, with no test in this file pinning the relationship.
    Suggested fix: add a code comment in JavaFootprint explaining that 0.9 is deliberately set equal to the recognizer threshold so that a trailing ';' alone never crosses it, and ideally reference the THRESHOLD constant so the dependency is visible. This protects the intent against future edits to either value.
Options

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

Comment with these commands to change:

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

Was this helpful? React with 👍 / 👎 | Gitar

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