feat(assist): add noDuplicateClasses assist action#8623
feat(assist): add noDuplicateClasses assist action#8623ematipico merged 2 commits intobiomejs:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 3872006 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 noDuplicateClasses assist for JSX and HTML that detects duplicate CSS classes and offers replacements preserving original quotes and whitespace. Introduces a shared class-dedup utility module (find_duplicate_classes, diagnostics) with unit tests, exposes shared modules for JS analyzers, makes AnyClassStringLike's visitor method public, registers an Assist category/group and assist/source codegen, adds options and wrapper types, test fixtures for JS/HTML, updates build/codegen to include assist/source, and extends RuleSource with an eslint-plugin variant. Possibly related PRs
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
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 (3)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (1)
66-72: Consider addingissue_numberfor tracking.Per guidelines, nursery rules benefit from an
issue_numberfield linking to the tracking issue. The PR objectives mention #6502 as the request for duplicate class detection.🔎 Suggested addition
pub NoDuplicateTailwindClasses { version: "next", name: "noDuplicateTailwindClasses", language: "jsx", recommended: false, fix_kind: FixKind::Safe, + issue_number: Some("6502"), }crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs (2)
53-59: Consider addingissue_numberfor tracking.Similar to the duplicate classes rule, adding an
issue_numberhelps track the feature request. The PR objectives reference #1274 as the Tailwind CSS support tracking issue.🔎 Suggested addition
pub NoUnnecessaryTailwindWhitespace { version: "next", name: "noUnnecessaryTailwindWhitespace", language: "jsx", recommended: false, fix_kind: FixKind::Safe, + issue_number: Some("1274"), }
79-86: Detection doesn't explicitly cover tabs or mixed whitespace.Line 82's
contains(" ")only detects double spaces. Tabs or mixed whitespace (e.g.,"flex\tp-4"or"flex \t p-4") will still be normalised bysplit_whitespace(), but the detection check and diagnostic won't explicitly mention them. The rule works correctly due to the guard at line 92, but the diagnostic message might not be fully accurate for tab-heavy inputs.If you'd like more precise detection:
🔎 Optional enhancement
- let has_multiple_spaces = value_str.contains(" "); + // Check for multiple consecutive whitespace characters (spaces, tabs, newlines) + let has_multiple_whitespace = value_str + .chars() + .zip(value_str.chars().skip(1)) + .any(|(a, b)| a.is_whitespace() && b.is_whitespace());And update the diagnostic message at line 111-112 accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/valid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (8)
.changeset/tailwind-utility-class-rules.mdcrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/valid.jsx
🧰 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_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
🧠 Learnings (22)
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs.changeset/tailwind-utility-class-rules.md
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Check if a variable is global using the semantic model to avoid false positives
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use helper functions like `map`, `filter`, and `and_then` to avoid deep indentation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Safe` in `declare_lint_rule!` for safe code actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.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 : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `version` field to `next` in `declare_lint_rule!` macro
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Use `rename_all = "camelCase"` in serde derive macro for rule options
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.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_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `domains` field in `declare_lint_rule!` to tag rules that belong to specific concepts like testing or frameworks
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `sources` field with `RuleSource` to cite ESLint or other rules that inspired the implementation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*valid* : Create test files prefixed with `valid` for code that should not trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsx
📚 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_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*invalid* : Create test files prefixed with `invalid` for code that should trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*.jsonc : Use `.jsonc` files to contain arrays of code snippet strings for snapshot tests
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/valid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `declare_lint_rule!` macro to declare analyzer rule types and implement the RuleMeta trait
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.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: Changesets should describe user-facing changes only; internal refactoring without behavior changes does not require a changeset
Applied to files:
.changeset/tailwind-utility-class-rules.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/tailwind-utility-class-rules.md
🔇 Additional comments (9)
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsx (1)
1-16: LGTM!Good coverage of valid cases: non-duplicates, variant-differentiated classes, similar-but-different utilities, empty strings, and single-class scenarios. Based on learnings, the
validprefix correctly indicates these should not trigger diagnostics.crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsx (1)
1-18: LGTM!Comprehensive invalid test cases covering leading/trailing whitespace, multiple consecutive spaces, combined scenarios, and multiline strings with newlines and tabs. Good coverage of the
classNameattribute variant too.crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs (1)
1-1: Visibility change looks appropriate.Making
any_class_string_likepublic enables the new Tailwind rules to reuseAnyClassStringLike. Since all consumers are within the same crate (biome_js_analyze), this is a reasonable approach. If external crate access becomes a concern later, consider usingpub(crate)instead.crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/valid.jsx (1)
1-12: LGTM!Straightforward valid cases with properly formatted whitespace. Good inclusion of the empty class and single class edge cases.
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsx (1)
1-16: LGTM!Solid invalid test coverage: exact duplicates, non-adjacent duplicates, variant-prefixed duplicates, multiple duplicates per string, and arbitrary value duplicates. Well structured!
.changeset/tailwind-utility-class-rules.md (1)
1-9: LGTM!Changeset correctly documents the two new nursery rules with appropriate links. Patch version bump is suitable for new nursery rules.
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (2)
106-122: Whitespace normalisation occurs as a side effect.Using
split_whitespace()andjoin(" ")means this rule will also normalise whitespace when removing duplicates. This is generally fine, but users may see whitespace fixes from this rule even whennoUnnecessaryTailwindWhitespaceis disabled. Consider documenting this in the rustdoc if intentional.
155-207: LGTM!The action implementation correctly handles all
AnyClassStringLikevariants and preserves quote styles. Good consistency with the existinguseSortedClassesrule.crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs (1)
131-183: LGTM!Action implementation correctly handles all
AnyClassStringLikevariants with proper quote style preservation. Consistent with other Tailwind rules in this PR.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (2)
144-148: Minor: Consider simpler pluralisation.The
<Emphasis>wrapper around the conditional "es" suffix is a bit unusual. Standard approach would be to conditionally include the whole word. Not blocking though — it works.
173-180: Inconsistent quote preservation forJsLiteralMemberName.Unlike
JsStringLiteralExpression(line 162-165) andJsxString(line 182-185), this branch doesn't attempt to detect the original quote style from the token. It falls back directly toctx.preferred_quote().Consider preserving the original quote style for consistency:
🔎 Suggested fix
AnyClassStringLike::JsLiteralMemberName(string_literal) => { - let replacement = js_literal_member_name(if ctx.preferred_quote().is_double() { + let is_double_quote = string_literal + .value_token() + .map(|token| token.text_trimmed().starts_with('"')) + .unwrap_or(ctx.preferred_quote().is_double()); + let replacement = js_literal_member_name(if is_double_quote { js_string_literal(deduplicated) } else { js_string_literal_single_quotes(deduplicated) }); mutation.replace_node(string_literal.clone(), replacement); }crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs (1)
158-165: Same quote preservation inconsistency forJsLiteralMemberName.As noted in
no_duplicate_tailwind_classes.rs, this branch doesn't detect the original quote style. Consider applying the same fix here for consistency.Also worth noting: the
action()implementations in both rules are nearly identical. A shared helper function could reduce duplication in future, but not blocking for this PR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noUnnecessaryTailwindWhitespace/invalid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rscrates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.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_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Commit rule work with message format `feat(biome_<language>_analyze): <ruleName>`
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`
📚 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_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Safe` in `declare_lint_rule!` for safe code actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `declare_lint_rule!` macro to declare analyzer rule types and implement the RuleMeta trait
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Unsafe` in `declare_lint_rule!` for unsafe code actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `version` field to `next` in `declare_lint_rule!` macro
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `deprecated` field to `declare_lint_rule!` macro when deprecating a rule
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `domains` field in `declare_lint_rule!` to tag rules that belong to specific concepts like testing or frameworks
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
🔇 Additional comments (3)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (2)
13-83: LGTM!Rule declaration follows Biome conventions nicely. Documentation is clear,
issue_numberis present for nursery rules, andfix_kindcorrectly indicates a safe fix. Based on learnings, this is spot on.
107-117: LGTM!Duplicate detection logic is correct. The linear search at line 110 is fine given typical duplicate counts are small.
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs (1)
12-61: LGTM!Rule declaration is well-structured with clear documentation and proper metadata. Based on learnings,
issue_numberis correctly included for nursery rules.
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
Outdated
Show resolved
Hide resolved
I really don't think we need a rule for this. We already |
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_unnecessary_tailwind_whitespace.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (1)
175-182: Consider preserving the original quote style for consistency.The
JsLiteralMemberNamecase usesctx.preferred_quote()directly without checking the original node's quote style, unlikeJsStringLiteralExpression(Line 164-167) andJsxString(Line 184-187) which both inspect the original token first. This inconsistency might unnecessarily change quote styles during the fix.🔎 Suggested refactor to preserve original quote style
AnyClassStringLike::JsLiteralMemberName(string_literal) => { - let replacement = js_literal_member_name(if ctx.preferred_quote().is_double() { + let is_double_quote = string_literal + .value() + .ok() + .and_then(|v| v.text().chars().next()) + .map(|c| c == '"') + .unwrap_or(ctx.preferred_quote().is_double()); + let replacement = js_literal_member_name(if is_double_quote { js_string_literal(deduplicated) } else { js_string_literal_single_quotes(deduplicated) }); mutation.replace_node(string_literal.clone(), replacement); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/tailwind-utility-class-rules.mdcrates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/tailwind-utility-class-rules.md
🧰 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_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Commit rule work with message format `feat(biome_<language>_analyze): <ruleName>`
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `sources` field with `RuleSource` to cite ESLint or other rules that inspired the implementation
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use language-specific rule names if the rule is meant for a specific language only
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Implement `action` function in Rule trait to provide code actions
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use generic rule names if the rule could potentially be implemented for multiple languages
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `version` field to `next` in `declare_lint_rule!` macro
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `language` field in `declare_lint_rule!` macro to the language the rule primarily applies to
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `declare_lint_rule!` macro to declare analyzer rule types and implement the RuleMeta trait
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should check syntax according to language specification and emit error diagnostics
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `language` to `jsx`, `ts`, or `tsx` for rules that only apply to specific JavaScript dialects
Applied to files:
crates/biome_analyze/src/rule.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `domains` field in `declare_lint_rule!` to tag rules that belong to specific concepts like testing or frameworks
Applied to files:
crates/biome_analyze/src/rule.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Parse rule functions must be prefixed with `parse_` and use the name defined in the grammar file, e.g., `parse_for_statement` or `parse_expression`
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-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.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_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Safe` in `declare_lint_rule!` for safe code actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
🔇 Additional comments (11)
crates/biome_analyze/src/rule.rs (5)
109-110: LGTM! Enum variant follows established pattern.The new
EslintBetterTailwindcssvariant is properly documented and structured consistently with other ESLint plugin variants in the codebase.
190-190: LGTM! Display implementation is consistent.The display name follows the
eslint-plugin-{name}convention used by other ESLint plugins.
276-276: LGTM! Pattern match correctly extracts rule name.The implementation follows the established pattern for extracting rule names from source variants.
320-322: LGTM! Namespacing follows plugin conventions.The
better-tailwindcss/{rule_name}format is consistent with how other ESLint plugins are namespaced in the codebase.
366-366: The URL format is correct.The upstream repository confirms this documentation structure exists and is accessible. Looks good to merge.
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (6)
1-13: LGTM!The imports are well-organised and all appear to be used appropriately for the rule implementation.
15-76: LGTM!The rule declaration correctly follows Biome conventions, and the
sourcesfield properly attributes the upstream ESLint rule as requested by the maintainer.
78-85: LGTM!The state structure appropriately uses boxed types for owned data.
93-131: LGTM!The duplicate detection logic correctly preserves first-occurrence order and efficiently tracks duplicates using
FxHashSet.
133-156: LGTM!The diagnostic message is clear, properly pluralised, and provides helpful guidance.
201-210: LGTM!The action creation follows the standard pattern with appropriate metadata and a clear message.
|
Thanks @ematipico and @Netail for the review feedback! I've addressed both points:
The PR now contains just the single noDuplicateTailwindClasses rule ported from https://github.com/schoero/eslint-plugin-better-tailwindcss. |
noDuplicateTailwindClasses nursery rule
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (1)
145-149: Curious<Emphasis>placement.The
<Emphasis>tag wraps just the "es" pluralisation suffix, which would render the "es" as bold/highlighted. Consider moving the emphasis toduplicates_stror removing it entirely for cleaner output.🔎 Suggested fix
markup! { - "Duplicate CSS utility class"<Emphasis>{ - if state.duplicates.len() > 1 { "es" } else { "" } - }</Emphasis>" detected: "{duplicates_str} + "Duplicate CSS utility class"{ + if state.duplicates.len() > 1 { "es" } else { "" } + }" detected: "{duplicates_str} },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.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_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Commit rule work with message format `feat(biome_<language>_analyze): <ruleName>`
📚 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_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Safe` in `declare_lint_rule!` for safe code actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `declare_lint_rule!` macro to declare analyzer rule types and implement the RuleMeta trait
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `sources` field with `RuleSource` to cite ESLint or other rules that inspired the implementation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `deprecated` field to `declare_lint_rule!` macro when deprecating a rule
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (2)
crates/biome_js_factory/src/make.rs (3)
js_string_literal(24-31)js_string_literal_single_quotes(34-41)js_template_chunk(63-70)crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs (3)
run(172-189)diagnostic(191-209)action(211-261)
🔇 Additional comments (4)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs (4)
15-76: Well-documented rule declaration.Good examples, proper attribution via
sources, and the options documentation is helpful for users already familiar withuseSortedClasses.
78-85: LGTM!State struct is well-documented with appropriate types.
93-130: Logic is sound.The duplicate detection correctly preserves first occurrences whilst collecting duplicates. The implicit whitespace normalisation via
split_whitespace+joinis a nice touch.
157-213: Action implementation handles all node types correctly.Quote style preservation is properly implemented for each variant, and the fallback to
preferred_quote/preferred_jsx_quoteis appropriate. Consistent with the existinguseSortedClassespattern.
|
Ready for re-review |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsx (1)
1-19: Solid test coverage for valid cases.The test cases appropriately cover the essential scenarios: no duplicates, similar-but-different classes (e.g.,
p-4vspx-4), empty strings, single classes, and both quote styles. This should prevent false positives.Optional: Consider adding function call test cases
Since the PR objectives mention the rule reuses
UseSortedClassesOptions(supportingfunctionsconfiguration), you might consider adding valid test cases for utility functions:// Valid: function calls with no duplicates clsx("flex", "p-4"); cn("text-red-500", "bg-white");This would ensure the rule handles function calls correctly when configured. However, if these are covered in the invalid test file or are intentionally omitted, feel free to disregard.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/invalid.jsx
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*valid* : Create test files prefixed with `valid` for code that should not trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*invalid* : Create test files prefixed with `invalid` for code that should trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateTailwindClasses/valid.jsx
dyc3
left a comment
There was a problem hiding this comment.
You should an HTML rule for this as well!
crates/biome_js_analyze/src/lint/nursery/no_duplicate_tailwind_classes.rs
Outdated
Show resolved
Hide resolved
d0ff300 to
f426f29
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx (1)
1-19: Consider adding valid template literal test cases.The invalid tests include template literals with duplicates. Adding corresponding valid cases would improve coverage:
// Valid: template literals without duplicates <div className={`flex p-4 ${condition}`} />; <div className={`${prefix} p-4 m-2 ${suffix}`} />;crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (1)
8-8: Consider usingFxHashSetfor consistency.The JS version of this rule uses
rustc_hash::FxHashSet, which is faster for small string keys. For consistency and performance:-use std::collections::HashSet; +use rustc_hash::FxHashSet;Then update the usages on lines 93-94 accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
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/noDuplicateClasses/invalid.html.snapis excluded by!**/*.snapand included by**crates/biome_html_analyze/tests/specs/nursery/noDuplicateClasses/valid.html.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsx.snapis excluded by!**/*.snapand included by**crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (9)
.changeset/no-duplicate-classes-rule.mdcrates/biome_analyze/src/rule.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_html_analyze/tests/specs/nursery/noDuplicateClasses/invalid.htmlcrates/biome_html_analyze/tests/specs/nursery/noDuplicateClasses/valid.htmlcrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
✅ Files skipped from review due to trivial changes (1)
- crates/biome_html_analyze/tests/specs/nursery/noDuplicateClasses/invalid.html
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_analyze/src/rule.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_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
🧠 Learnings (24)
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Applied to files:
.changeset/no-duplicate-classes-rule.mdcrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Check if a variable is global using the semantic model to avoid false positives
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use helper functions like `map`, `filter`, and `and_then` to avoid deep indentation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Safe` in `declare_lint_rule!` for safe code actions
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.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 : Use `TypeReference` instead of `Arc` for types that reference other types to avoid stale cache issues when modules are replaced
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `version` field to `next` in `declare_lint_rule!` macro
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Use `rename_all = "camelCase"` in serde derive macro for rule options
Applied to files:
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.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_js_analyze/src/lint/nursery/use_sorted_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*invalid* : Create test files prefixed with `invalid` for code that should trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsxcrates/biome_html_analyze/tests/specs/nursery/noDuplicateClasses/valid.htmlcrates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*valid* : Create test files prefixed with `valid` for code that should not trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsxcrates/biome_html_analyze/tests/specs/nursery/noDuplicateClasses/valid.htmlcrates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Set `language` to `jsx`, `ts`, or `tsx` for rules that only apply to specific JavaScript dialects
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*.jsonc : Use `.jsonc` files to contain arrays of code snippet strings for snapshot tests
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsxcrates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : The first paragraph of rule documentation must be a single line describing what the rule does
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `sources` field with `RuleSource` to cite ESLint or other rules that inspired the implementation
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 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/noDuplicateClasses/valid.html
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `domains` field in `declare_lint_rule!` to tag rules that belong to specific concepts like testing or frameworks
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Run `just l` to lint analyzer rule code
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.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: For new lint rules in changesets, show an example of invalid case in inline code or code block
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should check syntax according to language specification and emit error diagnostics
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
🧬 Code graph analysis (3)
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsx (1)
crates/biome_js_analyze/tests/specs/nursery/noUndeclaredEnvVars/validDynamicAccess.js (1)
prefix(8-8)
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (2)
crates/biome_analyze/src/rule.rs (2)
sources(642-645)same(255-260)crates/biome_html_factory/src/make.rs (1)
html_string_literal(10-17)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs (3)
crates/biome_js_factory/src/make.rs (3)
js_string_literal(24-31)js_string_literal_single_quotes(34-41)js_template_chunk(63-70)crates/biome_analyze/src/rule.rs (2)
same(255-260)recommended(627-630)crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (4)
run(69-119)diagnostic(121-144)state(123-127)action(146-175)
🔇 Additional comments (7)
crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs (1)
1-1: LGTM!Good call making
any_class_string_likepublic to allow reuse by the newNoDuplicateClassesrule, avoiding code duplication.crates/biome_html_analyze/tests/specs/nursery/noDuplicateClasses/valid.html (1)
1-16: LGTM!Good coverage of valid HTML class attribute scenarios. The test cases appropriately mirror the JSX tests.
.changeset/no-duplicate-classes-rule.md (1)
1-12: LGTM!Clear and informative changeset entry. Good attribution to the source plugin.
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/invalid.jsx (1)
1-23: LGTM!Comprehensive test coverage for invalid cases. Good inclusion of edge cases like arbitrary values, complex variant classes, and template literals.
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (1)
12-49: LGTM!Well-documented rule with clear examples. The
version: "next"andsourceswith.same()are correctly configured per guidelines.crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs (2)
200-228: LGTM!Thoughtful handling of template literal boundaries. The logic correctly preserves spaces when adjacent to template expressions, ensuring fixes like
${prefix} p-4 p-4 ${suffix}→${prefix} p-4 ${suffix}maintain proper spacing.
16-77: LGTM!Excellent documentation with clear examples. Good reuse of
UseSortedClassesOptionsfor consistent configuration across related rules.
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
|
Thanks for the feedback @dyc3! Addressing your points:
This contrasts with useSortedClasses which is more subjective (sorting preference). Finally, the JS rule shares options with useSortedClasses to control which attributes (class, className, custom) and functions (clsx, cn, cva, etc.) are checked. The HTML rule only checks class attributes since that's the standard HTML attribute. |
noDuplicateTailwindClasses nursery rulenoDuplicateClasses nursery rule for JS and HTML
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (2)
93-93: Consider usingFxHashSetfor consistency with the JS rule.The JS rule (line 106-107) uses
rustc_hash::FxHashSet, which is typically faster for small sets. Usingstd::collections::HashSethere works fine, butFxHashSetwould be more consistent across the codebase.Suggested change
-use std::collections::HashSet; +use rustc_hash::FxHashSet;And in
run():- let mut seen: HashSet<&str> = HashSet::default(); - let mut duplicate_set: HashSet<&str> = HashSet::default(); + let mut seen: FxHashSet<&str> = FxHashSet::default(); + let mut duplicate_set: FxHashSet<&str> = FxHashSet::default();
175-183: Helpful utility for single-quoted HTML strings.Clean helper function. Consider adding a brief rustdoc comment explaining when this is needed (i.e., when
make::html_string_literaldefaults to double quotes).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
-
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
🧰 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_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*valid* : Create test files prefixed with `valid` for code that should not trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*invalid* : Create test files prefixed with `invalid` for code that should trigger the rule
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
📚 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_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/tests/specs/**/*.jsonc : Use `.jsonc` files to contain arrays of code snippet strings for snapshot tests
Applied to files:
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.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_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `deprecated` field to `declare_lint_rule!` macro when deprecating a rule
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : The first paragraph of rule documentation must be a single line describing what the rule does
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Safe` in `declare_lint_rule!` for safe code actions
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `declare_lint_rule!` macro to declare analyzer rule types and implement the RuleMeta trait
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `sources` field with `RuleSource` to cite ESLint or other rules that inspired the implementation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Run `just l` to lint analyzer rule code
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.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: For new lint rules in changesets, show an example of invalid case in inline code or code block
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should check syntax according to language specification and emit error diagnostics
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.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_js_analyze/src/lint/nursery/no_duplicate_classes.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs (1)
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (4)
run(69-119)diagnostic(121-141)state(123-127)action(143-172)
🔇 Additional comments (14)
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (6)
1-11: LGTM!Imports are well-organised and minimal for the rule's requirements.
12-50: Well-documented rule declaration.The rustdoc is clear with good examples. Rule metadata is correct: nursery placement,
version: "next",FixKind::Safe, and properRuleSourceattribution with.same(). As per coding guidelines, inline rustdoc is the right approach here.
52-61: Clear state struct with inline documentation.The rustdoc comments on each field are helpful. One minor observation: the
is_single_quotefield is specific to the HTML rule (the JS rule doesn't need it since it handles quote detection differently inaction()).
69-119: Solid deduplication logic.The algorithm correctly preserves insertion order for the deduplicated result whilst tracking duplicates separately. Sorting duplicates (line 112) ensures deterministic diagnostic output across runs.
121-141: Clean diagnostic implementation.The pluralisation logic is now handled cleanly with a pre-formatted message string, addressing the previous review feedback about the
<Emphasis>wrapper.
143-172: Quote preservation in auto-fix looks correct.The action properly detects single vs double quotes and creates the replacement token accordingly. Using the helper function for single quotes keeps the code tidy.
crates/biome_js_analyze/tests/specs/nursery/noDuplicateClasses/valid.jsx (1)
1-23: Good test coverage for valid cases.The test file covers a solid range of scenarios: no duplicates, similar-but-different classes (e.g.,
p-4 px-4 py-4), empty/single class, quote styles, and template literals. This aligns with the learnings for creatingvalid-prefixed test files.crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs (7)
1-15: Well-structured imports.Using
FxHashSetfromrustc_hashis the right choice for performance. ReusingUseSortedClassesOptionsfrom the existinguseSortedClassesrule is excellent for configuration consistency.
16-77: Comprehensive rule documentation.Excellent rustdoc with clear examples for both invalid and valid cases. The options section nicely references the
useSortedClassesrule, making it clear to users that configuration is shared. Rule metadata is correct with.same()for the ESLint source.
79-86: Leaner state struct than the HTML counterpart.Sensibly omits
is_single_quotesince the JS rule detects quote style directly from the node inaction().
94-132: Clean run implementation leveraging shared infrastructure.The
should_visitcheck (line 98) properly delegates to the options, ensuring this rule respects the same attribute/function configuration asuseSortedClasses. The deduplication algorithm mirrors the HTML rule correctly.
134-154: Diagnostic matches HTML rule.Pluralisation and formatting are consistent with the HTML implementation. Good to see the same diagnostic style across both rules.
197-225: Careful handling of template chunk boundaries.The logic to preserve leading/trailing spaces when adjacent to template expressions is thoughtful. This ensures
\${prefix} foo bar ${suffix}`` doesn't lose the spaces that separate the interpolated values from the class names.One edge case to consider: if
deduplicatedis empty (all classes were duplicates of each other), you'd end up with just a space. However, sinceduplicate_set.is_empty()returns early on line 120, there's always at least one class indeduplicated_parts, so this is safe.
160-196: Quote style preservation across node types.Each match arm correctly detects the original quote style and uses the appropriate factory function. The fallback to
preferred_quote()/preferred_jsx_quote()when the token is unavailable is a sensible default.
|
@mldangelo from now on, can you please keep our issue template and stick to it? We aren't interested in future works, past works, etc. If you use AI, please make sure to at least do so as long as it aligns with our templates. Or write stuff yourself. Please update the template with the information we require. |
@ematipico appreciate the feedback! I removed references to other things I would like to port (is there a good place to discuss these or should I just open PRs?). I've also updated the description to follow the template. Let me know if there's anything else I should adjust. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
crates/biome_cli/src/execute/migrate/eslint_any_rule_to_biome.rsis excluded by!**/migrate/eslint_any_rule_to_biome.rsand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand 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 (4)
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_rule_options/src/lib.rscrates/biome_rule_options/src/no_duplicate_classes.rs
✅ Files skipped from review due to trivial changes (1)
- crates/biome_rule_options/src/lib.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/no_duplicate_classes.rscrates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
🧠 Learnings (26)
📓 Common learnings
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Use `deny_unknown_fields` in serde derive macro for rule options
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Wrap rule options fields in `Option<>` to properly track set and unset options during merge
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Rule options struct must derive `Deserializable`, `Serialize`, `Deserialize`, and optionally `JsonSchema`
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Use `rename_all = "camelCase"` in serde derive macro for rule options
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Implement `Merge` trait for rule options to support configuration inheritance
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/biome_rule_options/lib/**/*.rs : Rule options must be placed inside the `biome_rule_options` crate
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.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_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Rule documentation must include `## Options` section if the rule has options
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `options` code block property for rule-specific configuration snippets in documentation
Applied to files:
crates/biome_rule_options/src/no_duplicate_classes.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/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should perform static analysis of source code to detect invalid or error-prone patterns and emit diagnostics with proposed fixes
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Add `issue_number` field to `declare_lint_rule!` macro for work-in-progress rules
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/nursery/**/*.rs : Place new rules inside the `nursery` group during development
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).same()` when implementing a rule that matches the behavior of an ESLint rule
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.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_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Specify `fix_kind: FixKind::Safe` in `declare_lint_rule!` for safe code actions
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : The first paragraph of rule documentation must be a single line describing what the rule does
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `RuleSource::Eslint(...).inspired()` when implementing a rule inspired by but with different behavior than an ESLint rule
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Use `declare_lint_rule!` macro to declare analyzer rule types and implement the RuleMeta trait
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rscrates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `deprecated` field to `declare_lint_rule!` macro when deprecating a rule
Applied to files:
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Assist rules should detect refactoring opportunities and emit code action signals
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/**/*.rs : Add `sources` field with `RuleSource` to cite ESLint or other rules that inspired the implementation
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Run `just l` to lint analyzer rule code
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.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: For new lint rules in changesets, show an example of invalid case in inline code or code block
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
📚 Learning: 2025-12-19T12:53:30.413Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-12-19T12:53:30.413Z
Learning: Applies to crates/biome_analyze/**/*analyze/src/lint/**/*.rs : Lint rules should check syntax according to language specification and emit error diagnostics
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs
🧬 Code graph analysis (1)
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs (2)
crates/biome_js_factory/src/make.rs (3)
js_string_literal(24-31)js_string_literal_single_quotes(34-41)js_template_chunk(63-70)crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (4)
run(69-119)diagnostic(121-143)state(123-127)action(145-174)
🔇 Additional comments (5)
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs (2)
69-119: Deduplication logic is sound.The use of
HashSetfor tracking seen and duplicate classes is appropriate, and the logic correctly preserves the first occurrence of each class whilst identifying duplicates.
145-185: Quote-style preservation works correctly.The fix logic properly detects and maintains the original quote style (single vs double) when constructing the replacement token.
crates/biome_js_analyze/src/lint/nursery/no_duplicate_classes.rs (2)
88-92: Good reuse ofUseSortedClassesOptions.Sharing configuration with
useSortedClassesensures consistent handling ofattributesandfunctionsoptions across related rules.
199-227: Template chunk space preservation is correct.The logic to preserve leading/trailing spaces when a template chunk is adjacent to template expressions (e.g.
`${var} flex flex`) is necessary and correctly implemented. Without this, the fix would produce`${var}flex`instead of`${var} flex`.crates/biome_rule_options/src/no_duplicate_classes.rs (1)
1-6: This struct is generated as part of Biome's standard lint rule codegen pattern.Even empty or unused options structs are expected to exist in
biome_rule_options/for each rule, whether or not the corresponding rule actually references them. This is intentional and not an issue.
ematipico
left a comment
There was a problem hiding this comment.
The rules/actions require more work:
- diagnostics don't follow our standards
- newlines aren't handled
- sorting can break people's code
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
crates/biome_html_analyze/src/lint/nursery/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
I believe I addressed all three points:
Please let me know if you'd like to see anything else and happy new year! |
|
Hi, can this get a review when you all get a chance? Thank you! |
dcd1ee3 to
0e01e7e
Compare
|
Resolved merge conflicts again. This is ready for another look whenever you all have a moment. Thank you! |
ematipico
left a comment
There was a problem hiding this comment.
Awesome, thank you. I think we're almost there! Two things to address still: options and shared function
crates/biome_html_analyze/src/assist/source/no_duplicate_classes.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_html_analyze/src/assist/source/no_duplicate_classes.rs`:
- Around line 65-73: In run (RuleContext<Self>) when checking the attribute
name, replace the case-sensitive comparison on name_token.text_trimmed() with a
case-insensitive check using eq_ignore_ascii_case("class") so attributes like
CLASS or Class are treated as "class"; update the conditional that currently
returns None to use name_token.text_trimmed().eq_ignore_ascii_case("class") (or
its negation) so the rule applies regardless of attribute name casing.
🧹 Nitpick comments (1)
crates/biome_html_analyze/src/assist/source/no_duplicate_classes.rs (1)
99-123: Avoid hard‑coding the category string. Ifrule_category!()is available for assist rules, it keeps diagnostics aligned with rule metadata and prevents drift.♻️ Suggested refactor
- RuleDiagnostic::new( - category!("assist/source/noDuplicateClasses"), + RuleDiagnostic::new( + rule_category!(), node.range(), markup! { "This class string contains a duplicate class." }, ) @@ - RuleDiagnostic::new( - category!("assist/source/noDuplicateClasses"), + RuleDiagnostic::new( + rule_category!(), node.range(), markup! { "This class string contains duplicate classes." }, )
b17ff63 to
128e257
Compare
128e257 to
35cc8f0
Compare
Thank you so much for the review @ematipico Both have been addressed. I also simplified NoDuplicateClassesOptions by delegating serialization/deserialization to UseSortedClassesOptions instead of duplicating the visitor. It should be good to take another look. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/biome_html_analyze/src/assist.rs`:
- Around line 1-3: The file contains a duplicated crate-level doc comment string
"//! Generated file, do not edit by hand, see `xtask/codegen`" (appearing on
lines 1 and 3); remove the redundant occurrence so only a single crate-level
generated-file comment remains, or alternatively fix the code generator that
emits this comment to ensure it only writes the header once (search for the
exact doc-comment string in the generation logic or in assist.rs and deduplicate
it).
In `@crates/biome_js_analyze/src/assist/source/no_duplicate_classes.rs`:
- Line 1: Remove the unused import std::borrow::Cow from the top of
no_duplicate_classes.rs; locate the use declaration (use std::borrow::Cow;) and
delete it so there are no unused imports in the module (no other code changes
required).
🧹 Nitpick comments (5)
crates/biome_html_analyze/src/assist/source/no_duplicate_classes.rs (1)
104-133: Preferrule_category!()over manualcategory!()strings in diagnostics.Both the HTML and JS versions of this rule manually spell out the category string in
diagnostic(). Therule_category!()macro provides compile-time verification and avoids typo risk. Compare withuse_sorted_classes.rswhich usesrule_category!().Suggested change
- RuleDiagnostic::new( - category!("assist/source/noDuplicateClasses"), + RuleDiagnostic::new( + rule_category!(),Apply in both branches (lines 106 and 124).
Based on learnings: "Use the
rule_category!()macro in diagnostic implementations instead of manually parsing category strings to ensure compile-time verification and inject categories statically."crates/biome_js_analyze/src/shared/any_class_string_like.rs (1)
115-118: Minor:to_string()allocation for pattern matching.
tag.to_string()allocates aStringfrom the AST node. This is likely necessary here sincematch_functionneeds the full dotted expression (e.g.,tw.div), but worth noting per project conventions favouring&str/TokenTextwhere possible.crates/biome_js_analyze/src/assist/source/no_duplicate_classes.rs (3)
86-123: Userule_category!()instead of manualcategory!()string.Same issue as the HTML counterpart — prefer the compile-time–verified macro.
- category!("assist/source/noDuplicateClasses"), + rule_category!(),Apply on both lines 91 and 109.
Based on learnings: "Use the
rule_category!()macro in diagnostic implementations instead of manually parsing category strings to ensure compile-time verification and inject categories statically."
86-123: Diagnostic logic is duplicated between JS and HTML rules.The
diagnostic()implementations in both rules are nearly identical. As reviewer ematipico noted, extracting shared deduplication logic (including diagnostics construction) intobiome_analyze::sharedwould reduce this duplication. No action needed now if it's planned for a follow-up.
52-59: State struct placement aboveimpl Rule.Per project conventions, helper structs should be placed below the
impl Ruleblock, with the exception of node unions used in theQuerytype.DuplicateClassesStateis theStatetype, not aQueryunion. A minor nit — feel free to defer.Based on learnings: "all helper functions, structs, and enums must be placed below the
impl Ruleblock."
35cc8f0 to
49df46d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@xtask/codegen/src/generate_analyzer.rs`:
- Around line 238-244: generate_html_analyzer currently calls
generate_category("lint"/"assist") unconditionally; add the same
directory-existence guards used in other analyzers so you only call
generate_category if base_path.join("lint") and base_path.join("assist") exist.
Also make the function build and collect categories_and_groups like the other
generators and pass them into generate_build_script before calling
update_html_registry_builder; update references in generate_html_analyzer to
produce categories_and_groups, invoke
generate_build_script(categories_and_groups, &base_path) and then call
update_html_registry_builder(analyzers).
🧹 Nitpick comments (3)
crates/biome_analyze/src/shared/class_dedup.rs (1)
48-81: Consider pre-allocatingtokenswith estimated capacity.For large class strings (common in Tailwind-heavy codebases), a rough estimate like
value_str.len() / 4could avoid a few reallocations. Entirely optional — the current approach is correct.crates/biome_js_analyze/src/shared/any_class_string_like.rs (1)
115-120: Minor:to_string()allocation on the static member expression.Line 117 calls
tag.to_string()which heap-allocates. Per project conventions, prefer&strorTokenTextfor comparisons. That said, this is pre-existing code being moved, so feel free to defer.crates/biome_js_analyze/src/assist/source/no_duplicate_classes.rs (1)
86-123: Minor inconsistency in diagnostic formatting between single and multiple duplicates.Single duplicate (line 98) uses
<Emphasis>markup, but the multiple duplicates branch (line 116) wraps the joined list in plain double-quote characters. This results in different visual treatment depending on count.Not a blocker — the messages are clear either way. If you want consistency, you could wrap the list in
<Emphasis>as well:Optional diff
- .note(markup! { - "The classes "{duplicates_list}" appear multiple times." - }) + .note(markup! { + "The classes "<Emphasis>{duplicates_list}</Emphasis>" appear multiple times." + })
49df46d to
5a58b99
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@crates/biome_analyze/src/shared/class_dedup.rs`:
- Around line 41-58: The plural branch builds a plain comma-separated
duplicates_list string without wrapping each class name in Emphasis, causing
inconsistent rendering versus the singular branch; update the else branch that
constructs the RuleDiagnostic (the code using
duplicates.iter().map(...).collect::<Vec<&str>>().join(", ") and the subsequent
markup! note) to either (A) build the note markup dynamically so each duplicate
is inserted as <Emphasis>{...}</Emphasis> (compose a markup! with interleaved
Emphasis nodes instead of a single joined string), or (B) avoid joining and
instead call diagnostic.note(...) repeatedly for each item in duplicates (as
suggested) so each note wraps &**dup in <Emphasis>{...}</Emphasis>; target the
code that constructs duplicates_list, the RuleDiagnostic::new call, and the
.note(...) so the plural path mirrors the singular Emphasis formatting.
In `@crates/biome_html_analyze/src/assist/source.rs`:
- Around line 1-3: Remove the duplicated header line "//! Generated file, do not
edit by hand, see `xtask/codegen`" from
crates/biome_html_analyze/src/assist/source.rs (delete the redundant occurrence)
and then fix the code generator template in xtask/codegen so it emits that
header only once; search for the exact header string in the codegen templates
and in other generated source.rs outputs to ensure the template change prevents
future duplicates.
🧹 Nitpick comments (3)
crates/biome_rule_options/src/no_duplicate_classes.rs (1)
11-12: Consider#[serde(transparent)]to simplify.The manual
SerializeandDeserializeimpls delegate straight to the inner type —#[derive(Serialize, Deserialize)]with#[serde(transparent)]on the struct achieves the same with less boilerplate.♻️ Suggested simplification
-#[derive(Default, Clone, Debug, Eq, PartialEq)] +#[derive(Default, Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[serde(transparent)] pub struct NoDuplicateClassesOptions(UseSortedClassesOptions);Then remove the manual
impl Serialize(lines 29–36) andimpl<'de> Deserialize<'de>(lines 39–46).crates/biome_js_analyze/src/shared/any_class_string_like.rs (2)
115-120:to_string()allocates on every visit — consider avoiding.
tag.to_string()onJsStaticMemberExpressioncreates a heapStringeach time this path is hit. Since this runs per template chunk, it could add up.If there's a way to reconstruct the dotted name from
object()andmember()tokens without a fullDisplayallocation, that'd be preferable. If not, this is at least worth a// TODOnoting the allocation. As per coding guidelines: "Avoid string allocations by using&strorTokenTextfor comparisons instead of callingto_string()".
77-132:should_visitalways returnsSome(true)orNone— neverSome(false).The
Option<bool>return type suggests three states, but only two are used. A simplerOption<()>or justboolwould be more self-documenting. Not blocking since this is existing logic being moved, but worth noting for a future cleanup.
6ef098f to
19f6e85
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/biome_js_analyze/tests/specs/source/noDuplicateClasses/invalid.jsx`:
- Around line 33-38: Remove the duplicated utility-function test cases from
invalid.jsx: delete the cn("flex flex"); clsx("p-4 m-2 p-4"); and cn({ "flex
flex": isActive }); lines because functions aren't enabled in invalid.jsx (no
.options.json) and these cases are already covered in with-functions.jsx; ensure
only the non-function-based invalid tests remain in invalid.jsx so diagnostics
remain meaningful.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/shared/any_class_string_like.rs (1)
134-134: Nit:valuedoc comment could be a bit more descriptive.The doc comment for
valueis quite terse compared toshould_visit. Consider mentioning that it returns the inner text content (excluding quotes) of the class string node, which would help consumers understand the semantics without reading the match arms.
crates/biome_js_analyze/tests/specs/source/noDuplicateClasses/invalid.jsx
Show resolved
Hide resolved
Add a new assist action that detects and removes duplicate CSS classes in JSX class/className attributes, utility function calls (clsx, cn, cva), and HTML class attributes. - Shared class deduplication logic in biome_analyze::shared::class_dedup - Shared AnyClassStringLike type in biome_js_analyze::shared - Options type shared between JS and HTML rules via NoDuplicateClassesOptions - HTML assist support via codegen changes to generate_analyzer.rs - Inspired by eslint-plugin-better-tailwindcss/no-duplicate-classes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
19f6e85 to
eccfeb3
Compare
|
Thank you @ematipico and @dyc3! |
Summary
Adds the
noDuplicateClassesassist action to detect and remove duplicate CSS classes.For JSX files: Supports
class,classNameattributes and utility functions likeclsx,cn,cva.For HTML files: Checks
classattributes. This is the first assist action for HTML.Inspired by
eslint-plugin-better-tailwindcss/no-duplicate-classes.Test Plan
Snapshot tests for valid/invalid cases:
crates/biome_js_analyze/tests/specs/source/noDuplicateClasses/(JSX)crates/biome_html_analyze/tests/specs/source/noDuplicateClasses/(HTML)All tests passing.
Docs
Documentation included in rustdoc comments within the rule implementations.