-
-
Notifications
You must be signed in to change notification settings - Fork 714
feat(lint): port property-no-deprecated
rule from stylelint
#7683
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 38648ed 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 |
CodSpeed Performance ReportMerging #7683 will degrade performances by 11.68%Comparing Summary
Benchmarks breakdown
Footnotes
|
WalkthroughAdds a new nursery linter rule noDeprecatedProperties to Biome’s CSS analyser to detect deprecated CSS properties and suggest replacements where possible. Integrates the Nursery group into the Lint category. Implements rule logic, diagnostics, and an autofix action for property name replacements. Introduces configurable options via NoDeprecatedPropertiesOptions with an ignoreProperties list. Adds tests for valid, invalid, and ignored properties, plus rule options JSON. Updates rule options crate to expose the new options module. Includes a changeset entry. Suggested labels
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
crates/biome_configuration/src/analyzer/linter/rules.rs
is excluded by!**/rules.rs
and included by**
crates/biome_css_analyze/src/lint/nursery.rs
is excluded by!**/nursery.rs
and included by**
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.css.snap
is excluded by!**/*.snap
and included by**
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/invalid.css.snap
is excluded by!**/*.snap
and included by**
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/valid.css.snap
is excluded by!**/*.snap
and included by**
crates/biome_diagnostics_categories/src/categories.rs
is excluded by!**/categories.rs
and included by**
packages/@biomejs/backend-jsonrpc/src/workspace.ts
is excluded by!**/backend-jsonrpc/src/workspace.ts
and included by**
packages/@biomejs/biome/configuration_schema.json
is excluded by!**/configuration_schema.json
and included by**
📒 Files selected for processing (9)
.changeset/afraid-planes-dig.md
(1 hunks)crates/biome_css_analyze/src/lint.rs
(1 hunks)crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs
(1 hunks)crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.css
(1 hunks)crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.options.json
(1 hunks)crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/invalid.css
(1 hunks)crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/valid.css
(1 hunks)crates/biome_rule_options/src/lib.rs
(1 hunks)crates/biome_rule_options/src/no_deprecated_properties.rs
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
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.rs
crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.options.json
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/valid.css
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/invalid.css
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.css
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_css_analyze/src/lint.rs
crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs
crates/biome_rule_options/src/lib.rs
crates/biome_rule_options/src/no_deprecated_properties.rs
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.options.json
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/valid.css
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/invalid.css
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.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.rs
crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs
crates/biome_rule_options/src/lib.rs
crates/biome_rule_options/src/no_deprecated_properties.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.rs
crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs
crates/biome_rule_options/src/lib.rs
crates/biome_rule_options/src/no_deprecated_properties.rs
.changeset/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/**/*.md
: Create changesets using thejust new-changeset
command; do not author them manually
In changeset markdown, only use headers #### or #####
Changeset descriptions must end every sentence with a full stop (.)
For bug fixes, start the changeset description with a linked issue reference like “Fixed #1234”
Prefer past tense for what was done and present tense for current behavior in changesets
Files:
.changeset/afraid-planes-dig.md
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.options.json
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/valid.css
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/invalid.css
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.css
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : Place all new JavaScript lint rules in the nursery group under biome_js_analyze/lib/src/lint/nursery/<rule_name>.rs
📚 Learning: 2025-10-02T12:57:33.209Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/*.rs : Place all new JavaScript lint rules in the nursery group under biome_js_analyze/lib/src/lint/nursery/<rule_name>.rs
Applied to files:
crates/biome_css_analyze/src/lint.rs
crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs
📚 Learning: 2025-10-02T12:57:33.209Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/**/src/**/lint/**/*.rs : When deprecating a rule, add deprecated: "<reason>" in declare_lint_rule! with an explanatory message
Applied to files:
crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs
.changeset/afraid-planes-dig.md
📚 Learning: 2025-10-02T12:57:33.209Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
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_deprecated_properties.rs
📚 Learning: 2025-10-02T12:57:33.209Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/nursery/** : Place snapshot tests for new rules under tests/specs/nursery/<ruleName>/ with files prefixed by invalid* and valid*
Applied to files:
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.options.json
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/valid.css
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/invalid.css
📚 Learning: 2025-10-02T12:57:33.209Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.209Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/**/*.jsonc : Use .jsonc files to hold arrays of code snippets for snapshot tests; these snippets run in script mode (no import/export)
Applied to files:
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.options.json
📚 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/nursery/noDeprecatedProperties/ignoredProperties.options.json
🧬 Code graph analysis (3)
crates/biome_css_analyze/src/lint.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
Nursery
(1627-1772)
crates/biome_css_analyze/src/lint/nursery/no_deprecated_properties.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
NoDeprecatedPropertiesOptions
(8189-8191)crates/biome_css_factory/src/generated/node_factory.rs (1)
css_identifier
(774-779)
crates/biome_rule_options/src/no_deprecated_properties.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
NoDeprecatedPropertiesOptions
(8189-8191)
⏰ 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). (13)
- GitHub Check: Check JS Files
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Documentation
- GitHub Check: autofix
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_configuration)
🔇 Additional comments (9)
crates/biome_rule_options/src/lib.rs (1)
46-46
: LGTM!The module export is correctly placed alphabetically and follows the established naming convention.
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.options.json (1)
1-15
: LGTM!The test configuration correctly enables the rule and specifies the ignore list matching the test fixtures.
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/ignoredProperties.css (1)
1-21
: LGTM!The test fixture clearly demonstrates both ignored and non-ignored properties, matching the configuration in the options JSON.
crates/biome_css_analyze/src/lint.rs (2)
8-8
: LGTM!The nursery module is correctly added in alphabetical order and follows the established pattern.
11-11
: LGTM!The Nursery group is correctly integrated into the Lint category declaration.
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/invalid.css (1)
1-17
: LGTM!The test cases provide good coverage of different deprecated properties including standard properties, vendor-prefixed properties, and the presence of comments.
crates/biome_rule_options/src/no_deprecated_properties.rs (1)
1-9
: LGTM!The options struct follows all the coding guidelines: derives are correct, serde attributes are properly configured with camelCase renaming and deny_unknown_fields, and the schema feature is conditionally enabled. As per coding guidelines.
crates/biome_css_analyze/tests/specs/nursery/noDeprecatedProperties/valid.css (1)
7-12
: No changes needed: the rule correctly implements value-based acceptance for-webkit-box-orient
(accepted only when its value isvertical
)..changeset/afraid-planes-dig.md (1)
5-5
: Inconsistent tense in changeset description.Per coding guidelines, prefer past tense for what was done. Consider: "Added a nursery rule..." rather than "Added a nursery rule... that detects...". Alternatively, restructure to maintain past tense throughout.
Apply this diff:
-Added a nursery rule [`noDeprecatedProperties`](https://biomejs.dev/linter/rules/no-deprecated-properties/) that detects usages of deprecated CSS properties. The rule was ported from the [`property-no-deprecated`](https://stylelint.io/user-guide/rules/property-no-deprecated) rule in Stylelint. +Added a nursery rule [`noDeprecatedProperties`](https://biomejs.dev/linter/rules/no-deprecated-properties/) to detect usages of deprecated CSS properties. The rule was ported from the [`property-no-deprecated`](https://stylelint.io/user-guide/rules/property-no-deprecated) rule in Stylelint.⛔ Skipped due to learnings
Learnt from: CR PR: biomejs/biome#0 File: CONTRIBUTING.md:0-0 Timestamp: 2025-10-02T12:55:57.541Z Learning: Applies to .changeset/**/*.md : Prefer past tense for what was done and present tense for current behavior in changesets
let name = property.name().ok()?.to_trimmed_text(); | ||
let name = name.text(); | ||
|
||
if options | ||
.ignore_properties | ||
.iter() | ||
.any(|ignored_property| ignored_property.as_ref() == name) | ||
{ | ||
return Some(false); | ||
} | ||
|
||
Some(match name { | ||
// Allowed when the value is `vertical` | ||
"-webkit-box-orient" if property.value().to_trimmed_text() == "vertical" => false, | ||
|
||
"-khtml-box-align" | ||
| "-khtml-box-direction" | ||
| "-khtml-box-flex" | ||
| "-khtml-box-lines" | ||
| "-khtml-box-ordinal-group" | ||
| "-khtml-box-orient" | ||
| "-khtml-box-pack" | ||
| "-khtml-user-modify" | ||
| "-moz-box-align" | ||
| "-moz-box-direction" | ||
| "-moz-box-flex" | ||
| "-moz-box-lines" | ||
| "-moz-box-ordinal-group" | ||
| "-moz-box-orient" | ||
| "-moz-box-pack" | ||
| "-moz-user-modify" | ||
| "-ms-box-align" | ||
| "-ms-box-direction" | ||
| "-ms-box-flex" | ||
| "-ms-box-lines" | ||
| "-ms-box-ordinal-group" | ||
| "-ms-box-orient" | ||
| "-ms-box-pack" | ||
| "-webkit-box-align" | ||
| "-webkit-box-direction" | ||
| "-webkit-box-flex" | ||
| "-webkit-box-lines" | ||
| "-webkit-box-ordinal-group" | ||
| "-webkit-box-orient" | ||
| "-webkit-box-pack" | ||
| "-webkit-user-modify" | ||
| "grid-column-gap" | ||
| "grid-gap" | ||
| "grid-row-gap" | ||
| "ime-mode" | ||
| "page-break-after" | ||
| "page-break-before" | ||
| "page-break-inside" | ||
| "position-try-options" | ||
| "scroll-snap-coordinate" | ||
| "scroll-snap-destination" | ||
| "scroll-snap-margin-bottom" | ||
| "scroll-snap-margin-left" | ||
| "scroll-snap-margin-right" | ||
| "scroll-snap-margin-top" | ||
| "scroll-snap-margin" | ||
| "scroll-snap-points-x" | ||
| "scroll-snap-points-y" | ||
| "scroll-snap-type-x" | ||
| "scroll-snap-type-y" | ||
| "word-wrap" | ||
| "clip" => true, | ||
|
||
_ => false, | ||
}) |
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.
Normalise property names before matching
Right now we keep the source casing, so CLIP
or Word-Wrap
sail past the rule and the autofix never triggers. CSS property identifiers and keyword values are ASCII case-insensitive, so we have to fold both the lookup and the ignore list to a common case (and the replacement table too) to catch everything. A straight port of stylelint does this lowering. Please tighten it up like so:
- let name = property.name().ok()?.to_trimmed_text();
- let name = name.text();
-
- if options
- .ignore_properties
- .iter()
- .any(|ignored_property| ignored_property.as_ref() == name)
- {
- return Some(false);
- }
-
- Some(match name {
- // Allowed when the value is `vertical`
- "-webkit-box-orient" if property.value().to_trimmed_text() == "vertical" => false,
+ let name = property.name().ok()?.to_trimmed_text();
+ let name_text = name.text();
+
+ if options
+ .ignore_properties
+ .iter()
+ .any(|ignored_property| ignored_property.eq_ignore_ascii_case(name_text))
+ {
+ return Some(false);
+ }
+
+ let lower_name = name_text.to_ascii_lowercase();
+
+ let is_vertical = property
+ .value()
+ .to_trimmed_text()
+ .text()
+ .eq_ignore_ascii_case("vertical");
+
+ Some(match lower_name.as_str() {
+ // Allowed when the value is `vertical`
+ "-webkit-box-orient" if is_vertical => false,
@@
- | "clip" => true,
+ | "clip" => true,
@@
- Some(match property.name().ok()?.to_trimmed_text().text() {
+ let lower_name = property
+ .name()
+ .ok()?
+ .to_trimmed_text()
+ .text()
+ .to_ascii_lowercase();
+
+ Some(match lower_name.as_str() {
That way upper/mixed-case usage and ignores behave exactly like stylelint, and the suggested fixes appear for them too.
Also applies to: 205-229
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.
It would be better to do a case insensitive comparison to avoid the memory allocation
|
||
Some(match name { | ||
// Allowed when the value is `vertical` | ||
"-webkit-box-orient" if property.value().to_trimmed_text() == "vertical" => false, |
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.
Not sure if the compiler is able to optimise it, like this.
However I know that it's able to optimise code like this
struct DeprecatedProperties;
impl DeprecatedProperties {
fn matches(prop: &str) -> bool {
match prop {}
}
}
Summary
This pull request adds a new rule
noDeprecatedProperties
, ported from theproperty-no-deprecated
rule in stylelint.Test Plan
Added snapshot tests.
Docs
Added documentation comments (copy-pasted from stylelint's docs).