fix(useSemanticElements): don't flag elements and roles#9363
fix(useSemanticElements): don't flag elements and roles#9363
Conversation
🦋 Changeset detectedLatest commit: 8e97702 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 |
|
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 (2)
📒 Files selected for processing (3)
WalkthroughThe change updates the useSemanticElements lint rule to avoid reporting a diagnostic when a semantic HTML element already includes the matching role attribute (e.g., ). Such cases are now left to the noRedundantRoles rule. The patch adds an early-return and redundancy checks in the rule logic, updates tests to include semantic elements with matching roles, and adds a changeset for a patch release.Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
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_js_analyze/src/lint/a11y/use_semantic_elements.rs`:
- Around line 117-121: The current guard using is_already_semantic only compares
tag names (semantic_elements/related_elements .any(|instance|
instance.element.as_str() == element_name)), which causes false negatives for
constrained mappings (e.g., role -> input[type=checkbox]); update the comparison
to verify both tag and required attributes: iterate
semantic_elements.chain(related_elements) and for each instance check that
instance.element == element_name AND that all of instance.required_attributes
(or equivalent metadata on the mapping) are present and match in the current
node's attributes (use the node's attributes accessor where this check runs).
Replace the simple tag equality with this tag+required-attributes match to
ensure constrained elements are recognized correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 59287639-57dd-481f-ba2c-6584ed217bc1
⛔ Files ignored due to path filters (3)
crates/biome_js_analyze/tests/specs/a11y/useSemanticElements/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/a11y/useSemanticElements/invalid_self_closing.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/a11y/useSemanticElements/valid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/fix-use-semantic-elements-false-positive.mdcrates/biome_js_analyze/src/lint/a11y/use_semantic_elements.rscrates/biome_js_analyze/tests/specs/a11y/useSemanticElements/valid.jsx
Merging this PR will not alter performance
Comparing Footnotes
|
Summary
This PR closes #5212
There was some overlap with
noRedundantRoles. The new code now makes sure thatuseSemanticElementsdoesn't flag them.PR created with Claude Code.
Test Plan
Added new tests. Updated current snapshots
Docs
Updated the docs to reflect the change