fix: --stdin-file-path with nested configuration
#8239
Conversation
🦋 Changeset detectedLatest commit: 290e75a 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 |
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
32e6803 to
0d95d92
Compare
cormacrelf
changed the title
--stdin-file-path with nested configuration--stdin-file-path with nested configuration
This, basically, changed the semantics of stdin. That's a breaking change. There have been discussions around it. However we can't take this change lightly because there are users out there that rely on it. |
|
Warning Rate limit exceeded@cormacrelf has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (16)
WalkthroughNormalises the stdin file path by joining the provided Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/biome_cli/src/commands/scan_kind.rs (1)
7-12: Docs still mention stdin‑specific scan behaviour that no longer exists
get_forced_scan_kindnow ignoresroot_configuration_dir/working_dirand just matches onTraversalMode, but the comment onderive_best_scan_kindstill describes stdin‑based rules. It’d be good to update or trim that bullet so future readers don’t go hunting for logic that was intentionally removed.Also applies to: 24-61
crates/biome_cli/src/execute/std_in.rs (1)
5-41: Centralising stdin handling viaExecution+Stdinlooks solidDeriving
biome_path/contentonce fromStdinand then branching onExecution(is_format/is_check/is_lint/traversal_mode) makes the flow much clearer, and the “print content, then warning” pattern is a nice touch for long inputs. One small hygiene improvement you might consider: in the check/lint path we callopen_filebut, in the earlyis_ignored/is_protectedreturns, never callclose_file, so adding a matchingclose_filethere would keep the workspace state symmetric with the happy path even though this is a short‑lived CLI run.if file_features.is_ignored() { console.append(markup! {{content}}); // Write error last because files may generally be long console.error(markup! { <Warn>"The content was not fixed because the path `"{biome_path.as_str()}"` is ignored."</Warn> }); - return Ok(()); + workspace.close_file(CloseFileParams { + project_key, + path: biome_path.clone(), + })?; + return Ok(()); } if file_features.is_protected() { let protected_diagnostic = WorkspaceError::protected_file(biome_path.to_string()); … console.append(markup! {{content}}); - - return Ok(()); + workspace.close_file(CloseFileParams { + project_key, + path: biome_path.clone(), + })?; + return Ok(()); };Also applies to: 43-60, 81-113, 138-145, 257-266
crates/biome_cli/src/commands/mod.rs (1)
901-1011: stdin configuration path looks correct; consider setting a workspace dir for root‑only projectsThe new
runshort‑circuit for stdin plusconfigure_stdinachieves the goal: you load a root config, optionally overlay a nested config discovered viaConfigurationPathHint::StdinFilePath, merge CLI overrides, and then call intostd_in::runwithout doing any directory scanning. That’s a nice separation from the traversal‑based path.One small behavioural wrinkle: when there is no nested configuration,
project_directoryremainsNone, soupdate_settingsis invoked withworkspace_directory: Noneeven thoughconfiguration.is_root()will typically betrue. InWorkspaceServer::update_settingsthe root branch useslet directory = workspace_directory.unwrap_or_default();to find
.gitignore/.ignore, so in stdin mode you end up probing an empty path instead of the project root, which also deviates fromconfigure_workspace(where we pass an explicit project directory).You might want to mirror the workspace case by deriving a directory for the root‑only stdin scenario, e.g.:
@@ fn configure_stdin(...) - let configuration = self.merge_configuration(root_config, fs, console)?; + let root_configuration_dir = root_config + .directory_path + .clone() + .unwrap_or_else(|| working_dir.clone()); + let configuration = self.merge_configuration(root_config, fs, console)?; @@ - let result = workspace.update_settings(UpdateSettingsParams { - project_key: open_project_result.project_key, - workspace_directory: project_directory, - configuration, - })?; + let mut workspace_directory = project_directory; + if workspace_directory.is_none() && configuration.is_root() { + workspace_directory = Some(BiomePath::new(&root_configuration_dir)); + } + let result = workspace.update_settings(UpdateSettingsParams { + project_key: open_project_result.project_key, + workspace_directory, + configuration, + })?;That keeps stdin’s config walk behaviour while giving VCS/ignore handling a sensible base directory.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
crates/biome_cli/src/commands/check.rs(1 hunks)crates/biome_cli/src/commands/format.rs(1 hunks)crates/biome_cli/src/commands/lint.rs(1 hunks)crates/biome_cli/src/commands/mod.rs(5 hunks)crates/biome_cli/src/commands/scan_kind.rs(1 hunks)crates/biome_cli/src/commands/search.rs(1 hunks)crates/biome_cli/src/execute/mod.rs(2 hunks)crates/biome_cli/src/execute/std_in.rs(5 hunks)crates/biome_configuration/src/lib.rs(3 hunks)crates/biome_service/src/configuration.rs(6 hunks)crates/biome_service/src/workspace/server.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
crates/biome_cli/src/execute/std_in.rs (2)
packages/@biomejs/js-api/src/wasm.ts (2)
ProjectKey(2-2)BiomePath(1-1)crates/biome_cli/src/execute/mod.rs (3)
from(78-80)from(90-92)from(225-239)
crates/biome_service/src/workspace/server.rs (1)
crates/biome_service/src/configuration.rs (1)
load_nested_configuration(166-172)
crates/biome_cli/src/commands/mod.rs (1)
crates/biome_service/src/configuration.rs (2)
load_nested_configuration(166-172)load_configuration(157-163)
🔇 Additional comments (12)
crates/biome_cli/src/commands/format.rs (1)
146-151: Marking Console as unused here is fineRenaming the parameter to
_consoleis a tidy way to keep the trait signature while making it clear there are no side‑effects from the console in this path.crates/biome_cli/src/commands/lint.rs (1)
132-155: Unused Console parameter is correctly markedThe rename to
_consolekeeps the CommandRunner signature consistent while making it explicit that lint execution no longer depends on Console/STDIN here.crates/biome_cli/src/commands/search.rs (1)
60-77: Console decoupling looks goodUsing
_consolewhile keeping the Workspace dependency forparse_patternaligns with the new centralised stdin flow and doesn’t change search semantics.crates/biome_configuration/src/lib.rs (1)
685-707: NewStdinFilePathhint and helpers look consistent
StdinFilePathintegrates cleanly with Display andto_path_buf, and theor_elsehelper behaves as expected (only falling back fromNone), which should make stdin‑driven config resolution straightforward.Also applies to: 709-759
crates/biome_cli/src/commands/check.rs (1)
145-166: Check execution no longer depends on Console – all goodThe rename to
_consoleclearly signals that stdin/console state is now handled elsewhere while preserving the existing Check traversal behaviour.crates/biome_service/src/workspace/server.rs (1)
7-7: Good reuse ofload_nested_configurationin project config updatesSwitching
update_project_config_filesto callload_nested_configurationdirectly keeps the “non‑root config” loading logic in one place and preserves the existing behaviour and diagnostics handling. Import looks correct and matches the new helper.Also applies to: 2229-2239
crates/biome_service/src/configuration.rs (3)
156-173: Separated loaders for root and nested configs look sound
load_configurationand the newload_nested_configurationcleanly encode the “root vs non‑root” search by passingseek_root = Some(true/false)intoread_config, and both still funnel throughLoadedConfiguration::try_from_payload, so diagnostics andextendsresolution are preserved.
200-251:seek_root: Option<bool>and stdin path handling are reasonable, but worth keeping in mindUsing
seek_root: Option<bool>withis_some_and(|config| seek_root.is_none_or(|seek| seek == config.is_root()))nicely preserves the old “first config wins” behaviour for
None, while letting callers explicitly ask for only root (Some(true)) or only non‑root (Some(false)) configs. The newStdinFilePathbranches give stdin a distinct starting point for the config walk while still resolving externalextendsfrom the working directory. This all lines up with the new loaders and tests.
638-687: New stdin config test exercises the right edge cases
should_load_config_for_stdindoes a good job of proving thatload_configurationfinds the root config whileload_nested_configurationpicks up the nearest non‑root override for a stdin path, and the explicitIndentStyleassertions make the difference very clear. Nice, focused coverage.crates/biome_cli/src/commands/mod.rs (2)
23-24: CLI imports are aligned with the new stdin config flowPulling in
ConfigurationPathHintandload_nested_configurationhere matches howconfigure_stdinis implemented below and keeps all CLI‑side config wiring local to this module.Also applies to: 45-47
1248-1253:ConfiguredStdinis a sensible minimal carrierBundling just
ExecutionandProjectKeyintoConfiguredStdinkeeps the stdin path tidy and mirrorsConfiguredWorkspacewithout dragging along fields you don’t need in the non‑scanning case.crates/biome_cli/src/execute/mod.rs (1)
4-4: Exposingstd_into the CLI is consistent with the new flowPromoting
std_intopub(crate)socommands::modcan callstd_in::rundirectly is exactly what you want now that stdin handling has moved out ofTraversalMode. The workspace imports still line up with the JSON reporter code below.Also applies to: 30-33
CodSpeed Performance ReportMerging #8239 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.changeset/blue-parts-lie.md(1 hunks)crates/biome_cli/src/commands/check.rs(1 hunks)crates/biome_cli/src/commands/format.rs(1 hunks)crates/biome_cli/src/commands/lint.rs(1 hunks)crates/biome_cli/src/commands/scan_kind.rs(1 hunks)crates/biome_cli/src/commands/search.rs(1 hunks)crates/biome_cli/src/execute/mod.rs(2 hunks)crates/biome_cli/src/execute/std_in.rs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/biome_cli/src/commands/format.rs
- crates/biome_cli/src/commands/scan_kind.rs
- crates/biome_cli/src/commands/search.rs
- crates/biome_cli/src/commands/check.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
.changeset/blue-parts-lie.md
🪛 LanguageTool
.changeset/blue-parts-lie.md
[grammar] ~21-~21: There is an agreement error between ‘read’ and ‘config’. Insert ‘a(n)’ or change the noun to plural.
Context: ...from subdirectory/biome.json. Now, we read config as expected, reading from the nearest n...
(PRP_VB_NN)
⏰ 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). (4)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: Bench (biome_configuration)
- GitHub Check: autofix
🔇 Additional comments (5)
crates/biome_cli/src/commands/lint.rs (1)
132-156: LGTM — stdin decoupling is clean.The removal of stdin handling from the lint traversal construction aligns with the broader refactoring to centralise stdin processing. The
_consoleprefix correctly indicates the parameter is intentionally unused here.crates/biome_cli/src/execute/std_in.rs (2)
21-33: Well-structured stdin refactor.Deriving
biome_pathandcontentfrom theStdinparameter at the start ofrunis cleaner than threading them through multiple call sites. The parameter reordering (session, cli_options, mode, stdin, project_key) groups related concerns logically.
54-57: Good UX consideration.Writing the error message after the content output is sensible — users won't have to scroll up to find the actual file content after reading a brief warning.
crates/biome_cli/src/execute/mod.rs (2)
4-4: Appropriate visibility change.Making
std_invisible aspub(crate)supports the new stdin configuration flow introduced elsewhere in the PR whilst maintaining encapsulation within the crate.
95-181: Clean API simplification.Removing the
stdinfield from allTraversalModevariants (Check, Lint, Format, Search) simplifies the execution model and aligns with the goal of decoupling stdin handling from traversal logic. The change is systematic and consistent.
Almost missed your comment through the AI rabbit noise. Can you explain what broke / give an example of a setup that would behave differently now? I can only think of |
|
To be clear when I say scanning, I mean building a big list of files that need attention from biome. When in stdin mode, you should not be recursively walking the directory tree building up a huge list of files. That's just a huge waste. Since we can get the ignore behaviour without scanning, I did that. |
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
0d95d92 to
f48cd50
Compare
|
Indeed, all of the scanning code I deleted was "if stdin, use NoScanner scanner". I guess it wasn't really doing anything at all, right? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_cli/src/commands/mod.rs (1)
946-1011: Double‑check project root semantics inconfigure_stdinThe overall stdin flow looks good: you load a root config (respecting
--config-path), optionally load a nested config fromStdinFilePath, validate both, merge, then open a project and update its settings without doing a scan – exactly what the PR set out to do.Two small things worth sanity‑checking:
workspace_directorywhen using--config-path
Whencustom_config_pathis true,project_directorystaysNone, soworkspace.update_settingsseesworkspace_directory: None. In the non‑stdin path we always pass a concrete project dir (derived fromloaded_configuration.directory_path/working_dir). IfWorkspaceinternally assumes a non‑Noneworkspace directory for things like relative resolution, you might want to fall back toroot_config.directory_path(orworking_dir) here.
open_projectpath for stdin runs
Here you callopen_projectwithpath: BiomePath::new(stdin.as_path()), whereas the normal flow passes a directory (project_dir). IfOpenProjectParams::pathis conceptually “project root” rather than “arbitrary file”, it could be safer to instead pass the same directory you intend asworkspace_directory(e.g.project_directory.unwrap_or_else(|| BiomePath::new(working_dir))or similar).Both could be totally fine depending on
Workspace’s invariants, but it’s worth confirming we’re not introducing a slightly odd “project keyed by file path” special case for stdin.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.changeset/blue-parts-lie.md(1 hunks)crates/biome_cli/src/commands/check.rs(1 hunks)crates/biome_cli/src/commands/format.rs(1 hunks)crates/biome_cli/src/commands/lint.rs(1 hunks)crates/biome_cli/src/commands/mod.rs(5 hunks)crates/biome_cli/src/commands/scan_kind.rs(1 hunks)crates/biome_cli/src/commands/search.rs(1 hunks)crates/biome_cli/src/execute/mod.rs(2 hunks)crates/biome_cli/src/execute/std_in.rs(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/biome_cli/src/commands/scan_kind.rs
- crates/biome_cli/src/commands/format.rs
- crates/biome_cli/src/commands/search.rs
- crates/biome_cli/src/commands/lint.rs
- crates/biome_cli/src/commands/check.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
.changeset/blue-parts-lie.md
🧬 Code graph analysis (2)
crates/biome_cli/src/execute/std_in.rs (2)
crates/biome_cli/src/commands/mod.rs (2)
run(901-944)cli_options(636-656)packages/@biomejs/js-api/src/wasm.ts (1)
BiomePath(1-1)
crates/biome_cli/src/commands/mod.rs (1)
crates/biome_service/src/configuration.rs (2)
load_nested_configuration(166-172)load_configuration(157-163)
🪛 LanguageTool
.changeset/blue-parts-lie.md
[uncategorized] ~21-~21: A comma may be missing after the conjunctive/linking adverb ‘Thus’.
Context: ...uration from subdirectory/biome.json. Thus the output of stdin mode formatting dif...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🔇 Additional comments (4)
.changeset/blue-parts-lie.md (1)
5-24: Changelog entry reads clearly and matches the behaviour changeThe description now accurately explains how
--stdin-file-pathrespects nested configs and inherited root config, and the wording looks fine (the earlier “we read config” nit is already fixed). No changes needed here.crates/biome_cli/src/execute/std_in.rs (1)
21-34: New stdinrunwiring looks soundDeriving
BiomePathand content fromStdinup-front and then reusing them across format/check/lint branches keeps stdin handling nicely centralised and avoids any traversal coupling. The new warnings for ignored paths / disabled formatter, with “write error last” ordering, are a good UX touch for long files. I don’t see any functional landmines here.Also applies to: 52-58, 104-113, 138-144
crates/biome_cli/src/commands/mod.rs (1)
900-927: Stdin early‑return andConfiguredStdinintegration look consistentShort‑circuiting
runwhen stdin is present, and deferring toconfigure_stdin+std_in::run, cleanly separates “config walk + single‑file handling” from the normal traversal path.ConfiguredStdincarrying justExecutionandProjectKeykeeps the API minimal and avoids leaking stdin details into traversal logic. This all lines up nicely with the intent to decouple stdin from scanning.Also applies to: 1248-1253
crates/biome_cli/src/execute/mod.rs (1)
4-4: Module visibility and imports match new callersExposing
std_inaspub(crate)soCommandRunner::runcan delegate to it, and importingOpenFileParamsalongsidePatternId, are both in line with how the rest of the module uses those types. No issues from my side here.Also applies to: 30-33
I see your point and I agree with it, but unfortunately there's more to it. When VCS is enabled, we need the scanner to collect nested ignore files too. Stdin mode is also used inside editors, sometimes as an alternative to LSP formatting, if the editor doesn't support it. Which means, if a user opens a file inside a nested project, they should expect Biome to apply the correct configuration, or ignore the file if it's ignored. So, while the command needs to be fast, it also needs to be reliable and account for the user's configuration. In this last case, scanning isn't a waste, but a necessity |
|
Ah. I don't use the VCS ignore system because it's too slow on a particular monorepo -- i think it's the gitignore file scanner running on a gazillion directories. I will try to either add the scanner back on top of this or find out what went wrong with the original implementation. |
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
f48cd50 to
4834c1b
Compare
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
4834c1b to
8e8f2f2
Compare
|
@ematipico done. This was a much smaller diff too, just a path normalisation thing. I had to fix the logging that broke in #7531 as well. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/blue-parts-lie.md (1)
1-23: Consider referencing the linked issue.The changeset describes the fix clearly and uses correct tense throughout, which is great. You might want to add a reference to issue #8233 for traceability, if that's a convention in this project (e.g., "Fixes #8233" at the end).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/blue-parts-lie.md(1 hunks)crates/biome_cli/src/execute/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_cli/src/execute/mod.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Changeset descriptions should be user-facing, use past tense for actions taken (e.g., 'Added new feature'), and present tense for Biome behavior (e.g., 'Biome now supports...'). Include issue links, rule links, and code examples where applicable.
Files:
.changeset/blue-parts-lie.md
🧠 Learnings (2)
📚 Learning: 2025-11-24T18:03:52.013Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.013Z
Learning: Applies to **/.changeset/*.md : Changeset descriptions should be user-facing, use past tense for actions taken (e.g., 'Added new feature'), and present tense for Biome behavior (e.g., 'Biome now supports...'). Include issue links, rule links, and code examples where applicable.
Applied to files:
.changeset/blue-parts-lie.md
📚 Learning: 2025-11-24T18:03:52.014Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.014Z
Learning: For bugfix/feature PRs visible to Biome toolchain users or affecting published crates, create a changeset using the `just new-changeset` command with appropriate package selection, change type (major/minor/patch), and description.
Applied to files:
.changeset/blue-parts-lie.md
🪛 LanguageTool
.changeset/blue-parts-lie.md
[uncategorized] ~21-~21: A comma may be missing after the conjunctive/linking adverb ‘Thus’.
Context: ...uration from subdirectory/biome.json. Thus the output of stdin mode formatting dif...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
2 times, most recently
from
d6884d7 to
2d37b3d
Compare
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
2d37b3d to
c55e9b3
Compare
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Thank you! The bug fix looks solid, however I noticed some changes that aren't covered by the PR that should be reverted or explained in the PR description
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
c55e9b3 to
5803674
Compare
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
5803674 to
af186da
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/blue-parts-lie.md (1)
24-25: Consider adding comma before 'but' for grammatical correctness.The sentence connects two independent clauses and would read more naturally with a comma before "but".
Apply this diff:
-In addition, Biome now shows a warning if `--stdin-file-path` is provided but +In addition, Biome now shows a warning if `--stdin-file-path` is provided, but that path is ignored and therefore not formatted or fixed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_cli/tests/snapshots/main_commands_check/check_stdin_ignores_unknown_file_path.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_stdin_does_not_error_with_ignore_unknown_file_extensions.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (16)
.changeset/blue-parts-lie.md(1 hunks).github/workflows/pull_request.yml(1 hunks)biome.json(1 hunks)crates/biome_cli/src/execute/mod.rs(1 hunks)crates/biome_cli/src/execute/std_in.rs(3 hunks)e2e-tests/stdin-nested-config/app.js(1 hunks)e2e-tests/stdin-nested-config/app.js.formatted(1 hunks)e2e-tests/stdin-nested-config/biome.jsonc(1 hunks)e2e-tests/stdin-nested-config/donotformat.ts(1 hunks)e2e-tests/stdin-nested-config/subdirectory/biome.jsonc(1 hunks)e2e-tests/stdin-nested-config/subdirectory/lib.js(1 hunks)e2e-tests/stdin-nested-config/subdirectory/lib.js.formatted(1 hunks)e2e-tests/stdin-nested-config/subdirectory/typed.ts(1 hunks)e2e-tests/stdin-nested-config/subdirectory/typed.ts.formatted(1 hunks)e2e-tests/stdin-nested-config/test.sh(1 hunks)e2e-tests/test-all.sh(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- e2e-tests/stdin-nested-config/app.js.formatted
- e2e-tests/stdin-nested-config/subdirectory/typed.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_cli/src/execute/std_in.rs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use the Rustdbg!()macro for debugging output during test execution, and pass the--show-outputflag tocargo testto display debug output.
Use snapshot testing with theinstacrate for testing in Rust projects. Accept or reject snapshots usingcargo insta accept,cargo insta reject, orcargo insta review.
Write doc comments as doc tests in Rust using code blocks with assertions that will be executed during the testing phase.
Use rustdoc inline documentation for rules, assists, and their options. Create corresponding documentation PRs for other documentation updates against thenextbranch of the website repository.
Set theversionmetadata field in linter rule implementations to'next'for newly created rules. Update this field to the new version number when releasing a minor or major version.
Files:
crates/biome_cli/src/execute/mod.rs
**/.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Changeset descriptions should be user-facing, use past tense for actions taken (e.g., 'Added new feature'), and present tense for Biome behavior (e.g., 'Biome now supports...'). Include issue links, rule links, and code examples where applicable.
Files:
.changeset/blue-parts-lie.md
🧠 Learnings (29)
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
e2e-tests/stdin-nested-config/biome.jsonce2e-tests/stdin-nested-config/subdirectory/biome.jsoncbiome.json.changeset/blue-parts-lie.md
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/specs/**/options.json : Create an `options.json` file (formatted as `biome.json`) in test specification folders to apply non-default formatting options to all test files in that folder
Applied to files:
e2e-tests/stdin-nested-config/biome.jsonce2e-tests/stdin-nested-config/subdirectory/biome.jsonce2e-tests/stdin-nested-config/test.sh
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/tests/specs/**/*.{js,ts,tsx,jsx,json,css,graphql} : Test files should use 'invalid' or 'valid' prefixes to indicate whether they contain code reported by the rule
Applied to files:
e2e-tests/stdin-nested-config/biome.jsonce2e-tests/stdin-nested-config/subdirectory/biome.jsoncbiome.json
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/tests/specs/**/*.{js,ts,tsx,jsx,json,css,graphql} : Test files for rules should be placed inside 'tests/specs/' directory organized by group and rule name (e.g., 'tests/specs/nursery/myRuleName/')
Applied to files:
e2e-tests/stdin-nested-config/biome.jsonce2e-tests/stdin-nested-config/subdirectory/biome.jsoncbiome.json
📚 Learning: 2025-11-24T18:03:52.014Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.014Z
Learning: For bugfix/feature PRs visible to Biome toolchain users or affecting published crates, create a changeset using the `just new-changeset` command with appropriate package selection, change type (major/minor/patch), and description.
Applied to files:
e2e-tests/stdin-nested-config/biome.jsonc.changeset/blue-parts-lie.md
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation marked with 'options' should contain only rule-specific options in JSON/JSONC format, while 'full_options' contains complete biome.json configuration
Applied to files:
e2e-tests/stdin-nested-config/biome.jsonce2e-tests/stdin-nested-config/subdirectory/biome.jsonc.changeset/blue-parts-lie.md
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
e2e-tests/stdin-nested-config/biome.jsonce2e-tests/stdin-nested-config/test.sh
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Multi-file documentation snippets should use 'file=<path>' property to create an in-memory file system for testing cross-file rule behavior
Applied to files:
e2e-tests/stdin-nested-config/biome.jsoncbiome.jsoncrates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:03:52.013Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.013Z
Learning: Applies to **/*.rs : Use the Rust `dbg!()` macro for debugging output during test execution, and pass the `--show-output` flag to `cargo test` to display debug output.
Applied to files:
biome.json.github/workflows/pull_request.yml
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/Cargo.toml : Include development dependencies in `Cargo.toml` for formatter tests: `biome_formatter_test`, `biome_<language>_factory`, `biome_<language>_parser`, `biome_parser`, `biome_service`, `countme`, `iai`, `quickcheck`, `quickcheck_macros`, and `tests_macros`
Applied to files:
biome.json.github/workflows/pull_request.yml
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that overwhelmingly apply to a specific framework should be named using 'use<Framework>...' or 'no<Framework>...' prefix (e.g., `noVueReservedProps`)
Applied to files:
biome.json
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules ported from other ecosystems should include a 'sources' field in the 'declare_lint_rule!' macro with RuleSource metadata (e.g., '::ESLint')
Applied to files:
biome.json
📚 Learning: 2025-11-24T18:06:12.017Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.017Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Implement watcher tests for workspace methods in watcher.tests.rs and end-to-end tests in LSP tests
Applied to files:
biome.jsoncrates/biome_cli/src/execute/mod.rs.github/workflows/pull_request.yml
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation can use 'ignore' property to exclude snippets from automatic validation (use sparingly)
Applied to files:
biome.json
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that allow users to ban specific entities should use the 'noRestricted<Concept>' naming convention (e.g., `noRestrictedGlobals`)
Applied to files:
biome.json
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules that report code that can lead to runtime failures should use the 'noUnsafe<Concept>' naming convention (e.g., `noUnsafeOptionalChaining`)
Applied to files:
biome.json
📚 Learning: 2025-11-24T18:03:52.013Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.013Z
Learning: Applies to **/*.rs : Use snapshot testing with the `insta` crate for testing in Rust projects. Accept or reject snapshots using `cargo insta accept`, `cargo insta reject`, or `cargo insta review`.
Applied to files:
biome.json
📚 Learning: 2025-11-24T18:06:12.017Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.017Z
Learning: Applies to crates/biome_service/src/workspace*.rs : Implement the Workspace trait in the Biome Service to manage internal state of projects, including open documents, project layout instances, and module graph instances
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:06:12.017Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.017Z
Learning: Applies to crates/biome_service/src/workspace/server.rs : Use WorkspaceServer implementation for maintaining workspace state in daemon mode and CLI daemonless mode
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Avoid deep indentation in rule implementations by using Rust helper functions like 'map', 'filter', and 'and_then' instead of nested if-let statements
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules should be implemented with the 'impl Rule for RuleName' trait, including 'run' function that returns signals and optional 'diagnostic' and 'action' functions
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Use 'let else' pattern to reduce code branching when the rule's run function returns 'Vec'
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : When navigating CST, use the try operator '?' to handle 'Result' types when the rule's run function returns 'Option'
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:06:12.017Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.017Z
Learning: Applies to crates/biome_service/src/workspace/client.rs : Use WorkspaceClient implementation for creating connections to the daemon and communicating with WorkspaceServer
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : The first paragraph of rule documentation must be a single line and serves as the brief description for the rule overview page
Applied to files:
crates/biome_cli/src/execute/mod.rs
📚 Learning: 2025-11-24T18:06:12.017Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.017Z
Learning: Debug the WorkspaceWatcher by starting the daemon with cargo run --bin=biome -- start and running commands such as cargo run --bin=biome -- lint --use-server <path>
Applied to files:
crates/biome_cli/src/execute/mod.rs.github/workflows/pull_request.yml
📚 Learning: 2025-11-24T18:03:52.014Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.014Z
Learning: Before committing and opening a PR, run `just f` to format Rust and TOML files, `just l` to lint the whole project, and run appropriate code generation commands (`just gen-analyzer` for linter work, `just gen-bindings` for workspace work).
Applied to files:
e2e-tests/stdin-nested-config/test.sh
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/spec_tests.rs : Use the `tests_macros::gen_tests!` macro in `spec_tests.rs` to generate test functions for each specification file matching the pattern `tests/specs/<language>/**/*.<ext>`
Applied to files:
.github/workflows/pull_request.yml
📚 Learning: 2025-11-24T18:03:52.013Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.013Z
Learning: Applies to **/.changeset/*.md : Changeset descriptions should be user-facing, use past tense for actions taken (e.g., 'Added new feature'), and present tense for Biome behavior (e.g., 'Biome now supports...'). Include issue links, rule links, and code examples where applicable.
Applied to files:
.changeset/blue-parts-lie.md
🧬 Code graph analysis (2)
e2e-tests/stdin-nested-config/donotformat.ts (1)
e2e-tests/stdin-nested-config/app.js (1)
x(1-1)
crates/biome_cli/src/execute/mod.rs (1)
packages/@biomejs/js-api/src/wasm.ts (1)
BiomePath(1-1)
🪛 LanguageTool
.changeset/blue-parts-lie.md
[uncategorized] ~24-~24: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...rning if --stdin-file-path is provided but that path is ignored and therefore not ...
(COMMA_COMPOUND_SENTENCE_2)
🪛 Shellcheck (0.11.0)
e2e-tests/test-all.sh
[warning] 17-17: Quote the right-hand side of != in [[ ]] to prevent glob matching.
(SC2053)
e2e-tests/stdin-nested-config/test.sh
[warning] 5-5: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
⏰ 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). (7)
- GitHub Check: Documentation
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Test Node.js API
- GitHub Check: autofix
🔇 Additional comments (12)
biome.json (1)
36-36: LGTM!The re-inclusion pattern for e2e-tests is correct and necessary for the new stdin-nested-config test suite to be discovered properly.
e2e-tests/stdin-nested-config/app.js (1)
1-5: LGTM!The deliberately irregular formatting (excessive spacing and indentation) is correct for this test fixture, which validates the formatter's ability to normalize code via stdin with nested configuration.
e2e-tests/stdin-nested-config/subdirectory/lib.js (1)
1-5: LGTM!The intentionally inconsistent spacing serves as appropriate test data for validating nested configuration handling in stdin mode.
e2e-tests/stdin-nested-config/subdirectory/typed.ts.formatted (1)
1-5: LGTM!The formatted fixture correctly represents the expected output with proper spacing and indentation (indent width 2, consistent with the subdirectory's nested configuration).
e2e-tests/stdin-nested-config/donotformat.ts (1)
1-1: LGTM!The excessive spacing in this fixture is intentional for testing the scenario where formatting is skipped (ignored path or disabled formatter), validating that the warning message is shown correctly.
e2e-tests/stdin-nested-config/subdirectory/lib.js.formatted (1)
1-5: LGTM!The formatted fixture matches the expected output structure and formatting rules defined in the subdirectory's nested configuration.
crates/biome_cli/src/execute/mod.rs (1)
565-573: Based on my research and the code snippet provided, I can now assess the review comment. The web search confirms thatunwrap_or_default()onOption<PathBuf>returns an emptyPathBufwhen the value isNone. When an empty path is joined with the stdin path viaempty_path.join(stdin_path), the result is just the stdin path itself.This is problematic because:
- The comment explicitly states: "Biome path must starts_with(the project directory)" - indicating the path should include the working directory context
- The fallback behavior is inconsistent: When
working_directory()returnsNone, the resultingbiome_pathbecomes just the relative stdin path, violating the documented requirement- Configuration discovery is affected: The comment notes this is needed to "enable nested configuration discovery," which requires knowing the full project path context
The review comment's concern is valid and represents a potential bug. The current code uses
unwrap_or_default()as a silent fallback without ensuring the documented invariant ("must starts_with project directory") is maintained. This could lead to incorrect configuration discovery in edge cases where the working directory cannot be determined.
Verify that
working_directory()cannot returnNonein practice, or add error handling for the case where it does. The currentunwrap_or_default()silently violates the documented requirement that the biome path must start with the project directory. Either ensure thatworking_directory()always returns a validSomevalue (with defensive checks), or handle theNonecase explicitly with appropriate error reporting or a configurable fallback strategy.// Current code (lines 565-573): let working_dir = session .app .workspace .fs() .working_directory() .unwrap_or_default(); // Silent fallback to empty path - violates documented invariant let biome_path = BiomePath::new(working_dir.join(stdin.as_path()));e2e-tests/stdin-nested-config/subdirectory/biome.jsonc (1)
1-12: Configuration structure looks good for nested config testing.The nested config correctly sets
root: false, establishes include filters, and overrides formatter settings to demonstrate configuration discovery and override behaviour when using--stdin-file-path. The inline comment clarifying path semantics is helpful.e2e-tests/stdin-nested-config/biome.jsonc (1)
1-16: Root configuration appropriately structured for nested config e2e testing.Setting
root: trueand including**/biome.jsoncenables discovery of nested configs, whilstvcs.enabled: falsekeeps the test hermetic. The inclusion of**/*.jsbut exclusion of*.tsat root level (with nested inclusion in subdirectory) provides a solid test case for configuration override behaviour.e2e-tests/stdin-nested-config/test.sh (1)
1-28: Test structure and approach are sound.The script effectively tests nested config discovery by formatting files via stdin using
--stdin-file-pathand comparing output against reference.formattedfiles. The separate handling ofdonotformat.tsmakes sense if that file should remain unformatted due to nested config rules. The helper function reduces repetition nicely.Please confirm that all referenced
.formattedreference files exist in the test directory and thatdonotformat.tsis intentionally excluded from formatting per nested configuration.e2e-tests/test-all.sh (1)
1-26: Improved script robustness with directory handling and selective test execution.The updated script now properly manages working directory via
cd "$(dirname "$0")", enabling safe relative path resolution regardless of where it's invoked. The FILTER mechanism enables selective test execution (useful for debugging specific suites), andpushd/popdis safer than repeatedcd. The shift to explicitbash test.shinvocation aligns with updated shebang requirements across test scripts.This addresses the previous review comment from ematipico requesting coverage of this file's changes in the PR review—changes are now documented and reviewed.
.github/workflows/pull_request.yml (1)
104-104: Workflow step simplification is safe and appropriate.Delegating environment setup to the improved test-all.sh script (which now handles directory context internally) simplifies the workflow and improves maintainability. The script's proper shebang and
cdlogic ensure it works correctly regardless of invocation cwd.
| if [[ "$x" != $FILTER ]]; then | ||
| echo "Skipping $x" | ||
| continue | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Minor: Quote glob pattern to prevent unintended matching (SC2053).
The right-hand side of the != operator should be quoted to prevent glob matching:
- if [[ "$x" != $FILTER ]]; then
+ if [[ "$x" != "$FILTER" ]]; then📝 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.
| if [[ "$x" != $FILTER ]]; then | |
| echo "Skipping $x" | |
| continue | |
| fi | |
| if [[ "$x" != "$FILTER" ]]; then | |
| echo "Skipping $x" | |
| continue | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 17-17: Quote the right-hand side of != in [[ ]] to prevent glob matching.
(SC2053)
🤖 Prompt for AI Agents
In e2e-tests/test-all.sh around lines 17 to 20, the conditional compares $x to
$FILTER without quoting the right-hand side, which allows shell globbing
(SC2053); update the comparison to quote the FILTER variable (i.e., use a quoted
right-hand side) so the pattern is compared literally and not treated as a glob,
and keep the existing quoting of $x.
There was a problem hiding this comment.
No it should not
…ests This also runs the tests with bash, not sh. So we can do more advanced shell scripts in the various test.sh.
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
af186da to
189b888
Compare
This enables us to have explicit root: true without making the root biome project error out.
This fixes nested config discovery/matching with the filepath.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
e2e-tests/stdin-nested-config/test.sh (1)
4-5: Defer$TEMPexpansion in trap and quote itUsing double quotes here expands
$TEMPwhen the trap is set, and drops quoting when it runs. Better to defer expansion and keep the value quoted:-TEMP=$(mktemp) -trap "rm -f $TEMP" EXIT +TEMP=$(mktemp) +trap 'rm -f "$TEMP"' EXITThis matches ShellCheck’s SC2064 suggestion and is safer if the path ever contains odd characters.
🧹 Nitpick comments (2)
.changeset/blue-parts-lie.md (1)
5-25: Very minor comma/style nit in final sentenceThe wording is clear and the changeset matches the guidelines (past tense for the fix, present for behaviour). If you want to appease grammar pedants, you could add a comma before “but”:
-In addition, Biome now shows a warning if `--stdin-file-path` is provided but +In addition, Biome now shows a warning if `--stdin-file-path` is provided, butPurely optional; content is otherwise spot on.
e2e-tests/test-all.sh (1)
12-14: Intentional glob matching in FILTER; shellcheck can be ignored hereThe
FILTER="*$1*"plus[[ "$x" != $FILTER ]]gives the desired glob/substring match (see the “Glob matcher” comment). Quoting$FILTERwould silently change semantics to a literal compare and break filtering, so the current form is correct despite SC2053.If SC2053 is noisy in CI, consider annotating this with a one-line
# shellcheck disable=SC2053to document the intent.Also applies to: 24-29
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_cli/tests/snapshots/main_commands_check/check_stdin_ignores_unknown_file_path.snapis excluded by!**/*.snapand included by**crates/biome_cli/tests/snapshots/main_commands_format/format_stdin_does_not_error_with_ignore_unknown_file_extensions.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (16)
.changeset/blue-parts-lie.md(1 hunks).github/workflows/pull_request.yml(1 hunks)biome.json(1 hunks)crates/biome_cli/src/execute/mod.rs(1 hunks)crates/biome_cli/src/execute/std_in.rs(3 hunks)e2e-tests/stdin-nested-config/app.js(1 hunks)e2e-tests/stdin-nested-config/app.js.formatted(1 hunks)e2e-tests/stdin-nested-config/biome.jsonc(1 hunks)e2e-tests/stdin-nested-config/donotformat.ts(1 hunks)e2e-tests/stdin-nested-config/subdirectory/biome.jsonc(1 hunks)e2e-tests/stdin-nested-config/subdirectory/lib.js(1 hunks)e2e-tests/stdin-nested-config/subdirectory/lib.js.formatted(1 hunks)e2e-tests/stdin-nested-config/subdirectory/typed.ts(1 hunks)e2e-tests/stdin-nested-config/subdirectory/typed.ts.formatted(1 hunks)e2e-tests/stdin-nested-config/test.sh(1 hunks)e2e-tests/test-all.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- e2e-tests/stdin-nested-config/subdirectory/lib.js.formatted
- e2e-tests/stdin-nested-config/donotformat.ts
- e2e-tests/stdin-nested-config/subdirectory/lib.js
- crates/biome_cli/src/execute/mod.rs
- e2e-tests/stdin-nested-config/app.js.formatted
- .github/workflows/pull_request.yml
- biome.json
- e2e-tests/stdin-nested-config/biome.jsonc
- e2e-tests/stdin-nested-config/app.js
- e2e-tests/stdin-nested-config/subdirectory/typed.ts.formatted
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use the Rustdbg!()macro for debugging output during test execution, and pass the--show-outputflag tocargo testto display debug output.
Use snapshot testing with theinstacrate for testing in Rust projects. Accept or reject snapshots usingcargo insta accept,cargo insta reject, orcargo insta review.
Write doc comments as doc tests in Rust using code blocks with assertions that will be executed during the testing phase.
Use rustdoc inline documentation for rules, assists, and their options. Create corresponding documentation PRs for other documentation updates against thenextbranch of the website repository.
Set theversionmetadata field in linter rule implementations to'next'for newly created rules. Update this field to the new version number when releasing a minor or major version.
Files:
crates/biome_cli/src/execute/std_in.rs
**/.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Changeset descriptions should be user-facing, use past tense for actions taken (e.g., 'Added new feature'), and present tense for Biome behavior (e.g., 'Biome now supports...'). Include issue links, rule links, and code examples where applicable.
Files:
.changeset/blue-parts-lie.md
🧠 Learnings (19)
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/specs/**/options.json : Create an `options.json` file (formatted as `biome.json`) in test specification folders to apply non-default formatting options to all test files in that folder
Applied to files:
e2e-tests/stdin-nested-config/test.she2e-tests/stdin-nested-config/subdirectory/biome.jsonc
📚 Learning: 2025-09-25T12:32:59.003Z
Learnt from: arendjr
Repo: biomejs/biome PR: 7593
File: crates/biome_service/src/workspace/server.rs:1306-1306
Timestamp: 2025-09-25T12:32:59.003Z
Learning: In the biomejs/biome project, do not flag compilation errors during code review as they are handled by the existing test infrastructure and CI. Focus on other code quality aspects instead.
Applied to files:
e2e-tests/stdin-nested-config/test.she2e-tests/stdin-nested-config/subdirectory/biome.jsonc
📚 Learning: 2025-11-24T18:03:52.014Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.014Z
Learning: Before committing and opening a PR, run `just f` to format Rust and TOML files, `just l` to lint the whole project, and run appropriate code generation commands (`just gen-analyzer` for linter work, `just gen-bindings` for workspace work).
Applied to files:
e2e-tests/stdin-nested-config/test.sh
📚 Learning: 2025-11-24T18:05:20.343Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.343Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/src/context.rs : Define `<Language>FormatContext` struct in a `context.rs` file containing `comments` and `source_map` fields, implementing `FormatContext` and `CstFormatContext` traits
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Rules should only have severity set to 'error' if they report hard errors, dangerous code, or accessibility issues; use 'warn' for possibly erroneous code; use 'info' for stylistic suggestions
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:05:27.784Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Use the `dbg_write!` macro to debug formatter output instead of other logging methods
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Multi-file documentation snippets should use 'file=<path>' property to create an in-memory file system for testing cross-file rule behavior
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Deprecated rules must include a 'deprecated' field in the 'declare_lint_rule!' macro with the reason for deprecation
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation marked with 'expect_diagnostic' must emit exactly one diagnostic for the build system to generate it automatically
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation can use 'ignore' property to exclude snippets from automatic validation (use sparingly)
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:05:27.784Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:05:27.784Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.784Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Do not attempt to 'fix' the code; if a token/node is known to be mandatory but is missing, return `None` instead
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in rule documentation must specify a language and use 'expect_diagnostic' property for invalid snippets that should emit exactly one diagnostic
Applied to files:
crates/biome_cli/src/execute/std_in.rs
📚 Learning: 2025-11-24T18:03:52.014Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.014Z
Learning: For bugfix/feature PRs visible to Biome toolchain users or affecting published crates, create a changeset using the `just new-changeset` command with appropriate package selection, change type (major/minor/patch), and description.
Applied to files:
.changeset/blue-parts-lie.mde2e-tests/stdin-nested-config/subdirectory/biome.jsonc
📚 Learning: 2025-08-05T14:43:29.581Z
Learnt from: dyc3
Repo: biomejs/biome PR: 7081
File: packages/@biomejs/biome/configuration_schema.json:7765-7781
Timestamp: 2025-08-05T14:43:29.581Z
Learning: The file `packages/biomejs/biome/configuration_schema.json` is auto-generated and should not be manually edited or reviewed for schema issues; any changes should be made at the code generation source.
Applied to files:
.changeset/blue-parts-lie.mde2e-tests/stdin-nested-config/subdirectory/biome.jsonc
📚 Learning: 2025-11-24T18:03:52.013Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:03:52.013Z
Learning: Applies to **/.changeset/*.md : Changeset descriptions should be user-facing, use past tense for actions taken (e.g., 'Added new feature'), and present tense for Biome behavior (e.g., 'Biome now supports...'). Include issue links, rule links, and code examples where applicable.
Applied to files:
.changeset/blue-parts-lie.md
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/lib/src/**/!(mod).rs : Code blocks in documentation marked with 'options' should contain only rule-specific options in JSON/JSONC format, while 'full_options' contains complete biome.json configuration
Applied to files:
.changeset/blue-parts-lie.mde2e-tests/stdin-nested-config/subdirectory/biome.jsonc
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/tests/specs/**/*.{js,ts,tsx,jsx,json,css,graphql} : Test files for rules should be placed inside 'tests/specs/' directory organized by group and rule name (e.g., 'tests/specs/nursery/myRuleName/')
Applied to files:
e2e-tests/stdin-nested-config/subdirectory/biome.jsonc
📚 Learning: 2025-11-24T18:04:42.146Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:04:42.146Z
Learning: Applies to crates/biome_analyze/**/biome_*_analyze/tests/specs/**/*.{js,ts,tsx,jsx,json,css,graphql} : Test files should use 'invalid' or 'valid' prefixes to indicate whether they contain code reported by the rule
Applied to files:
e2e-tests/stdin-nested-config/subdirectory/biome.jsonc
🧬 Code graph analysis (2)
e2e-tests/stdin-nested-config/subdirectory/typed.ts (1)
e2e-tests/stdin-nested-config/subdirectory/lib.js (1)
y(1-1)
crates/biome_cli/src/execute/std_in.rs (1)
crates/biome_service/src/workspace.rs (1)
markup(1176-1178)
🪛 LanguageTool
.changeset/blue-parts-lie.md
[uncategorized] ~24-~24: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...rning if --stdin-file-path is provided but that path is ignored and therefore not ...
(COMMA_COMPOUND_SENTENCE_2)
🪛 Shellcheck (0.11.0)
e2e-tests/stdin-nested-config/test.sh
[warning] 5-5: Use single quotes, otherwise this expands now rather than when signalled.
(SC2064)
e2e-tests/test-all.sh
[warning] 26-26: Quote the right-hand side of != in [[ ]] to prevent glob matching.
(SC2053)
⏰ 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). (11)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Validate PR title
- GitHub Check: Test Node.js API
- GitHub Check: lint
- GitHub Check: autofix
🔇 Additional comments (4)
crates/biome_cli/src/execute/std_in.rs (1)
52-55: Stdin warnings and ordering look goodWriting the warnings after echoing the content is sensible for long inputs, and the three messages cleanly distinguish “ignored path” vs “formatter disabled” vs “not fixed”. No behavioural red flags from these additions.
Also applies to: 106-109, 138-141
e2e-tests/stdin-nested-config/subdirectory/typed.ts (1)
1-5: Fixture content looks fineThe odd spacing/indentation is appropriate here as unformatted input to exercise the formatter, and it mirrors the existing JS fixture.
e2e-tests/stdin-nested-config/subdirectory/biome.jsonc (2)
1-12: Configuration structure looks sound for nested-config e2e testing.The overall structure is well-suited for the PR's objectives:
root: falseenables proper config inheritance from parent configs (essential for testing nested overrides)files.includes: ["*.ts"]with the clarifying comment correctly documents that patterns are relative to this config's location- Formatter settings provide distinct values for test verification
Once the
extendssyntax is verified, this should serve the test scenario well.
3-3: The"extends": "//"syntax is correct and intentional.The
"//"is a documented Biome shortcut that tells Biome to extend the project root configuration. This is the proper pattern for nested folder configs to inherit root settings, making it exactly appropriate for this test scenario.
This tests the fix for the stdin with nested config issue.
cormacrelf
force-pushed
the
fix-stdin-nested-config
branch
from
189b888 to
290e75a
Compare
ematipico
left a comment
ematipico
left a comment
There was a problem hiding this comment.
Overall, it looks good. I left a comment we should resolve, if it's applicable
| .fs() | ||
| .working_directory() | ||
| .unwrap_or_default(); | ||
| let biome_path = BiomePath::new(working_dir.join(stdin.as_path())); |
There was a problem hiding this comment.
This operation might not be safe, because stdin.as_path might be an absolute path. We should check it before running the join operation
There was a problem hiding this comment.
Almost every path library I can think of, including camino, replaces the original path when you .join(absolute).
There was a problem hiding this comment.
Well there may be an edge case where the working dir has a symlink in it somewhere and someone passes an absolute path that resolves to the same file ultimately, but does not appear to be inside any project. But I am not sure we care.
2 participants
Summary
Fixes #8233. We were just missing a working_dir.join(), so biome was not able to find the project the stdin path was associated with. I don't know if that's exactly correct -- do we have to resolve the path of any symlinks or is cwd.join just fine?
Test Plan
I added an e2e test, and also improved the e2e test runner script. We can now:
cd e2e-testsfirst./e2e-tests/test-all.sh stdin, with glob matching{ "root": true }in test configs because e2e-tests is now ignored from the root biome.jsonDocs
This actually just makes the CLI behave as is already documented in the
stdin_file_pathCLI params.