Repository navigation
fix(validate): state sg-regex-needs-kind in the ast-grep rule schema - #682
Conversation
|
@claude /review |
|
Claude finished @theCodeDrift's task in 5m 35s —— View job Review completeReview mode: incremental — read 0 prior review thread(s) before reviewing.
Summary This is a well-scoped, carefully-documented fix. The core idea — stating the regex/kind pairing as Cross-file consistency is good: [New] One fragility concern flagged inline ( Other notes (not findings, just context for the author):
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The — AI Coding Agent |
The regex/kind pairing was enforced only by a hand-written walker in validate's requirements layer, so the rule schema said nothing about it and a rule looked valid until validate refused it. The pairing now lives in the schema. astGrepRuleJsonSchema layers an if/then onto every rule definition (SerializableRule, Relation) that points at one TasklessPinsKind definition: a sibling kind, a sibling `any` whose every branch pins a kind, or an `all` with a branch that does. zod cannot read if/then, so astGrepRuleSchema mirrors it with a check that walks the data against the same JSON Schema, reaching every position a rule can sit (including stopBy, nthChild.ofRule, utility arguments, fix.expandStart/expandEnd and rewriters). The check runs even when another field fails, and its issues carry the constraint id, so validate attributes them without matching on wording. getSchemaPayload now hands out the augmented schema. The message says how to find the kind (--debug-query=ast, and what a constraint's metavariable captures). The constraint summary and the create-sg-rule recipe (topic v15) now say "every regex, at any depth", with a Quirks entry, and the vendor contract pins the ast-grep behavior that entry describes. Closes #671
Also correct the vendor contract test header, which still named ast-grep 0.45.3 as the pinned version.
astGrepRuleSchema no longer reaches for zod's internal core._check to
attach a `when` to the pairing. It is now z.unknown().superRefine(),
which parses the upstream ast-grep schema with safeParse, forwards its
issues, and then adds one sg-regex-needs-kind issue per unanchored
regex. Because the outer schema accepts any input, the pairing still
runs when another field fails, without depending on `when`.
The public `.refine(fn, { when })` was the other candidate, but a
refinement raises a single issue with a fixed path and params, and the
pairing needs one issue per offending rule object.
Issues, their order, codes, paths, messages and params are unchanged.
The tests now pin that: the pairing is reported next to an unrelated
failure, and upstream failures match a plain z.fromJSONSchema parse.
180912e to
06935ee
Compare
…nd issue #682 moved sg-regex-needs-kind into the ast-grep rule schema, so the validateRequirements loop that appended the `taskless ast <file>` pointer is gone. Apply the pointer in validateSchema's formatter instead, to both the layer error and its attributed violation, and point the recipe's regex/kind section at `taskless ast` rather than --debug-query=ast.
…nd issue #682 moved sg-regex-needs-kind into the ast-grep rule schema, so the validateRequirements loop that appended the `taskless ast <file>` pointer is gone. Apply the pointer in validateSchema's formatter instead, to both the layer error and its attributed violation, and point the recipe's regex/kind section at `taskless ast` rather than --debug-query=ast.
Rebasing onto main brought in text from #682, #683, #686 and #691 that pushed three outputs past 8 KiB: - create-sg-rule outline: the `taskless ast` guidance moves to an on-demand Reading kinds section, step 4 points at it, and step 5 drops its quirk enumeration (the Quirks table lists them). - create-sg-rule pattern-quirks: the relational entries (follows/precedes, has/inside, not) move to a Relational quirks section; validate-rule names it beside the other two. - route outline: #686's note on recipes that limit questions moves to an on-demand Limits on asking section, with a one-line pointer in Preconditions.
Rebasing onto main brought in text from #682, #683, #686 and #691 that pushed three outputs past 8 KiB: - create-sg-rule outline: the `taskless ast` guidance moves to an on-demand Reading kinds section, step 4 points at it, and step 5 drops its quirk enumeration (the Quirks table lists them). - create-sg-rule pattern-quirks: the relational entries (follows/precedes, has/inside, not) move to a Relational quirks section; validate-rule names it beside the other two. - route outline: #686's note on recipes that limit questions moves to an on-demand Limits on asking section, with a one-line pointer in Preconditions.
Stack (root → tip):
Closes #671
What changed
astGrepRuleJsonSchema(insrc/schemas/ast-grep-rule.ts) layersif: {required: [regex]}, then: {$ref: TasklessPinsKind}onto both rule definitions in ast-grep's vendored schema (SerializableRule,Relation). The generated file is untouched, so upgrades keep working, and the layering throws if a vendor bump renames either definition.getSchemaPayload()now returns the augmented schema.TasklessPinsKind): a siblingkind; a siblinganywhose every branch pins a kind (recursively); or anallwith at least one branch that pins one. Akindinsidenotor a relation does not count.z.fromJSONSchemathrows onif/then, soastGrepRuleSchemaadds a check that walks the data against the same JSON Schema. That gives the same paths by construction, including positions the old walker missed (stopBy,nthChild.ofRule, utility arguments,fix.expandStart/expandEnd, rewriters). It is built withwhen: () => truebecause a plain.superRefineis skipped when another field already failed. Issues carryparams.constraintId, sovalidateattributessg-regex-needs-kindfrom the schema layer. The old Layer 2 walker is gone.anyoption, and how to find the kind (--debug-query=ast, and what a constraint's metavariable captures).create-sg-rulerecipe (topic v15) say "everyregex, at any depth". There is a new Quirks row and section, andassets/reference.jsonis regenerated.Tests
test/ast-grep-rule-schema.test.ts:constraints,utils,not, nestedhas/inside,precedes/follows,any/allitems, the rarer positions, and the sibling-anycases (accepted; refused with an unpinned or empty branch). Alsoall, nested pins, zod attribution, the issue still being reported alongside an unrelated schema failure, and the JSON Schema augmentation leaving everything else in the upstream schema unchanged.regexis an engine config error, a bare regex in a sub-rule runs, the sibling-anyconstraint example works, and$OBJ.$M()captures aproperty_identifier.findRegexWithoutKindexactly. Ajv is not added as a dependency.Notes for review
$schemacomment and no package export). The schema now states the pairing, but how an editor picks it up is a separate decision.