feat(biome_html_analyze): port useHeadingContent a11y rule to HTML#9716
feat(biome_html_analyze): port useHeadingContent a11y rule to HTML#9716dyc3 merged 7 commits intobiomejs:nextfrom
Conversation
Port the useHeadingContent accessibility rule from JSX to HTML. The rule enforces that heading elements (h1-h6) have content accessible to screen readers. Checks: - Empty headings (no text content) - Headings with aria-hidden (hidden from screen readers) - Headings with only aria-hidden children - Self-closing headings (no content possible) - Whitespace-only headings Allows: - Headings with text content - Headings with aria-label, aria-labelledby, or title - Headings with nested visible content - Headings with img alt text Handles case-insensitive matching for .html files and case-sensitive matching for Vue/Svelte/Astro components. Part of biomejs#8155
🦋 Changeset detectedLatest commit: 5868af7 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 |
|
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:
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 (1)
WalkthroughA 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 |
Merging this PR will degrade performance by 35.01%
Performance Changes
Comparing Footnotes
|
Add test coverage for component-based frameworks to verify: - Case-sensitive element name matching (lowercase h1-h6 only) - PascalCase components (H1, MyHeading) are correctly ignored - Invalid cases (empty headings, aria-hidden) produce diagnostics Adds 6 test files across vue/, svelte/, and astro/ subdirectories.
|
added test files for non- let me know if you need anything else 😄 |
| @@ -0,0 +1,17 @@ | |||
| <template> | |||
| <!-- should not generate diagnostics --> | |||
There was a problem hiding this comment.
The magic comments need to go at the very top of the file
| version: "next", | ||
| name: "useHeadingContent", | ||
| language: "html", | ||
| sources: &[RuleSource::EslintJsxA11y("heading-has-content").same()], |
There was a problem hiding this comment.
does https://html-eslint.org/ have a similar rule? we should add it as a rule source
- Move magic comments above <template> in Vue test files (valid.vue, invalid.vue) - Add @html-eslint/no-empty-headings as rule source for useHeadingContent - Add eslint migration mapping for @html-eslint/no-empty-headings - Update Vue test snapshots
|
Ah! I found the culprit. It was the html-eslint rule is called no-empty-headings. Added it as a rule source along with the eslint migration mapping.
Reference: https://html-eslint.org/docs/rules/no-empty-headings |
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/use_heading_content.rs`:
- Around line 156-161: The branch for AnyHtmlElement::HtmlElement only checks
children and misses paired (non-self-closing) custom components; update the
logic in the AnyHtmlElement::HtmlElement arm of has_accessible_content to mirror
the PascalCase and <Image> handling used in the self-closing branch so that
paired components (e.g., <CustomHeading></CustomHeading>) are treated as
potentially accessible. Specifically, in the AnyHtmlElement::HtmlElement case
within has_accessible_content, detect framework/custom component tags
(PascalCase or known component names like Image) before recursing into
element.children() and return true (or treat as accessible) the same way the
self-closing arm does. Apply the same change to the analogous block referenced
around lines 174-183 to ensure consistent handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e30fe9c-000d-4404-a490-e505cdd77288
⛔ Files ignored due to path filters (3)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/invalid.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/valid.vue.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_html_analyze/src/lint/a11y/use_heading_content.rscrates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/invalid.vuecrates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/valid.vue
✅ Files skipped from review due to trivial changes (2)
- crates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/valid.vue
- crates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/invalid.vue
<h1><MyComponent></MyComponent></h1> was falsely flagged as invalid because the HtmlElement arm only recursed into children without first checking for PascalCase custom components. The self-closing arm already handled this correctly. Mirror the PascalCase guard from the HtmlSelfClosingElement arm into the HtmlElement arm so both behave consistently. Fixes inconsistency reported by coderabbitai.
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/use_heading_content.rs`:
- Around line 121-123: The helper has_accessible_content currently receives only
is_astro and therefore applies PascalCase shortcuts and "img" matching without
source-type context; change the call sites to thread an is_html boolean (e.g.,
compute is_html = source_type.is_html()) and pass that into
has_accessible_content, then update has_accessible_content to use is_html to: 1)
only apply the PascalCase/component-name shortcut when NOT is_html, and 2) only
treat "img" case-insensitively (eq_ignore_ascii_case) when is_html (otherwise
use case-sensitive comparison); do the same source-type-aware fixes for the
other element-name checks referenced around the blocks at 153-173 and 183-193.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 21803ab4-7da4-418b-8147-a853ea145bf6
⛔ Files ignored due to path filters (1)
crates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/valid.vue.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_html_analyze/src/lint/a11y/use_heading_content.rscrates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/valid.vue
✅ Files skipped from review due to trivial changes (1)
- crates/biome_html_analyze/tests/specs/a11y/useHeadingContent/vue/valid.vue
…g on source type
In plain HTML files, all tag names are case-insensitive, so PascalCase has no
special meaning — a <Span> is just <span>, not a custom component. The
PascalCase 'treat as accessible component' shortcut must only fire for
component-based frameworks (Vue, Svelte, Astro).
For the native img check: HTML is case-insensitive so eq_ignore_ascii_case is
correct there, but in component files only lowercase 'img' is the native
element. Previously, <Img /> in Vue would match eq_ignore_ascii_case('img')
before reaching the PascalCase guard, incorrectly requiring an alt attribute.
Thread is_html through has_accessible_content and gate both shortcuts on it.
AnyHtmlElement covers text nodes, expressions, CDATA sections, and bogus elements in addition to actual tags. This caused the rule to fire for every content node in the document, producing a ~35% performance regression on large HTML files. AnyHtmlTagElement = HtmlOpeningElement | HtmlSelfClosingElement limits the query to actual tag nodes only, which is the correct scope for a heading rule. For HtmlOpeningElement nodes, the rule now navigates to the parent HtmlElement via syntax().parent() to access children and compute accessible content. Addresses: dyc3 review feedback on PR biomejs#9716
Description
Port the
useHeadingContentaccessibility rule from JSX to HTML, as part of #8155.The rule enforces that heading elements (
h1-h6) have content accessible to screen readers in HTML, Vue, Svelte, and Astro files.Checks
Flags as invalid:
<h1></h1>)<h2> </h2>)aria-hidden(hidden from screen readers entirely)aria-hiddenchildren<h1 />)aria-hiddenandaria-label(aria-hidden takes priority)Allows as valid:
aria-label,aria-labelledby, ortitle(accessible name)<img>withalttextImplementation notes
.htmlfiles (<H1>matches)get_truthy_aria_hidden_attribute,has_accessible_name, etc.)useAnchorContentTest plan
invalid.html: 10 test cases covering all invalid patternsvalid.html: 9 test cases covering all valid patterns