chore: validate diagnostics in examples of Promise-related rules#7403
Conversation
|
WalkthroughThis PR updates documentation and doctest annotations. In CONTRIBUTING.md, it adds a note (twice) that using file= is also useful to trigger type inference for single-file examples. In no_floating_promises.rs and no_misused_promises.rs, all invalid examples are annotated with expect_diagnostic and mapped to specific fixture files; valid examples reference dedicated files. No runtime logic or public API changes. Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
crates/biome_analyze/CONTRIBUTING.md (1)
1214-1215: Wording tweak for clarity (single-file phrasing).“individual files” reads a bit clunky. Suggest “single‑file examples” and “enable type inference”.
- This is also useful to trigger type inference even for examples that only consist of individual files. + This also helps enable type inference for single‑file examples.crates/biome_js_analyze/src/lint/nursery/no_misused_promises.rs (2)
31-64: Prefer TypeScript code fences for a TS-only rule.This rule declares
language: "ts"and relies on types. Usingtsfences across these invalid examples makes type inference deterministic and avoids JS-mode edge cases.-/// ```js,expect_diagnostic,file=promise-in-condition.js +/// ```ts,expect_diagnostic,file=promise-in-condition.ts @@ -/// ```js,expect_diagnostic,file=promise-in-ternary-condition.js +/// ```ts,expect_diagnostic,file=promise-in-ternary-condition.ts @@ -/// ```js,expect_diagnostic,file=promise-in-filter.js +/// ```ts,expect_diagnostic,file=promise-in-filter.ts @@ -/// ```js,expect_diagnostic,file=promise-while-condition.js +/// ```ts,expect_diagnostic,file=promise-while-condition.ts @@ -/// ```js,expect_diagnostic,file=spread-promise.js +/// ```ts,expect_diagnostic,file=spread-promise.ts @@ -/// ```js,expect_diagnostic,file=promise-in-forEach.js +/// ```ts,expect_diagnostic,file=promise-in-forEach.ts
67-82: Valid block: consistent and comprehensive.Using a single
file=valid-promises.jsblock is tidy and keeps the harness fast. If you adopt TS fences above, consider aligning this tovalid-promises.tsfor consistency.crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs (2)
55-58: Minor polish: make thePromise.allexample unambiguously a statement.A trailing semicolon avoids accidental continuation in concatenated snippets.
-/// Promise.all([p1, p2, p3]) +/// Promise.all([p1, p2, p3]);
116-152: Valid examples look good; consider declaringp1/p2/p3to aid type inference.Not required for this rule, but adding trivial declarations makes the snippet self‑contained.
/// ```ts,file=valid-examples.ts +/// const p1: Promise<unknown> = Promise.resolve(1); +/// const p2: Promise<unknown> = Promise.resolve(2); +/// const p3: Promise<unknown> = Promise.resolve(3); /// async function returnsPromise(): Promise<string> {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
crates/biome_analyze/CONTRIBUTING.md(1 hunks)crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs(7 hunks)crates/biome_js_analyze/src/lint/nursery/no_misused_promises.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_analyze/CONTRIBUTING.mdcrates/biome_js_analyze/src/lint/nursery/no_misused_promises.rscrates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_js_analyze/src/lint/nursery/no_misused_promises.rscrates/biome_js_analyze/src/lint/nursery/no_floating_promises.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_js_analyze/src/lint/nursery/no_misused_promises.rscrates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs
🧠 Learnings (4)
📚 Learning: 2025-09-05T06:39:46.267Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T06:39:46.267Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/*.rs : Use `file=<path>` in doc code blocks for multi-file test snippets when cross-file analysis is needed
Applied to files:
crates/biome_analyze/CONTRIBUTING.md
📚 Learning: 2025-09-05T06:39:46.267Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T06:39:46.267Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/*.rs : Rule documentation (Rust doc comments) must start with a single-line summary, include `## Examples` with `### Invalid` first then `### Valid`, and document options under `## Options`
Applied to files:
crates/biome_analyze/CONTRIBUTING.mdcrates/biome_js_analyze/src/lint/nursery/no_misused_promises.rscrates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs
📚 Learning: 2025-09-05T06:39:46.267Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T06:39:46.267Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/*.rs : Code blocks in rule docs must specify a language; invalid examples use `,expect_diagnostic` and must emit exactly one diagnostic; use `ignore` sparingly
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_misused_promises.rscrates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs
📚 Learning: 2025-09-05T06:39:46.267Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T06:39:46.267Z
Learning: Applies to crates/biome_analyze/crates/**/lib/src/**/*.rs : When showing rule-specific configuration in docs, use `json`/`jsonc` with `,options` (or `,full_options` for full biome.json); follow the modifier order `<lang>[,expect_diagnostic][,(options|full_options|use_options)][,ignore][,file=path]`
Applied to files:
crates/biome_js_analyze/src/lint/nursery/no_misused_promises.rs
⏰ 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). (24)
- GitHub Check: autofix
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Test Node.js API
- GitHub Check: Documentation
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
🔇 Additional comments (3)
crates/biome_js_analyze/src/lint/nursery/no_misused_promises.rs (1)
31-64: Good: diagnostics are now validated and mapped to fixture files.All invalid snippets use
expect_diagnosticandfile=…with the recommended modifier ordering. Nice one.crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs (2)
34-113: Nice: invalid examples annotated and scoped via file=…, order matches guidelines.These should each emit exactly one diagnostic under
noFloatingPromises. Good adherence to CONTRIBUTING.
34-152: Sanity-check doctests in CI
Ensure each invalid snippet emits exactly one diagnostic and valids stay clean. From the repo root, run:set -euo pipefail cargo test -p biome_js_analyze --doc --all-features rg -nP '```(js|ts)(?![ ,].*\b(expect_diagnostic|full_options|options|use_options|ignore|file=)\b)' -C0 crates/biome_js_analyze/src/lint/nursery/no_floating_promises.rs
CodSpeed Performance ReportMerging #7403 will not alter performanceComparing Summary
|
Summary
This adds annotations to the
noFloatingPromisesandnoMisusedPromisesrules so that we validate the diagnostics from their examples.Test Plan
CI should remain green.
Docs
Added a little explanation that the
file=<path>annotation can be useful beyond multi-file snippets, for instance to initialise type inference.