fix(cli): files in ignored folders are now correctly ignored #7275
Conversation
🦋 Changeset detectedLatest commit: f333689 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 |
WalkthroughThe PR refactors ignore/include evaluation across CLI traversal and workspace services. It introduces filesystem-aware checks and differentiates file vs directory include matching. Ignore decisions now support ancestor-aware evaluation by passing IgnoreKind::Ancestors through traversal, projects, and server layers. Public APIs in biome_service::projects and workspace::server are extended to accept a filesystem handle. Includes logic is split into is_file_included and is_dir_included. Tests are updated to reflect directory-focused patterns and Windows symlink directories. IgnoreKind derives Eq/PartialEq. A changeset documents the fix for explicitly passed files inside ignored folders. Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(no out-of-scope functional changes identified) Possibly related PRs
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{rs,toml}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
crates/biome_*/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/tests/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-08-11T11:53:15.299ZApplied to files:
📚 Learning: 2025-08-17T08:56:30.831ZApplied to files:
📚 Learning: 2025-08-11T11:53:15.299ZApplied to files:
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![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 (8)
.changeset/ancient-ancestors-ascent.md (1)
1-5: Add a minimal example to the changeset for clarity.The note is good and user-facing. A tiny example will help readers confirm the new behaviour at a glance.
Apply this diff to add an example section:
Fixed [#7268](https://github.com/biomejs/biome/issues/7268): Files that are explicitly passed as CLI arguments are now correctly ignored if they reside in an ignored folder. + +#### Example. +```json +// biome.json +{ "files": { "includes": ["**/*.ts", "!src/generated"] } } +``` + +```bash +# The following file was processed before; it is ignored now. +npx biome check src/generated/index.ts +```crates/biome_service/src/settings.rs (2)
314-316: Doc says “file only”, but implementation handles directories too.The function branches on is_dir(path), so the comment is misleading. Recommend updating the doc to reflect that directories are supported.
-/// `path` is expected to point to a file and not a directory. +/// `path` may refer to a file or a directory. +/// Directories are evaluated with `is_dir_included`, files with `is_file_included`.Also applies to: 326-330
951-958: Fix parameter name in doc comment (dir_path vs file_path).Nit: the prose still says “file_path” for a directory function.
-/// `file_path` must point to a directory. If it is a file, you should use +/// `dir_path` must point to a directory. If it is a file, you should use /// [Self::is_file_included()] instead.crates/biome_cli/tests/cases/included_files.rs (1)
100-106: Consider adding a deeper-nesting case (optional).An extra assertion with, say, "nested/folder/x.js" would guard against regressions in multi-level ancestry checks.
Happy to draft the additional test if you want it in this PR.
crates/biome_cli/src/execute/traverse.rs (1)
538-546: Optional perf win: pre-filter explicit CLI inputs.Given the slight overhead of ancestor checks, you could pre-filter the explicit inputs before scheduling traversal by calling
is_path_ignored(.. Ancestors)on each input (when it exists) and avoid enqueuing obviously ignored paths. Not required for this fix.If you'd like, I can sketch a small helper to pre-check inputs against
is_path_ignoredbeforescope.evaluate.crates/biome_service/src/workspace/server.rs (1)
577-582: Good: fs plumbed into top-level ignore checks.Passing the filesystem handle through
is_ignored_by_top_level_config(...)aligns server traversal with the new filesystem-aware settings logic. This should restore the expected ignore-on-ancestors behaviour for both dirs and files.If you fancy shaving a few keystrokes and repeated vtable lookups, consider caching
let fs = self.fs.as_ref();once in the function and reusing it at these call sites. Not critical.Also applies to: 611-616, 635-640, 644-649
crates/biome_service/src/projects.rs (2)
359-373: Ancestor inclusion check: works; consider a small readability tweak.The loop short-circuits correctly and stops at the project root. If you prefer a more declarative take:
Apply this diff for a tidier iteration:
- if ignore_kind == IgnoreKind::Ancestors { - for ancestor in path.ancestors().skip(1) { - if !is_included || ancestor == project_data.path { - break; - } - - is_included = is_included && includes.is_dir_included(ancestor) - } - } + if ignore_kind == IgnoreKind::Ancestors && is_included { + is_included = path + .ancestors() + .skip(1) + .take_while(|ancestor| *ancestor != project_data.path) + .all(|ancestor| includes.is_dir_included(ancestor)); + }
375-387: VCS root anchoring only for Ancestors—intentional?For VCS ignores we pass
root_path = Some(project_data.path)only whenIgnoreKind::Ancestors. If path-only checks also want root anchoring (arguably consistent), consider applying it there too. If this split is deliberate, a brief comment would save future head-scratching.Want me to add a note clarifying why
Pathdoesn’t anchor to root for VCS?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/biome_cli/tests/snapshots/main_cases_included_files/does_not_handle_files_in_ignored_folder.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (7)
.changeset/ancient-ancestors-ascent.md(1 hunks)crates/biome_cli/src/execute/traverse.rs(1 hunks)crates/biome_cli/tests/cases/included_files.rs(1 hunks)crates/biome_service/src/projects.rs(7 hunks)crates/biome_service/src/settings.rs(3 hunks)crates/biome_service/src/workspace.rs(1 hunks)crates/biome_service/src/workspace/server.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_cli/tests/cases/included_files.rscrates/biome_cli/src/execute/traverse.rscrates/biome_service/src/projects.rscrates/biome_service/src/settings.rscrates/biome_service/src/workspace.rscrates/biome_service/src/workspace/server.rs
crates/biome_*/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_cli/tests/cases/included_files.rscrates/biome_cli/src/execute/traverse.rscrates/biome_service/src/projects.rscrates/biome_service/src/settings.rscrates/biome_service/src/workspace.rscrates/biome_service/src/workspace/server.rs
**/tests/**
📄 CodeRabbit Inference Engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_cli/tests/cases/included_files.rs
.changeset/*.md
📄 CodeRabbit Inference Engine (CONTRIBUTING.md)
.changeset/*.md: Create changesets withjust new-changeset; store them in.changeset/with correct frontmatter (package keys and change type).
In changeset descriptions, follow content conventions: user-facing changes only; past tense for what you did; present tense for current behavior; link issues for fixes; link rules/assists; include representative code blocks; end every sentence with a period.
When adding headers in a changeset, only use #### or ##### levels.
Files:
.changeset/ancient-ancestors-ascent.md
crates/biome_service/src/workspace.rs
📄 CodeRabbit Inference Engine (crates/biome_service/CONTRIBUTING.md)
Implement the Workspace trait in src/workspace.rs
Files:
crates/biome_service/src/workspace.rs
crates/biome_service/src/workspace/server.rs
📄 CodeRabbit Inference Engine (crates/biome_service/CONTRIBUTING.md)
Use WorkspaceServer (src/workspace/server.rs) to maintain workspace state in daemon and CLI daemonless modes
Files:
crates/biome_service/src/workspace/server.rs
🧠 Learnings (10)
📚 Learning: 2025-08-17T08:56:30.822Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:56:30.822Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Use `biome_js_analyze/tests/quick_test.rs` for quick, ad-hoc testing; un-ignore the test and adjust the rule filter as needed
Applied to files:
crates/biome_cli/tests/cases/included_files.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:
crates/biome_cli/tests/cases/included_files.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/**/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_cli/tests/cases/included_files.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:
crates/biome_cli/tests/cases/included_files.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:
crates/biome_cli/tests/cases/included_files.rscrates/biome_service/src/projects.rs
📚 Learning: 2025-08-11T11:53:15.299Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.299Z
Learning: Applies to crates/biome_service/src/workspace.rs : Implement the Workspace trait in src/workspace.rs
Applied to files:
crates/biome_service/src/projects.rscrates/biome_service/src/workspace/server.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.rs : Keep workspace state in sync with the filesystem using WorkspaceWatcher; only active in daemon mode
Applied to files:
crates/biome_service/src/projects.rscrates/biome_service/src/workspace/server.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/client.rs : Use WorkspaceClient (src/workspace/client.rs) to connect to the daemon and communicate with WorkspaceServer
Applied to files:
crates/biome_service/src/projects.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/server.rs : Use WorkspaceServer (src/workspace/server.rs) to maintain workspace state in daemon and CLI daemonless modes
Applied to files:
crates/biome_service/src/projects.rs
📚 Learning: 2025-08-17T08:57:34.726Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-08-17T08:57:34.726Z
Learning: Applies to crates/biome_parser/xtask/codegen/src/*_kinds_src.rs : Add src/<language>_kinds_src.rs under xtask/codegen that returns a static KindSrc
Applied to files:
crates/biome_service/src/workspace.rs
🧬 Code Graph Analysis (1)
crates/biome_service/src/settings.rs (1)
crates/biome_service/src/lib.rs (1)
is_dir(78-80)
⏰ 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). (9)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- 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: Test Node.js API
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: autofix
🔇 Additional comments (13)
crates/biome_service/src/settings.rs (2)
942-949: Split file/dir include checks look good.The separation into is_file_included and matches_with_exceptions is clear and maintains previous “include-all when unset” semantics.
314-331: All clear: no lingeringis_includedcallsThe grep search returned zero matches, so the new
is_dir_included/is_file_includedAPI is used everywhere.crates/biome_service/src/workspace.rs (1)
1215-1226: Deriving Eq/PartialEq for IgnoreKind is spot on.This unblocks straightforward comparisons at call sites and keeps Path as the default. No concerns.
crates/biome_cli/tests/cases/included_files.rs (1)
100-106: Test update matches intended folder-ignore semantics.Switching to ["/*.js", "!/folder"] precisely exercises folder-level ignores while still including top-level files. Good coverage for the regression.
crates/biome_cli/src/execute/traverse.rs (1)
540-546: Using IgnoreKind::Ancestors for dirs/symlinks is the right call.This aligns traversal with ancestor-aware ignore semantics and fixes the “explicit file under ignored folder” case via directory handling. Nicely contained change.
crates/biome_service/src/workspace/server.rs (3)
606-617: Double-check KnownFiles + Update with Ancestors: do unknown file types slip in?When
scan_kindisKnownFiles/TargetedKnownFilesandignore_kindisAncestors, we only consult the parent dir’s top-level ignore; we don’t also gate onpath.is_required_during_scan(). For updates on files that aren’t already indexed, this might cause us to index a non-required file type if its parent isn’t ignored. If this is intentional, all good—otherwise, worth a test.Would you like me to draft a test that sends an update for a non-required file under a non-ignored parent and asserts it’s still ignored?
975-981: Good: get_file_features now uses filesystem-aware Projects API.Propagating
fsintoget_file_featuresensures include/ignore decisions align with file/dir semantics and ancestor rules.
996-1003: Good: path-level ignore checks now filesystem-aware.
is_path_ignoredforwardingfsintoProjects::is_ignored(...)brings CLI/API queries in line with the traversal behaviour. The special-case for the top-level config file remains intact.crates/biome_service/src/projects.rs (5)
7-7: Import looks right.Bringing in
FileSystemis needed for the new fs-parameterised APIs;ConfigNameis used inget_file_features.
141-152: Nice API uplift: Projects methods now filesystem-aware.Plumbing
fsinto:
is_ignored_by_top_level_configis_ignoredget_file_features- and the helper
unblocks correct file vs dir inclusion and ancestor evaluation. This is the right level to centralise the logic.
Also applies to: 156-164, 185-194, 341-347
359-363: Edge case: symlinks.We classify kind via
fs.path_is_dir(path). If symlinks are in play, confirmpath_is_dirfollows them as intended for include checks. If not, we might need a symlink-aware branch (only if you’ve seen oddities).Happy to add a focused test around a symlinked dir inside an ignored folder to confirm behaviour.
210-221: Top-level config file exemption: retained.Keeping
biome.json(c)at project root always included is spot on and mirrors server-side checks.
218-221: Good: unify feature gating with Ancestors semantics.Using
self.is_ignored(..., IgnoreKind::Ancestors)insideget_file_featuresensures explicit file queries respect ancestor folder ignores—exactly what #7268 needed.
|
Is the regression expected? |
|
No, I was looking into it, but I'm afraid I might need @siketyan 's help again :( The issue is due to symlink handling, but just by looking at the code I can't quite see why it would behave differently on Windows 🤷♂️ |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_fs/src/fs/os.rs (1)
361-392: Add targeted tests for Windows/relative symlink behaviourPlease add tests that assert:
path_kindon a symlink-to-dir returnsDirectory { is_symlink: false }.path_kindon a symlink-to-file returnsFile { is_symlink: false }.- Relative symlink targets are resolved against the link’s parent.
Using
TemporaryFsshould make these straightforward. If you’d like, I can draft them.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
crates/biome_fs/src/fs/os.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Format Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_fs/src/fs/os.rs
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_fs/src/fs/os.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Test Node.js API
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: autofix
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
siketyan
force-pushed
the
fix-nested-ignored-files
branch
from
2872a49 to
4fc08fa
Compare
Summary
Fixed #7268: Files that are explicitly passed as CLI arguments are now correctly ignored if they reside in an ignored folder.
Warning: This may have a slight but noticeable performance impact on traversal. We might be able to prevent this by pre-filtering the paths that are specified explicitly before traversal, but I wanted to fix the bug first.
Also fixes #7279.
Test Plan
Flawed test updated. Others should remain green.