JS-1995 Suppress S2486 on single-statement try blocks by erwan-leforestier-sonarsource · Pull Request #7445 · SonarSource/SonarJS

erwan-leforestier-sonarsource · 2026-07-02T15:49:19Z

Summary

Suppress S2486 when a try block contains a single top-level statement, which reduces noise on intentional try/fallback patterns. Keep reporting unused catch parameters for broader try blocks.

Changes

Skip the S2486 report when the enclosing try body has exactly one top-level statement
Extend the rule fixture with compliant single-statement cases and a noncompliant multi-statement case
Add a standalone FV that compares master and the review branch on the same sample

Summary by Gitar

Rule logic:
- Modified S2486 to exempt try blocks containing exactly one top-level statement, accounting for LabeledStatement wrappers.
Testing & Tooling:
- Updated build.yml to improve JS coverage artifacts handling.
- Updated several ruling test expectations following the logic change in S2486.

_{This will update automatically on new commits.}

hashicorp-vault-sonar-prod · 2026-07-02T15:49:36Z

JS-1995

github-actions · 2026-07-02T15:55:51Z

Ruling Report

Code no longer flagged (222 issues)

S2486

Ghost/core/client/app/controllers/feature.js:16

    14 |             try {
    15 |                 value = JSON.parse(this.get('setting.labs') || {});
>   16 |             } catch (err) {
    17 |                 value = {};
    18 |             }

Ghost/core/client/app/controllers/settings/navigation.js:59

    57 |         try {
    58 |             navItems = JSON.parse(this.get('model.navigation') || [{}]);
>   59 |         } catch (e) {
    60 |             navItems = [{}];
    61 |         }

Ghost/core/client/app/services/config.js:19

    17 |         try {
    18 |             return JSON.parse(val);
>   19 |         } catch (e) {
    20 |             return val;
    21 |         }

Ghost/core/client/app/utils/ajax.js:30

    28 |                 message =  request.responseJSON.error || 'Unknown Error';
    29 |             }
>   30 |         } catch (e) {
    31 |             msgDetail = request.status ? `${request.status} - ${request.statusText}` : 'Server was not available';
    32 |             message = `The server returned an error (${msgDetail}).`;

Ghost/core/server/api/settings.js:48

    46 |         try {
    47 |             labsValue = JSON.parse(settingsCache.labs.value);
>   48 |         } catch (e) {
    49 |             errors.logError.apply(this, errorMessages);
    50 |         }

Ghost/core/server/config/index.js:95

    93 |         try {
    94 |             pg = require('pg');
>   95 |         } catch (e) {
    96 |             pg = require('pg.js');
    97 |         }

Ghost/core/server/ghost-server.js:46

    44 |             try {
    45 |                 fs.unlinkSync(socketConfig.path);
>   46 |             } catch (e) {
    47 |                 // We can ignore this.
    48 |             }

Ghost/core/test/utils/index.js:325

   323 |             try {
   324 |                 data = JSON.parse(fileContents);
>  325 |             } catch (e) {
   326 |                 return new Error('Failed to parse the file');
   327 |             }

TypeScript/lib/tsc.js:2198

  2196 |                             stat = _fs.statSync(name);
  2197 |                         }
> 2198 |                         catch (e) {
  2199 |                             continue;
  2200 |                         }

TypeScript/lib/tsc.js:2310

  2308 |                         return _fs.statSync(path).mtime;
  2309 |                     }
> 2310 |                     catch (e) {
  2311 |                         return undefined;
  2312 |                     }

...and 212 more

📋 View full report

Code no longer flagged (222)

S2486

francois-mora-sonarsource · 2026-07-07T12:53:04Z

isSingleSimpleStatement() is still a bit too broad here. It now rejects labeled loops/switches, but it still treats a single nested block as “simple”, so this becomes compliant:

try {
  { foo(); bar(); }
} catch (err) {}

I verified that locally with a RuleTester repro: the current rule reports 0 issues for that shape. That feels outside the intended single-operation try/fallback pattern and turns a multi-step catch body into a false negative. Could we keep reporting for block wrappers as well, and add an invalid regression for that boundary next to the existing loop/switch cases?

erwan-leforestier-sonarsource · 2026-07-07T14:06:01Z

@francois-mora-sonarsource
Good catch, you were right. I tightened the exemption so wrapped blocks are still reported and added a regression test for that case.

Cast the catch clause parent to TryStatement at the call site and drop the redundant type check, and only resolve the parent when the caught variable is unused. Add fixture cases for assignment, variable declaration, throw and nested try single-statement bodies, plus an empty-try noncompliant boundary.

…bodies Loops and switch statements with an empty catch are not idiomatic best-effort patterns: they can hide multiple side-effecting operations that silently swallow errors. Exempt only when the single body statement is not a for/for-in/for-of/while/do-while/switch. Add fixture cases for each newly-flagged shape.

…emption A labeled loop like `outer: for (...)` has type LabeledStatement, not ForStatement, so it was slipping through the LOOP_OR_SWITCH_TYPES guard. Unwrap LabeledStatement nodes before the type check so that labeled loops and switches are correctly treated as non-exempt. Update ruling expected files to remove the 223 single-statement issues that S2486 no longer raises across the corpus. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

sonarqube-next · 2026-07-07T15:38:34Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

francois-mora-sonarsource

Looks good to me now.

The wrapped-block and labeled-loop boundaries are covered, the ruling delta stays aligned with the intended suppression scope, and the focused local regression check passes.

One non-blocking follow-up thought: depending on how we want to define “single statement”, we may later want to decide whether top-level IfStatement and nested TryStatement should also count as non-simple. I do not see that as a blocker for this PR as written.

gitar-bot · 2026-07-08T08:27:51Z

Code Review ✅ Approved 1 resolved / 1 findings

S2486 now exempts try blocks containing a single top-level statement, resolving the loop and switch guard bypass issues while reducing noise.

✅ 1 resolved

✅ Edge Case: Loop/switch guard bypassed by wrapping statements

📄 packages/analysis/src/jsts/rules/S2486/rule.ts:35-37
isSingleSimpleStatement only inspects body[0].type for direct loop/switch nodes. A single top-level statement that wraps a loop or switch escapes the guard and is treated as exempt (Compliant):

LabeledStatement wrapping a loop, e.g. outer: for (...) { ... } has type LabeledStatement, which is not in LOOP_OR_SWITCH_TYPES, so the report is suppressed even though the body is a loop.

A bare BlockStatement such as { doSomething(); doSomethingElse(); } counts as one top-level statement and is exempted, even though it contains multiple statements — the very case the multi-statement path is meant to catch.

These mirror the noncompliant forLoopBody/whileLoopBody/switchBody fixtures but slip through. If the intent is to exempt only genuinely trivial single-statement try bodies, consider unwrapping LabeledStatement/BlockStatement before the type check (and adding fixtures for these cases). Minor, since these are less common patterns, but they create inconsistent behavior with the new fixtures.

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}

github-actions Bot mentioned this pull request Jul 2, 2026

Update ruling results for PR #7445 #7446

Closed

This comment has been minimized.

Sign in to view

gitar-bot Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread packages/analysis/src/jsts/rules/S2486/rule.ts

erwan-leforestier-sonarsource force-pushed the fix/js-1995-s2486-single-statement-try branch from b19fe6a to 9008a7c Compare July 7, 2026 10:18

erwan-leforestier-sonarsource mentioned this pull request Jul 7, 2026

JS-2017 Fix JS coverage handoff for Sonar analysis #7481

Merged

erwan-leforestier-sonarsource marked this pull request as ready for review July 7, 2026 12:21

erwan-leforestier-sonarsource and others added 6 commits July 7, 2026 17:23

JS-1995 Ignore single-statement try blocks in S2486

7361362

Update ruling results

1959b7e

JS-1995 reject wrapped block try bodies

f8b5e76

erwan-leforestier-sonarsource force-pushed the fix/js-1995-s2486-single-statement-try branch from 7c239bd to f8b5e76 Compare July 7, 2026 15:23

francois-mora-sonarsource approved these changes Jul 7, 2026

View reviewed changes

erwan-leforestier-sonarsource merged commit f29bd92 into master Jul 8, 2026
44 checks passed

erwan-leforestier-sonarsource deleted the fix/js-1995-s2486-single-statement-try branch July 8, 2026 08:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

JS-1995 Suppress S2486 on single-statement try blocks#7445

JS-1995 Suppress S2486 on single-statement try blocks#7445
erwan-leforestier-sonarsource merged 6 commits into
masterfrom
fix/js-1995-s2486-single-statement-try

erwan-leforestier-sonarsource commented Jul 2, 2026 •

edited by gitar-bot Bot

Loading

Uh oh!

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026 •

edited by atlassian Bot

Loading

Uh oh!

github-actions Bot commented Jul 2, 2026 •

edited

Loading

Code no longer flagged (222)

Uh oh!

This comment has been minimized.

Uh oh!

francois-mora-sonarsource commented Jul 7, 2026

Uh oh!

erwan-leforestier-sonarsource commented Jul 7, 2026

Uh oh!

sonarqube-next Bot commented Jul 7, 2026

Uh oh!

francois-mora-sonarsource left a comment

Uh oh!

Uh oh!

gitar-bot Bot commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

erwan-leforestier-sonarsource commented Jul 2, 2026 • edited by gitar-bot Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Summary by Gitar

Uh oh!

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ruling Report

Code no longer flagged (222 issues)

S2486

Code no longer flagged (222)

Uh oh!

This comment has been minimized.

Uh oh!

francois-mora-sonarsource commented Jul 7, 2026

Uh oh!

erwan-leforestier-sonarsource commented Jul 7, 2026

Uh oh!

sonarqube-next Bot commented Jul 7, 2026

Quality Gate passed

Uh oh!

francois-mora-sonarsource left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gitar-bot Bot commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

erwan-leforestier-sonarsource commented Jul 2, 2026 •

edited by gitar-bot Bot

Loading

hashicorp-vault-sonar-prod Bot commented Jul 2, 2026 •

edited by atlassian Bot

Loading

github-actions Bot commented Jul 2, 2026 •

edited

Loading