feat(biome_html_analyze): add useButtonType rule (#8241)#8243
feat(biome_html_analyze): add useButtonType rule (#8241)#8243dyc3 merged 6 commits intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 00b7ef0 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 |
WalkthroughAdds a new HTML accessibility lint rule Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.changeset/witty-ears-hammer.md (1)
5-20: Clarify that the rule also validates thetypevalueThe implementation also flags invalid
typevalues (e.g.type="incorrectType"), not just missing attributes. It would be clearer to say it enforces a present and validtype(one of"button","submit","reset"), and optionally add atype="reset"example to the valid snippet.crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.html (1)
1-7: Good spread of invalid casesThis nicely covers missing, malformed and self‑closing
buttonelements for the rule’s branches. If you ever want to extend coverage, atype=""case could exercise the empty‑string path, but it’s not required for this PR.crates/biome_html_analyze/src/lint/a11y/use_button_type.rs (2)
9-39: Replace template docs with a concrete rule descriptionThe rule doc comment is still the generic template. It would be good to briefly describe that this HTML rule requires an explicit
typeattribute on<button>elements with a valid value ("button","submit","reset"), and mirror the invalid/valid examples you use in tests/changeset for consistency.
61-65: Fix typo intype_attribuevariable nameMinor nit:
type_attribueis misspelled. Renaming it totype_attribute(and updating the binding below) will make the code a bit clearer for future readers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/useButtonType/valid.html.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
.changeset/witty-ears-hammer.md(1 hunks)crates/biome_html_analyze/src/lint/a11y.rs(1 hunks)crates/biome_html_analyze/src/lint/a11y/use_button_type.rs(1 hunks)crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.html(1 hunks)crates/biome_html_analyze/tests/specs/a11y/useButtonType/valid.html(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
crates/biome_html_analyze/src/lint/a11y.rs (1)
crates/biome_html_syntax/src/element_ext.rs (1)
name(55-70)
crates/biome_html_analyze/src/lint/a11y/use_button_type.rs (2)
crates/biome_service/src/workspace.rs (1)
markup(1149-1151)crates/biome_analyze/src/rule.rs (3)
sources(610-613)same(246-251)recommended(595-598)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: End-to-end tests
- GitHub Check: Check Dependencies
- GitHub Check: autofix
🔇 Additional comments (2)
crates/biome_html_analyze/src/lint/a11y.rs (1)
8-10: A11y wiring forUseButtonTypelooks correctThe new module export and
A11ylint group registration forUseButtonTypematch the existing pattern; nothing further needed here.crates/biome_html_analyze/tests/specs/a11y/useButtonType/valid.html (1)
1-7: Valid samples match the rule’s intentThe examples cover all three allowed
typevalues and both standard and self‑closing forms; this looks sufficient for the happy‑path behaviour.
CodSpeed Performance ReportMerging #8243 will not alter performanceComparing Summary
Footnotes
|
Co-authored-by: Carson McManus <dyc3@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_html_analyze/src/lint/a11y/use_button_type.rs (1)
82-108: Diagnostic messages are clear and well-structured.The messages correctly use "attribute" terminology for HTML. The notes effectively explain the default behaviour and allowed values.
Minor nitpick: Line 100 has a trailing space before the closing quote. You can remove it if you'd like:
- "The default "<Emphasis>"type"</Emphasis>" of a button is "<Emphasis>"submit"</Emphasis>", which causes the submission of a form when placed inside a `form` element. " + "The default "<Emphasis>"type"</Emphasis>" of a button is "<Emphasis>"submit"</Emphasis>", which causes the submission of a form when placed inside a `form` element."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.html.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/witty-ears-hammer.md(1 hunks)crates/biome_html_analyze/src/lint/a11y/use_button_type.rs(1 hunks)crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_html_analyze/tests/specs/a11y/useButtonType/invalid.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-21T01:10:53.036Z
Learnt from: dyc3
Repo: biomejs/biome PR: 8171
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:125-137
Timestamp: 2025-11-21T01:10:53.036Z
Learning: In the Biome codebase, each lint rule has its own options type declaration (e.g., `type Options = RuleNameOptions`) as part of the codegen process, even if the options struct is empty or unused. This is standard practice and should not be flagged as an issue.
Applied to files:
.changeset/witty-ears-hammer.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: autofix
🔇 Additional comments (6)
crates/biome_html_analyze/src/lint/a11y/use_button_type.rs (5)
1-7: Imports look good.All necessary dependencies are present for the HTML lint rule implementation.
9-38: Rule declaration is well-structured.The metadata and documentation examples are clear and appropriate for this accessibility rule.
40-44: Constants and state definition look correct.The allowed button types match the HTML specification.
111-115: Helper function is correct.The case-insensitive check is appropriate for HTML element names.
71-73: No concerns found—code functions correctly per test specifications.The snapshot tests confirm that edge cases like
<button type>and<button type="">correctly generate diagnostics. The?operators on lines 71–73 aren't silently skipping diagnostics; the tests prove these cases are being caught and reported as expected..changeset/witty-ears-hammer.md (1)
1-22: Changeset documentation looks good.The examples clearly illustrate the rule's behaviour for both invalid and valid cases.
|
Sorry about the multiple commits, think I have fixed the recommendations now. Let me know if there is anything else needed. |
Summary
This PR adds the useButtonType rule to the HTML analyzer based on the existing JSX implementation.
Only AI tab completion was used in this PR.
Valid:
Invalid:
closes #8241
Test Plan
Added valid/invalid tests for this rule
Docs