fix(grit): match namespace and multi-specifier import patterns#9667
fix(grit): match namespace and multi-specifier import patterns#9667raashish1601 wants to merge 1 commit intobiomejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 96dcbe0 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 |
WalkthroughThis PR fixes GritQL pattern matching for import statements to align with Possibly related PRs
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. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_grit_patterns/tests/specs/ts/import_patterns.ts (1)
1-18: Please pin the$importbinding as well.
crates/biome_grit_patterns/tests/specs/ts/import_patterns.grit:1binds$import, but the current snapshot incrates/biome_grit_patterns/tests/specs/ts/import_patterns.snap:1-18only checks whole-statementmatched_ranges. With the new slot remapping incrates/biome_grit_patterns/src/grit_node_patterns.rs, this can still pass if namespace or multi-specifier imports bind the wrong subtree. A tiny rewrite/capture snapshot here would lock down the actual fix.Based on learnings: "All code changes MUST include appropriate tests: lint rules require snapshot tests in 'tests/specs/{group}/{rule}/', formatters require snapshot tests with valid/invalid cases, parsers require test files covering valid and error cases, and bug fixes require tests that reproduce and validate the fix."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_grit_patterns/tests/specs/ts/import_patterns.ts` around lines 1 - 18, Update the test to also pin the $import binding and assert its matched subtree so the snapshot validates the specific import node (not just whole-statement matched_ranges): modify the pattern in crates/biome_grit_patterns/tests/specs/ts/import_patterns.grit to capture $import for each import variant, then update crates/biome_grit_patterns/tests/specs/ts/import_patterns.snap to include the captured $import matched_range(s) (or a small rewrite/capture snapshot showing the $import subtree) so the new slot remapping in crates/biome_grit_patterns/src/grit_node_patterns.rs is exercised and locked down.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_grit_patterns/tests/specs/ts/import_patterns.ts`:
- Around line 1-18: Update the test to also pin the $import binding and assert
its matched subtree so the snapshot validates the specific import node (not just
whole-statement matched_ranges): modify the pattern in
crates/biome_grit_patterns/tests/specs/ts/import_patterns.grit to capture
$import for each import variant, then update
crates/biome_grit_patterns/tests/specs/ts/import_patterns.snap to include the
captured $import matched_range(s) (or a small rewrite/capture snapshot showing
the $import subtree) so the new slot remapping in
crates/biome_grit_patterns/src/grit_node_patterns.rs is exercised and locked
down.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bef6a934-cc58-43bb-8243-8ff58a0c7b72
⛔ Files ignored due to path filters (1)
crates/biome_grit_patterns/tests/specs/ts/import_patterns.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (4)
.changeset/odd-cycles-match.mdcrates/biome_grit_patterns/src/grit_code_snippet.rscrates/biome_grit_patterns/src/grit_node_patterns.rscrates/biome_grit_patterns/tests/specs/ts/import_patterns.ts
Merging this PR will degrade performance by 43.54%
Performance Changes
Comparing Footnotes
|
Summary
importdon't match all kinds of import statements #7727 regression fixture and expected snapshotTesting
git diff --checkcargo test -p biome_grit_patterns ...in this environment because the drive only had about 0.8 GB free and rustc/link failed withos error 112while creatingtarget/debugFixes #7727.