Skip to content

JS-1464 Fix S6819 false positives on ARIA composite widget patterns#6645

Open
sonar-nigel[bot] wants to merge 2 commits intomasterfrom
fix/JS-1464-fix-fp-on-s6819-aria-roles-required-for-custom-composite-widgets-sonnet
Open

JS-1464 Fix S6819 false positives on ARIA composite widget patterns#6645
sonar-nigel[bot] wants to merge 2 commits intomasterfrom
fix/JS-1464-fix-fp-on-s6819-aria-roles-required-for-custom-composite-widgets-sonnet

Conversation

@sonar-nigel
Copy link
Contributor

@sonar-nigel sonar-nigel bot commented Mar 19, 2026

Fix false positives in S6819 (aria-roles-required-for-custom-composite-widgets) where valid ARIA composite widget patterns were incorrectly flagged.

What changed

The decorator's isCustomTableWidget function was too narrow — it only handled table/grid container roles on div/span elements, missing other valid composite widget patterns. It has been renamed to isCustomCompositeWidget and extended:

  • Add listbox to COMPOSITE_CONTAINER_ROLES (alongside table, grid)
  • Add option to COMPOSITE_CHILD_ROLES (alongside row, gridcell, etc.)
  • Add COMPOSITE_INNER_ROLES (button, presentation, none) that are suppressed when nested inside any composite container
  • Remove the div/span element-name restriction, allowing ul/li and other HTML elements to form valid composite widgets

Why

ARIA composite widget patterns using listbox/option roles (including on ul/li elements) and inner roles like button nested inside composite containers were being incorrectly reported as violations. These are valid uses of ARIA roles when the semantic HTML alternative requires an incompatible parent context.

The RSPEC has been updated with a compliant listbox/option example to document this case.

Relates to JS-1464

Proposed rspec changes
diff --git a/rules/S6819/javascript/rule.adoc b/rules/S6819/javascript/rule.adoc
index 028f4ce..10692c3 100644
--- a/rules/S6819/javascript/rule.adoc
+++ b/rules/S6819/javascript/rule.adoc
@@ -12,6 +12,16 @@ include::../common/fix.adoc[]
 <button onClick={handleClick}>Click me</button>
 ----
 
+When a semantic HTML alternative requires a parent context that is incompatible with the custom widget structure, using ARIA roles is appropriate. For example, `<option>` requires a `<select>` parent, so a custom listbox widget using `<li role="option">` elements is compliant:
+
+[source,javascript,diff-id=2,diff-type=compliant]
+----
+<ul role="listbox">
+  <li role="option">First item</li>
+  <li role="option">Second item</li>
+</ul>
+----
+
 == Resources
 === Documentation

Vibe Bot added 2 commits March 19, 2026 14:16
Tests cover the scenario where ARIA composite widget roles (listbox/option
and inner roles like button) are incorrectly flagged when used on generic
HTML elements within composite widget structures. The tests verify that:
- div-based listbox/option composite widgets are not flagged
- ul/li-based listbox/option widgets are not flagged (no div/span restriction)
- role="button" inside a grid composite widget is not flagged
- orphan option and standalone listbox without descendants remain flagged

An upstream sentinel block verifies the underlying jsx-a11y rule still
raises on these patterns, ensuring the decorator suppression remains valid.

Relates to JS-1464
The decorator's isCustomTableWidget function only handled table/grid
container roles restricted to div/span elements, missing listbox/option
composite widgets and inner roles (like button) nested inside composite
containers.

The fix renames isCustomTableWidget to isCustomCompositeWidget and:
- Adds listbox to COMPOSITE_CONTAINER_ROLES (alongside table, grid)
- Adds option to COMPOSITE_CHILD_ROLES (alongside row, gridcell, etc.)
- Adds COMPOSITE_INNER_ROLES (button, presentation, none) that are
  suppressed when nested inside any composite container
- Removes the div/span element-name restriction from all traversals,
  allowing ul/li and other HTML elements to form valid composite widgets

The rspec is updated with a compliant listbox/option example clarifying
that ARIA roles are appropriate when the semantic HTML alternative
requires an incompatible parent context.

Relates to JS-1464
@github-actions
Copy link
Contributor

Ruling Report

No changes to ruling expected issues in this PR

@sonarqube-next
Copy link

@sonar-nigel sonar-nigel bot marked this pull request as ready for review March 19, 2026 16:01
@sonar-nigel sonar-nigel bot requested a review from a team March 19, 2026 16:01
@sonar-review-alpha
Copy link

sonar-review-alpha bot commented Mar 19, 2026

Summary

This PR fixes false positives in S6819 by extending ARIA composite widget pattern detection beyond tables/grids to include listbox/option patterns and inner roles like button nested inside composite containers.

Key changes:

  • Renamed isCustomTableWidgetisCustomCompositeWidget and generalized logic to handle three role categories:
    • Container roles: table, grid, listbox (new)
    • Child roles: row, gridcell, etc., + option (new)
    • Inner roles: button, presentation, none (new set)
  • Removed element-name restriction (was: div/span only) → now accepts ul/li and any HTML elements
  • Patterns like <ul role="listbox"><li role="option"> are now correctly recognized as valid ARIA patterns instead of violations

The fix targets a real gap: <option> requires a <select> parent in HTML, so custom listbox widgets using ARIA roles on alternative elements are appropriate and should not be flagged.

What reviewers should know

Reading order:

  1. Start at the role constants (COMPOSITE_CONTAINER_ROLES, etc.) — these are the configuration that changed
  2. Review the main function isCustomCompositeWidget() (line ~240) — the core logic is similar to before, just applied to more role combinations
  3. Check the sentinel test block (lines 22-41) — this validates that the upstream ESLint rule still raises on these patterns. If this ever fails, the decorator can be removed
  4. Review the new test cases (bottom of unit.test.ts) to see what patterns are now correctly handled

Non-obvious points:

  • The sentinel test is a safety mechanism: it ensures the decorator doesn't become orphaned if the upstream prefer-tag-over-role rule changes
  • The logic for container vs. child vs. inner roles is consistent with ARIA widget patterns: containers must have children, children/inner must have containers
  • No changes to how roles are extracted from JSX or how ancestors are found — only which role combinations are recognized

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonar-review-alpha
Copy link

sonar-review-alpha bot commented Mar 19, 2026

Summary

This PR expands S6819's false-positive suppression from table-specific patterns (div/span with table/grid roles) to a broader range of ARIA composite widget patterns. Key changes: (1) adds listbox/option roles to enable custom listbox widgets (especially ul/li patterns where <option> requires a <select> parent); (2) removes the div/span element-name restriction, allowing any HTML element to participate in composite widgets; (3) introduces COMPOSITE_INNER_ROLES (button, presentation, none) that are suppressed when nested inside any composite container. All helper functions and role sets have been renamed from "table" terminology to "composite" to reflect the broader scope.

What reviewers should know

Start here: The core logic is in isCustomCompositeWidget (line 240) — it now handles three categories: container roles (must have descendant children), child roles, and inner roles (all must have an ancestor container). Key decision: The new COMPOSITE_INNER_ROLES set is intentionally checked against ANY ancestor composite container, not paired with specific containers. This treats button/presentation/none specially — they're valid in any composite context. Watch for: The sentinel test (lines 23–40 in unit.test.ts) is crucial; it verifies the upstream ESLint rule still flags these patterns. If that test fails, it means the upstream rule has been fixed and the decorator can be simplified. Test coverage: Lines 287–336 validate the three main scenarios: div-based listbox/option, ul/li listbox/option (the primary fix), and inner roles nested in composites. Edge cases (orphaned option, empty listbox) correctly remain violations.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonar-review-alpha
Copy link

sonar-review-alpha bot commented Mar 19, 2026

Summary

Extends S6819 composite widget detection to handle listbox/option patterns on any HTML elements and adds support for inner roles (button, presentation, none) nested inside composite containers.

The key change is removing the element-name restriction that previously limited detection to div/span. This matters because custom listbox widgets often use ul/li elements (since <option> requires a <select> parent), which were being incorrectly flagged.

Adds a clever upstream sentinel test that verifies the upstream prefer-tag-over-role rule still raises on these patterns — an early-warning system if upstream behavior changes.

What reviewers should know

Start here: Review the three role sets (COMPOSITE_CONTAINER_ROLES, COMPOSITE_CHILD_ROLES, COMPOSITE_INNER_ROLES) and how they're used in isCustomCompositeWidget — the logic is now simpler (no element-name branching).

Key decisions to verify:

  • Element-name restriction removal: The old getJSXElementName helper and div/span checks are gone. Any element with the right role now counts. Verify this doesn't over-suppress legitimate violations.
  • Inner roles category: button, presentation, none inside composite containers are now valid. This covers things like decorative elements or drop indicators inside table/grid cells.
  • Ancestor search is recursive but still finds the nearest container: Uses existing findFirstMatchingAncestor helper.

Testing approach: Two test blocks:

  1. Upstream sentinel (lines 23–36) — ensures decorator is still needed. If upstream ESLint rule ever changes, this will fail.
  2. New test cases covering the three compliant patterns and two true positives (orphaned option, listbox without children).

No breaking changes: Only suppresses additional false positives; doesn't change what's actually reported as violations.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@francois-mora-sonarsource
Copy link
Contributor

LGTM - fixes S6819 false positives for ARIA composite widget patterns (listbox/option on div and ul/li elements, and inner roles like button nested inside composite containers) by extending the decorator's role sets and removing the div/span element-name restriction — #6645

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