Skip to content

JS-540 Persist ESLint-suppressed findings as accepted issues#7123

Draft
zglicz wants to merge 5 commits into
masterfrom
codex/js-540-eslint-suppressed-accepted
Draft

JS-540 Persist ESLint-suppressed findings as accepted issues#7123
zglicz wants to merge 5 commits into
masterfrom
codex/js-540-eslint-suppressed-accepted

Conversation

@zglicz
Copy link
Copy Markdown
Contributor

@zglicz zglicz commented May 29, 2026

Summary

  • persist ESLint-suppressed findings separately from normal issues
  • save suppressed findings as accepted issues when issue resolution is available
  • add JS/TS, gRPC, Java, and orchestrator coverage for ESLint-suppressed findings

Testing

  • CI

Summary by Gitar

  • Refactored analysis transformations:
    • Consolidated file result and issue processing logic into appendSuccessfulFileResult in packages/grpc/src/transformers/response.ts.
  • Improved suppression handling:
    • Simplified fallback resolution comment logic in packages/analysis/src/jsts/linter/issues/transform.ts using logical OR operator.
  • Test updates:
    • Updated numerous regression test expectations in its/ruling/src/test/expected/ to reflect changes in suppressed issue reporting.

This will update automatically on new commits.

@zglicz zglicz requested a review from a team May 29, 2026 08:57
@zglicz zglicz temporarily deployed to sca-checking May 29, 2026 08:57 — with GitHub Actions Inactive
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 29, 2026

Agentic Analysis: Early Results

Agentic Analysis and Context Augmentation are available on your project. Here are some issues that could have been prevented. Follow the links to learn how to put them into action.

3 issue(s) found across 2 file(s):

Rule File Line Message
javascript:S3504 its/plugin/projects/eslint-suppressed-issues/file.js 9 Unexpected var, use let or const instead.
java:S1874 its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/EslintSuppressedIssuesTest.java 90 Remove this use of "post"; it is deprecated.
java:S1874 its/plugin/tests/src/test/java/com/sonar/javascript/it/plugin/EslintSuppressedIssuesTest.java 96 Remove this use of "post"; it is deprecated.

Analyzed by SonarQube Agentic Analysis in 4.5 s

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown

hashicorp-vault-sonar-prod Bot commented May 29, 2026

JS-540

.getSensorContext()
.newIssueResolution()
.on(file)
.at(file.selectLine(issue.getLine()))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Bug: saveAcceptedIssueResolution crashes when issue.getLine() is 0

In saveAcceptedIssueResolution (line 654), file.selectLine(issue.getLine()) is called unconditionally. If a suppressed issue has line == 0 (the protobuf default for unset int32 fields), this will throw an IllegalArgumentException because SonarQube line indexing is 1-based. The existing saveIssue method at line 485 has a guard (issue.getLine() != 0) before calling selectLine, but saveAcceptedIssueResolution lacks this check. While the try/catch in saveSuppressedIssues will prevent a crash, the issue resolution will silently fail to save.

Add a guard for line == 0 before calling selectLine, matching the pattern in saveIssue:

private void saveAcceptedIssueResolution(JsTsContext<?> context, Issue issue, RuleKey ruleKey) {
  if (issue.getLine() == 0) {
    LOG.warn("Cannot save issue resolution for rule {} without a valid line", ruleKey);
    return;
  }
  context
    .getSensorContext()
    .newIssueResolution()
    .on(file)
    .at(file.selectLine(issue.getLine()))
    .status(IssueResolution.Status.DEFAULT)
    .forRules(List.of(ruleKey))
    .comment(issue.getResolutionComment())
    .save();
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 29, 2026

Ruling Report

No changes to ruling expected issues in this PR

@zglicz zglicz temporarily deployed to sca-checking May 29, 2026 09:37 — with GitHub Actions Inactive
@zglicz zglicz marked this pull request as draft May 29, 2026 12:28
@zglicz zglicz temporarily deployed to sca-checking May 29, 2026 14:22 — with GitHub Actions Inactive
@zglicz zglicz temporarily deployed to sca-checking May 29, 2026 15:34 — with GitHub Actions Inactive
@zglicz zglicz temporarily deployed to sca-checking May 29, 2026 16:38 — with GitHub Actions Inactive
@sonarqube-next
Copy link
Copy Markdown

@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented May 29, 2026

Code Review ⚠️ Changes requested 0 resolved / 1 findings

Persists ESLint-suppressed findings as accepted issues but triggers a crash in saveAcceptedIssueResolution when encountering a line number of 0.

⚠️ Bug: saveAcceptedIssueResolution crashes when issue.getLine() is 0

📄 sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisProcessor.java:654

In saveAcceptedIssueResolution (line 654), file.selectLine(issue.getLine()) is called unconditionally. If a suppressed issue has line == 0 (the protobuf default for unset int32 fields), this will throw an IllegalArgumentException because SonarQube line indexing is 1-based. The existing saveIssue method at line 485 has a guard (issue.getLine() != 0) before calling selectLine, but saveAcceptedIssueResolution lacks this check. While the try/catch in saveSuppressedIssues will prevent a crash, the issue resolution will silently fail to save.

Add a guard for line == 0 before calling selectLine, matching the pattern in saveIssue
private void saveAcceptedIssueResolution(JsTsContext<?> context, Issue issue, RuleKey ruleKey) {
  if (issue.getLine() == 0) {
    LOG.warn("Cannot save issue resolution for rule {} without a valid line", ruleKey);
    return;
  }
  context
    .getSensorContext()
    .newIssueResolution()
    .on(file)
    .at(file.selectLine(issue.getLine()))
    .status(IssueResolution.Status.DEFAULT)
    .forRules(List.of(ruleKey))
    .comment(issue.getResolutionComment())
    .save();
}
🤖 Prompt for agents
Code Review: Persists ESLint-suppressed findings as accepted issues but triggers a crash in `saveAcceptedIssueResolution` when encountering a line number of 0.

1. ⚠️ Bug: saveAcceptedIssueResolution crashes when issue.getLine() is 0
   Files: sonar-plugin/sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/analysis/AnalysisProcessor.java:654

   In `saveAcceptedIssueResolution` (line 654), `file.selectLine(issue.getLine())` is called unconditionally. If a suppressed issue has `line == 0` (the protobuf default for unset int32 fields), this will throw an `IllegalArgumentException` because SonarQube line indexing is 1-based. The existing `saveIssue` method at line 485 has a guard (`issue.getLine() != 0`) before calling `selectLine`, but `saveAcceptedIssueResolution` lacks this check. While the try/catch in `saveSuppressedIssues` will prevent a crash, the issue resolution will silently fail to save.

   Fix (Add a guard for line == 0 before calling selectLine, matching the pattern in saveIssue):
   private void saveAcceptedIssueResolution(JsTsContext<?> context, Issue issue, RuleKey ruleKey) {
     if (issue.getLine() == 0) {
       LOG.warn("Cannot save issue resolution for rule {} without a valid line", ruleKey);
       return;
     }
     context
       .getSensorContext()
       .newIssueResolution()
       .on(file)
       .at(file.selectLine(issue.getLine()))
       .status(IssueResolution.Status.DEFAULT)
       .forRules(List.of(ruleKey))
       .comment(issue.getResolutionComment())
       .save();
   }

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.
Unblock → Override a blocking verdict and allow merging.

Comment with these commands to change:

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

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.

1 participant