Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions packages/analysis/src/jsts/rules/S6819/decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const COMPOSITE_CHILD_ROLES = new Set([
* 1. Custom components (not standard HTML elements)
* 2. Valid ARIA patterns where semantic equivalents would lose functionality:
* - SVG with role="presentation"/"img" and aria-hidden="true" (decorative icons)
* - SVG with role="img" and a <title> child, aria-label, or aria-labelledby (semantic icons)
* - role="status" with aria-live (live region pattern)
* - role="slider" with complete aria-value* attributes
* - role="radio" with aria-checked
Expand Down Expand Up @@ -113,6 +114,7 @@ function isValidAriaPattern(node: TSESTree.JSXOpeningElement): boolean {

return (
isDecorativeSvg(elementName, role, attributes) ||
isSemanticSvgImg(elementName, role, attributes, node) ||
isLiveRegionStatus(role, attributes) ||
isCustomSlider(role, attributes) ||
isCustomRadio(role, attributes) ||
Expand All @@ -138,6 +140,52 @@ function isDecorativeSvg(
return ariaHiddenValue === true || ariaHiddenValue === 'true';
}

/**
* Checks if the element is a semantic SVG icon with role="img" and a proper accessible name.
*
* Inline SVG with role="img" is a WCAG-compliant pattern for icon components that need
* CSS class control, animation, or programmatic styling. It is not replaceable by <img>
* when the SVG provides an accessible name via <title>, aria-label, or aria-labelledby.
*/
function isSemanticSvgImg(
elementName: string | null,
role: string,
attributes: JSXOpeningElement['attributes'],
node: TSESTree.JSXOpeningElement,
): boolean {
if (elementName !== 'svg' || role !== 'img') {
return false;
}
const ariaLabelProp = getProp(attributes, 'aria-label');
if (ariaLabelProp && getLiteralPropValue(ariaLabelProp) !== '') {
return true;
}
const ariaLabelledbyProp = getProp(attributes, 'aria-labelledby');
if (ariaLabelledbyProp && getLiteralPropValue(ariaLabelledbyProp) !== '') {
Comment on lines +159 to +164
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.

getLiteralPropValue returns null for any non-literal prop value — dynamic expressions like aria-label={someVar}, aria-label={null}, or aria-label={undefined}. Because null !== '' is true, those cases all fall through and suppress the report.

This is intentionally more permissive than isDecorativeSvg, which uses === true || === 'true' (so a dynamic aria-hidden does not suppress). For aria-label, accepting unknowable dynamic values is a reasonable static-analysis trade-off — but aria-label={null} is a real pattern in conditional rendering (e.g. aria-label={active ? 'Close' : null}) and React silently drops the attribute at runtime, leaving the SVG with no accessible name.

Two things worth confirming:

  1. Is suppressing on any dynamic aria-label/aria-labelledby intentional, or should it require a string literal (mirroring how isDecorativeSvg handles aria-hidden)?
  2. If dynamic suppression is intentional, a test case documenting it (aria-label={someVar} in the valid block) would pin the behaviour and serve as an explicit notice that it differs from the decorative-SVG guard.
Suggested change
const ariaLabelProp = getProp(attributes, 'aria-label');
if (ariaLabelProp && getLiteralPropValue(ariaLabelProp) !== '') {
return true;
}
const ariaLabelledbyProp = getProp(attributes, 'aria-labelledby');
if (ariaLabelledbyProp && getLiteralPropValue(ariaLabelledbyProp) !== '') {
const ariaLabelProp = getProp(attributes, 'aria-label');
const ariaLabelValue = ariaLabelProp ? getLiteralPropValue(ariaLabelProp) : null;
if (typeof ariaLabelValue === 'string' && ariaLabelValue !== '') {
return true;
}
const ariaLabelledbyProp = getProp(attributes, 'aria-labelledby');
const ariaLabelledbyValue = ariaLabelledbyProp ? getLiteralPropValue(ariaLabelledbyProp) : null;
if (typeof ariaLabelledbyValue === 'string' && ariaLabelledbyValue !== '') {
return true;
}
  • Mark as noise

return true;
}
return hasTitleChild(node);
}

/**
* Checks if the JSX element has a direct <title> child element with non-empty content.
*/
function hasTitleChild(node: TSESTree.JSXOpeningElement): boolean {
const parent = node.parent;
if (parent?.type !== 'JSXElement') {
return false;
}
return parent.children.some(
child =>
child.type === 'JSXElement' &&
child.openingElement.name.type === 'JSXIdentifier' &&
child.openingElement.name.name === 'title' &&
child.children.some(
c => (c.type === 'JSXText' && c.value.trim() !== '') || c.type === 'JSXExpressionContainer',
),
);
Comment thread
sonar-review-alpha[bot] marked this conversation as resolved.
}

function isLiveRegionStatus(role: string, attributes: JSXOpeningElement['attributes']): boolean {
return role === 'status' && Boolean(getProp(attributes, 'aria-live'));
}
Expand Down
57 changes: 57 additions & 0 deletions packages/analysis/src/jsts/rules/S6819/unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,24 @@ describe('S6819 upstream sentinel', () => {
],
});
});

it('upstream prefer-tag-over-role raises on svg role="img" with accessible labels that decorator suppresses', () => {
const ruleTester = new NoTypeCheckingRuleTester();
ruleTester.run('prefer-tag-over-role', upstreamRule, {
valid: [],
invalid: [
// svg role="img" with aria-label — suppressed by decorator, raised by upstream
{ code: `<svg role="img" aria-label="Arrow Down"><path d="M12 4v16"/></svg>`, errors: 1 },
// svg role="img" with aria-labelledby — suppressed by decorator, raised by upstream
{
code: `<svg role="img" aria-labelledby="icon-title"><title id="icon-title">Icon</title></svg>`,
errors: 1,
},
// svg role="img" with <title> child — suppressed by decorator, raised by upstream
{ code: `<svg role="img"><title>Settings</title><path d="M10 10"/></svg>`, errors: 1 },
],
});
});
});

describe('S6819', () => {
Expand Down Expand Up @@ -283,6 +301,45 @@ describe('S6819', () => {
});
});

// JS-1465: inline SVG with role="img" and proper accessible name
it('should not flag SVG with role="img" when it has an accessible name', () => {
const ruleTester = new NoTypeCheckingRuleTester();

ruleTester.run('prefer-tag-over-role - semantic svg img', rule, {
valid: [
// svg role="img" with <title> child — WCAG-recommended inline SVG icon pattern
{
code: `<svg role="img" viewBox="0 0 24 24"><title>Arrow Down</title><path d="M12 4v16"/></svg>`,
},
// svg role="img" with aria-label
{
code: `<svg role="img" aria-label="Settings" viewBox="0 0 24 24"><path d="M10 10"/></svg>`,
},
// svg role="img" with aria-labelledby referencing a <title> child
{
code: `<svg role="img" aria-labelledby="brand-title"><title id="brand-title">Brand Name</title></svg>`,
},
],
invalid: [
// svg role="img" without an accessible name is still a true positive
{
code: `<svg role="img" viewBox="0 0 24 24"><path d="M5 12h14"/></svg>`,
errors: 1,
},
// svg role="img" with an explicit empty aria-label provides no accessible name
{
code: `<svg role="img" aria-label=""><path d="M5 12h14"/></svg>`,
errors: 1,
},
// svg role="img" with an empty <title> provides no accessible name
{
code: `<svg role="img"><title></title><path d="M5 12h14"/></svg>`,
errors: 1,
},
],
});
});

// JS-1464: listbox/option composite widgets
it('should not flag ARIA listbox/option composite widget roles', () => {
Comment thread
sonar-review-alpha[bot] marked this conversation as resolved.
const ruleTester = new NoTypeCheckingRuleTester();
Expand Down
Loading