chore: add migration for ignores scanner #7751
Conversation
|
| Name | Type |
|---|---|
| @biomejs/biome | Minor |
| @biomejs/cli-win32-x64 | Minor |
| @biomejs/cli-win32-arm64 | Minor |
| @biomejs/cli-darwin-x64 | Minor |
| @biomejs/cli-darwin-arm64 | Minor |
| @biomejs/cli-linux-x64 | Minor |
| @biomejs/cli-linux-arm64 | Minor |
| @biomejs/cli-linux-x64-musl | Minor |
| @biomejs/cli-linux-arm64-musl | Minor |
| @biomejs/wasm-web | Minor |
| @biomejs/wasm-bundler | Minor |
| @biomejs/wasm-nodejs | Minor |
| @biomejs/backend-jsonrpc | Patch |
| @biomejs/js-api | Major |
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
WalkthroughAdds a new migration rule Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/biome_migrate/src/analyzers/ignore_scanner.rs (2)
92-94: Remove unnecessary clone.The
array_value.clone()is unnecessary since you're only iterating over the elements.Apply this diff:
- let mut new_list: Vec<_> = - array_value.clone().elements().iter().flatten().collect(); + let mut new_list: Vec<_> = + array_value.elements().iter().flatten().collect();
98-105: Consider extracting separator generation logic.The comma-with-whitespace separator generation pattern is repeated three times (lines 98-105, 130-136, 150-155). Whilst not critical, a small helper function would reduce duplication.
Example:
fn make_separators(count: usize) -> Vec<SyntaxToken> { (0..count.saturating_sub(1)) .map(|_| token(T![,]).with_trailing_trivia([(TriviaPieceKind::Whitespace, " ")])) .collect() }Then replace each block with:
let separators = make_separators(new_list.len());Also applies to: 130-136, 150-155
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json.snapis excluded by!**/*.snapand included by**crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid2.json.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (5)
.changeset/forced-files-forget.md(1 hunks)crates/biome_migrate/src/analyzers.rs(3 hunks)crates/biome_migrate/src/analyzers/ignore_scanner.rs(1 hunks)crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json(1 hunks)crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid2.json(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_migrate/tests/specs/migrations/ignoreScanner/invalid2.jsoncrates/biome_migrate/src/analyzers.rscrates/biome_migrate/src/analyzers/ignore_scanner.rscrates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json
**/tests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place test files under a tests/ directory in each crate
Files:
crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid2.jsoncrates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json
.changeset/**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/**/*.md: Create changesets using thejust new-changesetcommand; do not author them manually
In changeset markdown, only use headers #### or #####
Changeset descriptions must end every sentence with a full stop (.)
For bug fixes, start the changeset description with a linked issue reference like “Fixed #1234”
Prefer past tense for what was done and present tense for current behavior in changesets
Files:
.changeset/forced-files-forget.md
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_migrate/src/analyzers.rscrates/biome_migrate/src/analyzers/ignore_scanner.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_migrate/src/analyzers.rscrates/biome_migrate/src/analyzers/ignore_scanner.rs
🧠 Learnings (2)
📚 Learning: 2025-10-02T12:57:33.228Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.228Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/**/*.jsonc : Use .jsonc files to hold arrays of code snippets for snapshot tests; these snippets run in script mode (no import/export)
Applied to files:
crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json
📚 Learning: 2025-10-02T12:57:33.228Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T12:57:33.228Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/specs/nursery/** : Place snapshot tests for new rules under tests/specs/nursery/<ruleName>/ with files prefixed by invalid* and valid*
Applied to files:
crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json
⏰ 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: End-to-end tests
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
- GitHub Check: autofix
🔇 Additional comments (7)
crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid2.json (1)
1-8: LGTM!Clean test fixture for the case where
includesdoesn't exist. This will validate that the migration creates["**", "!!foo", "!!bar"]as expected..changeset/forced-files-forget.md (1)
44-44: LGTM!Clear instruction for users to run the migration. Properly formatted per changeset guidelines.
crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json (1)
1-9: LGTM!Good test fixture for the case where
includesalready exists. Validates that the migration extends the existing array with the transformed ignores.crates/biome_migrate/src/analyzers.rs (1)
2-2: LGTM!Standard rule registration following the established pattern. Placement at the end of v2.0.0 rules is appropriate.
Also applies to: 15-15, 46-46
crates/biome_migrate/src/analyzers/ignore_scanner.rs (3)
1-19: LGTM!Standard migration rule declaration with appropriate version and name.
21-47: LGTM!The run method correctly identifies the target member, and the diagnostic message is clear and informative.
49-68: LGTM!Correctly transforms
experimentalScannerIgnoresentries by prefixing with!!. TheVecDequechoice enables efficient prepending of**later if needed.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/biome_migrate/src/analyzers/ignore_scanner.rs (1)
62-65: Good fix: force-ignore prefix added.Prefixing with !!**/ ensures matches at any depth and resolves the earlier review note. Nice.
🧹 Nitpick comments (3)
crates/biome_migrate/src/analyzers/ignore_scanner.rs (3)
14-19: Add a brief rustdoc for the rule (per repo guidelines).A short doc comment describing what this migration does and why will help future maintainers and meets the crate’s docs standards.
As per coding guidelines
31-36: run(): also ensure the member lives under files.Right now any experimentalScannerIgnores anywhere will signal. Guarding for the parent being files reduces false positives and wasted work.
91-117: Avoid no-op rewrites; optionally deduplicate.
- If there’s nothing to add, keep the existing includes node to minimise churn.
- Consider deduping to prevent duplicate patterns if users already have force-ignores.
Apply this minimal churn-avoidance diff:
- } else if text.text() == "includes" { + } else if text.text() == "includes" { + if experimental_ignore_list.is_empty() { + includes_found = true; + return Some(member); + } let array_value = member.value().ok()?.as_json_array_value()?.clone(); let mut new_list: Vec<_> = array_value.clone().elements().iter().flatten().collect(); let mut separators = vec![]; - new_list.extend(experimental_ignore_list.clone()); + new_list.extend(experimental_ignore_list.clone());If you’d like, I can provide a dedup version that builds a set of existing string values before extending.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid.json.snapis excluded by!**/*.snapand included by**crates/biome_migrate/tests/specs/migrations/ignoreScanner/invalid2.json.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (1)
crates/biome_migrate/src/analyzers/ignore_scanner.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_migrate/src/analyzers/ignore_scanner.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_migrate/src/analyzers/ignore_scanner.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_migrate/src/analyzers/ignore_scanner.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). (7)
- GitHub Check: autofix
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Documentation
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
🔇 Additional comments (1)
crates/biome_migrate/src/analyzers/ignore_scanner.rs (1)
159-165: Applicability::Always – confirm intent.Automated application changes formatting and may append duplicates. Confirm Always is desired for this migration; if not, consider MaybeIncorrect or use a guard to skip when no changes are needed.
| let mut experimental_ignore_list = member | ||
| .value() | ||
| .ok()? | ||
| .as_json_array_value()? | ||
| .elements() | ||
| .iter() | ||
| .flatten() | ||
| .filter_map(|element| { | ||
| let value = element.as_json_string_value()?; | ||
| let value = value.inner_string_text().ok()?; | ||
| let new_inner_string = format!("!!**/{}", value.text()); | ||
| Some(AnyJsonValue::JsonStringValue(json_string_value( | ||
| json_string_literal(new_inner_string.as_str()), | ||
| ))) | ||
| }) | ||
| .collect::<VecDeque<_>>(); |
There was a problem hiding this comment.
Don’t bail if the value isn’t an array; still remove the field.
Current code returns None when experimentalScannerIgnores isn’t an array, so the field isn’t removed despite the diagnostic. Treat non-array as empty and proceed.
Apply this diff:
- let mut experimental_ignore_list = member
- .value()
- .ok()?
- .as_json_array_value()?
- .elements()
- .iter()
- .flatten()
- .filter_map(|element| {
- let value = element.as_json_string_value()?;
- let value = value.inner_string_text().ok()?;
- let new_inner_string = format!("!!**/{}", value.text());
- Some(AnyJsonValue::JsonStringValue(json_string_value(
- json_string_literal(new_inner_string.as_str()),
- )))
- })
- .collect::<VecDeque<_>>();
+ let mut experimental_ignore_list = if let Some(array) =
+ member.value().ok().and_then(|v| v.as_json_array_value())
+ {
+ array
+ .elements()
+ .iter()
+ .flatten()
+ .filter_map(|element| {
+ let value = element.as_json_string_value()?;
+ let value = value.inner_string_text().ok()?;
+ let new_inner_string = format!("!!**/{}", value.text());
+ Some(AnyJsonValue::JsonStringValue(json_string_value(
+ json_string_literal(new_inner_string.as_str()),
+ )))
+ })
+ .collect::<VecDeque<_>>()
+ } else {
+ VecDeque::new()
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut experimental_ignore_list = member | |
| .value() | |
| .ok()? | |
| .as_json_array_value()? | |
| .elements() | |
| .iter() | |
| .flatten() | |
| .filter_map(|element| { | |
| let value = element.as_json_string_value()?; | |
| let value = value.inner_string_text().ok()?; | |
| let new_inner_string = format!("!!**/{}", value.text()); | |
| Some(AnyJsonValue::JsonStringValue(json_string_value( | |
| json_string_literal(new_inner_string.as_str()), | |
| ))) | |
| }) | |
| .collect::<VecDeque<_>>(); | |
| let mut experimental_ignore_list = if let Some(array) = | |
| member.value().ok().and_then(|v| v.as_json_array_value()) | |
| { | |
| array | |
| .elements() | |
| .iter() | |
| .flatten() | |
| .filter_map(|element| { | |
| let value = element.as_json_string_value()?; | |
| let value = value.inner_string_text().ok()?; | |
| let new_inner_string = format!("!!**/{}", value.text()); | |
| Some(AnyJsonValue::JsonStringValue(json_string_value( | |
| json_string_literal(new_inner_string.as_str()), | |
| ))) | |
| }) | |
| .collect::<VecDeque<_>>() | |
| } else { | |
| VecDeque::new() | |
| }; |
🤖 Prompt for AI Agents
In crates/biome_migrate/src/analyzers/ignore_scanner.rs around lines 52–67, the
current chain returns None when member.value() exists but isn't an array,
preventing the field from being removed; change it to treat non-array as an
empty list and continue. Concretely: obtain the value with member.value().ok()?
into a variable, attempt to get as_json_array_value(), and if that fails use an
empty iterator (or Vec/VecDeque) so the subsequent filter_map runs over zero
elements instead of bailing; keep the existing mapping that builds the
transformed JsonString values and collect into a VecDeque as before, then
proceed to remove or replace the field. Ensure no early return occurs when the
value is present but not an array.
There was a problem hiding this comment.
The suggestion isn't correct. The field is always an array. If it wasn't, it wouldn't be a valid configuration file
| let files_member = member | ||
| .syntax() | ||
| .ancestors() | ||
| .skip(1) | ||
| .find_map(JsonMember::cast)?; | ||
|
|
||
| // List without `experimentalScannerIgnores` |
There was a problem hiding this comment.
Guard against migrating non-files parents.
If experimentalScannerIgnores appears outside files, this will add includes to the wrong object. Verify the ancestor is files before mutating.
Apply this diff:
- let files_member = member
+ let files_member = member
.syntax()
.ancestors()
.skip(1)
.find_map(JsonMember::cast)?;
+ let files_name = files_member
+ .name()
+ .ok()?
+ .inner_string_text()
+ .ok()?;
+ if files_name.text() != "files" {
+ return None;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let files_member = member | |
| .syntax() | |
| .ancestors() | |
| .skip(1) | |
| .find_map(JsonMember::cast)?; | |
| // List without `experimentalScannerIgnores` | |
| let files_member = member | |
| .syntax() | |
| .ancestors() | |
| .skip(1) | |
| .find_map(JsonMember::cast)?; | |
| let files_name = files_member | |
| .name() | |
| .ok()? | |
| .inner_string_text() | |
| .ok()?; | |
| if files_name.text() != "files" { | |
| return None; | |
| } | |
| // List without `experimentalScannerIgnores` |
🤖 Prompt for AI Agents
In crates/biome_migrate/src/analyzers/ignore_scanner.rs around lines 69 to 75,
the current logic finds a parent JsonMember for a member and assumes it's the
"files" object; if experimentalScannerIgnores appears outside of files this will
mutate the wrong object. Change the ancestor search to ensure the found
JsonMember is the "files" member (e.g., check the member name/key equals "files"
or verify its parent is the files object) and only proceed to add includes when
that check passes; if the ancestor is not the files member, bail out or skip
mutation.
There was a problem hiding this comment.
The parent of experimentalScannerIgnores is always files, and this migration always runs on configuration files
Summary
This PR creates a migration for
experimentalScannerIgnoresTest Plan
Added two tests
Docs
Updated the changeset