+
Skip to content

Conversation

ryan-m-walker
Copy link
Contributor

Summary

This PR moves the get_test_services function from the xtask to the `biome_test_utils crate so that it can be shared more easily with the Biomejs website repo in the codegen process.

See this PR for reference

Test Plan

n/a

Docs

n/a

Copy link

changeset-bot bot commented Sep 8, 2025

⚠️ No Changeset found

Latest commit: 705f108

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Walkthrough

The patch adds biome_js_analyze = { workspace = true } to crates/biome_test_utils/Cargo.toml and introduces pub fn get_test_services<S: BuildHasher>(file_source: JsFileSource, files: &HashMap<String, String, S>) -> JsAnalyzerServices in crates/biome_test_utils/src/lib.rs. The new function builds an in-memory filesystem, parses package.json and tsconfig.json when present, constructs a ModuleGraph and ProjectLayout, updates the graph for provided JS/TS files, and returns JsAnalyzerServices (fast-path returns defaults for empty input). xtask/rules_check/src/lib.rs now imports and uses this shared get_test_services, removing its local implementation.

Possibly related PRs

Suggested labels

L-JavaScript

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and succinctly describes the primary change—moving the get_test_services function into the biome_test_utils crate—uses the standard “chore:” prefix, and gives reviewers immediate context without unnecessary detail.
Description Check ✅ Passed The description directly outlines the intent to relocate get_test_services for sharing with the Biomejs website repo, references the related PR for context, and aligns fully with the changes introduced in the pull request.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
		  - name: "Undocumented Breaking Changes"
			  mode: "warning"
			  instructions: |
				  Flag potential breaking changes that are not documented:
				  1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
				  2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
				  3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the A-Tooling Area: internal tools label Sep 8, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (4)
crates/biome_test_utils/src/lib.rs (4)

625-627: Be explicit with defaults to avoid inference surprises.

Make the empty-case construction of services explicit; it’s a touch clearer for readers and future refactors.

Apply:

-        return JsAnalyzerServices::from((Default::default(), Default::default(), file_source));
+        return JsAnalyzerServices::from((
+            Arc::<ModuleGraph>::default(),
+            Arc::<ProjectLayout>::default(),
+            file_source,
+        ));

635-637: Optionally normalise relative paths to an absolute root.

Normalising to /{path} matches how xtask builds test paths and makes module resolution predictable across callers.

Apply:

-        let path_buf = Utf8PathBuf::from(path);
+        let path_buf = if Utf8Path::new(path).is_absolute() {
+            Utf8PathBuf::from(path)
+        } else {
+            Utf8PathBuf::from("/").join(path)
+        };

658-660: Don’t unimplemented! on unknown manifests.

Failing hard here will abort docs checks for benign files (e.g. biome.json, jsconfig.json). Prefer ignoring unknown manifests.

Apply:

-                _ => unimplemented!("Unhandled manifest: {biome_path}"),
+                _ => {
+                    // Unknown manifest type for the module graph: ignore gracefully.
+                }

614-672: Nice extraction; consider tiny robustness tweaks.

Overall move is tidy and de-duplicates logic. After the small safety fixes above, this helper should be reusable by the website without surprises. If you want, I can add a couple of focused tests (empty map, relative manifest path, multi-file graph).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb0b38 and 6dcdc81.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock and included by **
📒 Files selected for processing (3)
  • crates/biome_test_utils/Cargo.toml (1 hunks)
  • crates/biome_test_utils/src/lib.rs (3 hunks)
  • xtask/rules_check/src/lib.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

  • crates/biome_test_utils/Cargo.toml
  • 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:

  • crates/biome_test_utils/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_test_utils/Cargo.toml
crates/*/Cargo.toml

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Place new crates under the crates/ directory

Files:

  • crates/biome_test_utils/Cargo.toml
**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Format all Rust source files before committing (just f)

Files:

  • crates/biome_test_utils/src/lib.rs
  • xtask/rules_check/src/lib.rs
🧠 Learnings (16)
📓 Common learnings
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" }
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
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js/jsx/ts/tsx)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/**/*.rs : Place all new rules in the nursery group under biome_js_analyze/lib/src/lint/nursery
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : In declare_lint_rule! metadata, set version: "next"
📚 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:

  • crates/biome_test_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:

  • crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-09-07T17:35:00.490Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.490Z
Learning: Applies to Cargo.toml : Define shared workspace dependencies in the root Cargo.toml using the [workspace.dependencies] table

Applied to files:

  • crates/biome_test_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.rs : Implement the Workspace trait in src/workspace.rs

Applied to files:

  • crates/biome_test_utils/Cargo.toml
  • xtask/rules_check/src/lib.rs
📚 Learning: 2025-09-07T17:35:00.490Z
Learnt from: CR
PR: biomejs/biome#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-09-07T17:35:00.490Z
Learning: Applies to crates/**/Cargo.toml : In internal crates, reference internal dependencies with workspace = true

Applied to files:

  • crates/biome_test_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:

  • crates/biome_test_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:

  • crates/biome_test_utils/Cargo.toml
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : In declare_lint_rule! metadata, set version: "next"

Applied to files:

  • crates/biome_test_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:

  • crates/biome_test_utils/Cargo.toml
  • xtask/rules_check/src/lib.rs
📚 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 : Use dbg_write! to debug and inspect the written IR elements during formatter development

Applied to files:

  • crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-11T11:46:05.836Z
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

Applied to files:

  • crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-09-05T09:13:58.901Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Prefer Box<[_]> over Vec<_> for Signals when emitting multiple diagnostics

Applied to files:

  • crates/biome_test_utils/src/lib.rs
📚 Learning: 2025-08-11T11:46:05.836Z
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 : Types implementing Diagnostic must also implement Debug (e.g., use #[derive(Debug, Diagnostic)])

Applied to files:

  • crates/biome_test_utils/src/lib.rs
📚 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
📚 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:

  • xtask/rules_check/src/lib.rs
🧬 Code graph analysis (2)
crates/biome_test_utils/src/lib.rs (4)
packages/@biomejs/js-api/src/wasm.ts (1)
  • BiomePath (1-1)
crates/biome_json_parser/src/lib.rs (1)
  • parse_json (23-26)
crates/biome_lsp/src/server.rs (1)
  • default (540-542)
crates/biome_module_graph/tests/snap/mod.rs (1)
  • new (21-27)
xtask/rules_check/src/lib.rs (1)
crates/biome_test_utils/src/lib.rs (2)
  • get_added_paths (199-220)
  • get_test_services (621-672)
⏰ 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: Parser conformance
  • GitHub Check: Test Node.js API
  • GitHub Check: Lint project (depot-windows-2022)
  • GitHub Check: Check Dependencies
  • GitHub Check: Test (depot-ubuntu-24.04-arm-16)
  • GitHub Check: Documentation
  • GitHub Check: Test (depot-windows-2022-16)
  • GitHub Check: Bench (biome_graphql_formatter)
  • GitHub Check: Bench (biome_module_graph)
  • GitHub Check: Bench (biome_json_parser)
  • GitHub Check: Bench (biome_graphql_parser)
  • GitHub Check: Bench (biome_configuration)
  • GitHub Check: Bench (biome_package)
  • GitHub Check: Bench (biome_html_formatter)
  • GitHub Check: Bench (biome_html_parser)
  • GitHub Check: Bench (biome_js_analyze)
  • GitHub Check: Bench (biome_css_parser)
  • GitHub Check: Bench (biome_json_formatter)
  • GitHub Check: Bench (biome_json_analyze)
  • GitHub Check: Bench (biome_js_parser)
  • GitHub Check: Bench (biome_css_analyze)
  • GitHub Check: Bench (biome_css_formatter)
  • GitHub Check: Bench (biome_js_formatter)
  • GitHub Check: autofix
🔇 Additional comments (7)
crates/biome_test_utils/Cargo.toml (1)

25-25: Dependency addition looks right.

Adding biome_js_analyze = { workspace = true } aligns with our workspace policy and the new usage in get_test_services.

crates/biome_test_utils/src/lib.rs (4)

10-11: New imports are appropriate.

MemoryFileSystem and JsAnalyzerServices are required for the new helper. All good.


14-14: Importing parse_json is correct.

Needed for manifest parsing in the new helper.


27-27: HashMap import: OK.

Used by the public helper signature.


30-30: BuildHasher import: OK.

Keeps the helper generic over the hasher.

xtask/rules_check/src/lib.rs (2)

29-29: Importing the shared helper is the right call.

Good consolidation; keeps xtask lean.


29-29: Resolved: only biome_test_utils::get_test_services is in use No other local definitions or re-exports of get_test_services were found across the Rust sources.

Comment on lines +642 to +645
path_buf.parent().unwrap().into(),
&parsed.syntax().as_send().unwrap(),
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid unwrap on parent() to prevent panics with relative paths.

path_buf.parent().unwrap() will panic for relative paths like "package.json". We already use a safer pattern elsewhere in this file; mirror it here.

Apply:

-                    layout.insert_serialized_node_manifest(
-                        path_buf.parent().unwrap().into(),
-                        &parsed.syntax().as_send().unwrap(),
-                    );
+                    let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default();
+                    layout.insert_serialized_node_manifest(
+                        dir,
+                        &parsed.syntax().as_send().unwrap(),
+                    );
-                    layout.insert_serialized_tsconfig(
-                        path_buf.parent().unwrap().into(),
-                        &parsed.syntax().as_send().unwrap(),
-                    );
+                    let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default();
+                    layout.insert_serialized_tsconfig(
+                        dir,
+                        &parsed.syntax().as_send().unwrap(),
+                    );

Also applies to: 654-656

🤖 Prompt for AI Agents
In crates/biome_test_utils/src/lib.rs around lines 642-645 (and also apply the
same fix at 654-656), replace the unconditional path_buf.parent().unwrap() with
a safe check: obtain the parent via if let Some(parent) = path_buf.parent() {
use parent.into() } else { return/continue/handle the no-parent case the same
way used elsewhere in this file (e.g., skip this insertion or use
Path::new(".").into() as appropriate) } so the code no longer panics for
relative paths like "package.json".

Copy link

codspeed-hq bot commented Sep 8, 2025

CodSpeed Performance Report

Merging #7438 will not alter performance

Comparing ryan-m-walker:chore/move-get-test-services-function-to-test-utils (705f108) with main (75b6a0d)

Summary

✅ 133 untouched benchmarks

@siketyan
Copy link
Member

siketyan commented Sep 8, 2025

@ryan-m-walker Can you address clippy errors?

Copy link
Contributor

@arendjr arendjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments on the functions docs.

ryan-m-walker and others added 2 commits September 8, 2025 12:21
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

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)

15-15: Cull unused imports to silence Clippy

These are no longer referenced here.

-use biome_fs::{BiomePath, MemoryFileSystem};
+use biome_fs::BiomePath;
- use biome_js_analyze::JsAnalyzerServices;
- use biome_module_graph::ModuleGraph;
- use biome_project_layout::ProjectLayout;
- use std::sync::Arc;

Also applies to: 17-17, 23-24, 36-36

♻️ Duplicate comments (1)
crates/biome_test_utils/src/lib.rs (1)

642-646: Don’t unwrap() parent dirs or syntax handles; handle relative paths and parse failures gracefully

parent().unwrap() will panic when path has no parent (e.g. "package.json"). as_send().unwrap() can also panic. Use safe fallbacks.

-                    layout.insert_serialized_node_manifest(
-                        path_buf.parent().unwrap().into(),
-                        &parsed.syntax().as_send().unwrap(),
-                    );
+                    let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default();
+                    if let Some(syntax) = parsed.syntax().as_send() {
+                        layout.insert_serialized_node_manifest(dir, &syntax);
+                    }
-                    layout.insert_serialized_tsconfig(
-                        path_buf.parent().unwrap().into(),
-                        &parsed.syntax().as_send().unwrap(),
-                    );
+                    let dir = path_buf.parent().map(|p| p.to_path_buf()).unwrap_or_default();
+                    if let Some(syntax) = parsed.syntax().as_send() {
+                        layout.insert_serialized_tsconfig(dir, &syntax);
+                    }

Also applies to: 654-656

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb0b38 and 256d5cc.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock and included by **
📒 Files selected for processing (3)
  • crates/biome_test_utils/Cargo.toml (1 hunks)
  • crates/biome_test_utils/src/lib.rs (3 hunks)
  • xtask/rules_check/src/lib.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

  • crates/biome_test_utils/src/lib.rs
  • crates/biome_test_utils/Cargo.toml
**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Format all Rust source files before committing (just f)

Files:

  • crates/biome_test_utils/src/lib.rs
  • xtask/rules_check/src/lib.rs
**/*.toml

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Format all TOML files before committing (just f, via taplo-cli)

Files:

  • crates/biome_test_utils/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_test_utils/Cargo.toml
crates/*/Cargo.toml

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Place new crates under the crates/ directory

Files:

  • crates/biome_test_utils/Cargo.toml
⏰ 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). (19)
  • GitHub Check: Test Node.js API
  • GitHub Check: Bench (biome_graphql_formatter)
  • GitHub Check: Bench (biome_package)
  • GitHub Check: Bench (biome_module_graph)
  • GitHub Check: Bench (biome_json_formatter)
  • GitHub Check: Bench (biome_json_analyze)
  • GitHub Check: Bench (biome_html_formatter)
  • GitHub Check: Bench (biome_configuration)
  • GitHub Check: Bench (biome_css_parser)
  • GitHub Check: Bench (biome_html_parser)
  • GitHub Check: Bench (biome_graphql_parser)
  • GitHub Check: Bench (biome_css_formatter)
  • GitHub Check: Bench (biome_json_parser)
  • GitHub Check: Bench (biome_css_analyze)
  • GitHub Check: Bench (biome_js_analyze)
  • GitHub Check: Bench (biome_js_formatter)
  • GitHub Check: Bench (biome_js_parser)
  • GitHub Check: Parser conformance
  • GitHub Check: autofix
🔇 Additional comments (1)
crates/biome_test_utils/Cargo.toml (1)

25-25: LGTM: workspace dep added correctly

biome_js_analyze = { workspace = true } matches the workspace dependency policy.

Comment on lines +627 to +628
return JsAnalyzerServices::from((Default::default(), Default::default(), file_source));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Unify return types: wrap defaults in Arc for consistency

Avoid mixing tuple types for From. Use Arc in the empty-path fast path too.

-    if files.is_empty() {
-        return JsAnalyzerServices::from((Default::default(), Default::default(), file_source));
-    }
+    if files.is_empty() {
+        let module_graph = Arc::new(ModuleGraph::default());
+        let layout = Arc::new(ProjectLayout::default());
+        return JsAnalyzerServices::from((module_graph, layout, file_source));
+    }
📝 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.

Suggested change
return JsAnalyzerServices::from((Default::default(), Default::default(), file_source));
}
if files.is_empty() {
let module_graph = Arc::new(ModuleGraph::default());
let layout = Arc::new(ProjectLayout::default());
return JsAnalyzerServices::from((module_graph, layout, file_source));
}
🤖 Prompt for AI Agents
In crates/biome_test_utils/src/lib.rs around lines 627-628, the fast-path return
uses raw Default tuple instead of wrapping defaults in Arc like other branches;
change the returned tuple to wrap the default values in Arc (e.g.,
Arc::new(Default::default()) for the shared components) so the From conversion
sees the same Arc-wrapped types as the non-empty-path path, ensuring consistent
return types.

Comment on lines +659 to +661
_ => unimplemented!("Unhandled manifest: {biome_path}"),
}
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace unimplemented! with a no-op to avoid panics in test utilities

Unknown manifests shouldn’t blow up the runner; just ignore them (the file still lands in the in-memory FS below).

-                _ => unimplemented!("Unhandled manifest: {biome_path}"),
+                _ => {
+                    // Ignore unknown manifests in tests; keep them in the in-memory FS.
+                }
📝 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.

Suggested change
_ => unimplemented!("Unhandled manifest: {biome_path}"),
}
} else {
_ => {
// Ignore unknown manifests in tests; keep them in the in-memory FS.
}
🤖 Prompt for AI Agents
In crates/biome_test_utils/src/lib.rs around lines 659-661, the match arm
currently calls unimplemented!("Unhandled manifest: {biome_path}") which causes
a panic for unknown manifests; replace that with a no-op so the runner ignores
unknown manifests (e.g., remove the unimplemented! call and leave the arm empty
or add a simple comment/log without panicking) so the file still lands in the
in-memory FS and tests do not abort.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
xtask/rules_check/src/lib.rs (2)

533-539: Sanity-check: wrong language passed to create_analyzer_options for CSS/GraphQL

These branches use JsonLanguage; they likely should use CssLanguage and GraphqlLanguage respectively.

-                let options = create_analyzer_options::<JsonLanguage>(
+                let options = create_analyzer_options::<CssLanguage>(
                     &workspace_settings,
                     project_key,
                     &file_path,
                     test,
                 );
-                let options = create_analyzer_options::<JsonLanguage>(
+                let options = create_analyzer_options::<GraphqlLanguage>(
                     &workspace_settings,
                     project_key,
                     &file_path,
                     test,
                 );

Also applies to: 579-585


341-343: Minor: prefer borrowing when building BiomePath

Avoids constructing by value; matches typical usage elsewhere.

-    let path = BiomePath::new(Utf8PathBuf::from(&file_path));
+    let path_buf = Utf8PathBuf::from(&file_path);
+    let path = BiomePath::new(&path_buf);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 256d5cc and 705f108.

📒 Files selected for processing (1)
  • xtask/rules_check/src/lib.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Format all Rust source files before committing (just f)

Files:

  • xtask/rules_check/src/lib.rs
🧠 Learnings (4)
📓 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/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.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: 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" }
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use quick_test.rs for ad-hoc testing; set RuleFilter to the rule (e.g., RuleFilter::Rule("nursery", "useAwesomeTrick")) and adjust SOURCE
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/lib/src/lint/nursery/**/*.rs : Place all new rules in the nursery group under biome_js_analyze/lib/src/lint/nursery
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : Set the language field in declare_lint_rule! to the most specific applicable language (js/jsx/ts/tsx)
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-09-05T09:13:58.901Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/**/src/**/*.rs : When porting a lint rule, set sources in declare_lint_rule! (RuleSource::Eslint(...).same() or .inspired())
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:50:12.090Z
Learning: Applies to crates/biome_js_type_info/{src,resolver,biome_module_graph/src}/**/*.rs : Implement and use type resolution via the TypeResolver trait; resolvers own TypeStore vectors and provide fast by-id and hashed lookups
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
📚 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:

  • 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/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs

Applied to files:

  • 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:

  • xtask/rules_check/src/lib.rs
🧬 Code graph analysis (1)
xtask/rules_check/src/lib.rs (1)
crates/biome_test_utils/src/lib.rs (1)
  • get_test_services (622-673)
⏰ 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). (25)
  • GitHub Check: Documentation
  • GitHub Check: Test (depot-ubuntu-24.04-arm-16)
  • GitHub Check: Test (depot-windows-2022-16)
  • GitHub Check: Check Dependencies
  • GitHub Check: Lint project (depot-windows-2022)
  • GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
  • GitHub Check: Bench (biome_html_parser)
  • GitHub Check: Bench (biome_configuration)
  • GitHub Check: Bench (biome_package)
  • GitHub Check: Bench (biome_json_parser)
  • GitHub Check: Test Node.js API
  • GitHub Check: Bench (biome_html_formatter)
  • GitHub Check: Bench (biome_module_graph)
  • GitHub Check: Bench (biome_json_formatter)
  • GitHub Check: Bench (biome_json_analyze)
  • GitHub Check: Bench (biome_graphql_formatter)
  • GitHub Check: Bench (biome_graphql_parser)
  • GitHub Check: Bench (biome_css_parser)
  • GitHub Check: Bench (biome_css_analyze)
  • GitHub Check: Bench (biome_css_formatter)
  • GitHub Check: Bench (biome_js_formatter)
  • GitHub Check: Bench (biome_js_parser)
  • GitHub Check: Bench (biome_js_analyze)
  • GitHub Check: autofix
  • GitHub Check: Parser conformance
🔇 Additional comments (4)
xtask/rules_check/src/lib.rs (4)

26-26: Switching to biome_test_utils::get_test_services is spot on

Good consolidation; removes duplication and makes reuse easy. Nothing else to do here.


15-15: BiomePath import is correct

Used in create_analyzer_options; all good.


20-20: Narrowing to JsonParserOptions only

Clean import; avoids pulling parse_json unnecessarily.


10-10: Don’t remove the Console import
The Console trait is needed for the console.println(...) calls on EnvConsole, so the original use biome_console::{Console, markup}; must remain.

Likely an incorrect or invalid review comment.

@ryan-m-walker
Copy link
Contributor Author

@siketyan done!

@arendjr arendjr merged commit 755ea73 into biomejs:main Sep 9, 2025
30 checks passed
kedevked pushed a commit to kedevked/biome that referenced this pull request Sep 22, 2025
…iomejs#7438)

Co-authored-by: Arend van Beelen jr. <arend@arendjr.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Tooling Area: internal tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载