feat(html/noPositiveTabindex): port noPositiveTabindex rule to HTML#8501
feat(html/noPositiveTabindex): port noPositiveTabindex rule to HTML#8501dyc3 merged 2 commits intobiomejs:nextfrom
Conversation
Port the existing JSX noPositiveTabindex rule to HTML. The rule prevents the usage of positive integers on the tabindex attribute, which can disrupt natural keyboard navigation order. Contributes to biomejs#8155
🦋 Changeset detectedLatest commit: c419b89 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 |
CodSpeed Performance ReportMerging #8501 will not alter performanceComparing Summary
Footnotes
|
WalkthroughThis PR adds an HTML accessibility lint rule Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (9)
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: 0
🧹 Nitpick comments (2)
.changeset/html-no-positive-tabindex.md (1)
5-5: Consider adding a code example.As per coding guidelines for changesets, rule additions should include a code example to help users understand what the rule detects.
🔎 Suggested enhancement:
-Added [`noPositiveTabindex`](https://biomejs.dev/linter/rules/no-positive-tabindex/) rule for HTML. This rule prevents the usage of positive integers on the `tabindex` attribute, which can disrupt natural keyboard navigation order. +Added [`noPositiveTabindex`](https://biomejs.dev/linter/rules/no-positive-tabindex/) rule for HTML. This rule prevents the usage of positive integers on the `tabindex` attribute, which can disrupt natural keyboard navigation order. + +```html +<div tabindex="1"></div> +```crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs (1)
52-55: Consider reducing visibility.
NoPositiveTabindexStateappears to be used only within this module. You could tighten encapsulation withpub(crate)unless there's a convention requiringpubfor rule state structs.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_html_analyze/tests/specs/a11y/noPositiveTabindex/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/a11y/noPositiveTabindex/valid.html.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 (5)
.changeset/html-no-positive-tabindex.md(1 hunks)crates/biome_html_analyze/src/lint/a11y.rs(1 hunks)crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs(1 hunks)crates/biome_html_analyze/tests/specs/a11y/noPositiveTabindex/invalid.html(1 hunks)crates/biome_html_analyze/tests/specs/a11y/noPositiveTabindex/valid.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
crates/**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update inline rustdoc documentation for rules, assists, and their options when adding new features or changing existing features in Rust crates
Files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Write changesets that are concise (1-3 sentences), user-focused, use past tense for actions taken and present tense for Biome behavior, include code examples for rules, and end sentences with periods
Files:
.changeset/html-no-positive-tabindex.md
🧠 Learnings (24)
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : New rules must be placed inside the `nursery` group as an incubation space exempt from semantic versioning, with promotion to appropriate groups done during minor/major releases
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noRestricted` prefix for rules that report user-banned entities (e.g., `noRestrictedGlobals`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUndeclared` prefix for rules that report undefined entities (e.g., `noUndeclaredVariables`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnknown` prefix for rules that report mistyped entities in CSS (e.g., `noUnknownUnit`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnsafe` prefix for rules that report code leading to runtime failures (e.g., `noUnsafeOptionalChaining`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `declare_lint_rule!` macro must include metadata fields: `version` (set to 'next'), `name` (rule identifier), `language` (applicable language), `recommended` (boolean), and optional fields like `severity`, `fix_kind`, `sources`, `domains`, and `deprecated`
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Deprecate rules by adding a `deprecated` field to the `declare_lint_rule!` macro with a message explaining the reason for deprecation (e.g., 'Use the rule noAnotherVar')
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUnused` prefix for rules that report unused entities (e.g., `noUnusedVariables`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noRedundant` prefix for rules that report redundant code (e.g., `noRedundantUseStrict`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noEmpty` prefix for rules that report empty code (e.g., `noEmptyBlockStatements`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Rules should use the `use` prefix naming convention when the sole intention is to mandate a single concept (e.g., `useValidLang` to enforce valid HTML lang attribute values)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noMisleading` prefix for rules that report valid but potentially misleading code (e.g., `noMisleadingCharacterClass`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Rules should use the `no` prefix naming convention when the sole intention is to forbid a single concept (e.g., `noDebugger` to disallow debugger statements)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rscrates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Framework-specific rules should be named using the `use` or `no` prefix followed by the framework name (e.g., `noVueReservedProps`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noUseless` prefix for rules that report unnecessary code that could be removed or simplified (e.g., `noUselessConstructor`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Set rule severity to `error` for correctness/security/a11y rules, `warn` for suspicious/performance rules, `info` for style/complexity rules, and `info` for actions
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noInvalid` prefix for rules that report runtime errors from mistyping (e.g., `noInvalidConstructorSuper`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Code actions must specify a `fix_kind` field in the `declare_lint_rule!` macro as either `FixKind::Safe` or `FixKind::Unsafe` to indicate whether fixes always preserve program behavior
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules must be implemented using the `Rule` trait with type parameters: `Query` (node type to analyze), `State` (information for signals), `Signals` (return type from run function), and `Options` (rule configuration)
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `run` function of a lint rule should return `Option<Self::State>` or an iterable like `Vec<Self::State>` or `Box<[Self::State]>` to signal zero or more diagnostics
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : In rule documentation code blocks, mark invalid examples with the `expect_diagnostic` property and valid examples without it; each invalid example must emit exactly one diagnostic
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `diagnostic` function must return a `RuleDiagnostic` that defines the message reported to the user using the `markup!` macro
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : The `action` function must return a `JsRuleAction` (or equivalent language-specific action type) with category `ctx.action_category(ctx.category(), ctx.group())` and applicability from `ctx.metadata().applicability()`
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
📚 Learning: 2025-11-27T23:04:02.022Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-27T23:04:02.022Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Use the `noConstant` prefix for rules that report computations always evaluated to the same value (e.g., `noConstantMathMinMaxClamp`)
Applied to files:
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs
🧬 Code graph analysis (1)
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs (2)
crates/biome_analyze/src/rule.rs (3)
sources(630-633)same(249-254)recommended(615-618)crates/biome_html_syntax/src/attr_ext.rs (1)
string_value(9-19)
⏰ 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). (1)
- GitHub Check: Lint project (depot-windows-2022)
🔇 Additional comments (8)
crates/biome_html_analyze/tests/specs/a11y/noPositiveTabindex/valid.html (1)
1-7: Good test coverage for the valid cases.The test file covers the essential valid scenarios:
tabindex="0",tabindex="-1", and elements without the attribute.Consider adding edge cases like
tabindex=""ortabindex="abc"to document how the rule handles non-integer values (which are treated as valid per the implementation).crates/biome_html_analyze/tests/specs/a11y/noPositiveTabindex/invalid.html (1)
1-6: Solid test coverage for invalid cases.Good variety of positive tabindex values across different HTML element types. This ensures the rule correctly triggers diagnostics for the full range of invalid scenarios.
crates/biome_html_analyze/src/lint/a11y/no_positive_tabindex.rs (5)
10-50: Well-documented rule with proper metadata.The rule declaration follows all conventions:
version: "next",severity: Severity::Errorfor a11y rules,fix_kind: FixKind::Unsafe(appropriate since removing the attribute alters tab behaviour), and proper source attribution. The WCAG 2.4.3 reference is a nice touch.Based on learnings, this implementation aligns with the required
declare_lint_rule!metadata fields.
63-79: Clean implementation with proper early returns.Good use of the
?operator for clean control flow. The logic correctly extracts the tabindex value and validates it.
81-99: Helpful diagnostic messages.The primary message is clear, and the two notes provide excellent context about why positive tabindex values are problematic and what to use instead.
101-111: Fix implementation is correct.The fix removes the attribute entirely, which aligns with the
Unsafedesignation — users should verify this doesn't break their intended tab order.
114-123: Sensible validation logic.The approach of treating parse failures as valid (since browsers ignore non-integers) is pragmatic. The
trim()handles whitespace nicely.crates/biome_html_analyze/src/lint/a11y.rs (1)
9-9: Module correctly integrated via codegen—all aspects check out.The file is properly generated via
xtask/codegen, and the rule registration at line 9 and line 15 are both correct. Theno_positive_tabindexmodule is alphabetically ordered, the rule is properly registered in thedeclare_lint_group!macro, and the rule implementation follows all guidelines: it uses the correct "no" prefix, has version set to "next", specifies severity asError(appropriate for a11y rules), includesfix_kind: FixKind::Unsafe, provides documentation with proper HTML code-block tags, and references the source. No issues found.Also applies to: 15–15
Summary
Contributes to #8155
Ports the existing JSX
noPositiveTabindexrule to HTML. This rule prevents the usage of positive integers on thetabindexattribute, which can disrupt natural keyboard navigation order.Changes:
noPositiveTabindexrule for HTML incrates/biome_html_analyze/src/lint/a11y/tabindexattributeTest plan
invalid.htmlwith positive tabindex values (1, 5, 2, 10, 99)valid.htmlwith valid tabindex values (0, -1)AI Assistance Disclosure
I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.