-
-
Notifications
You must be signed in to change notification settings - Fork 839
feat(analyze/html): add noDuplicateAttributes #8653
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: e24f0a2 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 |
WalkthroughAdds a new HTML/Vue lint rule Possibly related PRs
Suggested labels
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
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🧠 Learnings (22)📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-11-24T18:05:42.356ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-11-24T18:04:57.309ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-11-21T01:10:53.059ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-12-04T13:29:49.287ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-12-31T15:35:32.899ZApplied to files:
📚 Learning: 2025-12-22T09:26:56.943ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2026-01-02T14:58:16.536ZApplied to files:
📚 Learning: 2025-11-24T18:04:57.309ZApplied to files:
🧬 Code graph analysis (1)crates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs (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). (12)
🔇 Additional comments (8)
Comment |
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 (3)
crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.html (1)
1-6: Consider additional valid test cases.The current test coverage is correct but minimal. Consider optionally adding test cases such as:
- Attributes with values:
<div foo="a" bar="b"></div>- More than two distinct attributes
- Mixed boolean and valued attributes
However, if these are covered elsewhere or deemed unnecessary, the current coverage is acceptable.
crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.html (1)
7-8: Clarify test expectations for case-sensitive non-duplicates.This file is named
invalid.htmland the top comment states "should generate diagnostics", yet lines 7–8 explicitly expect NO diagnostic (case-sensitive comparison). Consider either:
- Moving this case to
valid.htmlfor consistency with the naming convention, or- Updating the top comment to clarify that this file tests both triggering and non-triggering scenarios.
Based on learnings, test files typically use
invalid/validprefixes to separate cases that should and should not trigger the rule.crates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs (1)
164-168: Consider explicit match forVueVOnDirective.The
v-on:clicklonghand form falls into the_ => Nonecatch-all, which works correctly but isn't as explicit as theVueVOnShorthandDirectivecase. For clarity and to catch future enum variants at compile time, consider matching it explicitly:🔎 Suggested change
// Ignore all v-on and shorthand @event handlers. AnyVueDirective::VueVOnShorthandDirective(_) => None, + AnyVueDirective::VueVOnDirective(_) => None, _ => None,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_html_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.vue.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/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 (9)
.changeset/add-no-duplicate-attributes-rule.mdcrates/biome_analyze/src/rule.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rscrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.htmlcrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.vuecrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.htmlcrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.vuecrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
�� Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_rule_options/src/lib.rscrates/biome_analyze/src/rule.rscrates/biome_rule_options/src/no_duplicate_attributes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
🧠 Learnings (31)
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Rule options must be defined in the `biome_rule_options` crate with a file named after the rule
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_analyze/src/rule.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/*.rs : No module may copy or clone data from another module in the module graph, not even behind an `Arc`
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Use `Option<_>` wrapper for rule option fields to enable proper merging of configurations
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Apply `#[serde(deny_unknown_fields)]` to rule option structs to enforce strict configuration validation
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Implement `biome_deserialize::Merge` for rule option types to define how shared and user configurations are merged
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Rule option types must derive `Deserializable`, `Serialize`, `Deserialize`, and optionally `JsonSchema` traits
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Apply `#[serde(rename_all = "camelCase")]` to rule option structs to match JSON configuration naming convention
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Rule names should use the `no` prefix when the rule's sole intention is to forbid a single concept
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_attributes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : New rules must be placed inside the `nursery` group before promotion to other groups
Applied to files:
crates/biome_rule_options/src/lib.rs.changeset/add-no-duplicate-attributes-rule.mdcrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Deprecated rules must include a `deprecated` field in the `declare_lint_rule!` macro with an explanation of what rule to use instead
Applied to files:
crates/biome_rule_options/src/lib.rscrates/biome_analyze/src/rule.rscrates/biome_rule_options/src/no_duplicate_attributes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/* : Create test files with `invalid` and `valid` prefixes to represent code that should and should not trigger the rule
Applied to files:
crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.vuecrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.htmlcrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.htmlcrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.vue
📚 Learning: 2025-12-04T13:29:49.287Z
Learnt from: dyc3
Repo: biomejs/biome PR: 8291
File: crates/biome_html_formatter/tests/specs/prettier/vue/html-vue/elastic-header.html:10-10
Timestamp: 2025-12-04T13:29:49.287Z
Learning: Files under `crates/biome_html_formatter/tests/specs/prettier` are test fixtures synced from Prettier and should not receive detailed code quality reviews (e.g., HTTP vs HTTPS, formatting suggestions, etc.). These files are test data meant to validate formatter behavior and should be preserved as-is.
Applied to files:
crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.htmlcrates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.html
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 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/add-no-duplicate-attributes-rule.md
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: For new nursery rules, send PRs to the maintenance branch `main`
Applied to files:
.changeset/add-no-duplicate-attributes-rule.md
📚 Learning: 2025-12-31T15:35:41.261Z
Learnt from: dyc3
Repo: biomejs/biome PR: 8639
File: crates/biome_js_analyze/src/lint/nursery/no_excessive_lines_per_file.rs:101-108
Timestamp: 2025-12-31T15:35:41.261Z
Learning: In crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs, the `issue_number` field in `declare_lint_rule!` macro is optional and the vast majority of nursery rules do not need it. Do not recommend adding `issue_number` unless there's a specific reason.
Applied to files:
.changeset/add-no-duplicate-attributes-rule.md
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: For rule changes in changesets, clearly demonstrate what is now invalid that wasn't before, or vice versa
Applied to files:
.changeset/add-no-duplicate-attributes-rule.md
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : When porting rules from other linters, use `sources` metadata with `RuleSource::Eslint().same()` for identical behavior or `.inspired()` for different behavior
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `declare_lint_rule!` macro with a `version` field set to `next` for new rules
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : The first paragraph of rule documentation must be written in a single line to ensure proper rendering in the rules overview table
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Rule names should use the `use` prefix when the rule's sole intention is to mandate a single concept
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use the `Semantic<T>` query type to access semantic information about bindings, references, and scope within a rule
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement the `action` function and add `fix_kind` metadata to the rule macro if the rule provides code actions
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Code blocks in rule documentation must specify a language identifier and be tagged with `expect_diagnostic` for invalid examples or remain untagged for valid examples
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Invalid code snippets in rule documentation must emit exactly one diagnostic
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `Markup!` macro for diagnostic messages and code action descriptions to ensure proper formatting
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Define `FormatHtmlSyntaxNode` struct in a `cst.rs` file implementing `FormatRule<HtmlSyntaxNode>`, `AsFormat<HtmlFormatContext>`, and `IntoFormat<HtmlFormatContext>` traits using the provided boilerplate code
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2026-01-02T14:58:16.514Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.514Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `rule_category!()` macro to refer to the diagnostic category instead of dynamically parsing its string name
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: Applies to **/*.rs : Use inline rustdoc documentation for rules, assists, and their options
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-11-21T01:10:53.059Z
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.059Z
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:
crates/biome_rule_options/src/no_duplicate_attributes.rs
📚 Learning: 2025-12-22T09:26:56.943Z
Learnt from: ematipico
Repo: biomejs/biome PR: 8537
File: crates/biome_js_analyze/src/lint/nursery/no_leaked_render.rs:167-210
Timestamp: 2025-12-22T09:26:56.943Z
Learning: When defining lint rules (declare_lint_rule!), only specify fix_kind if the rule implements an action(...) function. Rules that only emit diagnostics without a code fix should omit fix_kind. This applies to all Rust lint rule definitions under crates/.../src/lint (e.g., crates/biome_js_analyze/src/lint/...).
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
📚 Learning: 2025-12-31T15:35:32.899Z
Learnt from: dyc3
Repo: biomejs/biome PR: 8639
File: crates/biome_js_analyze/src/lint/nursery/no_excessive_lines_per_file.rs:101-108
Timestamp: 2025-12-31T15:35:32.899Z
Learning: In Rust lint rules under the nursery category, the issue_number field in declare_lint_rule! is optional and should not be added unless there is a compelling reason. In code reviews, verify that no unnecessary issue_number is included in nursery lint declarations. Only add issue_number if there is an explicit, justified reason (e.g., tracked issue for external observers). This guidance broadly applies to all nursery lint rule files, not just the single file.
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs
🧬 Code graph analysis (1)
crates/biome_analyze/src/rule.rs (1)
crates/biome_analyze/src/matcher.rs (1)
rule_name(121-123)
⏰ 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). (12)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test Node.js API
- GitHub Check: End-to-end tests
- GitHub Check: autofix
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Documentation
- GitHub Check: Validate rules documentation
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Check JS Files
🔇 Additional comments (9)
.changeset/add-no-duplicate-attributes-rule.md (1)
1-5: LGTM!The changeset correctly documents the new nursery rule at patch level with a clear description.
crates/biome_rule_options/src/lib.rs (1)
56-56: Correct module declaration.The module is properly declared in alphabetical order. Note that Line 1 indicates this file is generated by
xtask/codegen, so ensure the codegen is re-run to persist this change.crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/invalid.vue (1)
1-16: LGTM! Well-documented Vue test cases.The test coverage is excellent:
- Duplicate detection with Vue directive syntax (
:foovsfoo,v-bind:foovs:foo)- Multiple duplicate scenarios for
classandstyle- Clear comments documenting ignored cases (event handlers, dynamic arguments)
The ignored cases align with Vue's behaviour where multiple event handlers and dynamic arguments have special semantics.
crates/biome_analyze/src/rule.rs (1)
173-174: The html-eslint URL structure is correct:https://html-eslint.org/docs/rules/<rule-name>/. The code properly follows the established pattern for ESLint plugin sources.crates/biome_html_analyze/tests/specs/nursery/noDuplicateAttributes/valid.vue (1)
1-13: LGTM!Good test coverage for valid scenarios — covers plain attributes, Vue class/style bindings (which are correctly ignored), and dynamic arguments. The test cases align well with the rule's documented behaviour.
crates/biome_rule_options/src/no_duplicate_attributes.rs (1)
1-6: LGTM!The options struct follows all conventions: correct derives, serde attributes with
deny_unknown_fieldsandcamelCaserenaming, and conditionalJsonSchemafor the schema feature. Empty options struct is fine for rules without configurable options.crates/biome_html_analyze/src/lint/nursery/no_duplicate_attributes.rs (3)
10-60: LGTM!Well-structured rule declaration with clear documentation. The examples correctly demonstrate invalid and valid cases, and the
sourcesmetadata properly references the upstream ESLint rules with.same()indicating identical behaviour.
67-113: LGTM!Clean implementation. The
HashSetapproach for duplicate detection is efficient, and the diagnostic provides helpful context with the notes suggesting remediation. The use ofBox<[Self::State]>for returning multiple violations is appropriate.
62-65: Thepubvisibility is the standard pattern across all lint rules in the codebase—there are nopub(crate)State structs in any rule implementations. SinceStateis an associated type in theRuletrait, public visibility is necessary and appropriate.Likely an incorrect or invalid review comment.
Merging this PR will not alter performanceSummary
Comparing Footnotes
|
| /// Rules from [Eslint Plugin Turbo](https://github.com/vercel/turborepo/tree/main/packages/eslint-plugin-turbo) | ||
| EslintTurbo(&'static str), | ||
| /// Rules from [html-eslint](https://html-eslint.org/) | ||
| HtmlEslint(&'static str), |
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.
EslintHtml instead perhaps? I think as all of them start with Eslint
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.
That's how the project is named though. I think its because eslint-plugin-html is a different thing that only allows linting js in html, and does not implement any html rules itself. It's a bit weird.
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.
As a nursery rule, I think it's fine, however we should consider having one single rule.
We already https://biomejs.dev/linter/rules/no-duplicate-jsx-props/ which does the same thing.
|
|
||
| 1 │ <!-- should generate diagnostics --> | ||
| 2 │ | ||
| > 3 │ <div foo="a" foo="b"></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.
We could add a code frame to show where the other attribute is
| "Duplicate attribute '"<Emphasis>{name}</Emphasis>"'." | ||
| }, | ||
| ) | ||
| .note("Each attribute name must be unique within a single element.").note( |
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.
The diagnostic doesn't explain why they must be unique
|
@coderabbitai can you create an issue to track the merging of https://biomejs.dev/linter/rules/no-duplicate-jsx-props/ and the new rule created in this PR? Ideally we should deprecate the former in favour of the latter, once it's promoted |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 44 @ematipico: I've created issue #8677 to track the merging of �� Learnings used |
c4418e7 to
e24f0a2
Compare
Summary
Originally, this was going to be a direct port of https://eslint.vuejs.org/rules/no-duplicate-attributes.html (and it still is, to an extent). But I figured that it would make more sense for it to be more generic.
First draft written by gpt 5.2, and I cleaned it up manually.
I intentionally didn't use case insensitive equality to compare the attributes, because the source rule doesn't do this, and the formatter will deal with the casing of attributes. We can also expand this to cover svelte things better in the future.
Test Plan
snapshots
Docs