feat(html/a11y): port noAriaUnsupportedElements to HTML#9491
feat(html/a11y): port noAriaUnsupportedElements to HTML#9491ematipico merged 5 commits intobiomejs:nextfrom
Conversation
…dElements HTML rule Add missing #[derive(Debug)] on AttributeKind and RuleState, reorder struct before impl block, and add test cases for multiple aria attrs on one element and invalid aria-* attributes.
🦋 Changeset detectedLatest commit: 18bb0fd 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
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a changeset and a new HTML accessibility lint rule 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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_html_analyze/src/lint/a11y/no_aria_unsupported_elements.rs (1)
68-71: Consider storing the attribute reference inRuleState.Storing only
AttributeKindmeansactionmust re-search for the offending attribute. Storing a reference or identifier of the actual attribute would eliminate the need to duplicate the search logic and prevent the inconsistency noted above.pub struct RuleState { attribute_kind: AttributeKind, attribute: AnyHtmlAttribute, // or appropriate type }🤖 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_aria_unsupported_elements.rs` around lines 68 - 71, The RuleState currently only stores attribute_kind which forces the action code to re-search for the offending attribute; add a field to RuleState (e.g., attribute or attribute_id / AnyHtmlAttribute-like identifier) and populate it where RuleState is constructed so consumers like the action closure can use the stored attribute directly instead of re-querying; update all places that build RuleState (constructors/factory call sites) and adjust action logic to reference RuleState.attribute (or attribute_id) and remove the duplicated search, taking care to satisfy any required lifetimes or type parameters for the attribute type.
🤖 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_aria_unsupported_elements.rs`:
- Around line 133-145: The action function currently matches attributes by name
using starts_with("aria-")/role which can pick up invalid aria-* names; change
the attribute selection in action (in fn action) to mirror run's validation: for
each attribute, if attribute.as_html_attribute() yields a name, parse it with
AriaAttribute::from_str() and only treat it as an ARIA attribute when the parser
returns Some(valid_attribute) (or allow the special-case "role" as run does);
this ensures action skips invalid aria-* names like aria-foo just as run does.
---
Nitpick comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_aria_unsupported_elements.rs`:
- Around line 68-71: The RuleState currently only stores attribute_kind which
forces the action code to re-search for the offending attribute; add a field to
RuleState (e.g., attribute or attribute_id / AnyHtmlAttribute-like identifier)
and populate it where RuleState is constructed so consumers like the action
closure can use the stored attribute directly instead of re-querying; update all
places that build RuleState (constructors/factory call sites) and adjust action
logic to reference RuleState.attribute (or attribute_id) and remove the
duplicated search, taking care to satisfy any required lifetimes or type
parameters for the attribute type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31532de7-92e5-4b0e-81c8-1c12f2bebd28
⛔ Files ignored due to path filters (2)
crates/biome_html_analyze/tests/specs/a11y/noAriaUnsupportedElements/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noAriaUnsupportedElements/valid.html.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/html-no-aria-unsupported-elements.mdcrates/biome_html_analyze/src/lint/a11y/no_aria_unsupported_elements.rscrates/biome_html_analyze/tests/specs/a11y/noAriaUnsupportedElements/invalid.htmlcrates/biome_html_analyze/tests/specs/a11y/noAriaUnsupportedElements/valid.html
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
The rule lacks in tests. We should test case sensitivity and components for Vue/Svelte/Astro
The `action` function used only `starts_with("aria-")` to find
attributes to remove, while `run` also checked `AriaAttribute::from_str()`
to ensure the attribute is a valid ARIA attribute. This inconsistency
meant that for an element like `<meta aria-foo="bar" aria-hidden="true">`,
`action` would remove `aria-foo` (invalid, not flagged by `run`) instead
of `aria-hidden` (valid, actually flagged).
Add the same `AriaAttribute::from_str()` validation to `action` and add
a test case covering this scenario.
…/Astro tests Fix case sensitivity for uppercase aria attributes like ARIA-LABEL by lowercasing attribute names before prefix and AriaAttribute::from_str checks. Add test coverage for uppercase element names (META, STYLE, SCRIPT) and uppercase attribute names (ROLE, ARIA-LABEL). Add test specs for Vue, Svelte, and Astro file formats.
| let attribute_name_text = attribute_name.text_trimmed(); | ||
| let attribute_name_lower = attribute_name_text.to_lowercase(); |
There was a problem hiding this comment.
These allocate memory, use TokenText instead
| let attribute_name_text = attribute_name.text_trimmed(); | ||
|
|
||
| let attribute_name_lower = attribute_name_text.to_lowercase(); |
There was a problem hiding this comment.
Same thing here it allocates memory
…non-HTML files Use token_text_trimmed() and to_ascii_lowercase_cow() instead of text_trimmed() and to_lowercase() to avoid unnecessary memory allocations. Check HtmlFileSource to match element names case-insensitively in HTML files but case-sensitively in Vue/Svelte/Astro, so PascalCase components like <Meta> are not flagged in framework files. Add PascalCase component tests to Vue/Svelte/Astro valid specs.
IxxyDev
force-pushed
the
feat/html-no-aria-unsupported-elements
branch
from
119172e to
18bb0fd
Compare
Merging this PR will degrade performance by 27.26%
Performance Changes
Comparing 
Footnotes
|
3 participants
Summary
Ported the
noAriaUnsupportedElementslint rule from JSX to HTML.roleandaria-*attributes on HTML elements that do not support ARIA:<meta>,<html>,<script>, and<style>.aria-*attributes.Test Plan
cargo test -p biome_html_analyze -- no_aria_unsupported_elements— 2 spec tests pass (valid + invalid HTML)roleandaria-*attributesrole/aria-*on supported elements (<div>,<input>) are not flaggedroleandaria-hidden— only first attribute is reported (consistent with JSX version)aria-fooattribute on<meta>— not flagged (consistent with JSX version)Docs
Documentation is inline in the rule's rustdoc.