Skip to content

SONARJAVA-6455 S7467: FP on broken semantics - #5853

Merged
asya-vorobeva merged 1 commit into
masterfrom
s7467-fp-when-broken-semantics
Aug 14, 2026
Merged

SONARJAVA-6455 S7467: FP on broken semantics#5853
asya-vorobeva merged 1 commit into
masterfrom
s7467-fp-when-broken-semantics

Conversation

@asya-vorobeva

Copy link
Copy Markdown
Contributor

Without semantics, catch parameter usages() is always empty, causing false positives. Guard with context.getSemanticModel() != null.

Without semantics, catch parameter usages() is always empty, causing
false positives. Guard with context.getSemanticModel() != null.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

SONARJAVA-6455

@gitar-bot

gitar-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Guards catch parameter usage checks with a semantic model null check to prevent false positives when semantics are absent. No issues found.

Options

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

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

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 7 days with no activity. If there is no activity in the next 7 days it will be closed automatically

@github-actions github-actions Bot added the stale label Aug 8, 2026

@francois-mora-sonarsource francois-mora-sonarsource left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — the guard is correct for the case it targets (semantic model entirely absent), and the flipped verifyNoIssues() in test_without_semantics covers it.

One non-blocking note on test coverage. The added executor_run_compliant() sample doesn't exercise the change: it's analyzed with the full default classpath, so e resolves and has a usage, and it passes on the parent commit too. It also repeats the lambda-usage case already at line 117.

While checking that, I found a separate FP on partial semantics that this PR doesn't cover — usages() is also empty when the parameter binds fine but its usage sits in a lambda body ECJ couldn't bind. Reproduced against this branch:

void usedInsideLambdaWithUnresolvedFunctionalInterface() {
  try {
    Integer.parseInt("x");
  } catch (NumberFormatException e) { // FP: "Replace e with an unnamed pattern"
    Unknown.run(() -> System.out.println(e.getMessage()));
  }
}

NumberFormatException resolves, the semantic model is present, so the new guard passes — but the usage inside the lambda was never registered. Note this is not fixable by checking v.symbol().isUnknown() instead (I tried; the symbol is known here). It needs something like detecting unresolved constructs in the parameter's scope, so it's a design question rather than a condition tweak.

Worth a follow-up ticket rather than blocking this one. Happy to file it with the reproducer.

@asya-vorobeva

Copy link
Copy Markdown
Contributor Author

Approving — the guard is correct for the case it targets (semantic model entirely absent), and the flipped verifyNoIssues() in test_without_semantics covers it.

One non-blocking note on test coverage. The added executor_run_compliant() sample doesn't exercise the change: it's analyzed with the full default classpath, so e resolves and has a usage, and it passes on the parent commit too. It also repeats the lambda-usage case already at line 117.

Absolutely agree, and I've added this test case only to illustrate customer's issue.

While checking that, I found a separate FP on partial semantics that this PR doesn't cover — usages() is also empty when the parameter binds fine but its usage sits in a lambda body ECJ couldn't bind. Reproduced against this branch:

void usedInsideLambdaWithUnresolvedFunctionalInterface() {
  try {
    Integer.parseInt("x");
  } catch (NumberFormatException e) { // FP: "Replace e with an unnamed pattern"
    Unknown.run(() -> System.out.println(e.getMessage()));
  }
}

NumberFormatException resolves, the semantic model is present, so the new guard passes — but the usage inside the lambda was never registered. Note this is not fixable by checking v.symbol().isUnknown() instead (I tried; the symbol is known here). It needs something like detecting unresolved constructs in the parameter's scope, so it's a design question rather than a condition tweak.

Worth a follow-up ticket rather than blocking this one. Happy to file it with the reproducer.

Yep, it's well-known problem for many rules. Good point, I'll create JIRA ticket to investigate and fix this point.

@asya-vorobeva
asya-vorobeva merged commit c9f819a into master Aug 14, 2026
18 checks passed
@asya-vorobeva
asya-vorobeva deleted the s7467-fp-when-broken-semantics branch August 14, 2026 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants