-
-
Notifications
You must be signed in to change notification settings - Fork 714
fix(noDuplicateProperties): false positives in @container
and @starting-style
at-rules
#7581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…rting-style` at-rules
🦋 Changeset detectedLatest commit: 3968566 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe PR treats Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1).changeset/*.md📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-09-07T17:35:00.517Z
Applied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/biome_css_semantic/src/events.rs (1)
244-247
: Missing RuleEnd for new rule starts (unbalanced events).You push RuleStart for CSS_CONTAINER_AT_RULE and CSS_STARTING_STYLE_AT_RULE, but don’t emit RuleEnd on leave. This can skew scoping and any consumers relying on balanced start/end events.
Apply this diff to balance starts/ends:
- if matches!( - node.kind(), - CSS_QUALIFIED_RULE | CSS_NESTED_QUALIFIED_RULE | CSS_MEDIA_AT_RULE - ) { + if matches!( + node.kind(), + CSS_QUALIFIED_RULE + | CSS_NESTED_QUALIFIED_RULE + | CSS_CONTAINER_AT_RULE + | CSS_MEDIA_AT_RULE + | CSS_STARTING_STYLE_AT_RULE + ) { self.stash.push_back(SemanticEvent::RuleEnd); if self.is_in_root_selector { self.stash.push_back(SemanticEvent::RootSelectorEnd); self.is_in_root_selector = false; } }Note: CSS_SUPPORTS_AT_RULE is also started but not ended today; consider including it in a follow-up for consistency.
🧹 Nitpick comments (4)
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/valid.css (1)
34-46
: Valid cases for @starting-style and @container look spot on.These ensure declarations scoped by these at-rules aren’t treated as duplicates of the outer scope. Nice.
If you want to harden this further later: add a case with two sibling @container queries (different conditions) to guard against cross-container false positives.
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/invalid.css (1)
63-77
: Great invalid coverage within the same at-rule scope.The duplicates inside @starting-style/@container blocks should indeed be flagged. Looks good.
Consider adding a variant mixing a duplicate and a distinct property in the same at-rule block to assert per-property grouping behaviour.
.changeset/thin-cases-dig.md (1)
5-5
: Add a tiny example to the changeset for clarity.The entry reads well and follows the style guide. A minimal code block helps users immediately recognise the fix.
Apply this diff to append an example:
Fixed [#7470](https://github.com/biomejs/biome/issues/7470): Fixed false positives for [`noDuplicateProperties`](https://biomejs.dev/linter/rules/no-duplicate-properties/) in `@container` and `@starting-style` at-rules. + +Example of the corrected behaviour. + +```css +/* no diagnostics */ +a { + opacity: 1; + @starting-style { opacity: 0; } +} + +a { + display: block; + @container (min-width: 600px) { display: none; } +} +```crates/biome_css_semantic/src/semantic_model/model.rs (1)
171-172
: Extending AnyRuleStart with container/starting-style is correct.This keeps the semantic model in step with event extraction and the linter’s needs.
Optional: mirror
is_media_rule
withis_container_rule
/is_starting_style_rule
helpers if callers start branching on these.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/invalid.css.snap
is excluded by!**/*.snap
and included by**
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/valid.css.snap
is excluded by!**/*.snap
and included by**
📒 Files selected for processing (5)
.changeset/thin-cases-dig.md
(1 hunks)crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/invalid.css
(1 hunks)crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/valid.css
(1 hunks)crates/biome_css_semantic/src/events.rs
(1 hunks)crates/biome_css_semantic/src/semantic_model/model.rs
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md
: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period
Files:
.changeset/thin-cases-dig.md
crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Files:
crates/biome_css_semantic/src/events.rs
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/valid.css
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/invalid.css
crates/biome_css_semantic/src/semantic_model/model.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_semantic/src/events.rs
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/valid.css
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/invalid.css
crates/biome_css_semantic/src/semantic_model/model.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_css_semantic/src/events.rs
crates/biome_css_semantic/src/semantic_model/model.rs
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/valid.css
crates/biome_css_analyze/tests/specs/suspicious/noDuplicateProperties/invalid.css
🧠 Learnings (5)
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : If a rule returns a code action (implements `action`), add `fix_kind` in `declare_lint_rule!` and use `ctx.metadata().applicability()` when building the action
Applied to files:
crates/biome_css_semantic/src/events.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/**/src/{lint,assist}/**/*.rs : In declare_lint_rule! macros, set `version: "next"` for new or updated rules
Applied to files:
crates/biome_css_semantic/src/events.rs
📚 Learning: 2025-09-10T08:05:22.867Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-10T08:05:22.867Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/**/*.rs : Define per-rule options types in `biome_rule_options` crate (one file per rule, e.g., `lib/use_my_rule.rs`)
Applied to files:
crates/biome_css_semantic/src/events.rs
crates/biome_css_semantic/src/semantic_model/model.rs
📚 Learning: 2025-08-11T11:48:27.774Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.774Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/src/cst.rs : Define FormatHtmlSyntaxNode in cst.rs and implement FormatRule<HtmlSyntaxNode>, plus AsFormat and IntoFormat for HtmlSyntaxNode using the provided mapping code
Applied to files:
crates/biome_css_semantic/src/semantic_model/model.rs
📚 Learning: 2025-08-17T08:57:34.751Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:57:34.751Z
Learning: Applies to crates/biome_parser/xtask/codegen/*.ungram : Union node names must start with Any* (e.g., AnyHtmlAttribute)
Applied to files:
crates/biome_css_semantic/src/semantic_model/model.rs
🧬 Code graph analysis (1)
crates/biome_css_semantic/src/events.rs (1)
crates/biome_css_analyze/src/assist/source/use_sorted_properties.rs (1)
kind
(241-276)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test Node.js API
- GitHub Check: Documentation
- GitHub Check: autofix
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
🔇 Additional comments (3)
crates/biome_css_semantic/src/events.rs (1)
66-69
: Good call: treat @container and @starting-style as rule starts.This aligns event extraction with the semantic model and unblocks the linter fix.
crates/biome_css_semantic/src/semantic_model/model.rs (2)
2-6
: Imports updated to include container/starting-style at-rules — all good.
175-183
: text_trimmed_range handling is complete for new variants.Consistent with other arms; no gaps spotted.
.changeset/thin-cases-dig.md
Outdated
"@biomejs/biome": patch | ||
--- | ||
|
||
Fixed [#7470](https://github.com/biomejs/biome/issues/7470): Fixed false positives for [`noDuplicateProperties`](https://biomejs.dev/linter/rules/no-duplicate-properties/) in `@container` and `@starting-style` at-rules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed [#7470](https://github.com/biomejs/biome/issues/7470): Fixed false positives for [`noDuplicateProperties`](https://biomejs.dev/linter/rules/no-duplicate-properties/) in `@container` and `@starting-style` at-rules. | |
Fixed [#7470](https://github.com/biomejs/biome/issues/7470): solved a false positives for [`noDuplicateProperties`](https://biomejs.dev/linter/rules/no-duplicate-properties/) in `@container` and `@starting-style` at-rules, where ... |
It's not very what was the false positive, could you please explain it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1a83fe8 I added an example to the changeset, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/thin-cases-dig.md
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md
: In changeset files, only use #### or ##### headers; avoid other header levels
Changeset descriptions should use past tense for what you did (e.g., "Added...")
Describe current Biome behavior in present tense within changesets (e.g., "Biome now supports...")
For bug fixes in changesets, start with a link to the issue (e.g., "Fixed #1234: ...")
When referencing rules or assists in changesets, include links to their documentation pages
Include a minimal code block in the changeset when applicable to demonstrate the change
End every sentence in the changeset description with a period
Files:
.changeset/thin-cases-dig.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: autofix
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary
Fixed #7470 and a false positive in
@starting-style
(playground link).Test Plan
@container
and@starting-style
.