[sg] Trailing $$$ breaks sg rules

[thecodedrift]

Needs to be part of ast grep's rule validation system:

a trailing $$$ in an ast-grep pattern doesn't match a zero-argument call, which is an easy way to write a rule that looks right and matches nothing

found on nightly

[thecodedrift]

Investigated. The claim as worded does not reproduce — but a strictly worse version of it does, one arity up from where it was placed.

What actually happens (ast-grep 0.41.0, the version we bundle)

Source: foo(); foo(1); foo(1,2); foo(1,2,3);

pattern	matches
foo($$$)	all four, including foo()
foo($A, $$$)	only foo(1,2), foo(1,2,3) — not foo(1)
foo($$$, $A)	only foo(1)
foo($A, $$$, $B)	only foo(1,2)

So a standalone trailing $$$ is fine — eval($$$ARGS) and every other rule we ship is correct. The trap is a $$$ that follows a comma-separated sibling: it silently requires ≥1 node. A leading $$$ before a bare metavariable is worse still — it binds exactly zero, collapsing the pattern to fixed arity.

$$$ and $$$ARGS behave identically. Reproduces in Python too. Not fixed in 0.45.2 (tested against current latest; identical output) — upgrading won't help.

Why

--debug-query=pattern shows the mechanism:

foo($A, $$$)  ->  arguments: (  MetaVar $A  ,  MetaVar $$$  )

The , is a node in the pattern. Default strictness is smart, under which every node in the pattern must be matched (should_skip_goal() returns false for Terminal). foo(1) has no comma, so the match fails. $$$ matches zero nodes just fine — the separator is what fails. Statement blocks are unaffected because ; lives inside the statement node, so there's no mandatory unnamed sibling.

Upstream: ast-grep/ast-grep#1365, closed 2024-08-01 as working-as-intended with "use strictness: ast". The docs only ever say "matches zero or more AST nodes" — that omission is the trap.

Author-side fix, verified against our bundled binary. strictness goes inside the pattern object; at rule level ast-grep rejects it as an unknown field. It is already accepted by our vendored schema (generated/ast-grep-rule-schema.json, $defs.PatternStyle):

rule:
  pattern:
    context: fetch($URL, $$$REST)
    selector: call_expression
    strictness: ast

For the leading-$$$ case strictness: ast does not help; use any: [{pattern: foo($A)}, {pattern: foo($A, $$$REST)}].

Blast radius

Zero shipped rules affected. Every $$$ in this repo is the sole element of its argument list — eval($$$ARGS) in example/, console.warn($$$) / Function($$$) / DOMPurify.sanitize($$$) in verify-examples.ts and the test fixtures. No comma-adjacent hits anywhere.

The exposure is entirely prospective. No recipe mentions $$$ at all, and create-sg-rule.txt:47-52 points agents at the upstream docs that omit the caveat. fetch($URL, $$$), useEffect($FN, $$$), res.status($C).json($$$) are exactly what gets written next.

The bigger gap this exposed

verify Layer 2 checks that a -test.yml exists, never what's in it — and ast-grep test passes an empty invalid: bucket (1 passed; 0 failed, exit 0). End to end:

$ taskless test .taskless/rules/sg/no-fetch-opts --json
{"ok":true,"rules":[{"engine":"sg","ruleId":"no-fetch-opts","ok":true,"errors":[],"ran":true}]}
$ taskless check --json     # source contains fetch(url)
{"success":true,"results":[]}

A rule that flags nothing reports ok: true, ran: true.

We already close this on the Vale side. rules/vale/verify.ts:118-149 defines ValeFixtureCoverage ("both" | "pass-only" | "fail-only" | "none") and inspect.ts:244-248 fails any rule whose fixtures !== "both" — "half a claim". The comment there says the quiet part: a rule with only pass/ fixtures passes with an empty missingFailures without ever demonstrating that it fires. The sg branch of that same function has no equivalent, and openspec/specs/cli-rule-validation/spec.md specifies the Vale bucket rule with no sg counterpart — so the spec is incomplete in a way that makes today's sg behavior look correct.

Proposal

1. sg fixture-coverage check — mirror Vale: a rule whose invalid: bucket is empty across all its -test.yml files has never been shown to fire, and must not pass test. This reads the author's own structured YAML with the parser already imported in verify.ts, derives nothing, and generalizes past $$$ to any rule that matches nothing. It also makes the trap loud at the right moment — adding a zero-arg case to invalid: produces a clean [Missing] Expect rule to report issues, but none found in: fetch(url).
2. Recipe change — a $$$-and-separators note in create-sg-rule.txt and improve-rule.txt, plus a requirement that fixtures cover the arity boundary when a pattern contains $$$; and a strictness: ast entry in RULE_EXAMPLES so the safe form reaches agents through verify --schema.

Neither half works alone: (2) teaches it, (1) makes the un-taught case impossible to ship silently.

Deliberately not proposing a static regex lint over pattern strings. That's what .conventions/STYLEGUIDE-CODE.md warns against — and even implemented perfectly, the ≥1 semantics is sometimes exactly what the author meant (fetch called with options), so it could only ever be an often-wrong warning, on a LayerResult that has no warnings channel today.

Tests

Nothing today covers an empty bucket — every sg case in verify.test.ts populates invalid:. Two additions: one case per coverage state in verify.test.ts, and a vendor-contract test in ast-grep-vendor-contract.test.ts pinning the $$$/comma behavior against the bundled binary, so an ast-grep upgrade that changes it is loud rather than a rule that quietly starts over-firing.