Reject root multi-metavariable patterns by morgan-coded · Pull Request #2727 · ast-grep/ast-grep

morgan-coded · 2026-06-16T02:52:42Z

Root $$$META patterns were still accepted at pattern creation time, even though they hit the same root-pattern problem discussed in #2697.

This rejects both named and bare root multi-metavariable patterns up front. In-list $$$ behavior is unchanged.

I checked the redesign with formatting, pattern tests, core/config tests, the full cargo test suite, clippy, and git diff --check.

Fixes #2697

Summary by CodeRabbit

Bug Fixes
- Pattern validation now properly rejects root-level multi meta variables, preventing invalid pattern configurations from being created.
Tests
- Expanded test coverage for pattern validation to verify root-level multi meta variable rejection while maintaining support for nested multi meta variables.

coderabbitai · 2026-06-16T02:52:55Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6fc37c75-bd6c-4e59-aaa1-544148bd9154

📥 Commits

Reviewing files that changed from the base of the PR and between cc12304 and 8e36ec1.

📒 Files selected for processing (1)

crates/core/src/matcher/pattern.rs

📝 Walkthrough

Walkthrough

PatternBuilder::build now validates the parsed pattern root and returns a new PatternError::RootMultiMetaVar(String) error when the root node is a multi meta variable (MultiCapture or Multiple). Non-root multi meta variables remain unaffected. Tests cover rejection and acceptance cases.

Changes

Root multi meta variable validation in PatternBuilder

Layer / File(s)	Summary
`PatternError` variant and `build` validation `crates/core/src/matcher/pattern.rs`	Adds `PatternError::RootMultiMetaVar(String)` enum variant and inserts a post-parse check in `PatternBuilder::build` that matches on the pattern root; returns the new error for `MetaVariable::MultiCapture` and `MetaVariable::Multiple`, otherwise returns `Ok(pattern)` as before.
Unit tests `crates/core/src/matcher/pattern.rs`	New tests assert `$$$A` and `$$$PARAMS` at the root are rejected with `RootMultiMetaVar`; `$A` and `$_` are accepted; `foo($$$A, c)` with a non-root multi meta variable is accepted and retains its existing match behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐇 Hippity-hop, I checked the root,
A $$$VAR alone? That's a no-hoot!
Three dollar signs must share the stage,
They can't stand alone on pattern's page.
Now RootMultiMetaVar tells you clear —
Snuggle inside a list, my dear! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main change: rejecting root multi-metavariable patterns during validation.
Linked Issues check	✅ Passed	The PR directly addresses issue `#2697` by rejecting invalid root multi-metavariable patterns at validation time, preventing the runtime blank output bug described in the issue.
Out of Scope Changes check	✅ Passed	All changes are focused on pattern validation: the new error variant, rejection logic, and test coverage for root multi-metavariables are all within scope of fixing the reported issue.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds validation to reject patterns where a multi-capture meta variable ($$$...) is used as the entire pattern, returning a dedicated error and covering the behavior with tests.

Changes:

Build patterns first, then reject $$$... when it is the root node of the pattern.
Introduce a new PatternError::RootMultiMetaVar error variant with a specific message.
Add unit tests for rejecting root multi meta vars while keeping existing in-list behavior unchanged.

Comments suppressed due to low confidence (1)

crates/core/src/matcher/pattern.rs:1

Adding a new variant to a public Rust enum is a SemVer-breaking change for downstream crates that perform exhaustive matching on PatternError. If PatternError is part of the public API, consider marking it #[non_exhaustive] (and updating internal matches accordingly), or otherwise ensure this lands with an appropriate breaking-release/version bump policy.

use crate::language::Language;

codecov · 2026-06-16T02:55:17Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.68%. Comparing base (cc12304) to head (8e36ec1).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2727      +/-   ##
==========================================
+ Coverage   86.67%   86.68%   +0.01%     
==========================================
  Files         120      120              
  Lines       20636    20654      +18     
==========================================
+ Hits        17886    17904      +18     
  Misses       2750     2750

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fix: reject root multi-metavariable patterns

8e36ec1

Copilot AI review requested due to automatic review settings June 16, 2026 02:52

Copilot AI reviewed Jun 16, 2026

View reviewed changes

HerringtonDarkholme approved these changes Jun 21, 2026

View reviewed changes

HerringtonDarkholme added this pull request to the merge queue Jun 21, 2026

Merged via the queue into ast-grep:main with commit a8b7061 Jun 21, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Reject root multi-metavariable patterns#2727

Reject root multi-metavariable patterns#2727
HerringtonDarkholme merged 1 commit into
ast-grep:mainfrom
morgan-coded:fix/2697-reject-root-multi-metavar

morgan-coded commented Jun 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

codecov Bot commented Jun 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

morgan-coded commented Jun 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

codecov Bot commented Jun 16, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

morgan-coded commented Jun 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading