feat(biome_graphql_analyze): implement useDeprecatedDate
#7620
Conversation
🦋 Changeset detectedLatest commit: 4c3824a 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 #7620 will not alter performanceComparing Summary
Footnotes
|
WalkthroughAdds a new nursery GraphQL lint rule Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)crates/biome_*/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{rs,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
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). (23)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs (4)
88-93: Enforce strictYYYY‑MM‑DDbefore/while parsing.The code calls
.parse()without format gating, but the note says the argument “must match theYYYY‑MM‑DDformat”. Add a lightweight pre‑check (length + hyphen positions + digits) or use a format‑specific parser, then fall back toInvalidon mismatch.Also applies to: 135-136
68-74: De‑duplicate argument name resolution.The “default to
deletionDatewhen empty” logic is duplicated. Compute it once (helper function or store it inState) and reuse in bothrunanddiagnostic.- let argument_name = ctx - .options() - .argument_name - .is_empty() - .then(|| "deletionDate".into()) - .unwrap_or(ctx.options().argument_name.clone()); + let argument_name = resolve_argument_name(ctx.options());Also applies to: 106-112
44-48: Limit enum visibility.
DeprecatedDateIssuedoesn’t need to be public; keep it private to avoid expanding the public surface.-pub enum DeprecatedDateIssue { +enum DeprecatedDateIssue {
50-55: TrimStatepayload (optional).
diagnosticgets the span fromctx.query()and doesn’t use the storedGraphqlDirective. Considertype State = DeprecatedDateIssueto reduce allocations and state churn.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockand included by**crates/biome_configuration/src/analyzer/linter/rules.rsis excluded by!**/rules.rsand included by**crates/biome_diagnostics_categories/src/categories.rsis excluded by!**/categories.rsand included by**crates/biome_graphql_analyze/src/lint/nursery.rsis excluded by!**/nursery.rsand included by**crates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphql.snapis excluded by!**/*.snapand included by**packages/@biomejs/biome/configuration_schema.jsonis excluded by!**/configuration_schema.jsonand included by**
📒 Files selected for processing (11)
.changeset/ninety-hotels-grin.md(1 hunks)Cargo.toml(1 hunks)crates/biome_graphql_analyze/Cargo.toml(1 hunks)crates/biome_graphql_analyze/src/lint.rs(1 hunks)crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs(1 hunks)crates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphql(1 hunks)crates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/valid.graphql(1 hunks)crates/biome_graphql_syntax/src/lib.rs(2 hunks)crates/biome_graphql_syntax/src/string_value_ext.rs(2 hunks)crates/biome_rule_options/src/lib.rs(1 hunks)crates/biome_rule_options/src/use_deprecated_date.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_rule_options/src/lib.rscrates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/valid.graphqlcrates/biome_graphql_syntax/src/string_value_ext.rscrates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rscrates/biome_graphql_analyze/src/lint.rscrates/biome_graphql_syntax/src/lib.rscrates/biome_rule_options/src/use_deprecated_date.rscrates/biome_graphql_analyze/Cargo.toml
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_rule_options/src/lib.rscrates/biome_graphql_syntax/src/string_value_ext.rscrates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rscrates/biome_graphql_analyze/src/lint.rscrates/biome_graphql_syntax/src/lib.rscrates/biome_rule_options/src/use_deprecated_date.rs
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_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/valid.graphqlcrates/biome_graphql_syntax/src/string_value_ext.rscrates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rscrates/biome_graphql_analyze/src/lint.rscrates/biome_graphql_syntax/src/lib.rscrates/biome_graphql_analyze/Cargo.toml
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphqlcrates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/valid.graphql
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all TOML files before committing (just f, via taplo-cli)
Files:
Cargo.tomlcrates/biome_graphql_analyze/Cargo.toml
Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Define shared workspace dependencies in the root Cargo.toml using the [workspace.dependencies] table
Files:
Cargo.toml
crates/**/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
crates/**/Cargo.toml: In internal crates, reference internal dependencies with workspace = true
Use path dependencies for dev-dependencies in internal crates to avoid requiring published versions
Files:
crates/biome_graphql_analyze/Cargo.toml
crates/*/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Place new crates under the crates/ directory
Files:
crates/biome_graphql_analyze/Cargo.toml
.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/ninety-hotels-grin.md
🧠 Learnings (6)
📓 Common learnings
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 : When deprecating a rule, add `deprecated: "<reason>"` to `declare_lint_rule!`
📚 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 : When deprecating a rule, add `deprecated: "<reason>"` to `declare_lint_rule!`
Applied to files:
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs.changeset/ninety-hotels-grin.md
📚 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_graphql_analyze/src/lint/nursery/use_deprecated_date.rscrates/biome_graphql_analyze/src/lint.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/**/nursery/**/*.rs : Place all new rules in the nursery group (implement rule files under a `.../src/*/nursery/` directory)
Applied to files:
crates/biome_graphql_analyze/src/lint.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 : Avoid deep indentation and panics; prefer `?`, `ok()?`, and combinators (`map`, `and_then`, `filter`) over nested `if let`/`unwrap`/`expect`
Applied to files:
crates/biome_graphql_analyze/src/lint.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 : Options types must implement serialization/deserialization and schema support using derives: `Serialize`, `Deserialize`, `Deserializable`, and `#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]`
Applied to files:
crates/biome_rule_options/src/use_deprecated_date.rs
🧬 Code graph analysis (3)
crates/biome_graphql_syntax/src/string_value_ext.rs (1)
crates/biome_graphql_syntax/src/lib.rs (1)
inner_string_text(119-128)
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs (1)
crates/biome_analyze/src/rule.rs (3)
sources(575-578)same(244-249)recommended(560-563)
crates/biome_graphql_syntax/src/lib.rs (1)
crates/biome_graphql_syntax/src/string_value_ext.rs (1)
inner_string_text(20-22)
⏰ 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). (23)
- GitHub Check: Check Dependencies
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Check JS Files
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Test Node.js API
- GitHub Check: autofix
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
Outdated
Show resolved
Hide resolved
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
https://github.com/biomejs/biome/pull/7620/files#diff-dc2003cef1c68fb2632517e6837aba01d97ca6fa976306765dda643f679504d6R68-R73
Is there an easier way to set a default value for a string option?
CodeRabbit suggested the answer 😁 |
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
Outdated
Show resolved
Hide resolved
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
Outdated
Show resolved
Hide resolved
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
Outdated
Show resolved
Hide resolved
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
Outdated
Show resolved
Hide resolved
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
Outdated
Show resolved
Hide resolved
08c4323 to
4977673
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs (1)
92-96: UTC “today” looks good.Thanks for switching to a UTC‑based clock; this removes local‑TZ flakiness from date comparisons.
🧹 Nitpick comments (3)
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs (3)
62-75: Avoid allocating for name comparisons.Using to_trimmed_string() allocates. Prefer borrowing token text (e.g. trimmed_text on the name token) and compare to &str to keep this allocation‑free. Same for the argument name check.
Can you switch both name comparisons to borrowed text APIs available on GraphQL name/value tokens?
70-70: Trim clones from options.You don’t need to clone the option string; borrow it as &str for both the comparison and diagnostics.
- let argument_name = ctx.options().argument_name.clone(); + let argument_name = ctx.options().argument_name.as_str();- let argument_name = ctx.options().argument_name.clone(); + let argument_name = ctx.options().argument_name.as_str();Also applies to: 103-103
85-90: Streamline parsing and comparison.The is_err() + ok()? pattern is a bit clunky. Parse once, branch, then compare with the parsed value.
- let due_date_value = argument_string_value.inner_string_text().ok()?; - let due_date = due_date_value.text().parse(); - - if due_date.is_err() { - return Some((DeprecatedDateIssue::Invalid, node.clone())); - } + let due_date_value = argument_string_value.inner_string_text().ok()?; + let due_date: jiff::civil::Date = match due_date_value.text().parse() { + Ok(d) => d, + Err(_) => return Some((DeprecatedDateIssue::Invalid, node.clone())), + }; let now = Timestamp::now().to_zoned(TimeZone::UTC).date(); - if now > due_date.ok()? { + if now > due_date { return Some((DeprecatedDateIssue::Due, node.clone())); }Also applies to: 94-96
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphql.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs(1 hunks)crates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_graphql_analyze/tests/specs/nursery/useDeprecatedDate/invalid.graphql
🧰 Additional context used
📓 Path-based instructions (3)
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_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
🧠 Learnings (1)
📚 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 : When deprecating a rule, add `deprecated: "<reason>"` to `declare_lint_rule!`
Applied to files:
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs
🧬 Code graph analysis (1)
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs (1)
crates/biome_analyze/src/rule.rs (3)
sources(575-578)same(244-249)recommended(560-563)
⏰ 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). (23)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: autofix
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Check JS Files
🔇 Additional comments (3)
crates/biome_graphql_analyze/src/lint/nursery/use_deprecated_date.rs (3)
10-43: Bold move, clean landing.Rule metadata wiring looks correct: name, language, source mapping and non‑recommended status all make sense. Nice use of .same() to attribute source.
130-141: Grammar fix landed.“The deprecation has passed its due date.” reads well and is user‑friendly.
127-129: Format message is accurate
jiff::civil::Date::FromStrstrictly enforces zero-padded YYYY-MM-DD (e.g. “2024-06-19”), so the existing “must match theYYYY-MM-DDformat” wording is correct.
f7c6a1f to
fbb7f3f
Compare
Summary
Implement Eslint Graphql's
require-deprecation-dateRelated # #3303
Test Plan
Docs