-
-
Notifications
You must be signed in to change notification settings - Fork 781
feat(noUnknownAtRule): add ignore option
#7745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 4ab6e2a 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 an ignore list to the CSS linter rule suspicious.noUnknownAtRules. Introduces a case-insensitive should_ignore(name, options) and updates NoUnknownAtRules::run to compute the at-rule name once, skip processing when the name matches ignore, and avoid recomputing the token text. Adds Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs (1)
10-50: Consider documenting theignoreoption.The rule documentation explains the rule well but doesn't mention the new
ignoreoption. Adding an example would help users discover this feature.Consider adding a section like this after the existing examples:
/// ```css /// @media (max-width: 960px) { /// body { /// font-size: 13px; /// } /// } /// ``` +/// +/// ## Options +/// +/// Allows ignoring specific at-rules by name (case-insensitive): +/// +/// ```json +/// { +/// "linter": { +/// "rules": { +/// "suspicious": { +/// "noUnknownAtRules": { +/// "options": { +/// "ignore": ["custom-at-rule", "my-framework-rule"] +/// } +/// } +/// } +/// } +/// } +/// } +/// ``` pub NoUnknownAtRules {
📜 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/noUnknownAtRules/valid_with_ignore.css.snapis excluded by!**/*.snapand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (4)
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs(2 hunks)crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css(1 hunks)crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.json(1 hunks)crates/biome_rule_options/src/no_unknown_at_rules.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
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_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.jsoncrates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rscrates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.jsoncrates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rscrates/biome_rule_options/src/no_unknown_at_rules.rscrates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.jsoncrates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rscrates/biome_rule_options/src/no_unknown_at_rules.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rscrates/biome_rule_options/src/no_unknown_at_rules.rs
🧠 Learnings (2)
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
PR: biomejs/biome#7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.json
📚 Learning: 2025-10-02T12:57:33.228Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.228Z
Learning: Applies to crates/biome_analyze/crates/biome_rule_options/lib/*.rs : Define per-rule options in biome_rule_options/lib/<rule>.rs with a dedicated options struct/enum (camelCase serde names, deny_unknown_fields, default) and derive Serialize/Deserialize/Deserializable (and schemars JsonSchema when schema feature is on)
Applied to files:
crates/biome_rule_options/src/no_unknown_at_rules.rs
🧬 Code graph analysis (2)
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
NoUnknownAtRulesOptions(8558-8558)
crates/biome_rule_options/src/no_unknown_at_rules.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
NoUnknownAtRulesOptions(8558-8558)
⏰ 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). (10)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: autofix
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
🔇 Additional comments (5)
crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.json (1)
1-15: LGTM!The test configuration is well-structured and correctly specifies the ignore list for the custom at-rules used in the test file.
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs (2)
56-64: LGTM!The case-insensitive comparison is appropriate for CSS at-rules. The logic is clear and correct.
83-93: Good optimisation!Precomputing the name and reusing it avoids redundant work. The early return for ignored rules is clean.
crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css (1)
1-10: LGTM!The test cases effectively demonstrate the ignore feature with various at-rule names and cases.
crates/biome_rule_options/src/no_unknown_at_rules.rs (1)
3-10: LGTM!The options struct follows the established patterns with proper serde configuration. The
skip_serializing_ifattribute is appropriate for omitting empty ignore lists, and the documentation correctly notes case-insensitive matching.Based on learnings
CodSpeed Performance ReportMerging #7745 will not alter performanceComparing Summary
Footnotes
|
ematipico
left a 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.
Two important things are missing:
- changeset
- docs must be updated to reflect the new option
|
Oh yeah, whoops. I'll fix that up. |
bff2b1d to
be2b0e2
Compare
crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css.snap
Show resolved
Hide resolved
244ce72 to
50c6ff2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
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_analyze/src/lint/suspicious/no_unknown_at_rules.rs (1)
8-8: Includeignorein generatedNoUnknownAtRulesOptions
Theworkspace.tsinterface is auto-generated; adjustxtask/codegento add the newignoreoption.
🧹 Nitpick comments (1)
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs (1)
82-90: Consider using iterator combinators.The function works correctly, but could be more concise:
-fn should_ignore(name: &str, options: &NoUnknownAtRulesOptions) -> bool { - for ignore_pattern in &options.ignore { - if name.eq_ignore_ascii_case(ignore_pattern) { - return true; - } - } - false -} +fn should_ignore(name: &str, options: &NoUnknownAtRulesOptions) -> bool { + options.ignore.iter().any(|p| name.eq_ignore_ascii_case(p)) +}
📜 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/noUnknownAtRules/valid_with_ignore.css.snapis excluded by!**/*.snapand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (5)
.changeset/breezy-suns-leave.md(1 hunks)crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs(3 hunks)crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css(1 hunks)crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.json(1 hunks)crates/biome_rule_options/src/no_unknown_at_rules.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- .changeset/breezy-suns-leave.md
- crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.options.json
- crates/biome_css_analyze/tests/specs/suspicious/noUnknownAtRules/valid_with_ignore.css
- crates/biome_rule_options/src/no_unknown_at_rules.rs
🧰 Additional context used
📓 Path-based instructions (4)
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_analyze/src/lint/suspicious/no_unknown_at_rules.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs
🧬 Code graph analysis (1)
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
NoUnknownAtRulesOptions(8558-8558)
⏰ 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). (10)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: autofix
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_formatter)
🔇 Additional comments (2)
crates/biome_css_analyze/src/lint/suspicious/no_unknown_at_rules.rs (2)
35-67: Documentation looks solid.The new options section clearly explains the
ignorefeature, including case-insensitive matching behaviour. The examples are helpful.
109-118: Well done—name computed once, no waste.The refactoring to compute the name early and check the ignore list before constructing state is clean and efficient.
|
The CI failure looks unrelated. The tests for |
Summary
This adds a simple escape hatch for the
noUnknownAtRulesrule. This was brought up in #7223, and I thought it would be a good idea.Test Plan
Added a test
Docs
updated the doc comment