-
-
Notifications
You must be signed in to change notification settings - Fork 724
chore: introduce ruledoc utils #7541
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
|
230593b
to
516d2d4
Compare
/// | ||
/// The builder can be reused to create cheap instances of analyzer services | ||
/// for multiple code blocks. | ||
pub struct AnalyzerServicesBuilder { |
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.
I refactored a little bit how we create services for code block analysis. Previously, this was done ad-hoc, which resulted in indexing of the module graph and project layout for every block separately. Now, when multi-file rules are analysed, we index once and reuse the module graph and project layout between code blocks.
WalkthroughAdds a new workspace crate crates/biome_ruledoc_utils, introducing CodeBlock parsing and an AnalyzerServicesBuilder to construct in-memory analyser services. Updates xtask/rules_check to use CodeBlock and AnalyzerServicesBuilder, replacing the previous CodeBlockTest flow and options handling. Removes get_test_services from biome_test_utils and switches dependencies in xtask/rules_check to biome_ruledoc_utils. Adjusts workspace members in the root Cargo.toml. Minor documentation example tweaks in use_sorted_keys.rs. No changes to existing public APIs outside the moved/replaced test harness types. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
/// | ||
/// ```js,expect_diff | ||
/// { | ||
/// const obj = { |
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.
This fixes some diagnostics that were previously not caught by just lint-rules
. Now they get successfully detected, because the CodeBlock parser now supports expect_diff
the same way as the website.
CodSpeed Performance ReportMerging #7541 will not alter performanceComparing Summary
|
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xtask/rules_check/src/lib.rs (1)
469-474
: Add a post-analysis guard forexpect_diff
.Fail when no diagnostic (hence no diff) was produced.
Apply this diff:
if diagnostics.has_error { bail!("Please resolve the error in the above code block(s).") } + if test.expect_diff && diagnostics.all_diagnostics.is_empty() { + bail!( + "Analysis of '{group}/{rule}' on the following code block returned no diff where one was expected.\n\n{code}" + ); + }
♻️ Duplicate comments (1)
xtask/rules_check/src/lib.rs (1)
785-791
: Nice: single shared services builder per section.Avoids repeated indexing; good call.
🧹 Nitpick comments (6)
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (1)
66-86
: Nit: add a trailing semicolon for consistency.The third example ends with
}
while the others use};
. It’s valid JS, but consistency helps prevent tiny diffs later.Apply this diff to align it:
- /// } + /// };crates/biome_test_utils/src/lib.rs (1)
172-194
: Consider documenting the new replacement pattern.With
get_test_services
removed, developers will need to use the newAnalyzerServicesBuilder
frombiome_ruledoc_utils
. Consider adding a comment here or updating the module documentation to guide users to the new approach.Add a doc comment like:
/// Creates a module graph that is initialized for the given `input_file`. /// /// It uses an [OsFileSystem] initialized for the directory in which the test /// file resides and inserts all files from that directory, so that files /// importing each other within that directory will be picked up correctly. /// /// The `project_layout` should be initialized in advance if you want any /// manifest files to be discovered. +/// +/// Note: For creating analyzer services in tests, use `AnalyzerServicesBuilder` +/// from `biome_ruledoc_utils` instead of the previously available `get_test_services`. pub fn module_graph_for_test_file(crates/biome_ruledoc_utils/Cargo.toml (1)
1-12
: Mark this crate as private to avoid accidental publish.Given it depends on internal path crates (e.g. biome_test_utils), set publish = false.
Apply this diff:
[package] authors.workspace = true categories.workspace = true description = "Utilities for running code blocks in rule docs" edition.workspace = true homepage.workspace = true keywords.workspace = true license.workspace = true name = "biome_ruledoc_utils" repository.workspace = true version = "0.0.1" +publish = false
crates/biome_ruledoc_utils/src/lib.rs (1)
14-15
: Import Utf8Path for the new fallbacks.Needed by the suggested changes.
Apply this diff:
-use camino::Utf8PathBuf; +use camino::{Utf8Path, Utf8PathBuf};crates/biome_ruledoc_utils/src/codeblock.rs (2)
621-636
: Docstring token mismatch: usefull_options
(underscore), notfull-options
.The parser accepts
full_options
, so update the comment to avoid confusion.Apply this diff:
- // ```json,full-options + // ```json,full_options
96-111
: Minor: derive file path cheaply.
file_path()
allocates a String even when only borrowing. If this hits hot paths, consider returning Cow<'_, str>. Non-blocking.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
and included by**
📒 Files selected for processing (8)
Cargo.toml
(1 hunks)crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
(2 hunks)crates/biome_ruledoc_utils/Cargo.toml
(1 hunks)crates/biome_ruledoc_utils/src/codeblock.rs
(1 hunks)crates/biome_ruledoc_utils/src/lib.rs
(1 hunks)crates/biome_test_utils/src/lib.rs
(1 hunks)xtask/rules_check/Cargo.toml
(1 hunks)xtask/rules_check/src/lib.rs
(23 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
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/assist/source/use_sorted_keys.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
crates/biome_ruledoc_utils/src/lib.rs
crates/biome_ruledoc_utils/src/codeblock.rs
crates/biome_ruledoc_utils/Cargo.toml
crates/biome_test_utils/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all Rust source files before committing (just f)
Files:
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
crates/biome_ruledoc_utils/src/lib.rs
crates/biome_ruledoc_utils/src/codeblock.rs
xtask/rules_check/src/lib.rs
crates/biome_test_utils/src/lib.rs
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format all TOML files before committing (just f, via taplo-cli)
Files:
Cargo.toml
xtask/rules_check/Cargo.toml
crates/biome_ruledoc_utils/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_ruledoc_utils/Cargo.toml
crates/*/Cargo.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Place new crates under the crates/ directory
Files:
crates/biome_ruledoc_utils/Cargo.toml
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace.rs : Implement the Workspace trait in src/workspace.rs
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Learnt from: CR
PR: biomejs/biome#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:55:30.118Z
Learning: Applies to crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/** : Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
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`)
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 : Code blocks in rule docs must specify language; invalid snippets require `expect_diagnostic`; use `options`/`full_options`/`use_options` markers as appropriate
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/server.rs : Use WorkspaceServer (src/workspace/server.rs) to maintain workspace state in daemon and CLI daemonless modes
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)
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_js_analyze/tests/quick_test.rs : Quick test: edit `tests/quick_test.rs`, remove or comment `#[ignore]`, set `SOURCE`, and adjust `RuleFilter`
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Applies to Cargo.toml : Define shared workspace dependencies in the root Cargo.toml using the [workspace.dependencies] table
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_diagnostics/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:46:05.836Z
Learning: Applies to crates/biome_diagnostics/**/*.rs : Use helper advice types from biome_diagnostics::v2 (CodeFrameAdvice, CommandAdvice, DiffAdvice, LogAdvice) when suitable
📚 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_js_analyze/lib/src/{lint,assist}/**/*.rs : When banning globals (e.g., `noConsoleLog`), check the semantic model to avoid false positives from locally shadowed bindings
Applied to files:
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
📚 Learning: 2025-09-07T17:35:00.517Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Update documentation when features change; for rules/assists/options, use inline Rust rustdoc
Applied to files:
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs
xtask/rules_check/src/lib.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace.rs : Implement the Workspace trait in src/workspace.rs
Applied to files:
Cargo.toml
xtask/rules_check/Cargo.toml
crates/biome_ruledoc_utils/Cargo.toml
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/Cargo.toml : Add biome_js_formatter as a path dependency in Cargo.toml: biome_js_formatter = { version = "0.0.1", path = "../biome_js_formatter" }
Applied to files:
Cargo.toml
crates/biome_ruledoc_utils/Cargo.toml
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-09-07T17:35:00.517Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Applies to Cargo.toml : Define shared workspace dependencies in the root Cargo.toml using the [workspace.dependencies] table
Applied to files:
Cargo.toml
xtask/rules_check/Cargo.toml
crates/biome_ruledoc_utils/Cargo.toml
📚 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/Cargo.toml : Add the specified dev-dependencies (biome_formatter_test, biome_html_factory, biome_html_parser, biome_parser, biome_service, countme with feature enable, iai 0.1.1, quickcheck, quickcheck_macros, tests_macros) to Cargo.toml under [dev-dependencies]
Applied to files:
Cargo.toml
xtask/rules_check/Cargo.toml
crates/biome_ruledoc_utils/Cargo.toml
crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-09-07T17:35:00.517Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.517Z
Learning: Applies to crates/**/Cargo.toml : In internal crates, reference internal dependencies with workspace = true
Applied to files:
Cargo.toml
crates/biome_ruledoc_utils/Cargo.toml
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Applied to files:
Cargo.toml
xtask/rules_check/Cargo.toml
crates/biome_ruledoc_utils/Cargo.toml
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
Cargo.toml
crates/biome_test_utils/src/lib.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/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
Cargo.toml
📚 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:
Cargo.toml
xtask/rules_check/Cargo.toml
crates/biome_ruledoc_utils/src/codeblock.rs
crates/biome_ruledoc_utils/Cargo.toml
xtask/rules_check/src/lib.rs
📚 Learning: 2025-08-17T08:55:30.118Z
Learnt from: CR
PR: biomejs/biome#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T08:55:30.118Z
Learning: Applies to crates/biome_*_{syntax,parser,formatter,analyze,factory,semantic}/** : Maintain the per-language crate structure: biome_{lang}_{syntax,parser,formatter,analyze,factory,semantic}
Applied to files:
Cargo.toml
crates/biome_ruledoc_utils/Cargo.toml
📚 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/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
Cargo.toml
📚 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/lib.rs : Place the plumbing traits and impls (AsFormat, IntoFormat, FormattedIterExt, and their Iterator adapters) in the biome_html_formatter crate’s lib.rs
Applied to files:
Cargo.toml
📚 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/crates/biome_*_syntax/** : Create a new crate named biome_<language>_syntax under crates/
Applied to files:
Cargo.toml
📚 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_js_analyze/tests/quick_test.rs : Quick test: edit `tests/quick_test.rs`, remove or comment `#[ignore]`, set `SOURCE`, and adjust `RuleFilter`
Applied to files:
xtask/rules_check/Cargo.toml
xtask/rules_check/src/lib.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 : Code blocks in rule docs must specify language; invalid snippets require `expect_diagnostic`; use `options`/`full_options`/`use_options` markers as appropriate
Applied to files:
crates/biome_ruledoc_utils/src/codeblock.rs
xtask/rules_check/src/lib.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_ruledoc_utils/Cargo.toml
📚 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 : Recommended rules with domains are enabled only when users enable the matching domains; use `domains` metadata judiciously
Applied to files:
crates/biome_ruledoc_utils/Cargo.toml
📚 Learning: 2025-08-11T11:48:52.001Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:52.001Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Build an intermediate IR via the provided helper APIs when formatting
Applied to files:
crates/biome_test_utils/src/lib.rs
🧬 Code graph analysis (3)
crates/biome_ruledoc_utils/src/lib.rs (2)
crates/biome_json_parser/src/lib.rs (1)
parse_json
(23-26)crates/biome_test_utils/src/lib.rs (1)
get_added_paths
(197-218)
crates/biome_ruledoc_utils/src/codeblock.rs (3)
packages/@biomejs/js-api/src/wasm.ts (1)
BiomePath
(1-1)crates/biome_test_utils/src/lib.rs (2)
settings
(168-168)create_analyzer_options
(45-127)crates/biome_service/src/file_handlers/mod.rs (1)
from_extension
(176-178)
xtask/rules_check/src/lib.rs (2)
crates/biome_ruledoc_utils/src/codeblock.rs (2)
file_path
(108-111)from_str
(117-160)crates/biome_ruledoc_utils/src/lib.rs (1)
from_files
(36-90)
⏰ 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). (5)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_formatter)
🔇 Additional comments (8)
crates/biome_js_analyze/src/assist/source/use_sorted_keys.rs (2)
46-51
: Good call switching to a bound object.Making the example a valid statement (
const obj = {…}
) avoids parser ambiguity and plays nicer with ruledoc execution.
54-63
: Example consistency looks solid.Same improvement here; keeps the snippets executable and clearer for readers.
crates/biome_test_utils/src/lib.rs (2)
3-6
: LGTM! Clean import organisation.The imports are well-organised with the removal of
HashMap
andMemoryFileSystem
aligning with the elimination ofget_test_services
.
14-14
: Verify removal of MemoryFileSystem doesn't break tests.crates/biome_test_utils/src/lib.rs removed MemoryFileSystem from imports; confirm no tests still call get_test_services or reference MemoryFileSystem — run a repo-wide search (e.g. rg -n "get_test_services|MemoryFileSystem" --type rust) and, if found, update those tests to use biome_ruledoc_utils test helpers.
Cargo.toml (1)
186-186
: Workspace wire-up looks good.biome_ruledoc_utils is correctly added under workspace.dependencies for internal use.
xtask/rules_check/Cargo.toml (1)
32-32
: Correct dependency pivot to biome_ruledoc_utils.Clean swap-in; no stray biome_test_utils here. Ship it.
crates/biome_ruledoc_utils/Cargo.toml (1)
13-26
: Workspace-scoped deps look consistent.All internal deps use
workspace = true
; no version drifts.crates/biome_ruledoc_utils/src/lib.rs (1)
82-85
: get_added_paths() asserts on parse errors.This is fine for dev tooling, but it will panic on bad examples. If you want resilience, consider collecting diagnostics instead.
Do you want this to hard-fail on malformed JS in ruledoc examples, or should we degrade gracefully here?
let parsed = parse_json(&src, JsonParserOptions::default()); | ||
layout.insert_serialized_node_manifest( | ||
path_buf.parent().unwrap().into(), | ||
&parsed.syntax().as_send().unwrap(), | ||
); | ||
} | ||
Some("tsconfig.json") => { |
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.
Avoid unwrap that can panic on paths without a parent.
path_buf.parent().unwrap()
will panic if a manifest is at the FS root or a bare filename slips in. Use a safe fallback.
Apply this diff:
- layout.insert_serialized_node_manifest(
- path_buf.parent().unwrap().into(),
- &parsed.syntax().as_send().unwrap(),
- );
+ let manifest_dir = path_buf.parent().unwrap_or(camino::Utf8Path::new("/"));
+ layout.insert_serialized_node_manifest(
+ manifest_dir.into(),
+ &parsed.syntax().as_send().expect("sendable JSON syntax"),
+ );
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let parsed = parse_json(&src, JsonParserOptions::default()); | |
layout.insert_serialized_node_manifest( | |
path_buf.parent().unwrap().into(), | |
&parsed.syntax().as_send().unwrap(), | |
); | |
} | |
Some("tsconfig.json") => { | |
let parsed = parse_json(&src, JsonParserOptions::default()); | |
let manifest_dir = path_buf.parent().unwrap_or(camino::Utf8Path::new("/")); | |
layout.insert_serialized_node_manifest( | |
manifest_dir.into(), | |
&parsed.syntax().as_send().expect("sendable JSON syntax"), | |
); | |
} | |
Some("tsconfig.json") => { |
🤖 Prompt for AI Agents
In crates/biome_ruledoc_utils/src/lib.rs around lines 55 to 61, avoid the
panic-prone call path_buf.parent().unwrap(); instead check for a parent and use
a safe fallback (for example PathBuf::from(".") or an empty PathBuf) when None;
implement this by replacing the unwrap with an if let or match that obtains the
parent path when present and converts it to a PathBuf (or uses the chosen
fallback) before calling layout.insert_serialized_node_manifest so the code
never panics on root or bare filenames.
let parsed = parse_json( | ||
&src, | ||
JsonParserOptions::default() | ||
.with_allow_comments() | ||
.with_allow_trailing_commas(), | ||
); | ||
layout.insert_serialized_tsconfig( | ||
path_buf.parent().unwrap().into(), | ||
&parsed.syntax().as_send().unwrap(), | ||
); |
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.
Same unwrap issue for ts/jsconfig; also support jsconfig.json.
Harden parent handling and accept jsconfig.json equivalently.
Apply this diff:
- Some("tsconfig.json") => {
+ Some("tsconfig.json") | Some("jsconfig.json") => {
let parsed = parse_json(
&src,
JsonParserOptions::default()
.with_allow_comments()
.with_allow_trailing_commas(),
);
- layout.insert_serialized_tsconfig(
- path_buf.parent().unwrap().into(),
- &parsed.syntax().as_send().unwrap(),
- );
+ let manifest_dir = path_buf.parent().unwrap_or(camino::Utf8Path::new("/"));
+ layout.insert_serialized_tsconfig(
+ manifest_dir.into(),
+ &parsed.syntax().as_send().expect("sendable JSONC syntax"),
+ );
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let parsed = parse_json( | |
&src, | |
JsonParserOptions::default() | |
.with_allow_comments() | |
.with_allow_trailing_commas(), | |
); | |
layout.insert_serialized_tsconfig( | |
path_buf.parent().unwrap().into(), | |
&parsed.syntax().as_send().unwrap(), | |
); | |
Some("tsconfig.json") | Some("jsconfig.json") => { | |
let parsed = parse_json( | |
&src, | |
JsonParserOptions::default() | |
.with_allow_comments() | |
.with_allow_trailing_commas(), | |
); | |
let manifest_dir = path_buf.parent().unwrap_or(camino::Utf8Path::new("/")); | |
layout.insert_serialized_tsconfig( | |
manifest_dir.into(), | |
&parsed.syntax().as_send().expect("sendable JSONC syntax"), | |
); | |
} |
_ => unimplemented!("Unhandled manifest: {biome_path}"), | ||
} |
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.
Don’t unimplemented!
on unknown manifests.
This will blow up in tooling. Prefer ignoring or warning.
Apply this diff:
- _ => unimplemented!("Unhandled manifest: {biome_path}"),
+ _ => {
+ // Ignore other manifests (lockfiles etc.) for now.
+ continue;
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
_ => unimplemented!("Unhandled manifest: {biome_path}"), | |
} | |
_ => { | |
// Ignore other manifests (lockfiles etc.) for now. | |
continue; | |
}, | |
} |
🤖 Prompt for AI Agents
In crates/biome_ruledoc_utils/src/lib.rs around lines 73–74, the match arm uses
unimplemented!("Unhandled manifest: {biome_path}") which panics; change this to
a non-panicking behavior by emitting a warning (e.g. log::warn! or eprintln!
with the biome_path) and skipping the unknown manifest instead of crashing —
adjust the control flow to return an appropriate Ok/None or continue so the
function signature (Result/Option) remains satisfied and tooling won't blow up.
} else if self.test.expect_diff { | ||
// ...or there is no diff... | ||
if self.all_diagnostics.is_empty() { | ||
self.has_error = true; | ||
bail!( | ||
"Analysis of '{group}/{rule}' on the following code block returned no diff where one was expected.\n\n{code}", | ||
); | ||
} |
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.
expect_diff
check here is ineffective.
This branch runs only when a diagnostic exists; it won’t catch the “no diff produced” case (zero diagnostics).
Apply this diff to assert after analysis completes (see next comment), and remove this inner check:
- } else if self.test.expect_diff {
- // ...or there is no diff...
- if self.all_diagnostics.is_empty() {
- self.has_error = true;
- bail!(
- "Analysis of '{group}/{rule}' on the following code block returned no diff where one was expected.\n\n{code}",
- );
- }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
} else if self.test.expect_diff { | |
// ...or there is no diff... | |
if self.all_diagnostics.is_empty() { | |
self.has_error = true; | |
bail!( | |
"Analysis of '{group}/{rule}' on the following code block returned no diff where one was expected.\n\n{code}", | |
); | |
} |
🤖 Prompt for AI Agents
In xtask/rules_check/src/lib.rs around lines 194 to 201 the branch checking
self.test.expect_diff is ineffective because it only runs when diagnostics exist
and therefore cannot catch the “no diff produced” case; remove this inner check
and instead add an assertion after the full analysis completes that if
test.expect_diff is true then fail when all_diagnostics.is_empty() (set
has_error and bail with the same message), ensuring the check runs regardless of
whether any diagnostics were emitted during analysis.
Summary
Creates a new crate
biome_ruledoc_utils
, and moved some of the code for parsing ruledoc code blocks there, as well as the logic for creating analyzer services evaluating the code blocks. This avoids a bunch of code duplication between codegen in the main repository and the website repository.The website is updated here: biomejs/website#3129
Test Plan
just lint-rules
still works.Docs
N/A