feat(biome_html_analyze): port noRedundantRoles a11y rule to HTML#9564
feat(biome_html_analyze): port noRedundantRoles a11y rule to HTML#9564dyc3 merged 10 commits intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 0d5579c The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Linter
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new HTML accessibility lint rule Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/invalid.html (1)
1-25: Consider adding test coverage for attribute-dependent implicit roles.The PR objectives mention handling
<input type="...">,<th scope="...">,<a href="...">,<img alt="...">,<section>with accessible name, and<select>with size/multiple. However, the invalid test cases don't exercise these attribute-dependent scenarios.Consider adding cases such as:
<input type="checkbox" role="checkbox"><input type="radio" role="radio"><th scope="col" role="columnheader"><select role="combobox"><section aria-label="Section" role="region">Also note: Line 6
<form role="form">may be a false positive—per the WAI-ARIA spec,<form>only has an implicit role offormwhen it has an accessible name. Without one, it has no implicit role.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/invalid.html` around lines 1 - 25, Add attribute-dependent invalid cases and fix the form false positive: add tests exercising implicit roles that depend on attributes such as an <input type="checkbox" role="checkbox"> and <input type="radio" role="radio">, a table header like <th scope="col" role="columnheader">, a <select role="combobox"> (including size/multiple variations), and a landmark <section aria-label="Section" role="region"> to ensure attribute-driven implicit roles are covered; also update the existing <form role="form"> test to either supply an accessible name (e.g., aria-label) so it legitimately has an implicit form role or remove/mark it as valid since a bare <form> without an accessible name does not have an implicit role per the WAI-ARIA spec.crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs (1)
85-86: Minor: redundant.to_string()calls.Both
role_valueandelement_name_strare alreadyStringtypes (created via.to_lowercase()), so.to_string()is unnecessary here.✨ Suggested fix
if explicit_role == implicit_role { return Some(RuleState { redundant_attribute: role_attribute, - role_value: role_value.to_string(), - element_name: element_name_str.to_string(), + role_value, + element_name: element_name_str, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs` around lines 85 - 86, The two fields role_value and element_name in the struct construction are being assigned using redundant .to_string() calls even though role_value and element_name_str are already String (from .to_lowercase()); remove the unnecessary .to_string() calls and assign role_value and element_name_str directly (e.g., role_value: role_value, element_name: element_name_str) in the code inside no_redundant_roles.rs to eliminate the extra allocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs`:
- Around line 85-86: The two fields role_value and element_name in the struct
construction are being assigned using redundant .to_string() calls even though
role_value and element_name_str are already String (from .to_lowercase());
remove the unnecessary .to_string() calls and assign role_value and
element_name_str directly (e.g., role_value: role_value, element_name:
element_name_str) in the code inside no_redundant_roles.rs to eliminate the
extra allocations.
In `@crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/invalid.html`:
- Around line 1-25: Add attribute-dependent invalid cases and fix the form false
positive: add tests exercising implicit roles that depend on attributes such as
an <input type="checkbox" role="checkbox"> and <input type="radio"
role="radio">, a table header like <th scope="col" role="columnheader">, a
<select role="combobox"> (including size/multiple variations), and a landmark
<section aria-label="Section" role="region"> to ensure attribute-driven implicit
roles are covered; also update the existing <form role="form"> test to either
supply an accessible name (e.g., aria-label) so it legitimately has an implicit
form role or remove/mark it as valid since a bare <form> without an accessible
name does not have an implicit role per the WAI-ARIA spec.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 410e8733-01e7-4e68-b88e-4dee5b3fb8fe
📒 Files selected for processing (3)
crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rscrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/invalid.htmlcrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/valid.html
crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/invalid.html
Show resolved
Hide resolved
|
Thanks for the thorough review. Pushed fixes for everything:
One thing I couldn't do cleanly: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs`:
- Around line 181-214: The matching for HTML enumerated attributes in the "th"
and "input" branches is case-sensitive, so values like TYPE="CHECKBOX" or
SCoPe="COL" are missed; before matching use a normalized ASCII-case-insensitive
value (e.g., call to_ascii_lowercase or equivalent) for the results of
get_attribute_value for "type" and "scope" and then match on that normalized
string when producing AriaRole variants in the th and input match arms (retain
the existing list of cases and the list-check via
node.find_attribute_by_name("list")); also add mixed-case fixtures for at least
one input type branch and one th scope branch to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8815a7e8-8501-42f1-8e28-ab8b18b58aa8
📒 Files selected for processing (10)
.changeset/port-no-redundant-roles-html.mdcrates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rscrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/astro/invalid.astrocrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/astro/valid.astrocrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/invalid.htmlcrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/svelte/invalid.sveltecrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/svelte/valid.sveltecrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/valid.htmlcrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/vue/invalid.vuecrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/vue/valid.vue
✅ Files skipped from review due to trivial changes (7)
- .changeset/port-no-redundant-roles-html.md
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/astro/invalid.astro
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/vue/valid.vue
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/vue/invalid.vue
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/svelte/valid.svelte
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/svelte/invalid.svelte
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/astro/valid.astro
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
You can dereference TokenText or call .text() on it to get a &str. You can also easily get a &str from a Cow string.
We really shouldn't allocate String unless its truly unavoidable, and I'm pretty confident its avoidable here.
crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/svelte/valid.svelte
Outdated
Show resolved
Hide resolved
|
Pushed the fixes. Main changes:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs (1)
130-263: Consider reusingbiome_aria::AriaRoles::get_implicit_role.This helper duplicates much of the logic in
crates/biome_aria/src/roles.rs. While the current implementation is correct and self-contained, sharing code would reduce maintenance burden and ensure consistency as the spec evolves.Notable differences from
biome_aria:
- Missing
menuitemhandling (checkbox/radio variants)- Adds
address,details,fieldsetto Group (which biome_aria also has, so that's fine)If reuse isn't straightforward due to the
Elementtrait abstraction, this can be deferred.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs` around lines 130 - 263, The get_implicit_role_for_element function duplicates ARIA role logic; replace its long match with a single call to the shared implementation (biome_aria::AriaRoles::get_implicit_role) to avoid drift: update get_implicit_role_for_element to adapt the AnyHtmlElement/Element-trait access to whatever API biome_aria expects (convert node attributes/name as needed) and return the Option<AriaRole> from AriaRoles::get_implicit_role, ensuring you also account for menuitem checkbox/radio variants if biome_aria exposes them (or map them afterward) so behavior remains equivalent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs`:
- Around line 130-263: The get_implicit_role_for_element function duplicates
ARIA role logic; replace its long match with a single call to the shared
implementation (biome_aria::AriaRoles::get_implicit_role) to avoid drift: update
get_implicit_role_for_element to adapt the AnyHtmlElement/Element-trait access
to whatever API biome_aria expects (convert node attributes/name as needed) and
return the Option<AriaRole> from AriaRoles::get_implicit_role, ensuring you also
account for menuitem checkbox/radio variants if biome_aria exposes them (or map
them afterward) so behavior remains equivalent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f2ae581-e287-48ea-abfb-5e630c847281
📒 Files selected for processing (5)
.changeset/port-no-redundant-roles-html.mdcrates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rscrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/astro/valid.astrocrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/svelte/valid.sveltecrates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/vue/valid.vue
✅ Files skipped from review due to trivial changes (4)
- .changeset/port-no-redundant-roles-html.md
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/astro/valid.astro
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/svelte/valid.svelte
- crates/biome_html_analyze/tests/specs/a11y/noRedundantRoles/vue/valid.vue
|
Token text is an owned type. You can just pass it in the state.
I'm pretty sure it doesn't do that |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs`:
- Around line 325-341: The mapping currently returns AriaRole::Generic for
"header" and "footer" unconditionally; update the implicit-role logic in
no_redundant_roles (the code block that checks element_name and returns
AriaRole::Generic) to inspect ancestor elements: if element_name is "header" and
no ancestor is one of "article", "aside", "main", "nav", or "section" return
AriaRole::Banner; if element_name is "footer" and no such ancestor exists return
AriaRole::ContentInfo; otherwise fall back to AriaRole::Generic. Implement
ancestor traversal using the document/element context available in the same
function (or helper) so the header/footer cases are context-sensitive.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b816a423-02fc-4365-8249-9aedef52689d
📒 Files selected for processing (1)
crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs
| } else if element_name.eq_ignore_ascii_case("b") | ||
| || element_name.eq_ignore_ascii_case("bdi") | ||
| || element_name.eq_ignore_ascii_case("bdo") | ||
| || element_name.eq_ignore_ascii_case("body") | ||
| || element_name.eq_ignore_ascii_case("data") | ||
| || element_name.eq_ignore_ascii_case("div") | ||
| || element_name.eq_ignore_ascii_case("i") | ||
| || element_name.eq_ignore_ascii_case("q") | ||
| || element_name.eq_ignore_ascii_case("samp") | ||
| || element_name.eq_ignore_ascii_case("small") | ||
| || element_name.eq_ignore_ascii_case("span") | ||
| || element_name.eq_ignore_ascii_case("u") | ||
| || element_name.eq_ignore_ascii_case("pre") | ||
| || element_name.eq_ignore_ascii_case("header") | ||
| || element_name.eq_ignore_ascii_case("footer") | ||
| { | ||
| AriaRole::Generic |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's existing infrastructure for ancestor traversal in HTML rules
rg -n "parent|ancestor" --type rust crates/biome_html_analyze/src/lint/Repository: biomejs/biome
Length of output: 4008
🏁 Script executed:
# First, let's examine the full no_redundant_roles.rs file to understand context handling
wc -l crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rsRepository: biomejs/biome
Length of output: 122
🏁 Script executed:
# Check if there are comments about header/footer limitations in the file
rg -n "header|footer" crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs -A 2 -B 2Repository: biomejs/biome
Length of output: 658
🏁 Script executed:
# Look for the JSX equivalent rule to check if it documents this limitation
fd -t f "no_redundant_roles" crates/biome_js_analyze/Repository: biomejs/biome
Length of output: 116
🌐 Web query:
WAI-ARIA header footer implicit roles context dependent banner contentinfo
💡 Result:
In WAI-ARIA, the HTML header and footer elements have implicit landmark roles that are context-dependent, specifically when used as direct descendants of the body element. The header element implies the banner role when it is in the context of a body element (i.e., a top-level page header containing site-oriented content like logos or global navigation). Similarly, the footer element implies the contentinfo role when it is in the context of a body element (i.e., a page footer with copyrights, privacy links, etc.). These implicit roles do not apply if header or footer are nested inside other elements like section, article, nav, main, or aside. In such cases, they do not convey landmark semantics and require explicit role attributes if needed. banner and contentinfo are landmark roles limited to one per page at the top level (unless nested in document/application iframes), and they help assistive technologies navigate page structure.
Citations:
- 1: https://www.w3.org/WAI/ARIA/apg/practices/landmark-regions/
- 2: https://www.w3.org/WAI/ARIA/apg/patterns/landmarks/examples/contentinfo.html
- 3: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Contentinfo_role
- 4: https://www.w3.org/TR/wai-aria/states_and_properties
- 5: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/landmark_role
- 6: https://www.w3.org/WAI/tutorials/page-structure/sections/
🏁 Script executed:
# Read the no_redundant_roles.rs file to understand full context
head -100 crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rsRepository: biomejs/biome
Length of output: 2904
🏁 Script executed:
# Read around the header/footer section to see if there are any comments
sed -n '200,352p' crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rsRepository: biomejs/biome
Length of output: 6821
🏁 Script executed:
# Check the JSX rule for comparison
head -150 crates/biome_js_analyze/src/lint/a11y/no_redundant_roles.rsRepository: biomejs/biome
Length of output: 3599
🏁 Script executed:
# Check if the JSX aria_roles service handles context for header/footer
rg -n "get_implicit_role|header|footer" crates/biome_js_analyze/src/services/aria/ -A 3 -B 1Repository: biomejs/biome
Length of output: 139
🏁 Script executed:
# Find where the aria service is located
find crates -type d -name "aria" 2>/dev/nullRepository: biomejs/biome
Length of output: 39
🏁 Script executed:
# Search for get_implicit_role implementation
rg -n "get_implicit_role" --type rust crates/Repository: biomejs/biome
Length of output: 1033
🏁 Script executed:
# Check the biome_aria implementation of get_implicit_role
cat -n crates/biome_aria/src/roles.rs | head -50Repository: biomejs/biome
Length of output: 2440
🏁 Script executed:
# Read more of the roles.rs to understand get_implicit_role implementation
sed -n '1,100p' crates/biome_aria/src/roles.rsRepository: biomejs/biome
Length of output: 4312
🏁 Script executed:
# Search for header/footer in the aria metadata
rg -n "header|footer" crates/biome_aria/src/roles.rs -A 2 -B 2Repository: biomejs/biome
Length of output: 1850
Context-dependent roles for <header> and <footer> are not handled.
Per WAI-ARIA, <header> has implicit role banner (and <footer> has contentinfo) when not nested in <article>, <aside>, <main>, <nav>, or <section>. The current implementation unconditionally returns Generic for both. This is a documented limitation in the aria metadata layer: ancestor traversal would be needed to determine the correct implicit role.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs` around lines
325 - 341, The mapping currently returns AriaRole::Generic for "header" and
"footer" unconditionally; update the implicit-role logic in no_redundant_roles
(the code block that checks element_name and returns AriaRole::Generic) to
inspect ancestor elements: if element_name is "header" and no ancestor is one of
"article", "aside", "main", "nav", or "section" return AriaRole::Banner; if
element_name is "footer" and no such ancestor exists return
AriaRole::ContentInfo; otherwise fall back to AriaRole::Generic. Implement
ancestor traversal using the document/element context available in the same
function (or helper) so the header/footer cases are context-sensitive.
|
there's still no |
Merging this PR will degrade performance by 24.27%
Performance Changes
Comparing 
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs (1)
251-274:⚠️ Potential issue | 🟡 Minor
scopeandtypestill need ASCII-case normalisation.Lines 251-274 compare raw attribute text, so
scope="COL"andtype="CHECKBOX"still dodge the rule. Please normalise these enumerated values before matching, and add one mixed-case fixture per branch so it stays fixed this time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs` around lines 251 - 274, The attribute comparisons for scope and type use raw attribute text so mixed-case values slip through; update get_attribute_value calls in the branch that determines AriaRole to normalize ASCII case (e.g., map the Option<String> to to_ascii_lowercase) before doing match on scope_value.as_deref() and type_value.as_deref(), keeping the existing match arms (Some("col"), Some("checkbox"), etc.), and also add one test fixture per branch using a mixed-case attribute (e.g., scope="CoL", type="ChEcKbOx") to ensure the rule catches case variations; refer to get_attribute_value, scope, type, AriaRole, and node.find_attribute_by_name to locate the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs`:
- Around line 283-290: The element-to-role mapping for anchors misses the "link"
element, causing <link href role="link"> to be treated as Generic; update the
conditional in the mapper that checks element_name (the branch that currently
tests "a" and "area") to also accept "link" (e.g., include
element_name.eq_ignore_ascii_case("link")) so that when
node.find_attribute_by_name("href").is_some() the function returns
AriaRole::Link; alternatively refactor this mapping to call the shared mapper in
crates::biome_aria::roles.rs to avoid future drift.
- Around line 77-85: The PascalCase early-return should only run for framework
template files, not plain HTML; update the check around
element_name.text().as_bytes().first().is_some_and(u8::is_ascii_uppercase) to
first verify the file is a framework template via ctx.file_path() (e.g., ends
with .vue, .svelte, .astro) and only then skip by returning None. Locate the
PascalCase check in no_redundant_roles (the block using element_name and the
early return) and wrap it with a guard that inspects ctx.file_path() for those
template extensions before applying the uppercase-first-char heuristic.
---
Duplicate comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs`:
- Around line 251-274: The attribute comparisons for scope and type use raw
attribute text so mixed-case values slip through; update get_attribute_value
calls in the branch that determines AriaRole to normalize ASCII case (e.g., map
the Option<String> to to_ascii_lowercase) before doing match on
scope_value.as_deref() and type_value.as_deref(), keeping the existing match
arms (Some("col"), Some("checkbox"), etc.), and also add one test fixture per
branch using a mixed-case attribute (e.g., scope="CoL", type="ChEcKbOx") to
ensure the rule catches case variations; refer to get_attribute_value, scope,
type, AriaRole, and node.find_attribute_by_name to locate the code to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 63ca808c-87f1-4b34-8c60-6f6a61b375db
⛔ Files ignored due to path filters (1)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**
📒 Files selected for processing (1)
crates/biome_html_analyze/src/lint/a11y/no_redundant_roles.rs
|
|
Addressed the remaining feedback:
All 8 tests pass. |
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
note the CI failures, and please address the codspeed perf regression
|
Fixed both CI issues:
Net result: 75 lines added, 156 removed. Match table is back, allocations are minimized, and the disallowed method is gone. |
dyc3
left a comment
dyc3
left a comment
There was a problem hiding this comment.
I believe the current benchmark is low signal because its so small. Looks good.
|
@ctonneslan Its pretty obvious you've been using ai (which you should have disclosed, per our contributing guidelines and pr template, please do so in the future), but just curious, what model were you using? |
dyc3
force-pushed
the
port-no-redundant-roles-html-v2
branch
from
2ecdd0b to
0bcd5fe
Compare
|
Opus 4.6? |
Port the noRedundantRoles lint rule from JSX to HTML, as part of the umbrella issue biomejs#8155. The rule flags explicit role attributes that match the element's implicit ARIA role (e.g. <button role="button">) and provides an unsafe fix to remove the redundant attribute. Includes implicit role resolution for all standard HTML elements, including attribute-dependent cases (input type, th scope, a href, img alt, section with accessible name, select size/multiple). Ref: biomejs#8155
- Add minor changeset
- Add HtmlEslint('no-redundant-role') as second rule source
- Skip component elements (uppercase) in Vue/Svelte/Astro files
- Store TextRange instead of HtmlAttribute in RuleState
- Remove unnecessary String allocations in RuleState
- Add magic comments to test files
- Add Vue, Svelte, and Astro test files
- Prefix unused state param in action
- Work with &str from TokenText directly instead of calling .to_lowercase() (HTML tag names are already lowercase from parser) - Only allocate String in RuleState for the diagnostic message - Use past tense and add docs link in changeset - Add <Button role="button"> as valid test case in Vue/Svelte/Astro to verify component elements are correctly skipped
- Store TokenText (element name) and Text (role value) directly in RuleState instead of allocating Strings. Both are owned types. - Use eq_ignore_ascii_case for tag name matching since the HTML parser preserves original casing from source (doesn't lowercase). - Switched from match arms to if-else chain since eq_ignore_ascii_case can't be used in match patterns.
- Removed the PascalCase early return. Components like <MyComponent> naturally return None from get_implicit_role_for_element since they don't match any tag name. The old skip would have incorrectly bypassed <BUTTON role="button"> in plain HTML files. - Added 'link' to the a/area implicit role branch to match the shared mapper in biome_aria/src/roles.rs.
… skip - Generate and accept insta snapshots for all test files (HTML, Vue, Svelte, Astro) - Normalize input type and th scope attribute values with to_ascii_lowercase per HTML spec (enumerated attributes are ASCII case-insensitive) - Gate the PascalCase component skip to non-HTML files using ctx.source_type::<HtmlFileSource>().is_html() so <BUTTON> in plain HTML still gets matched while <Button> in Vue/Svelte/Astro is correctly skipped
- Replace disallowed to_ascii_lowercase() with to_ascii_lowercase_cow() from biome_string_case - Lowercase the element name once in run() with to_ascii_lowercase_cow, then pass the Cow<str> to get_implicit_role_for_element which uses a match table - This restores the match-based dispatch (faster than the if-else chain) while still handling case-insensitive HTML tag names - Also lowercase input type and th scope values with the same cow-based approach
dyc3
force-pushed
the
port-no-redundant-roles-html-v2
branch
from
2de8e67 to
0d5579c
Compare
2 participants
Summary
Ports the
noRedundantRolesaccessibility lint rule from JSX to HTML, as part of #8155.The rule flags explicit
roleattributes that match the element's implicit ARIA role (e.g.<button role="button">) and provides an unsafe fix action to remove the redundant attribute.Implementation
Ast<AnyHtmlElement>(no special services needed)get_implicit_role_for_elementhelper that maps HTML tag names to their implicit ARIA roles per the WAI-ARIA spec<input type="...">— checkbox, radio, button, textbox, etc.<th scope="col|row">— columnheader vs rowheader<a href="...">— link vs generic<img alt="...">— img vs presentation<section>with accessible name — region vs generic<select>with size/multiple — combobox vs listboxTest plan
invalid.html— 25 cases covering all common elements with redundant rolesvalid.html— elements with different roles, no roles, and edge cases (a without href, img with empty alt)Changeset
minor
Ref: #8155