fix(grit): make $filename metavariable accessible in GritQL plugins#8755
fix(grit): make $filename metavariable accessible in GritQL plugins#8755dyc3 merged 9 commits intobiomejs:mainfrom
Conversation
This fixes GitHub issue biomejs#6670 where the $filename metavariable was not accessible when using where clauses in GritQL patterns. The root cause was that global variables ($filename, $program, etc.) and local pattern variables were both stored in scope 0. When the auto-wrap bubble pattern entered scope 0, it would reset ALL variable values to None, including the $filename that was set by FilePattern. The fix separates these scopes: - Scope 0: Global variables only ($new_files, $program, $filename, $absolute_filename) - Scope 1: Local pattern variables (user-defined variables like $args, $message) Now when the auto-wrap bubble enters scope 1, it resets local pattern variables (giving each match a fresh state) while preserving global variables in scope 0. Also adds Pattern::Variable(Variable::file_name()) to wrap_pattern_in_file to ensure $filename is properly bound when FilePattern matches. Fixes biomejs#6670 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 0d02f10 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 |
WalkthroughEnsures a two‑scope variable model: scope 0 for global metavariables and scope 1 for local pattern variables. NodeCompilationContext::new now guarantees a global scope exists, sets scope_index to the computed index, and appends a local scope. Auto-wrap uses context.scope_index for bubble PatternDefinition and binds FilePattern filenames with Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 (1)
crates/biome_grit_patterns/tests/quick_test.rs (1)
35-36: Consider usingdbg!()for debugging output.Per coding guidelines, prefer
dbg!()overprintln!for debugging output in Rust tests. This applies to lines 35-36 and 73-74.♻️ Suggested change
- println!("Logs explicit: {:?}", logs); - println!("Effects explicit: {effects:#?}"); + dbg!(&logs); + dbg!(&effects);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
crates/biome_grit_patterns/tests/specs/ts/filenameMatch.snapis excluded by!**/*.snapand included by**crates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (8)
crates/biome_grit_patterns/src/grit_query.rscrates/biome_grit_patterns/src/pattern_compiler/auto_wrap.rscrates/biome_grit_patterns/src/pattern_compiler/compilation_context.rscrates/biome_grit_patterns/tests/quick_test.rscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_grit_patterns/tests/quick_test.rscrates/biome_grit_patterns/src/pattern_compiler/auto_wrap.rscrates/biome_grit_patterns/src/pattern_compiler/compilation_context.rscrates/biome_grit_patterns/src/grit_query.rs
🧠 Learnings (17)
📚 Learning: 2025-12-04T13:29:49.287Z
Learnt from: dyc3
Repo: biomejs/biome PR: 8291
File: crates/biome_html_formatter/tests/specs/prettier/vue/html-vue/elastic-header.html:10-10
Timestamp: 2025-12-04T13:29:49.287Z
Learning: Files under `crates/biome_html_formatter/tests/specs/prettier` are test fixtures synced from Prettier and should not receive detailed code quality reviews (e.g., HTTP vs HTTPS, formatting suggestions, etc.). These files are test data meant to validate formatter behavior and should be preserved as-is.
Applied to files:
crates/biome_grit_patterns/tests/specs/ts/filenameMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/* : Create test files with `invalid` and `valid` prefixes to represent code that should and should not trigger the rule
Applied to files:
crates/biome_grit_patterns/tests/specs/ts/filenameMatch.tscrates/biome_grit_patterns/tests/quick_test.rscrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/*.jsonc : Use `.jsonc` format for test files containing multiple code snippets, where each snippet is a string in an array
Applied to files:
crates/biome_grit_patterns/tests/specs/ts/filenameMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/* : Test snapshot output files must be placed in `tests/specs/` folder with subfolders matching the rule group and rule name
Applied to files:
crates/biome_grit_patterns/tests/specs/ts/filenameMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
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:
crates/biome_grit_patterns/tests/specs/ts/filenameMatch.tscrates/biome_grit_patterns/tests/quick_test.rscrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/language.rs : Implement `TestFormatLanguage` trait in `tests/language.rs` for the formatter's test language
Applied to files:
crates/biome_grit_patterns/tests/quick_test.rscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
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:
crates/biome_grit_patterns/tests/quick_test.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Create test infrastructure with `tests/specs` folder structure and `spec_test.rs`, `spec_tests.rs`, and `language.rs` files in test directories
Applied to files:
crates/biome_grit_patterns/tests/quick_test.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
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:
crates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.gritcrates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.tscrates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
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_grit_patterns/src/pattern_compiler/auto_wrap.rscrates/biome_grit_patterns/src/pattern_compiler/compilation_context.rscrates/biome_grit_patterns/src/grit_query.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Check if a variable is global using the semantic model before reporting diagnostics for rules that ban global functions or variables
Applied to files:
crates/biome_grit_patterns/src/grit_query.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use the `Semantic<T>` query type to access semantic information about bindings, references, and scope within a rule
Applied to files:
crates/biome_grit_patterns/src/grit_query.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Use `declare_node_union!` macro to query multiple node types together to avoid redundant traversal passes
Applied to files:
crates/biome_grit_patterns/src/grit_query.rs
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/local_inference.rs : Implement local inference in dedicated modules to derive type definitions from expressions without context of surrounding scopes
Applied to files:
crates/biome_grit_patterns/src/grit_query.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/*_analyze/**/src/lint/**/*.rs : Implement custom `Queryable` and `Visitor` types for rules that require deep inspection of child nodes to avoid inefficient traversals
Applied to files:
crates/biome_grit_patterns/src/grit_query.rs
📚 Learning: 2025-11-24T18:06:03.545Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:03.545Z
Learning: Applies to crates/biome_parser/**/src/**/*.rs : Use `ConditionalParsedSyntax` for syntax that is only valid in specific contexts (e.g., strict mode, file types, language versions) and call `or_invalid_to_bogus()` to convert to a bogus node if not supported
Applied to files:
crates/biome_grit_patterns/src/grit_query.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/biome_rule_options/lib/**/*.rs : Rule options must be defined in the `biome_rule_options` crate with a file named after the rule
Applied to files:
crates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit
🧬 Code graph analysis (3)
crates/biome_grit_patterns/tests/quick_test.rs (4)
crates/biome_grit_parser/src/lib.rs (1)
parse_grit(18-21)crates/biome_grit_patterns/src/grit_query.rs (4)
from_node(112-206)new(315-320)new(349-354)new(367-375)crates/biome_grit_patterns/src/pattern_compiler/compilation_context.rs (2)
new(27-40)new(71-79)crates/biome_js_syntax/src/file_source.rs (1)
ts(185-192)
crates/biome_grit_patterns/src/pattern_compiler/auto_wrap.rs (1)
crates/biome_formatter_test/src/spec.rs (1)
file_name(100-102)
crates/biome_grit_patterns/src/grit_query.rs (1)
crates/biome_grit_patterns/src/pattern_compiler/compilation_context.rs (3)
new(27-40)new(71-79)new_with_scope(81-97)
🔇 Additional comments (10)
crates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.grit (1)
1-3: LGTM!Solid negative test case. The regex
.*\.tsx$will correctly filter out.tsfiles, validating that$filenameconstraints work as expected with the new scoping fix.crates/biome_grit_patterns/src/pattern_compiler/auto_wrap.rs (2)
410-416: Nice fix for the scope separation.Using
context.scope_index(scope 1) for the bubble ensures local pattern variables are reset without clobbering globals like$filenamein scope 0. The comment explains the rationale clearly.
419-426: Core fix for$filenamebinding.Switching from
Pattern::ToptoPattern::Variable(Variable::file_name())ensures the filename is actually bound whenFilePatternmatches, making it accessible inwhereclauses. This directly addresses issue #6670.crates/biome_grit_patterns/tests/specs/ts/filenameNoMatch.ts (1)
1-2: LGTM!Clear test fixture. The
.tsextension won't match the.tsx$regex infilenameNoMatch.grit, confirming the negative case works.crates/biome_grit_patterns/tests/specs/ts/filenameMatch.grit (1)
1-3: LGTM!Good positive test case for validating
$filenamebinding works.Minor note: the regex
.*\.ts$will also match.mts,.cts, and.tsxfiles (since they end with "ts"). For a stricter match, you could use.*\.ts$with a negative lookahead or.*[^x]\.ts$, but for this test's purpose it's fine as-is.crates/biome_grit_patterns/tests/specs/ts/filenameMatch.ts (1)
1-2: LGTM!Clean test fixture. Both
console.logcalls should match whenfilenameMatch.gritruns against this.tsfile, validating the fix works end-to-end.crates/biome_grit_patterns/src/pattern_compiler/compilation_context.rs (1)
71-97: LGTM!Clean refactor introducing
new_with_scopewhile maintaining backward compatibility via delegation fromnew. The existing documentation forscope_indexandvars_arrayadequately explains the scoping model.crates/biome_grit_patterns/tests/quick_test.rs (1)
44-78: Good coverage of the core fix.This test directly validates the originally failing pattern from issue #6670. The structure is clear and the assertion confirms the fix works.
crates/biome_grit_patterns/src/grit_query.rs (2)
138-156: Well-documented scope separation.The inline comments clearly explain the two-scope model and rationale. This is the crux of the fix—separating globals (scope 0) from locals (scope 1) so the auto-wrap bubble doesn't clobber
$filename.
166-173: Correct scope initialisation.Using scope 1 for local pattern variables ensures definitions and patterns are compiled in the right context, preserving global variables in scope 0.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change println! to dbg! in quick_test.rs for debugging output - Simplify changeset to be user-focused without jargon - Add issue link to changeset
|
@dyc3 thanks for the review, pushed a few changes now. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_grit_patterns/tests/quick_test.rs (1)
67-67: Preferdbg!()for consistency.Lines 35-36 and 76-77 use
dbg!(), but this line usesprintln!. As per coding guidelines, preferdbg!()for debugging output in Rust tests.♻️ Suggested fix
- println!("Compiled pattern: {:?}", query.pattern); + dbg!(&query.pattern);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fix-filename-metavariable.mdcrates/biome_grit_patterns/tests/quick_test.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_grit_patterns/tests/quick_test.rs
🧠 Learnings (5)
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
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:
crates/biome_grit_patterns/tests/quick_test.rs
📚 Learning: 2026-01-02T14:58:16.536Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-01-02T14:58:16.536Z
Learning: Applies to crates/biome_analyze/**/tests/specs/**/* : Create test files with `invalid` and `valid` prefixes to represent code that should and should not trigger the rule
Applied to files:
crates/biome_grit_patterns/tests/quick_test.rs
📚 Learning: 2025-11-24T18:06:12.048Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:06:12.048Z
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:
crates/biome_grit_patterns/tests/quick_test.rs
📚 Learning: 2025-11-24T18:05:20.371Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:20.371Z
Learning: Applies to crates/biome_formatter/**/biome_*_formatter/tests/language.rs : Implement `TestFormatLanguage` trait in `tests/language.rs` for the formatter's test language
Applied to files:
crates/biome_grit_patterns/tests/quick_test.rs
📚 Learning: 2025-12-21T21:15:03.796Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-12-21T21:15:03.796Z
Learning: In changesets, start with a link to the issue when fixing a bug (e.g., 'Fixed [#4444](link): ...')
Applied to files:
.changeset/fix-filename-metavariable.md
🧬 Code graph analysis (1)
crates/biome_grit_patterns/tests/quick_test.rs (3)
crates/biome_grit_parser/src/lib.rs (1)
parse_grit(18-21)crates/biome_grit_patterns/src/grit_query.rs (4)
from_node(111-205)new(314-319)new(348-353)new(366-374)crates/biome_js_syntax/src/file_source.rs (1)
ts(185-192)
🔇 Additional comments (3)
.changeset/fix-filename-metavariable.md (1)
1-7: LGTM!Changeset is well-formatted, starts with the issue link as expected, and the description is clear for users. Based on learnings, this follows the convention for bug fix changesets.
crates/biome_grit_patterns/tests/quick_test.rs (2)
8-43: LGTM!Test is well-structured and uses
dbg!()for debugging output as per coding guidelines. Good coverage for the explicit file pattern case.
69-84: LGTM!Test logic correctly validates that
$filenameis bound in auto-wrapped where clauses. Good regression test for issue #6670.
| // Scope 1: Local pattern variables (initially empty) | ||
| Vec::new(), | ||
| ]; | ||
| let mut global_vars: BTreeMap<String, usize> = GLOBAL_VARS | ||
| .iter() | ||
| .map(|(global_var, index)| ((*global_var).to_string(), *index)) | ||
| .collect(); | ||
| let mut diagnostics = Vec::new(); | ||
|
|
||
| // We're not in a local scope yet, so this map is kinda useless. | ||
| // It's just there because all node compilers expect one. | ||
| // Local variables map - used for pattern variables in scope 1 | ||
| let mut vars = BTreeMap::new(); | ||
|
|
||
| let mut node_context = NodeCompilationContext::new( | ||
| let mut node_context = NodeCompilationContext::new_with_scope( | ||
| &context, | ||
| &mut vars, | ||
| &mut vars_array, | ||
| &mut global_vars, | ||
| &mut diagnostics, | ||
| 1, // Use scope 1 for local pattern variables | ||
| ); |
There was a problem hiding this comment.
These changes seem unrelated to the issue. What do you @arturalkaim ?
There was a problem hiding this comment.
not really, this is the core of the fix
Previously, both global variables ($filename, $program, etc.) and local pattern variables were in scope 0. When the auto-wrap bubble pattern entered scope 0 via enter_scope(0), it reset ALL variable values to None - including $filename that FilePattern had just set.
Now when the bubble enters scope 1, it resets local pattern variables (giving each match a fresh state) while preserving global variables in scope 0.
Without this scope separation, the $filename fix in auto_wrap.rs alone doesn't work - the value still gets reset when the bubble enters the scope.
There was a problem hiding this comment.
Yeah using scope 1 here seems correct and matches the original GritQL implementation.
morgante
left a comment
morgante
left a comment
There was a problem hiding this comment.
Let's fix the new() method to prevent creating incorrect contexts in the first place - the scope index of a new context should always default to the end of the vars array.
| // Scope 1: Local pattern variables (initially empty) | ||
| Vec::new(), | ||
| ]; | ||
| let mut global_vars: BTreeMap<String, usize> = GLOBAL_VARS | ||
| .iter() | ||
| .map(|(global_var, index)| ((*global_var).to_string(), *index)) | ||
| .collect(); | ||
| let mut diagnostics = Vec::new(); | ||
|
|
||
| // We're not in a local scope yet, so this map is kinda useless. | ||
| // It's just there because all node compilers expect one. | ||
| // Local variables map - used for pattern variables in scope 1 | ||
| let mut vars = BTreeMap::new(); | ||
|
|
||
| let mut node_context = NodeCompilationContext::new( | ||
| let mut node_context = NodeCompilationContext::new_with_scope( | ||
| &context, | ||
| &mut vars, | ||
| &mut vars_array, | ||
| &mut global_vars, | ||
| &mut diagnostics, | ||
| 1, // Use scope 1 for local pattern variables | ||
| ); |
There was a problem hiding this comment.
Yeah using scope 1 here seems correct and matches the original GritQL implementation.
| ) | ||
| } | ||
|
|
||
| pub(crate) fn new_with_scope( |
There was a problem hiding this comment.
Shouldn't need this actually, just use the length of vars_array when choosing the scope index.
See the equivalent logic here: https://github.com/biomejs/gritql/blob/main/crates/core/src/pattern_compiler/mod.rs#L8-L15
There was a problem hiding this comment.
True, then this one is not needed.
Address review feedback from @morgante: - Remove new_with_scope() method - Use vars_array.len() as scope index in new(), matching original GritQL - See: https://github.com/biomejs/gritql/blob/main/crates/core/src/pattern_compiler/mod.rs#L8-L15 Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @crates/biome_grit_patterns/src/pattern_compiler/compilation_context.rs:
- Around line 78-84: NodeCompilationContext::new currently assumes callers
pre-populate vars_array with a global scope, but test_pattern_from_node in
snippet_compiler.rs passes an empty vars_array which makes scope_index 0 and
breaks global/local separation; fix by making NodeCompilationContext::new ensure
vars_array has a global scope (if vars_array.is_empty() push Vec::new()), set
scope_index accordingly, and add a short rustdoc on NodeCompilationContext::new
noting that it will create a global scope when none is provided so callers need
not pre-populate.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/biome_grit_patterns/src/grit_query.rscrates/biome_grit_patterns/src/pattern_compiler/compilation_context.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/biome_grit_patterns/src/grit_query.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Use inline rustdoc documentation for rules, assists, and their options
Use thedbg!()macro for debugging output in Rust tests and code
Use doc tests (doctest) format with code blocks in rustdoc comments; ensure assertions pass in tests
Files:
crates/biome_grit_patterns/src/pattern_compiler/compilation_context.rs
🧠 Learnings (1)
📚 Learning: 2025-11-24T18:05:42.356Z
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_type_info/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:42.356Z
Learning: Applies to crates/biome_js_type_info/**/local_inference.rs : Implement local inference in dedicated modules to derive type definitions from expressions without context of surrounding scopes
Applied to files:
crates/biome_grit_patterns/src/pattern_compiler/compilation_context.rs
4 participants
This PR was written primarily by Claude Code.
Summary
This PR fixes GitHub issue #6670 where the metavariable was not accessible when using clauses in GritQL patterns.
For example, this pattern was not working:
Root Cause
Global variables (
,, etc.) and local pattern variables were both stored in scope 0. When the auto-wrap bubble pattern entered scope 0, it would reset ALL variable values toNone, including the `` that was set byFilePattern.Solution
Separate the scopes:
,,,),)Now when the auto-wrap bubble enters scope 1, it resets local pattern variables (giving each match a fresh state) while preserving global variables in scope 0.
Additionally,
wrap_pattern_in_filenow usesPattern::Variable(Variable::file_name())to ensure `` is properly bound whenFilePatternmatches.Test plan
filenameMatch.gritthat verifies `` matches.tsfilesfilenameNoMatch.gritthat verifies `` doesn't match.tsxfor `.ts` filesquick_test.rsfor the `` bindingFixes #6670
Docs
N/A - This is a bug fix for an existing feature, no documentation changes required.