-
-
Notifications
You must be signed in to change notification settings - Fork 714
fix: remove option, combobox, listbox from semantic element suggestions #7129
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: c6d5338 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
WalkthroughThis change updates the Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. All modifications are directly related to the objectives in the linked issue. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (24)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
88aae32
to
3522caf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/new-shrimps-reply.md (1)
5-5
: Add missing full stop.Per the changeset guidelines, sentences should end with a full stop.
-Fix: remove option, combobox, listbox roles from [useSemanticElements](https://biomejs.dev/linter/rules/use-semantic-elements/) suggestions +Fix: remove option, combobox, listbox roles from [useSemanticElements](https://biomejs.dev/linter/rules/use-semantic-elements/) suggestions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/new-shrimps-reply.md
(1 hunks)crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Format code (Rust + TOML) using
just format
Format Rust and TOML files using
just f
(alias forjust format
).
Files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
crates/biome_*/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Core crates must be located in
/crates/biome_*/
Files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
**/*.rs
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Update documentation for new features or changes, using inline rustdoc for rules, assists, and their options.
Files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
.changeset/*.md
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
.changeset/*.md
: Changeset descriptions should be about user-facing changes, use past tense for what you did, present tense for Biome behavior, reference issues/rules/assists with links, include code blocks when applicable, and end every sentence with a full stop.
Headers in changesets should use####
or#####
only; other headers may break the changelog.
Files:
.changeset/new-shrimps-reply.md
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : Update the `language` field in `declare_lint_rule!` to match the language the rule applies to (e.g., `js`, `jsx`, `ts`, `tsx`).
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : When a rule requires semantic information, use the `Semantic<>` query type and access the semantic model via `ctx.model()`.
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : If a lint rule is ported from another ecosystem (e.g., ESLint), add a `sources` metadata field in `declare_lint_rule!` referencing the original rule.
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/{use*,no*}_*{vue,react,angular,svelte}*.rs : If a rule overwhelmingly applies to a specific framework, it should be named using the `use<Framework>...` or `no<Framework>...` pattern (e.g., `noVueReservedProps`).
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : If a rule provides a code action, add the `fix_kind` metadata to the `declare_lint_rule!` macro and implement the `action` function.
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : when a rule requires semantic in...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : When a rule requires semantic information, use the `Semantic<>` query type and access the semantic model via `ctx.model()`.
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
.changeset/new-shrimps-reply.md
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : update the `language` field in `...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : Update the `language` field in `declare_lint_rule!` to match the language the rule applies to (e.g., `js`, `jsx`, `ts`, `tsx`).
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
.changeset/new-shrimps-reply.md
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : if a rule provides a code action...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : If a rule provides a code action, add the `fix_kind` metadata to the `declare_lint_rule!` macro and implement the `action` function.
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
.changeset/new-shrimps-reply.md
📚 Learning: applies to crates/biome_analyze/biome_rule_options/lib/*.rs : use boxed slices (`box<[box]>`) i...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/*.rs : Use boxed slices (`Box<[Box<str>]>`) instead of `Vec<String>` for string arrays in rule options to save memory.
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
.changeset/new-shrimps-reply.md
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : when banning certain functions o...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : When banning certain functions or variables, always check if the variable is global using the semantic model to avoid false positives.
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : if a lint rule is ported from an...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : If a lint rule is ported from another ecosystem (e.g., ESLint), add a `sources` metadata field in `declare_lint_rule!` referencing the original rule.
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
.changeset/new-shrimps-reply.md
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : when implementing the `run` func...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : When implementing the `run` function for a rule, prefer transforming `Result` into `Option` and using the `?` operator for concise code.
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/{use*,no*}_*{vue,react,angular,svelte}*...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/{use*,no*}_*{vue,react,angular,svelte}*.rs : If a rule overwhelmingly applies to a specific framework, it should be named using the `use<Framework>...` or `no<Framework>...` pattern (e.g., `noVueReservedProps`).
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
.changeset/new-shrimps-reply.md
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : the `declare_lint_rule!` macro m...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : The `declare_lint_rule!` macro must be used to declare an analyzer rule type and implement the RuleMeta trait.
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
📚 Learning: applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : assign the `domains` field in `d...
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:50:18.454Z
Learning: Applies to crates/biome_analyze/biome_*/lib/src/lint/nursery/*.rs : Assign the `domains` field in `declare_lint_rule!` if the rule belongs to a specific domain (e.g., testing, framework).
Applied to files:
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs
📚 Learning: the file `packages/@biomejs/biome/configuration_schema.json` is auto-generated and should not be man...
Learnt from: dyc3
PR: biomejs/biome#7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
.changeset/new-shrimps-reply.md
📚 Learning: applies to .changeset/*.md : changeset descriptions should be about user-facing changes, use past te...
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:49:35.574Z
Learning: Applies to .changeset/*.md : Changeset descriptions should be about user-facing changes, use past tense for what you did, present tense for Biome behavior, reference issues/rules/assists with links, include code blocks when applicable, and end every sentence with a full stop.
Applied to files:
.changeset/new-shrimps-reply.md
📚 Learning: applies to crates/biome_lint_rules/**/*.rs : linter rules must have a `version` metadata field in th...
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-08-05T14:49:35.574Z
Learning: Applies to crates/biome_lint_rules/**/*.rs : Linter rules must have a `version` metadata field in their implementation, set to "next" for new rules and updated to the new version when releasing.
Applied to files:
.changeset/new-shrimps-reply.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). (18)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: autofix
🔇 Additional comments (1)
crates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rs (1)
91-98
: LGTM! Well-implemented solution.The exclusion logic is correctly placed and follows the existing pattern. The detailed comments explain the rationale well, referencing the W3C documentation. This effectively addresses the accessibility concerns raised in issue #6951.
3522caf
to
5b27e7e
Compare
22 │ <div role="link" ></div> | ||
23 │ <div role="list" ></div> | ||
> 24 │ <div role="listbox" ></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth noting this wasn’t correct in the first place. <select>
gets combobox
by default, and will only be read as a listbox when using <select multiple>
. And nobody’s using that (hopefully). We live in a society
CodSpeed Performance ReportMerging #7129 will not alter performanceComparing Summary
|
Summary
Fixes #6951
Test Plan
Docs
Bugfix?