JS-2013 Align S6842 with the ARIA in HTML conformance table by erwan-leforestier-sonarsource · Pull Request #7484 · SonarSource/SonarJS

erwan-leforestier-sonarsource · 2026-07-07T12:06:09Z

Summary

Align S6842 with the full ARIA in HTML conformance table, not only the composite-widget overrides. jsx-a11y's element→roles allowlist cannot express context, so the rule is now decorated: config.ts keeps the flat, spec-derived allowlists and decorator.ts handles everything the allowlist cannot. This brings the JS/TS analyzer to parity with the sonar-html implementation (SONARHTML-403) — both now agree with the spec on every element/role pair.

Changes

config.ts: enumerated allowlists per the conformance table — ul/ol/menu (drop treegrid), nav, h1–h6, fieldset, td, progress; li is left to the decorator.
decorator.ts (new): suppresses the combinations the table permits but the allowlist cannot describe:
- elements that accept any role (table/tbody/thead/tfoot, text-level elements such as p, blockquote, time, output, …);
- context-sensitive elements — li (only restricted when a parent exposes a list role), img (interactive roles allowed only with an accessible name), figure (restricted only with a figcaption), label (restricted only when associated with a control);
- toolbar treated as a structure role (non-interactive), consistent with sonar-html and the WAI-ARIA role taxonomy.

Fixes vs. the previous revision

Removes false positives on nav, headings, named img, and any-role elements.
Restores the missing treegrid-on-ul/ol report (spec forbids it).
Drops the incorrect table[role="button"] / tbody[role="button"] true positives — the table family accepts any role.

Tests

unit.test.ts rewritten as a spec matrix (valid + invalid) covering every case above.

hashicorp-vault-sonar-prod · 2026-07-07T12:06:34Z

JS-2013

github-actions · 2026-07-07T12:13:08Z

Ruling Report

Code no longer flagged (225 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 215 more

New issues flagged (2 issues)

S8968

vitest/test/snapshots/test/fixtures/obsolete/src/test1.test.ts:4

     2 | 
     3 | it('foo', () => {
>    4 |   if (process.env.TEST_OBSOLETE) return
     5 |   expect("foo").toMatchSnapshot();
     6 | })

vitest/test/snapshots/test/fixtures/obsolete/src/test1.test.ts:9

     7 | 
     8 | it('fuu', () => {
>    9 |   if (process.env.TEST_OBSOLETE) return
    10 |   expect("fuu").toMatchSnapshot();
    11 | })

📋 View full report

Code no longer flagged (225)

S2486

S6842

desktop/app/src/ui/lib/vertical-segmented-control/segmented-item.tsx:71

S8784

New issues flagged (2)

S8968

Extend beyond the composite-widget overrides to the full conformance table by decorating the jsx-a11y rule. config.ts keeps the flat, spec-derived allowlists; decorator.ts handles what the allowlist cannot express: - elements that accept any role (table family, text-level elements); - context-sensitive elements: li (parent list role), img (accessible name), figure (figcaption), label (association); - toolbar treated as a structure role (non-interactive), matching sonar-html. Fixes the previous false positives (nav, headings, named img, any-role elements) and the treegrid-on-ul/ol false negative, and drops the incorrect table[role=button] true positives so both analyzers agree with the spec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

gitar-bot · 2026-07-08T10:38:56Z

Code Review 👍 Approved with suggestions 1 resolved / 3 findings

Aligns S6842 with the ARIA in HTML conformance table using a new decorator to handle context-sensitive roles. Address the minor limitations regarding non-literal accessible names and global toolbar role allowance to ensure complete rule coverage.

💡 Edge Case: img name/role check ignores non-literal attribute values

📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:149 📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:177-188 📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:201-215

In decorate, hasAccessibleName/hasNonEmptyAttribute and the role lookup rely on getLiteralPropValue(getProp(...)), which returns undefined for any non-literal (JSX expression) attribute value. For <img alt={label} role="button" /> or <img aria-label={t('logo')} role="tab" />, the accessible name is provided dynamically, so hasAccessibleName() returns false and isRoleAllowedBySpec reports a false positive even though the image is correctly named at runtime. Common React code passes alt/aria-label as expressions, so this can be a frequent false positive. Consider treating an image whose alt/aria-label/aria-labelledby prop is present but non-literal as (potentially) named — e.g. check for prop presence via getProp and only require a non-empty value when the value is a literal string.

💡 Edge Case: toolbar role suppressed globally before context checks

📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:138-141 📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:145-156

isRoleAllowedBySpec returns true for role === 'toolbar' before any element/context checks, so role="toolbar" is allowed on every element. This means <ul role="list"><li role="toolbar">…</li></ul> is not reported, even though a list item whose parent exposes a list role is restricted to listitem by the conformance table (the same context the code enforces for other roles via parentExposesListRole). If treating toolbar as a non-interactive structure role is intentional and matches sonar-html this may be acceptable, but the unconditional early return bypasses the context-sensitive handling for li (and img/figure/label). Consider whether the toolbar allowance should be scoped rather than global.

✅ 1 resolved

✅ Quality: deepStrictEqual test is brittle to upstream jsx-a11y changes

📄 packages/analysis/src/jsts/rules/S6842/config.ts:42-44 📄 packages/analysis/src/jsts/rules/S6842/unit.test.ts:52-57
The equality test deepStrictEqual(allowlist, EXPECTED_ALLOWLIST) depends on both the exact contents and the exact ordering of the upstream jsx-a11y no-noninteractive-element-to-interactive-role defaults.

Two sources of fragility:

Ordering: config builds table as [...new Set([...upstreamAllowlist.table, ...TABLE_COMPOSITE_ROLES])], whose insertion order only equals the test's hand-written TABLE_ALLOWED_ROLES = ['grid', ...LIST_CONTAINER_COMPOSITE_ROLES, 'treegrid'] as long as upstreamAllowlist.table is exactly ['grid']. If a future jsx-a11y release reorders or adds a table role, the arrays diverge and the assertion fails even though behavior is correct.

Inconsistent dedup: config dedups ul/ol with new Set([...upstreamAllowlist.ul, 'toolbar']), but the test's EXPECTED_ALLOWLIST.ul = [...UPSTREAM_ALLOWLIST.ul, 'toolbar'] does not dedup. If upstream ever adds 'toolbar' to ul/ol, config yields one entry while the expectation yields two, breaking the test.

Suggestion: make the test build its expectation the same way the config does (spread upstream + apply the same Set dedup / same ordering), or assert membership as sets rather than order-sensitive arrays, so the test tracks intent rather than incidental ordering of the upstream package.

🤖 Prompt for agents

Code Review: Aligns S6842 with the ARIA in HTML conformance table using a new decorator to handle context-sensitive roles. Address the minor limitations regarding non-literal accessible names and global `toolbar` role allowance to ensure complete rule coverage.

1. 💡 Edge Case: img name/role check ignores non-literal attribute values
   Files: packages/analysis/src/jsts/rules/S6842/decorator.ts:149, packages/analysis/src/jsts/rules/S6842/decorator.ts:177-188, packages/analysis/src/jsts/rules/S6842/decorator.ts:201-215

   In `decorate`, `hasAccessibleName`/`hasNonEmptyAttribute` and the role lookup rely on `getLiteralPropValue(getProp(...))`, which returns `undefined` for any non-literal (JSX expression) attribute value. For `<img alt={label} role="button" />` or `<img aria-label={t('logo')} role="tab" />`, the accessible name is provided dynamically, so `hasAccessibleName()` returns false and `isRoleAllowedBySpec` reports a false positive even though the image is correctly named at runtime. Common React code passes `alt`/`aria-label` as expressions, so this can be a frequent false positive. Consider treating an image whose `alt`/`aria-label`/`aria-labelledby` prop is present but non-literal as (potentially) named — e.g. check for prop presence via `getProp` and only require a non-empty value when the value is a literal string.

2. 💡 Edge Case: toolbar role suppressed globally before context checks
   Files: packages/analysis/src/jsts/rules/S6842/decorator.ts:138-141, packages/analysis/src/jsts/rules/S6842/decorator.ts:145-156

   `isRoleAllowedBySpec` returns `true` for `role === 'toolbar'` before any element/context checks, so `role="toolbar"` is allowed on every element. This means `<ul role="list"><li role="toolbar">…</li></ul>` is not reported, even though a list item whose parent exposes a `list` role is restricted to `listitem` by the conformance table (the same context the code enforces for other roles via `parentExposesListRole`). If treating `toolbar` as a non-interactive structure role is intentional and matches sonar-html this may be acceptable, but the unconditional early return bypasses the context-sensitive handling for `li` (and `img`/`figure`/`label`). Consider whether the `toolbar` allowance should be scoped rather than global.

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}

gitar-bot · 2026-07-08T12:31:06Z

Code Review ✅ Approved 3 resolved / 3 findings

Aligns S6842 with the full ARIA in HTML conformance table by separating static allowlists from complex contextual decorations. Consider updating the img accessible name check to support non-literal attribute values, as the current implementation only recognizes static literals.

✅ 3 resolved

✅ Quality: deepStrictEqual test is brittle to upstream jsx-a11y changes

📄 packages/analysis/src/jsts/rules/S6842/config.ts:42-44 📄 packages/analysis/src/jsts/rules/S6842/unit.test.ts:52-57
The equality test deepStrictEqual(allowlist, EXPECTED_ALLOWLIST) depends on both the exact contents and the exact ordering of the upstream jsx-a11y no-noninteractive-element-to-interactive-role defaults.

Two sources of fragility:

Ordering: config builds table as [...new Set([...upstreamAllowlist.table, ...TABLE_COMPOSITE_ROLES])], whose insertion order only equals the test's hand-written TABLE_ALLOWED_ROLES = ['grid', ...LIST_CONTAINER_COMPOSITE_ROLES, 'treegrid'] as long as upstreamAllowlist.table is exactly ['grid']. If a future jsx-a11y release reorders or adds a table role, the arrays diverge and the assertion fails even though behavior is correct.

Inconsistent dedup: config dedups ul/ol with new Set([...upstreamAllowlist.ul, 'toolbar']), but the test's EXPECTED_ALLOWLIST.ul = [...UPSTREAM_ALLOWLIST.ul, 'toolbar'] does not dedup. If upstream ever adds 'toolbar' to ul/ol, config yields one entry while the expectation yields two, breaking the test.

Suggestion: make the test build its expectation the same way the config does (spread upstream + apply the same Set dedup / same ordering), or assert membership as sets rather than order-sensitive arrays, so the test tracks intent rather than incidental ordering of the upstream package.

✅ Edge Case: toolbar role suppressed globally before context checks

📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:138-141 📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:145-156
isRoleAllowedBySpec returns true for role === 'toolbar' before any element/context checks, so role="toolbar" is allowed on every element. This means <ul role="list"><li role="toolbar">…</li></ul> is not reported, even though a list item whose parent exposes a list role is restricted to listitem by the conformance table (the same context the code enforces for other roles via parentExposesListRole). If treating toolbar as a non-interactive structure role is intentional and matches sonar-html this may be acceptable, but the unconditional early return bypasses the context-sensitive handling for li (and img/figure/label). Consider whether the toolbar allowance should be scoped rather than global.

✅ Edge Case: img name/role check ignores non-literal attribute values

📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:149 📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:177-188 📄 packages/analysis/src/jsts/rules/S6842/decorator.ts:201-215
In decorate, hasAccessibleName/hasNonEmptyAttribute and the role lookup rely on getLiteralPropValue(getProp(...)), which returns undefined for any non-literal (JSX expression) attribute value. For <img alt={label} role="button" /> or <img aria-label={t('logo')} role="tab" />, the accessible name is provided dynamically, so hasAccessibleName() returns false and isRoleAllowedBySpec reports a false positive even though the image is correctly named at runtime. Common React code passes alt/aria-label as expressions, so this can be a frequent false positive. Consider treating an image whose alt/aria-label/aria-labelledby prop is present but non-literal as (potentially) named — e.g. check for prop presence via getProp and only require a non-empty value when the value is a literal string.

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 · 2026-07-08T12:41:56Z

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

gitar-bot Bot reviewed Jul 7, 2026

View reviewed changes

Comment thread packages/analysis/src/jsts/rules/S6842/config.ts Outdated

This comment has been minimized.

Sign in to view

JS-2013 Add S6842 spec-compliant overrides

3e622bd

erwan-leforestier-sonarsource force-pushed the fix/js-2013-s6842-aria-html-overrides branch from 4a8f2d7 to 3e622bd Compare July 7, 2026 15:24

erwan-leforestier-sonarsource changed the title ~~JS-2013 Allow S6842 spec-compliant composite overrides~~ JS-2013 Align S6842 with the ARIA in HTML conformance table Jul 8, 2026

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

Update ruling results for PR #7484 #7493

Closed

gitar-bot Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread packages/analysis/src/jsts/rules/S6842/decorator.ts

gitar-bot Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread packages/analysis/src/jsts/rules/S6842/decorator.ts Outdated

JS-2013 Fix S6842 img accessible names and ruling

3ebd5c2

gitar-bot Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread packages/analysis/src/jsts/rules/S6842/decorator.ts

Fix SonarQube findings on S6842 follow-up

0ed57ea

erwan-leforestier-sonarsource added 2 commits July 8, 2026 14:13

JS-2013 Restrict S6842 toolbar override to list containers

96968d8

CI: retrigger after transient Artifactory failure

e307172

erwan-leforestier-sonarsource marked this pull request as ready for review July 8, 2026 14:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

JS-2013 Align S6842 with the ARIA in HTML conformance table#7484

JS-2013 Align S6842 with the ARIA in HTML conformance table#7484
erwan-leforestier-sonarsource wants to merge 6 commits into
masterfrom
fix/js-2013-s6842-aria-html-overrides

erwan-leforestier-sonarsource commented Jul 7, 2026 •

edited

Loading

Uh oh!

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

edited by atlassian Bot

Loading

Uh oh!

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Code no longer flagged (225)

New issues flagged (2)

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

gitar-bot Bot commented Jul 8, 2026

Uh oh!

gitar-bot Bot commented Jul 8, 2026

Uh oh!

sonarqube-next Bot commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

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

Uh oh!

Summary

Changes

Fixes vs. the previous revision

Tests

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Ruling Report

Code no longer flagged (225 issues)

S2486

New issues flagged (2 issues)

S8968

Code no longer flagged (225)

New issues flagged (2)

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

gitar-bot Bot commented Jul 8, 2026

Uh oh!

gitar-bot Bot commented Jul 8, 2026

Uh oh!

sonarqube-next Bot commented Jul 8, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

erwan-leforestier-sonarsource commented Jul 7, 2026 •

edited

Loading

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

edited by atlassian Bot

Loading

github-actions Bot commented Jul 7, 2026 •

edited

Loading