feat(html): port noLabelWithoutControl to HTML#9921
Conversation
Adds an HTML visitor for the noLabelWithoutControl a11y rule so it fires on <label> elements in .html, .vue, .svelte, and .astro files. Follows the same two-condition check as the JSX version: the label must have accessible text content (text, aria-label, aria-labelledby, or nested img alt) and an associated control (for attribute or nested input/select/ textarea/button/meter/output/progress). Element names are matched case-insensitively in .html files and case-sensitively in component framework files, per established pattern. Co-Authored-By: Calvin Magezi <calvinmagezi@outlook.com>
🦋 Changeset detectedLatest commit: c629daa 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 the HTML variant of the 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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/biome_html_analyze/src/lint/a11y/no_label_without_control.rs (1)
85-85:same()looks a bit too bold here.This port intentionally differs from the upstream JSX rule (
foronly, nohtmlFor, and the JSX-specific component options are not wired), soinspired()reads more accurate thansame(). Tiny metadata detail, but future-you will thank present-you.Based on learnings: "Use
RuleSource::Eslint("rule-name").same()when porting an ESLint rule with matching behavior" and "UseRuleSource::Eslint("rule-name").inspired()when porting an ESLint rule with different behavior or options".🤖 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_label_without_control.rs` at line 85, The RuleSource metadata currently uses RuleSource::EslintJsxA11y("label-has-associated-control").same(), which claims an identical port but this lint intentionally deviates from the upstream JSX rule; change the metadata to use inspired() instead of same(), i.e. replace the call to same() on RuleSource::EslintJsxA11y("label-has-associated-control") with inspired() so the rule is marked as inspired rather than identical.
🤖 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_label_without_control.rs`:
- Around line 163-200: The current check in element_has_accessible_label is too
permissive because has_accessible_name and
html_self_closing_element_has_accessible_name accept title and other
aria/title-bearing children; update the logic so the label is considered
accessible only if (1) the element itself has an explicit aria-name (only
aria-label or aria-labelledby, not title) or (2) it has non-empty plain text
descendants (text nodes) or (3) it contains a nested img with a non-empty alt.
Concretely: replace the top-level has_accessible_name call in
element_has_accessible_label with a new helper that only checks
aria-label/aria-labelledby (exclude title), and change
has_accessible_text_in_children to (a) keep AnyHtmlContent -> is_non_empty_text,
(b) for HtmlElement children do not treat their aria/title/aria-label as text —
only recurse into their children, and (c) for self-closing children only
consider img -> html_self_closing_element_has_non_empty_attribute(..., "alt")
and remove the use of html_self_closing_element_has_accessible_name; use
html_element_has_truthy_aria_hidden as before to skip hidden subtrees.
- Around line 219-221: The helper element_has_for_attribute currently treats any
presence of a for attribute as valid even when its value is empty or whitespace;
update element_has_for_attribute (in no_label_without_control.rs) to require a
non-empty, non-whitespace value (e.g., reuse the project’s existing
non-empty-attribute check or trim the attribute value and ensure it's not empty)
so labels with for="" or for=" " are treated as unassociated.
In `@crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/valid.html`:
- Around line 1-34: Add framework fixtures covering the new source-type branch:
create parallel test files for .vue, .svelte, and .astro that mirror this HTML
spec but include cases that exercise case-sensitive matching (e.g., PascalCase
<Label> vs lowercase <label>, and lowercase vs uppercase control tags like
<input> vs <INPUT>), ensuring .html remains case-insensitive while
.vue/.svelte/.astro assert case-sensitive behavior; update snapshot fixtures so
the new branch is exercised by tests for the unique examples shown (e.g.,
<label>/<Label>, <input>/<INPUT>, and the template expression {{ labelText }}).
---
Nitpick comments:
In `@crates/biome_html_analyze/src/lint/a11y/no_label_without_control.rs`:
- Line 85: The RuleSource metadata currently uses
RuleSource::EslintJsxA11y("label-has-associated-control").same(), which claims
an identical port but this lint intentionally deviates from the upstream JSX
rule; change the metadata to use inspired() instead of same(), i.e. replace the
call to same() on RuleSource::EslintJsxA11y("label-has-associated-control") with
inspired() so the rule is marked as inspired rather than identical.
🪄 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: 1af303eb-5708-47d6-b29d-a4c3f9615059
📒 Files selected for processing (4)
.changeset/add-no-label-without-control-html.mdcrates/biome_html_analyze/src/lint/a11y/no_label_without_control.rscrates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/invalid.htmlcrates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/valid.html
- Change RuleSource metadata from .same() to .inspired() — this HTML port intentionally differs from the upstream JSX rule (no htmlFor, no component options), so inspired() is more accurate than same() - Exclude title attribute from accessible-label check: title is not a reliable substitute for visible label text; only aria-label and aria-labelledby are accepted - Remove html_self_closing_element_has_accessible_name fallback in has_accessible_text_in_children: only img with a non-empty alt contributes accessible text; other self-closing elements' aria/title describe the control, not the label's own text content - Require non-empty, non-whitespace value for for attribute: for="" and for=" " cannot reference any control's DOM ID and are now treated as unassociated - Add two new invalid fixtures covering for="" and for=" " cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed the CodeRabbit review feedback in 963e47a:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/invalid.html (1)
9-13: Consider adding an explicit empty-img altinvalid case.Given the rule now treats only
<img alt="...">as self-closing text contribution, it’d be worth adding a guard case like<label for="x"><img alt="" /></label>(or whitespace-only alt) to lock that behaviour in.🤖 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/noLabelWithoutControl/invalid.html` around lines 9 - 13, Add an explicit invalid test case showing an empty-image alt inside a label so the rule's handling of self-closing text contributions is locked in: modify the invalid.html spec to include a label that references an id and contains <img alt=""> (and optionally a whitespace-only alt variant) as a child (e.g. <label for="x"><img alt="" /></label>) to assert that an empty alt does not provide accessible label text for the referenced control.
🤖 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/tests/specs/a11y/noLabelWithoutControl/invalid.html`:
- Around line 9-13: Add an explicit invalid test case showing an empty-image alt
inside a label so the rule's handling of self-closing text contributions is
locked in: modify the invalid.html spec to include a label that references an id
and contains <img alt=""> (and optionally a whitespace-only alt variant) as a
child (e.g. <label for="x"><img alt="" /></label>) to assert that an empty alt
does not provide accessible label text for the referenced control.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4ace6739-1993-4316-9e81-d7e02d24cb83
📒 Files selected for processing (2)
crates/biome_html_analyze/src/lint/a11y/no_label_without_control.rscrates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/invalid.html
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_html_analyze/src/lint/a11y/no_label_without_control.rs
…ge cases
Per ematipico and CodeRabbit feedback:
- Add .vue, .svelte, and .astro test fixtures exercising the case-sensitive
matching path for framework files:
- valid: lowercase label/controls, PascalCase <Label> ignored (custom component),
template expressions {{ expr }} / {expr} as accessible text
- invalid: uppercase <INPUT> not matched as control (case-sensitive),
PascalCase <Input> not matched as control
- Add two invalid HTML fixtures locking in empty img alt behaviour:
<img alt=""> and <img alt=" "> do not contribute accessible label text
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks @ematipico! Added the framework fixtures in 31cacc1: New test files:
Also added two invalid HTML fixtures per CodeRabbit's nitpick: |
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
@CalvinMagezi could you take a look at the CI failures? |
…xtures
Add generated insta snapshots for the .vue, .svelte, and .astro framework
fixture files added in the previous commit. Also removes the
{{ labelText }} test case from valid.html — template interpolations belong
in framework fixtures, not plain HTML (which has no interpolation parser).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in beef313 — the framework fixture files (.vue/.svelte/.astro) were committed without their insta snapshot files. Also removed |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/valid.html (1)
1-32: Nice fixture set — please add one positive<img alt>case for completeness.You already cover empty/whitespace
altin invalid fixtures, but this valid file should also include a non-emptyaltpath (e.g.<label><img alt="A label" /><input /></label>) so the “img-as-accessible-text” success branch is locked down too. Tiny addition, big future-proofing.🤖 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/noLabelWithoutControl/valid.html` around lines 1 - 32, Add a positive case that exercises the "img-as-accessible-text" success path by including a label that contains a non-empty-alt image plus a form control so the parser treats the img’s alt as accessible text; specifically, inside the existing valid.html fixture add a <label> element that wraps an <img> with a non-empty alt attribute and an associated control like <input> to ensure the img-alt branch is tested (e.g., a label containing an img with alt="A label" and an input).
🤖 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/tests/specs/a11y/noLabelWithoutControl/valid.html`:
- Around line 1-32: Add a positive case that exercises the
"img-as-accessible-text" success path by including a label that contains a
non-empty-alt image plus a form control so the parser treats the img’s alt as
accessible text; specifically, inside the existing valid.html fixture add a
<label> element that wraps an <img> with a non-empty alt attribute and an
associated control like <input> to ensure the img-alt branch is tested (e.g., a
label containing an img with alt="A label" and an input).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d8f19e3-b7d7-46c1-97e6-1bf02cde9d50
⛔ Files ignored due to path filters (11)
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/noLabelWithoutControl/astro/invalid.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/astro/valid.astro.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/svelte/invalid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/svelte/valid.svelte.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/valid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/vue/invalid.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/vue/valid.vue.snapis excluded by!**/*.snapand included by**packages/@biomejs/backend-jsonrpc/src/workspace.tsis excluded by!**/backend-jsonrpc/src/workspace.tsand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (1)
crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/valid.html
|
@CalvinMagezi still linting hasn't been addressed |
- Collapse the `if let Some(tag_name) { if tag_name == "img" }` into a
single `if let ... && ...` to satisfy clippy::collapsible_if
- Add `<label><img alt="A label" /><input /></label>` to valid.html
so there is a positive test for the img-as-accessible-text path
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@ematipico Fixed the clippy lint in d60712c — collapsed the nested All 8 tests pass, |
ematipico
added
the
M-Likely Agent
The JSX rule already maps "jsx-a11y/label-has-associated-control" to noLabelWithoutControl (line 1712). The HTML rule's .inspired() source auto-generated a second arm for the same pattern, causing an unreachable-pattern error. Remove the duplicate. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
One more fix in 5729d75 — the
|
|
Please wait merging the PR, the code isn't that great and I got sick of this agent |
Change the rule source from .inspired() to .same() to match the convention used by all other HTML rules that port from JSX a11y rules. With .inspired(), the codegen produces a second match arm (with include_inspired check) that conflicts with the JSX rule's existing .same() arm, causing an unreachable-pattern error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The autofix bot re-added the .inspired() arm before the .same() source change landed. Remove it again — codegen confirms no duplicate is produced with .same(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Two more fixes in c629daa:
Both |
|
Understood, sorry about the churn @ematipico. I should have caught the clippy and codegen issues before the first push rather than iterating in CI. Happy to hand this off if you'd prefer to clean it up, or I can squash and address any remaining feedback in a single commit whenever you're ready. |
3 participants
Summary
Ports the
noLabelWithoutControla11y lint rule to HTML, resolving the relevant item in the umbrella issue #8155.The rule now fires on
<label>elements in.html,.vue,.svelte, and.astrofiles using the same two-condition check as the existing JSX version:aria-label,aria-labelledby, or a nested<img alt="...">)forattribute or a nested control element (input,select,textarea,button,meter,output,progress)Key differences from the JSX rule
for(nothtmlFor— that's JSX-specific React mapping).htmlfiles and case-sensitively in component framework files (.vue,.svelte,.astro), consistent with the pattern used inuseHeadingContent,useAnchorContent, etc.inputComponentsandlabelComponentsoptions fromNoLabelWithoutControlOptionsare not wired up for HTML (they're JSX-specific custom component names), but the shared options struct is retained so no schema changes are neededTest plan
crates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/invalid.html— cases that should emit a diagnosticcrates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/valid.html— cases that should pass cleanly.snap) will be generated by runningcargo test -p biome_html_analyzejust gen-analyzerto regenerate the JSON configuration schema and documentationFiles changed
crates/biome_html_analyze/src/lint/a11y/no_label_without_control.rs— new rule implementationcrates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/invalid.html— invalid test fixturescrates/biome_html_analyze/tests/specs/a11y/noLabelWithoutControl/valid.html— valid test fixtures.changeset/add-no-label-without-control-html.md— minor changesetCloses part of #8155.
🤖 Generated with Claude Code