fix(lint): extend matcher for GritCodeSnippet to cover edge case #7786

daivinhtran · 2025-10-18T16:06:54Z

For

`$fn($args)` where {
    $fn <: `buildConfig`
}

$fn($args) is compiled into a GritNodePattern with kind JS_CALL_EXPRESSION. $fn binds to a JS_IDENTIFIER_EXPRESSION node.

`buildConfig` creates GritCodeSnippet with these patterns

  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JS_IDENTIFIER_BINDING)
  - JsSyntaxKind(JS_NAME)
  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JS_LITERAL_MEMBER_NAME)
  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JS_LITERAL_MEMBER_NAME)
  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JSX_TEXT)
  - JsSyntaxKind(JSX_NAME)
  - JsSyntaxKind(JS_IDENTIFIER_BINDING)
  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JS_IDENTIFIER_BINDING)
  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JS_REFERENCE_IDENTIFIER)
  - JsSyntaxKind(JS_IDENTIFIER_BINDING)
  - JsSyntaxKind(JS_LITERAL_MEMBER_NAME)

Given CST for buildConfig() is

    0: JS_EXPRESSION_STATEMENT@0..14
      0: JS_CALL_EXPRESSION@0..13 
        0: JS_IDENTIFIER_EXPRESSION@0..11
          0: JS_REFERENCE_IDENTIFIER@0..11
            0: IDENT@0..11 "buildConfig" [] []
        1: (empty)
        2: (empty)
        3: JS_CALL_ARGUMENTS@11..13
          0: L_PAREN@11..12 "(" [] []
          1: JS_CALL_ARGUMENT_LIST@12..12
          2: R_PAREN@12..13 ")" [] []
      1: SEMICOLON@13..14 ";" [] []

The node kind of buildConfig is JS_IDENTIFIER_EXPRESSION which is not part of the available patterns.

My fix is just making GritCodeSnippet's matcher more flexible by detecting when JS_IDENTIFIER_EXPRESSION has a single child and unwraps to JS_REFERENCE_IDENTIFIER.

I'm not pleased with the fix. Without the fix, there are other workarounds

`$fn($args)` where {
    $fn <: "buildConfig" // using string quotation
}

`$fn` where {
    $fn <: `buildConfig`
}

There could be more principled approach. With on-going effort in js plugin, I'm not sure if it's worth diving into that now.

Fixes: #7601

changeset-bot · 2025-10-18T16:06:57Z

🦋 Changeset detected

Latest commit: 3f20c4e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@biomejs/biome	Patch
@biomejs/cli-win32-x64	Patch
@biomejs/cli-win32-arm64	Patch
@biomejs/cli-darwin-x64	Patch
@biomejs/cli-darwin-arm64	Patch
@biomejs/cli-linux-x64	Patch
@biomejs/cli-linux-arm64	Patch
@biomejs/cli-linux-x64-musl	Patch
@biomejs/cli-linux-arm64-musl	Patch
@biomejs/wasm-web	Patch
@biomejs/wasm-bundler	Patch
@biomejs/wasm-nodejs	Patch
@biomejs/backend-jsonrpc	Patch

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

crates/biome_grit_patterns/tests/specs/ts/matchBacktickSnippet.snap

coderabbitai · 2025-10-18T16:10:30Z

Walkthrough

Adds Grit test snippets and corresponding TypeScript test lines that exercise matching of function-call shapes for backtick-delimited and string-literal patterns targeting buildConfig(...). Updates grit_code_snippet.rs to add an exact-kind fast-path and to attempt matching against the child of single-child JS_IDENTIFIER_EXPRESSION wrapper nodes. Adds a unit test test_snippet_node_from_tree in snippet_compiler.rs. No changes to public APIs or exported entities.

Suggested labels

L-JavaScript

Suggested reviewers

arendjr
ematipico

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "fix(lint): extend matcher for GritCodeSnippet to cover edge case" clearly and specifically describes the primary change in the pull request. It identifies both the component being modified (GritCodeSnippet matcher) and the nature of the change (extending to cover an edge case). The title is concise, uses conventional commit prefixes appropriately, and would provide useful context to someone reviewing the project history. The phrasing is concrete and avoids vague terminology.
Linked Issues Check	✅ Passed	The changes directly address issue #7601 by extending the GritCodeSnippet matcher to handle JS_IDENTIFIER_EXPRESSION nodes with a single child. The implementation adds a fast-path for exact kind matches and wrapper-node handling that unwraps JS_IDENTIFIER_EXPRESSION to its child binding, enabling the Grit plugin to correctly detect and flag `buildConfig()` calls as required. Test specs and a snapshot test are included to verify the fix works for backtick and string-literal matching patterns, fulfilling the acceptance criteria of detecting `buildConfig()` usage correctly.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8914c7 and 3f20c4e.

📒 Files selected for processing (2)

.changeset/ripe-rabbits-build.md (1 hunks)
crates/biome_grit_patterns/src/grit_code_snippet.rs (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

crates/biome_grit_patterns/src/grit_code_snippet.rs
.changeset/ripe-rabbits-build.md

⏰ 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). (2)

GitHub Check: Test Node.js API
GitHub Check: autofix

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs (1)

497-515: Good snapshot test; consider adding the call-form too.

This nicely pins the CST for the identifier case. For extra confidence around the reported edge, add a sibling test for buildConfig() to cover the call-expression path here as well (you already have TS specs, but a unit keeps it close to the compiler).

crates/biome_grit_patterns/src/grit_code_snippet.rs (1)

45-75: Apply the suggested optimisations to avoid double-executing patterns and trim the O(n) traversal.

The review is spot-on. The code does execute the exact-kind pattern twice (once in fast-path, again in the second loop if unsuccessful), and node.children().count() == 1 is unnecessarily expensive. The narrowing of the single-child unwrap to only kinds that appear in self.patterns is also a worthwhile safety improvement—currently it could attempt matching against any wrapper, but the suggested approach keeps it focused.

Tests exist (matchBacktickSnippet and matchStringLiteral) to help validate your changes. Apply the suggested diff and run cargo test -p biome_grit_patterns locally to confirm nothing regresses.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7cf8e7 and bbbf993.

⛔ Files ignored due to path filters (1)

crates/biome_grit_patterns/tests/specs/ts/matchBacktickSnippet.snap is excluded by !**/*.snap and included by **

📒 Files selected for processing (2)

crates/biome_grit_patterns/src/grit_code_snippet.rs (2 hunks)
crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs
crates/biome_grit_patterns/src/grit_code_snippet.rs

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Format Rust files before committing (e.g., via just f which formats Rust)
Document rules, assists, and options with inline rustdoc in source

Files:

crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs
crates/biome_grit_patterns/src/grit_code_snippet.rs

🧬 Code graph analysis (2)

crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs (2)

crates/biome_grit_patterns/src/grit_node.rs (1)

node (207-209)

crates/biome_grit_patterns/src/grit_target_node.rs (1)

node (639-641)

crates/biome_grit_patterns/src/grit_code_snippet.rs (1)

crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs (1)

node (258-261)

⏰ 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). (3)

GitHub Check: Check Dependencies
GitHub Check: autofix
GitHub Check: Test Node.js API

🔇 Additional comments (1)

crates/biome_grit_patterns/src/grit_code_snippet.rs (1)

10-10: Remove unused AstNode import on line 10.

The code doesn't use AstNode—methods like first_child() and children() are concrete implementations on GritTargetNode, not trait methods. Drop the import to keep things tidy.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

crates/biome_grit_patterns/src/grit_code_snippet.rs (2)
52-59: Consider avoiding redundant pattern execution.

When the exact-kind pattern exists but fails to execute on line 47, this loop will re-try the same pattern. Whilst not incorrect, it's wasteful.

Consider either:

Tracking which pattern was already attempted and skipping it here, or

Restructuring to avoid the redundancy

64-71: Optimise the single-child check.

The condition node.children().count() == 1 iterates through all children, which is inefficient. Since you already have first_child(), check whether a second child exists instead.

Apply this diff to improve performance:
-        if let Some(child) = node.first_child()  && node.children().count() == 1 {
+        if let Some(child) = node.first_child() && node.children().nth(1).is_none() {
             let child_binding = GritResolvedPattern::from_node_binding(child);
             for (_, pattern) in &self.patterns {
                 if pattern.execute(&child_binding, state, context, logs)? {

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bbbf993 and 7847ab2.

📒 Files selected for processing (1)

crates/biome_grit_patterns/src/grit_code_snippet.rs (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

crates/biome_grit_patterns/src/grit_code_snippet.rs

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Format Rust files before committing (e.g., via just f which formats Rust)
Document rules, assists, and options with inline rustdoc in source

Files:

crates/biome_grit_patterns/src/grit_code_snippet.rs

🧬 Code graph analysis (1)

crates/biome_grit_patterns/src/grit_code_snippet.rs (1)

crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs (1)

node (258-261)

🪛 GitHub Actions: autofix.ci

crates/biome_grit_patterns/src/grit_code_snippet.rs

[error] 75-75: Build failed due to syntax error: unexpected closing delimiter '}' (this delimiter might not be properly closed...).

🪛 GitHub Actions: Pull request Node.js

crates/biome_grit_patterns/src/grit_code_snippet.rs

[error] 75-75: unexpected closing delimiter: }

[error] 75-75: This error prevents building the crate. Could be caused by a mismatched or missing opening/closing delimiter in the surrounding code.

⏰ 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). (2)

GitHub Check: Test (depot-windows-2022-16)
GitHub Check: Lint project (depot-windows-2022)

🔇 Additional comments (1)

crates/biome_grit_patterns/src/grit_code_snippet.rs (1)

10-10: Import addition looks good.

The AstNode import is necessary for the new child-node unwrapping logic below.

crates/biome_grit_patterns/src/grit_code_snippet.rs

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

crates/biome_grit_patterns/src/grit_code_snippet.rs (1)
53-65: Wrapper-node handling correctly addresses the reported issue.

The logic properly unwraps single-child JS_IDENTIFIER_EXPRESSION nodes to match against the inner JS_REFERENCE_IDENTIFIER, which aligns with the fix objective.

Optional micro-optimisation: Line 57's node.children().count() == 1 iterates all children. Since you've already got first_child() in hand, you could check child.next_sibling().is_none() instead—but the current approach is perfectly clear and unlikely to be a bottleneck.
-        if node.kind() == GritTargetSyntaxKind::JsSyntaxKind(JsSyntaxKind::JS_IDENTIFIER_EXPRESSION)
-            && let Some(child) = node.first_child()
-            && node.children().count() == 1
+        if node.kind() == GritTargetSyntaxKind::JsSyntaxKind(JsSyntaxKind::JS_IDENTIFIER_EXPRESSION)
+            && let Some(child) = node.first_child()
+            && child.next_sibling().is_none()

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7847ab2 and a8914c7.

📒 Files selected for processing (2)

.changeset/ripe-rabbits-build.md (1 hunks)
crates/biome_grit_patterns/src/grit_code_snippet.rs (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

crates/biome_grit_patterns/src/grit_code_snippet.rs

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Format Rust files before committing (e.g., via just f which formats Rust)
Document rules, assists, and options with inline rustdoc in source

Files:

crates/biome_grit_patterns/src/grit_code_snippet.rs

.changeset/*.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period

Files:

.changeset/ripe-rabbits-build.md

⏰ 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). (10)

GitHub Check: Documentation
GitHub Check: Test Node.js API
GitHub Check: Check Dependencies
GitHub Check: Test (depot-ubuntu-24.04-arm-16)
GitHub Check: Test (depot-windows-2022-16)
GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
GitHub Check: Lint project (depot-windows-2022)
GitHub Check: End-to-end tests
GitHub Check: autofix
GitHub Check: lint

🔇 Additional comments (3)

crates/biome_grit_patterns/src/grit_code_snippet.rs (3)

4-4: LGTM – imports support the new functionality.

The additions of JsSyntaxKind and AstNode are necessary for the kind check and node traversal in the wrapper-handling logic.

Also applies to: 11-11

46-51: Fast-path optimisation looks good.

The exact-kind matching with early return is a sensible optimisation. The if let pattern with && chaining is clean and idiomatic.

67-67: Correct fallback behaviour.

Returning Ok(false) when neither the fast-path nor wrapper handling succeeds is the expected fallback.

.changeset/ripe-rabbits-build.md

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8914c7 and 82162ac.

📒 Files selected for processing (2)

.changeset/ripe-rabbits-build.md (1 hunks)
crates/biome_grit_patterns/src/grit_code_snippet.rs (2 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

.changeset/*.md

📄 CodeRabbit inference engine (CONTRIBUTING.md)

.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period

Files:

.changeset/ripe-rabbits-build.md

crates/biome_*/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place core crates under /crates/biome_*/

Files:

crates/biome_grit_patterns/src/grit_code_snippet.rs

**/*.rs

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.rs: Format Rust files before committing (e.g., via just f which formats Rust)
Document rules, assists, and options with inline rustdoc in source

Files:

crates/biome_grit_patterns/src/grit_code_snippet.rs

🧬 Code graph analysis (1)

crates/biome_grit_patterns/src/grit_code_snippet.rs (1)

crates/biome_grit_patterns/src/pattern_compiler/snippet_compiler.rs (1)

node (258-261)

⏰ 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: Documentation
GitHub Check: Test (depot-windows-2022-16)
GitHub Check: Test (depot-ubuntu-24.04-arm-16)
GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
GitHub Check: Check Dependencies
GitHub Check: Lint project (depot-windows-2022)
GitHub Check: autofix

🔇 Additional comments (4)

crates/biome_grit_patterns/src/grit_code_snippet.rs (4)

4-4: Imports look correct.

Both JsSyntaxKind (used at line 55) and AstNode (provides first_child/next_sibling methods used at lines 56-57) are necessary for the new logic.

Also applies to: 11-11

46-51: Fast path optimisation is sound.

Checking exact kind match first and returning early on success is a sensible performance improvement.

53-65: Wrapper-node handling correctly addresses the issue.

The logic properly checks for a single-child JS_IDENTIFIER_EXPRESSION, unwraps to the child, and attempts matching across all patterns. This directly fixes the buildConfig() detection issue described in #7601.

67-67: Fallback behaviour is correct.

Returning false when neither fast path nor wrapper match succeeds maintains the expected default behaviour.

.changeset/ripe-rabbits-build.md

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Add test

c7cf8e7

daivinhtran marked this pull request as draft October 18, 2025 16:07

github-actions bot added A-Parser Area: parser L-Grit Language: GritQL labels Oct 18, 2025

daivinhtran commented Oct 18, 2025

View reviewed changes

crates/biome_grit_patterns/tests/specs/ts/matchBacktickSnippet.snap Outdated Show resolved Hide resolved

daivinhtran added 2 commits October 20, 2025 18:01

Make GritCodeSnippet matcher for flexible

4a19cf4

Remove tests

b1a196d

github-actions bot removed the A-Parser Area: parser label Oct 20, 2025

daivinhtran added 2 commits October 20, 2025 18:14

Make matcher for strict

2984ff7

Add comment

bbbf993

daivinhtran changed the title ~~Tests to reproduce #7601~~ fix(plugin): Extend matcher for GritCodeSnippet to cover edge case Oct 20, 2025

daivinhtran marked this pull request as ready for review October 20, 2025 11:24

Fix clippy warning

7847ab2

coderabbitai bot reviewed Oct 20, 2025

View reviewed changes

crates/biome_grit_patterns/src/grit_code_snippet.rs Outdated Show resolved Hide resolved

daivinhtran added 2 commits October 20, 2025 18:38

Add changeset

b72a374

Fix clippy warnings and make matcher stricter

a8914c7

coderabbitai bot reviewed Oct 20, 2025

View reviewed changes

.changeset/ripe-rabbits-build.md Outdated Show resolved Hide resolved

Address comment

82162ac

daivinhtran changed the title ~~fix(plugin): Extend matcher for GritCodeSnippet to cover edge case~~ fix(lint): Extend matcher for GritCodeSnippet to cover edge case Oct 20, 2025

daivinhtran changed the title ~~fix(lint): Extend matcher for GritCodeSnippet to cover edge case~~ fix(lint): extend matcher for GritCodeSnippet to cover edge case Oct 20, 2025

coderabbitai bot reviewed Oct 20, 2025

View reviewed changes

.changeset/ripe-rabbits-build.md Outdated Show resolved Hide resolved

Update .changeset/ripe-rabbits-build.md

3f20c4e

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(lint): extend matcher for GritCodeSnippet to cover edge case #7786

fix(lint): extend matcher for GritCodeSnippet to cover edge case #7786

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

fix(lint): extend matcher for GritCodeSnippet to cover edge case #7786

Are you sure you want to change the base?

fix(lint): extend matcher for GritCodeSnippet to cover edge case #7786

Conversation

Uh oh!

Uh oh!

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

Walkthrough

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant