From 79e7010499b8757fd41c89ec2d620e75b35b042f Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 11 Nov 2026 18:27:23 -0800 Subject: [PATCH 1/3] fix(validate): state sg-regex-needs-kind in the ast-grep rule schema 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 --- .changeset/sg-regex-needs-kind-schema.md | 5 + packages/cli/assets/reference.json | 4 +- packages/cli/src/agent/create-sg-rule.md | 52 ++- packages/cli/src/rules/constraints.ts | 4 +- packages/cli/src/rules/verify.ts | 61 ++-- packages/cli/src/schemas/ast-grep-rule.ts | 327 ++++++++++++++---- .../cli/test/ast-grep-rule-schema.test.ts | 261 ++++++++++++++ .../cli/test/ast-grep-vendor-contract.test.ts | 80 +++++ packages/cli/test/constraints.test.ts | 2 +- packages/cli/test/verify.test.ts | 11 +- 10 files changed, 690 insertions(+), 117 deletions(-) create mode 100644 .changeset/sg-regex-needs-kind-schema.md create mode 100644 packages/cli/test/ast-grep-rule-schema.test.ts diff --git a/.changeset/sg-regex-needs-kind-schema.md b/.changeset/sg-regex-needs-kind-schema.md new file mode 100644 index 000000000..1c753b235 --- /dev/null +++ b/.changeset/sg-regex-needs-kind-schema.md @@ -0,0 +1,5 @@ +--- +"@taskless/cli": patch +--- + +`sg-regex-needs-kind` now lives in the ast-grep rule schema. The schema the CLI publishes states the pairing as `if`/`then` on every rule definition, and `validate` reports it from the schema layer with the same paths, so a schema-aware tool flags exactly what `validate` refuses. Every `regex`, at any depth, is covered: `rule`, `constraints`, `utils`, every `any`/`all`/`not`/`has`/`inside`/`precedes`/`follows` sub-rule, and the rarer positions (`stopBy`, `nthChild.ofRule`, utility arguments, `fix.expandStart`/`expandEnd`, rewriters). A sibling `any:` whose every branch sets a `kind` now anchors the regex, so a constraint no longer has to be split into one branch per kind, and an `all:` with one branch that sets a `kind` counts too. The message says how to find the kind, and it is reported even when another field also fails the schema. The `create-sg-rule` recipe (topic v15) lists the rule under Quirks. diff --git a/packages/cli/assets/reference.json b/packages/cli/assets/reference.json index c167008eb..c08c16f30 100644 --- a/packages/cli/assets/reference.json +++ b/packages/cli/assets/reference.json @@ -44,8 +44,8 @@ "id": "sg-regex-needs-kind", "engine": "sg", "enforcedBy": "validate", - "summary": "A `regex` needs a sibling `kind`, in `rule`, `constraints` and `utils`.", - "rationale": "A regex match with no kind to anchor it is ambiguous and slow: it is applied to every node rather than to the one shape the author meant. ast-grep accepts it, so the engine is not the thing that will tell you." + "summary": "Every `regex`, at any depth, needs a `kind` beside it: a sibling `kind`, or a sibling `any` whose every branch sets one. This covers `rule`, `constraints`, `utils` and every sub-rule beneath them (`any`, `all`, `not`, `has`, `inside`, `precedes`, `follows`).", + "rationale": "A regex match with no kind to anchor it is ambiguous and slow: it is applied to every node rather than to the one shape the author meant. ast-grep refuses it only at the top of `rule` and runs it in any sub-rule, so beyond that one spot the engine is not the thing that will tell you. The pairing is part of the rule schema Taskless publishes (`if`/`then` on every rule definition), so a schema-aware editor flags the same paths `validate` does." }, { "id": "sg-language-accepted", diff --git a/packages/cli/src/agent/create-sg-rule.md b/packages/cli/src/agent/create-sg-rule.md index d1d37ba58..4eb6baea2 100644 --- a/packages/cli/src/agent/create-sg-rule.md +++ b/packages/cli/src/agent/create-sg-rule.md @@ -1,4 +1,4 @@ -# Topic: create-sg-rule (CLI v%(CLI_VERSION)s / topic v14) +# Topic: create-sg-rule (CLI v%(CLI_VERSION)s / topic v15) %(ONE_COMMAND_PER_CALL)s @@ -163,8 +163,9 @@ whole rule. | a case didn't behave as expected | fix the rule pattern OR the test case | | the rule's scope excluded a case | move the fixture to a path `files:` admits | - A `regex` without an accompanying `kind` fails validation, the two - always travel together. + Every `regex`, at any depth, needs a `kind` beside it, or + validation fails. That includes `constraints`, `utils`, and every + `not`, `has`, `inside`, `any` and `all` sub-rule; see **Quirks**. Pass a directory above a rule and every rule beneath it is checked, one entry per rule in the report. `.taskless/rules/sg` covers every @@ -188,9 +189,9 @@ whole rule. ## Quirks -Each of these comes from a grammar or from ast-grep itself, so a rule -written carefully from the upstream reference still hits them. They -fail in one of two ways: +Each of these, except the first, comes from a grammar or from ast-grep +itself, so a rule written carefully from the upstream reference still +hits them. Those fail in one of two ways: - **Config error.** The scan exits 8 and names the problem. Loud, but it aborts config parsing, so every other `sg` rule in the project @@ -208,6 +209,7 @@ sometimes what the author meant. Read it and decide. | quirk | symptom | `validate` reports it | |------------------------------------------------|-------------------------|-----------------------------| +| a `regex` without a `kind`, at any depth | refused by `validate` | yes | | `language:` copied from `detect --json` | config error, or notice | yes | | `TypeScript` declared over `.tsx` files | silent | only if `files:` names .tsx | | an inline construct named in a `Markdown` rule | config error, or silent | the `kind:` form only | @@ -217,6 +219,44 @@ sometimes what the author meant. Read it and decide. | `follows` or `precedes` without `stopBy: end` | silent | as a notice | | a `not` containing a bare metavariable | silent | no | +### Every `regex`, at any depth, needs a `kind` + +This one is a Taskless rule, wider than ast-grep's own check. ast-grep +refuses a bare `regex` only at the top of `rule` (a config error); in +any sub-rule it runs it against the text of every node it reaches, +which is ambiguous and slow. `validate` refuses a `regex` with no +`kind` beside it anywhere it appears: the top of `rule`, +`constraints.`, `utils`, `not`, `has`/`inside` (however deeply +nested), `precedes`/`follows`, and each `any`/`all` branch. + +A sibling `any:` whose every branch sets a `kind` counts, so a +metavariable that may capture one of several kinds keeps a single +regex. A callee is an `identifier` in `rpc()` and a +`member_expression` in `client.rpc()`: + +```yaml +rule: + pattern: "$CALLEE()" +constraints: + CALLEE: + any: + - kind: identifier + - kind: member_expression + regex: "(^|\\.)rpc$" +``` + +An `all:` with one branch that sets a `kind` counts too. A `kind` +inside `not`, `has` or another relation does not, because it describes +a different node. + +To find the kind, run ast-grep with `--debug-query=ast` on a sample of +the code and take the named node the regex should match. Under +`constraints`, that is the node the metavariable captures, which is +not always the obvious one: the method `$M` captures in `$OBJ.$M()` is +a `property_identifier` in TypeScript, not an `identifier`. Read it off +the dump rather than guessing, and if the metavariable can capture +several kinds, list them in a sibling `any:` as above. + ### `language:` copied from `detect --json` `detect --json` reports the *repository's* languages, in a different diff --git a/packages/cli/src/rules/constraints.ts b/packages/cli/src/rules/constraints.ts index e8a65b4c6..9c4f7bb20 100644 --- a/packages/cli/src/rules/constraints.ts +++ b/packages/cli/src/rules/constraints.ts @@ -82,9 +82,9 @@ export const RULE_CONSTRAINTS = [ engine: "sg", enforcedBy: "validate", summary: - "A `regex` needs a sibling `kind`, in `rule`, `constraints` and `utils`.", + "Every `regex`, at any depth, needs a `kind` beside it: a sibling `kind`, or a sibling `any` whose every branch sets one. This covers `rule`, `constraints`, `utils` and every sub-rule beneath them (`any`, `all`, `not`, `has`, `inside`, `precedes`, `follows`).", rationale: - "A regex match with no kind to anchor it is ambiguous and slow: it is applied to every node rather than to the one shape the author meant. ast-grep accepts it, so the engine is not the thing that will tell you.", + "A regex match with no kind to anchor it is ambiguous and slow: it is applied to every node rather than to the one shape the author meant. ast-grep refuses it only at the top of `rule` and runs it in any sub-rule, so beyond that one spot the engine is not the thing that will tell you. The pairing is part of the rule schema Taskless publishes (`if`/`then` on every rule definition), so a schema-aware editor flags the same paths `validate` does.", }, { id: "sg-language-accepted", diff --git a/packages/cli/src/rules/verify.ts b/packages/cli/src/rules/verify.ts index 2e6447869..c4d74928b 100644 --- a/packages/cli/src/rules/verify.ts +++ b/packages/cli/src/rules/verify.ts @@ -3,9 +3,10 @@ import { readFile } from "node:fs/promises"; import { parse } from "yaml"; import { + astGrepRuleJsonSchema, astGrepRuleSchema, + REGEX_NEEDS_KIND, TASKLESS_REQUIRED_FIELDS, - findRegexWithoutKind, } from "../schemas/ast-grep-rule"; import { pathPrefixed, schemaLayer } from "../schemas/layer"; import { @@ -28,7 +29,6 @@ import { import { ensureTasklessDirectory } from "../filesystem/directory"; import { ruleFilePath } from "./engines"; import { RULE_TESTS_DIRECTORY, RULES_DIRECTORY } from "./layout"; -import astGrepJsonSchema from "../generated/ast-grep-rule-schema.json"; import { RULE_EXAMPLES } from "./verify-examples"; import { violate, type RuleViolation } from "./constraints"; import { isValidRuleId } from "./validate-id"; @@ -105,14 +105,14 @@ export interface VerifyResult { export function getSchemaPayload(): Record { return { - astGrepSchema: astGrepJsonSchema, + astGrepSchema: astGrepRuleJsonSchema, tasklessRequirements: { requiredFields: [...TASKLESS_REQUIRED_FIELDS], rules: [ { name: "regex-requires-kind", description: - "Rules using `regex` in any position must also specify `kind` at the same level. Without kind, regex matches against node text which is ambiguous and slow.", + "Every `regex`, at any depth (`rule`, `constraints`, `utils`, and every sub-rule beneath them), needs a `kind` beside it: a sibling `kind`, or a sibling `any` whose every branch sets `kind`. `astGrepSchema` states this as `if`/`then` on its rule definitions. Without kind, regex matches against node text which is ambiguous and slow.", }, ], }, @@ -127,7 +127,24 @@ function validateSchema(ruleData: unknown): LayerResult { // way whichever engine wrote it. `pathPrefixed` is the half that differs: // ast-grep's messages come from an upstream JSON Schema and are generic // ("expected string"), so the path is what makes them actionable. - return schemaLayer(astGrepRuleSchema.safeParse(ruleData), pathPrefixed); + const result = astGrepRuleSchema.safeParse(ruleData); + const layer = schemaLayer(result, pathPrefixed); + // The regex/kind pairing lives in the schema (see `astGrepRuleJsonSchema`), + // and its issues say which constraint they are, so they are attributed + // here rather than re-derived from their wording. + const violations: RuleViolation[] = []; + for (const issue of result.error?.issues ?? []) { + if ( + issue.code === "custom" && + issue.params?.constraintId === REGEX_NEEDS_KIND + ) { + violations.push({ + constraintId: REGEX_NEEDS_KIND, + message: pathPrefixed(issue), + }); + } + } + return { ...layer, violations }; } /** @@ -309,38 +326,6 @@ async function validateRequirements( } } - // Check regex-requires-kind in rule and other top-level rule containers - for (const key of ["rule", "constraints", "utils"]) { - const container = ruleData[key]; - if ( - container && - typeof container === "object" && - !Array.isArray(container) - ) { - if (key === "rule") { - for (const message of findRegexWithoutKind( - container as Record - )) { - violate(collected, "sg-regex-needs-kind", message); - } - } else { - // constraints/utils are Record - for (const [name, value] of Object.entries( - container as Record - )) { - if (value && typeof value === "object" && !Array.isArray(value)) { - for (const message of findRegexWithoutKind( - value as Record, - `${key}.${name}` - )) { - violate(collected, "sg-regex-needs-kind", message); - } - } - } - } - } - } - // `strictness:` at the top level is accepted by ast-grep and ignored, so the // pattern keeps narrowing exactly as before. Inside `rule:` the same key is a // schema error; up here only this check can say so. @@ -601,7 +586,7 @@ export async function verifyRule( const schemaResult: SchemaLayerResult = { valid: schemaErrors.length === 0, errors: schemaErrors, - violations: language.violations, + violations: [...(parsed.violations ?? []), ...language.violations], notices: [...language.notices, ...findSgTraps(ruleRecord)], }; diff --git a/packages/cli/src/schemas/ast-grep-rule.ts b/packages/cli/src/schemas/ast-grep-rule.ts index 5a86ed1db..36e7f2258 100644 --- a/packages/cli/src/schemas/ast-grep-rule.ts +++ b/packages/cli/src/schemas/ast-grep-rule.ts @@ -1,82 +1,283 @@ import { z } from "zod"; +import * as core from "zod/v4/core"; -import astGrepSchema from "../generated/ast-grep-rule-schema.json"; +import upstreamSchema from "../generated/ast-grep-rule-schema.json"; +import type { RuleConstraintId } from "../rules/constraints"; /** - * Zod schema for ast-grep rule validation, derived from the official - * ast-grep JSON Schema via `z.fromJSONSchema()`. This gives us full - * validation coverage matching the upstream spec. + * The `$defs` entries in ast-grep's schema that describe a rule object, and so + * the only places a `regex` can appear. `SerializableRule` is every plain rule + * (`rule`, `constraints.*`, `utils.*`, `any`/`all` items, `not`, a rule-form + * `stopBy`, `nthChild.ofRule`, utility arguments, rewriters); `Relation` is the + * same object with `stopBy` and `field` added (`has`, `inside`, `precedes`, + * `follows`, a fix's `expandStart`/`expandEnd`). + */ +const RULE_DEFINITIONS: readonly string[] = ["SerializableRule", "Relation"]; + +/** Our own `$defs` entry, prefixed so an upstream name can never collide. */ +const PINS_KIND_DEFINITION = "TasklessPinsKind"; + +/** The constraint the pairing reports under. See `rules/constraints.ts`. */ +export const REGEX_NEEDS_KIND = + "sg-regex-needs-kind" satisfies RuleConstraintId; + +/** + * What the pairing tells an author to do, shared by the JSON Schema + * description an editor shows and the message `validate` prints. + */ +export const REGEX_NEEDS_KIND_HINT = + 'Every "regex", at any depth, needs a "kind" beside it: a sibling `kind:`, ' + + "or a sibling `any:` whose every branch sets `kind:` (or an `all:` with a " + + "branch that does). Without one the regex is tested against the text of " + + "every node, which is ambiguous and slow. To find the kind, run ast-grep " + + "with `--debug-query=ast` on a sample of the code and take the named node " + + "the regex should match; under `constraints`, that is the node the " + + "metavariable captures."; + +type JsonObject = Record; + +function isObject(value: unknown): value is JsonObject { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * ast-grep's rule schema with the regex/kind pairing written into it. + * + * The upstream schema is vendored verbatim (`scripts/fetch-ast-grep-schema.ts` + * overwrites it on every upgrade), so the pairing is layered on here rather + * than edited into the generated file. Every rule-object definition gains + * `if: { required: [regex] }, then: TasklessPinsKind`, and `TasklessPinsKind` + * is the one statement of what anchors a regex: + * + * - a `kind` in the same object, or + * - an `any` whose every branch pins a kind, since whichever branch matched, + * the node has one of those kinds, or + * - an `all` with at least one branch that pins a kind, since every branch + * matched. * - * The raw JSON Schema is also embedded in `getSchemaPayload()`'s - * `astGrepSchema` field, for agent-facing consumption. There is no `--schema` - * CLI flag any more — it was removed CLI-wide in favor of embedding schema - * content in recipes — so nothing today reaches this via a command a user - * types; the comment used to say `rule verify --schema`, which named both a - * flag and a command form that no longer exist. + * `if`/`then` rather than 2020-12's `dependentSchemas`: both say the same + * thing, and `if`/`then` is the one YAML and JSON language servers have + * supported longest. + * + * zod cannot consume either keyword (`z.fromJSONSchema` throws on both), so + * {@link astGrepRuleSchema} is built from the upstream schema and mirrors this + * with a refinement. {@link findRegexWithoutKind} walks the data against this + * same schema, so the two report the same paths by construction. */ -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument -export const astGrepRuleSchema = z.fromJSONSchema(astGrepSchema as any); +function withRegexNeedsKind(schema: JsonObject): JsonObject { + const augmented = structuredClone(schema); + const definitions = augmented.$defs; + if (!isObject(definitions)) { + throw new Error("ast-grep rule schema has no $defs"); + } + for (const name of RULE_DEFINITIONS) { + const definition = definitions[name]; + // A vendor upgrade that renames either definition, or starts using + // `if`/`then` itself, must fail loudly here rather than ship a schema + // that silently stopped saying this. + if (!isObject(definition)) { + throw new Error(`ast-grep rule schema has no $defs.${name}`); + } + if ("if" in definition || "then" in definition) { + throw new Error(`ast-grep rule schema already uses if/then on ${name}`); + } + definition.if = { required: ["regex"] }; + // A JSON Schema keyword on a plain data object, never awaited. + // eslint-disable-next-line unicorn/no-thenable + definition.then = { $ref: `#/$defs/${PINS_KIND_DEFINITION}` }; + } + definitions[PINS_KIND_DEFINITION] = { + description: `Taskless (${REGEX_NEEDS_KIND}): ${REGEX_NEEDS_KIND_HINT}`, + anyOf: [ + { required: ["kind"] }, + { + required: ["any"], + properties: { + any: { + type: "array", + minItems: 1, + items: { $ref: `#/$defs/${PINS_KIND_DEFINITION}` }, + }, + }, + }, + { + required: ["all"], + properties: { + all: { + type: "array", + contains: { $ref: `#/$defs/${PINS_KIND_DEFINITION}` }, + }, + }, + }, + ], + }; + return augmented; +} /** - * Taskless-specific required fields, beyond what ast-grep requires. - * ast-grep only requires `rule` + `language`; Taskless also requires - * `id`, `severity`, and `message` for meaningful rule output. + * ast-grep's rule JSON Schema, plus the Taskless regex/kind pairing. + * + * This is the schema to hand anything schema-aware: an editor, or an agent + * reading `getSchemaPayload()`'s `astGrepSchema`. It flags a `regex` without + * a kind at exactly the paths `validate` refuses. */ -export const TASKLESS_REQUIRED_FIELDS = [ - "id", - "language", - "severity", - "message", - "rule", -] as const; +export const astGrepRuleJsonSchema = withRegexNeedsKind( + upstreamSchema as JsonObject +); /** - * Check if a rule object uses `regex` anywhere without a sibling `kind`. - * Recursively walks the rule tree. + * Whether a rule object fixes the kind of the node it matches, by the + * definition in `TasklessPinsKind` above. */ -export function findRegexWithoutKind( - object: Record, - path = "" -): string[] { - const errors: string[] = []; - - if ("regex" in object && !("kind" in object)) { - errors.push( - `${path || "rule"}: uses "regex" without a sibling "kind" field. Regex matches without kind are ambiguous and slow.` - ); +function pinsKind(rule: JsonObject): boolean { + if ("kind" in rule) return true; + const { any, all } = rule; + if ( + Array.isArray(any) && + any.length > 0 && + any.every((branch) => isObject(branch) && pinsKind(branch)) + ) { + return true; } + return ( + Array.isArray(all) && + all.some((branch) => isObject(branch) && pinsKind(branch)) + ); +} - // Recurse into known composite fields - for (const key of [ - "any", - "all", - "not", - "has", - "inside", - "precedes", - "follows", - ]) { - const value = object[key]; - if (Array.isArray(value)) { - for (const [index, item] of value.entries()) { - if (item && typeof item === "object" && !Array.isArray(item)) { - errors.push( - ...findRegexWithoutKind( - item as Record, - `${path || "rule"}.${key}[${String(index)}]` - ) - ); - } +export interface RegexWithoutKind { + /** Where the offending rule object sits, as zod reports paths. */ + path: (string | number)[]; + message: string; +} + +const DEFINITION_PREFIX = "#/$defs/"; + +/** + * Visit `value` against `schema`, reporting every rule object that uses + * `regex` without pinning a kind. + * + * Schema-directed rather than a hand-listed set of keys, so it reaches every + * position ast-grep's schema allows a rule (including `stopBy`, `nthChild`'s + * `ofRule`, utility arguments, a fix's `expandStart`/`expandEnd`, and + * rewriters) and cannot drift from the JSON Schema an editor applies. + */ +function visit( + definitions: JsonObject, + schema: unknown, + value: unknown, + path: (string | number)[], + found: Map +): void { + if (!isObject(schema)) return; + + let node = schema; + const reference = schema.$ref; + if ( + typeof reference === "string" && + reference.startsWith(DEFINITION_PREFIX) + ) { + const name = reference.slice(DEFINITION_PREFIX.length); + const resolved = definitions[name]; + if (!isObject(resolved)) return; + node = resolved; + if ( + RULE_DEFINITIONS.includes(name) && + isObject(value) && + "regex" in value && + !pinsKind(value) + ) { + // Keyed by path: an `anyOf` can reach the same object twice. + found.set(JSON.stringify(path), { path, message: REGEX_NEEDS_KIND_HINT }); + } + } + + for (const keyword of ["anyOf", "oneOf", "allOf"]) { + const branches = node[keyword]; + if (Array.isArray(branches)) { + for (const branch of branches) { + visit(definitions, branch, value, path, found); } - } else if (value && typeof value === "object" && !Array.isArray(value)) { - errors.push( - ...findRegexWithoutKind( - value as Record, - `${path || "rule"}.${key}` - ) - ); } } - return errors; + if (isObject(value)) { + const properties = isObject(node.properties) ? node.properties : {}; + for (const [key, child] of Object.entries(value)) { + const childSchema = + key in properties ? properties[key] : node.additionalProperties; + visit(definitions, childSchema, child, [...path, key], found); + } + } else if (Array.isArray(value)) { + for (const [index, item] of value.entries()) { + visit(definitions, node.items, item, [...path, index], found); + } + } +} + +/** + * Every rule object in a parsed ast-grep rule file that uses `regex` without + * a kind to anchor it: in `rule`, `constraints`, `utils`, and every composite + * and relational sub-rule beneath them, at any depth. + */ +export function findRegexWithoutKind(ruleFile: unknown): RegexWithoutKind[] { + const found = new Map(); + visit( + astGrepRuleJsonSchema.$defs as JsonObject, + astGrepRuleJsonSchema, + ruleFile, + [], + found + ); + return [...found.values()]; } + +/** + * The zod half of the regex/kind pairing: one issue per path + * {@link findRegexWithoutKind} reports, each carrying `params.constraintId` + * so `validate` can attribute it to `sg-regex-needs-kind` without matching on + * wording. + * + * Built with `when: () => true` because a plain `.superRefine()` is skipped + * whenever the base schema already failed: a rule with a mistyped `severity` + * would hide its regex problem until the next round, which is the round this + * check exists to save. + */ +const regexNeedsKindCheck = core._check( + (payload) => { + for (const { path, message } of findRegexWithoutKind(payload.value)) { + payload.issues.push({ + code: "custom", + path, + message, + input: payload.value, + params: { constraintId: REGEX_NEEDS_KIND }, + }); + } + }, + { when: () => true } +); + +/** + * Zod schema for ast-grep rule validation, derived from the official + * ast-grep JSON Schema via `z.fromJSONSchema()`, plus the check that mirrors + * the regex/kind pairing {@link astGrepRuleJsonSchema} adds. zod cannot read + * the `if`/`then` that states it there, so the upstream schema is the base. + */ +export const astGrepRuleSchema = z + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + .fromJSONSchema(upstreamSchema as any) + .check(regexNeedsKindCheck); + +/** + * Taskless-specific required fields, beyond what ast-grep requires. + * ast-grep only requires `rule` + `language`; Taskless also requires + * `id`, `severity`, and `message` for meaningful rule output. + */ +export const TASKLESS_REQUIRED_FIELDS = [ + "id", + "language", + "severity", + "message", + "rule", +] as const; diff --git a/packages/cli/test/ast-grep-rule-schema.test.ts b/packages/cli/test/ast-grep-rule-schema.test.ts new file mode 100644 index 000000000..1b38f3c8c --- /dev/null +++ b/packages/cli/test/ast-grep-rule-schema.test.ts @@ -0,0 +1,261 @@ +import { describe, expect, it } from "vitest"; + +import upstreamSchema from "../src/generated/ast-grep-rule-schema.json"; +import { + astGrepRuleJsonSchema, + astGrepRuleSchema, + findRegexWithoutKind, +} from "../src/schemas/ast-grep-rule"; + +/** + * The regex/kind pairing (`sg-regex-needs-kind`), stated once in the JSON + * Schema and mirrored by the zod check `validate` runs. See taskless/cli#671. + */ + +const BASE = { + id: "probe", + language: "TypeScript", + severity: "error", + message: "probe", +}; + +/** The dotted paths `validate` would refuse, for a rule file body. */ +function refused(file: Record): string[] { + return findRegexWithoutKind({ ...BASE, ...file }).map(({ path }) => + path.join(".") + ); +} + +describe("findRegexWithoutKind", () => { + it("accepts a regex with a sibling kind", () => { + expect(refused({ rule: { kind: "identifier", regex: "^a$" } })).toEqual([]); + }); + + it("refuses a regex at the top of `rule`", () => { + expect(refused({ rule: { regex: "^a$" } })).toEqual(["rule"]); + }); + + it("refuses a regex inside `constraints.`", () => { + expect( + refused({ + rule: { pattern: "foo($A)" }, + constraints: { A: { regex: "^a$" } }, + }) + ).toEqual(["constraints.A"]); + }); + + it("refuses a regex inside `utils.`", () => { + expect( + refused({ + rule: { matches: "is-a" }, + utils: { "is-a": { regex: "^a$" } }, + }) + ).toEqual(["utils.is-a"]); + }); + + it("refuses a regex inside `not`", () => { + expect( + refused({ rule: { kind: "identifier", not: { regex: "^a$" } } }) + ).toEqual(["rule.not"]); + }); + + it("refuses a regex in nested `has` and `inside` sub-rules", () => { + expect( + refused({ + rule: { + kind: "call_expression", + has: { + field: "function", + kind: "member_expression", + has: { regex: "^rpc$" }, + }, + inside: { regex: "^async" }, + }, + }) + ).toEqual(["rule.has.has", "rule.inside"]); + }); + + it("refuses a regex in `precedes`, `follows`, and `any`/`all` items", () => { + expect( + refused({ + rule: { + kind: "identifier", + precedes: { regex: "a" }, + follows: { regex: "b" }, + any: [{ kind: "x" }, { regex: "c" }], + all: [{ kind: "x" }, { regex: "d" }], + }, + }) + ).toEqual(["rule.precedes", "rule.follows", "rule.any.1", "rule.all.1"]); + }); + + it("reaches every other rule position the schema allows", () => { + expect( + refused({ + rule: { + kind: "identifier", + has: { kind: "x", stopBy: { regex: "a" } }, + nthChild: { position: 1, ofRule: { regex: "b" } }, + matches: { "util-call": { ARG: { regex: "c" } } }, + }, + fix: { template: "", expandEnd: { regex: "d" } }, + rewriters: [{ id: "rw", rule: { regex: "e" } }], + }) + ).toEqual([ + "rule.has.stopBy", + "rule.nthChild.ofRule", + "rule.matches.util-call.ARG", + "fix.expandEnd", + "rewriters.0.rule", + ]); + }); + + describe("kind supplied through a sibling `any` or `all`", () => { + it("accepts a sibling `any` whose every branch pins a kind", () => { + expect( + refused({ + rule: { pattern: "foo($X)" }, + constraints: { + X: { + any: [{ kind: "identifier" }, { kind: "property_identifier" }], + regex: "^rpc$", + }, + }, + }) + ).toEqual([]); + }); + + it("refuses a sibling `any` with a branch that pins no kind", () => { + expect( + refused({ + rule: { pattern: "foo($X)" }, + constraints: { + X: { + any: [{ kind: "identifier" }, { pattern: "$A.$B" }], + regex: "^rpc$", + }, + }, + }) + ).toEqual(["constraints.X"]); + }); + + it("refuses an empty sibling `any`", () => { + expect(refused({ rule: { any: [], regex: "^a$" } })).toEqual(["rule"]); + }); + + it("accepts a sibling `all` with one branch that pins a kind", () => { + expect( + refused({ + rule: { + all: [{ pattern: "$A" }, { kind: "identifier" }], + regex: "a", + }, + }) + ).toEqual([]); + }); + + it("accepts kinds pinned through nested `any`/`all`", () => { + expect( + refused({ + rule: { + any: [ + { kind: "identifier" }, + { all: [{ has: { kind: "x" } }, { kind: "member_expression" }] }, + { any: [{ kind: "a" }, { kind: "b" }] }, + ], + regex: "^rpc", + }, + }) + ).toEqual([]); + }); + + it("does not count a kind inside `not` or a relation", () => { + expect( + refused({ + rule: { + all: [{ not: { kind: "x" } }, { has: { kind: "y" } }], + regex: "a", + }, + }) + ).toEqual(["rule"]); + }); + }); +}); + +describe("astGrepRuleSchema", () => { + it("reports every unanchored regex as an attributed issue", () => { + const result = astGrepRuleSchema.safeParse({ + ...BASE, + rule: { kind: "identifier", has: { regex: "a" } }, + constraints: { A: { regex: "b" } }, + }); + expect(result.success).toBe(false); + const issues = result.error?.issues ?? []; + expect(issues.map((issue) => issue.path.join("."))).toEqual([ + "rule.has", + "constraints.A", + ]); + for (const issue of issues) { + expect(issue.code).toBe("custom"); + expect(issue.message).toMatch(/--debug-query=ast/); + expect(issue.code === "custom" && issue.params).toEqual({ + constraintId: "sg-regex-needs-kind", + }); + } + }); + + // A plain `.superRefine()` would be skipped here, hiding the regex problem + // behind an unrelated one for a whole extra round. + it("still reports the pairing when another field fails the schema", () => { + const result = astGrepRuleSchema.safeParse({ + ...BASE, + severity: 3, + rule: { regex: "a" }, + }); + const paths = (result.error?.issues ?? []).map((issue) => + issue.path.join(".") + ); + expect(paths).toContain("severity"); + expect(paths).toContain("rule"); + }); + + it("accepts a regex anchored by a sibling `any` of kinds", () => { + expect( + astGrepRuleSchema.safeParse({ + ...BASE, + rule: { any: [{ kind: "a" }, { kind: "b" }], regex: "x" }, + }).success + ).toBe(true); + }); +}); + +describe("astGrepRuleJsonSchema", () => { + const definitions = astGrepRuleJsonSchema.$defs as Record< + string, + Record + >; + + it("states the pairing on every rule definition", () => { + for (const name of ["SerializableRule", "Relation"]) { + expect(definitions[name]?.if).toEqual({ required: ["regex"] }); + expect(definitions[name]?.then).toEqual({ + $ref: "#/$defs/TasklessPinsKind", + }); + } + expect(definitions.TasklessPinsKind?.description).toMatch( + /sg-regex-needs-kind/ + ); + }); + + it("changes nothing else about the upstream schema", () => { + const stripped = structuredClone(astGrepRuleJsonSchema) as { + $defs: Record>; + }; + delete stripped.$defs.TasklessPinsKind; + for (const name of ["SerializableRule", "Relation"]) { + delete stripped.$defs[name]?.if; + delete stripped.$defs[name]?.then; + } + expect(stripped).toEqual(upstreamSchema); + }); +}); diff --git a/packages/cli/test/ast-grep-vendor-contract.test.ts b/packages/cli/test/ast-grep-vendor-contract.test.ts index f2a59bc14..8f0d5211a 100644 --- a/packages/cli/test/ast-grep-vendor-contract.test.ts +++ b/packages/cli/test/ast-grep-vendor-contract.test.ts @@ -1216,6 +1216,86 @@ withSg("ast-grep vendor contract", () => { * same features showed no difference at all, because neither bug fires * unless a metavariable is bound in the specific position the fix changed. */ + /** + * THE REGEX/KIND PAIRING, MEASURED AT 0.51.1. + * + * `sg-regex-needs-kind` is a Taskless rule, and `create-sg-rule.md` says + * under Quirks how far ast-grep's own check goes: it refuses a bare regex at + * the top of `rule` and runs one anywhere else. The recipe also tells + * authors that a sibling `any:` of kinds anchors a constraint's regex, and + * that the method `$OBJ.$M()` captures is a `property_identifier`. Each of + * those is ast-grep's behavior, so each is pinned here. + */ + describe("regex and kind", () => { + const callSource = [ + "rpc();", + "client.rpc();", + "other();", + "client.rpcx();", + "", + ].join("\n"); + + it("refuses a bare regex at the top of `rule` as a config error", () => { + const outcome = grammarFindings(' regex: "^rpc"', callSource); + expect(outcome.status).toBe(8); + expect(outcome.stderr).toContain( + "Rule must specify a set of AST kinds to match" + ); + }); + + it("runs a bare regex in a sub-rule, against any node's text", () => { + // Nothing upstream refuses this shape, which is why the house rule + // reaches past the top of `rule`. `rpc$` matches the callee whatever + // its kind: an identifier in one call, a member_expression in the other. + expect( + grammarMatches( + [ + " kind: call_expression", + ' has: { field: function, regex: "rpc$" }', + ].join("\n"), + callSource + ) + ).toEqual(["rpc()", "client.rpc()"]); + }); + + it("anchors a constraint's regex with a sibling `any:` of kinds", () => { + // The recipe's example verbatim: one regex, two kinds. + expect( + grammarMatches( + [ + ' pattern: "$CALLEE()"', + "constraints:", + " CALLEE:", + " any:", + " - kind: identifier", + " - kind: member_expression", + String.raw` regex: "(^|\\.)rpc$"`, + ].join("\n"), + callSource + ) + ).toEqual(["rpc()", "client.rpc()"]); + }); + + it("captures the method in `$OBJ.$M()` as a property_identifier", () => { + const [asIdentifier, asProperty] = [ + "identifier", + "property_identifier", + ].map((kind) => + [ + ' pattern: "$OBJ.$M()"', + "constraints:", + " M:", + ` kind: ${kind}`, + ' regex: "^rpc$"', + ].join("\n") + ); + expect(grammarMatches(asIdentifier ?? "", callSource)).toEqual([]); + expect(grammarMatches(asProperty ?? "", callSource)).toEqual([ + "client.rpc()", + ]); + }); + }); + describe("Haskell names a type synonym `type_synonym`", () => { // RENAMED AT 0.51.0, which moved tree-sitter-haskell from 0.23.1 to // 0.24.1. Upstream fixed a typo in the kind (`fix!: typo: 'synomym' -> diff --git a/packages/cli/test/constraints.test.ts b/packages/cli/test/constraints.test.ts index 418995ae2..412c69867 100644 --- a/packages/cli/test/constraints.test.ts +++ b/packages/cli/test/constraints.test.ts @@ -191,7 +191,7 @@ const VIOLATIONS: Record = { rule: { ...VALID_RULE, rule: { regex: "^eval$" } }, fixture: VALID_FIXTURE, }), - names: /uses "regex" without a sibling "kind" field/, + names: /rule: Every "regex", at any depth, needs a "kind" beside it/, }, "sg-language-accepted": { run: () => diff --git a/packages/cli/test/verify.test.ts b/packages/cli/test/verify.test.ts index 8768fbfa0..76b619124 100644 --- a/packages/cli/test/verify.test.ts +++ b/packages/cli/test/verify.test.ts @@ -286,12 +286,13 @@ describe("verifyRule", () => { const result = await verifyRule(temporaryDirectory, "regex-no-kind"); - expect(result.requirements.valid).toBe(false); - expect(result.requirements.errors).toContainEqual( - expect.stringContaining("regex") + // The pairing is part of the schema now, so Layer 1 reports it. + expect(result.schema.valid).toBe(false); + expect(result.schema.errors).toContainEqual( + expect.stringMatching(/^rule: Every "regex", at any depth/) ); - expect(result.requirements.errors).toContainEqual( - expect.stringContaining("kind") + expect(result.schema.violations).toContainEqual( + expect.objectContaining({ constraintId: "sg-regex-needs-kind" }) ); }); From d69dbcab43b81b44c3e75f90645540a4e160d86b Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 11 Nov 2026 18:47:30 -0800 Subject: [PATCH 2/3] docs(schema): note why the regex/kind check uses zod's internal _check Also correct the vendor contract test header, which still named ast-grep 0.45.3 as the pinned version. --- packages/cli/src/schemas/ast-grep-rule.ts | 7 +++++++ packages/cli/test/ast-grep-vendor-contract.test.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/schemas/ast-grep-rule.ts b/packages/cli/src/schemas/ast-grep-rule.ts index 36e7f2258..b410e78c4 100644 --- a/packages/cli/src/schemas/ast-grep-rule.ts +++ b/packages/cli/src/schemas/ast-grep-rule.ts @@ -242,6 +242,13 @@ export function findRegexWithoutKind(ruleFile: unknown): RegexWithoutKind[] { * whenever the base schema already failed: a rule with a mistyped `severity` * would hide its regex problem until the next round, which is the round this * check exists to save. + * + * `core._check` is a leading-underscore export of `zod/v4/core`, so zod does + * not promise it across minor releases. It is used on purpose: the public + * `z.check()` (zod 4.3.6) builds a check with no params, so it cannot carry + * `when`. If a zod bump reshapes `_check`, the "still reports the pairing + * when another field fails the schema" case in + * `test/ast-grep-rule-schema.test.ts` fails rather than letting this go quiet. */ const regexNeedsKindCheck = core._check( (payload) => { diff --git a/packages/cli/test/ast-grep-vendor-contract.test.ts b/packages/cli/test/ast-grep-vendor-contract.test.ts index 8f0d5211a..c34882150 100644 --- a/packages/cli/test/ast-grep-vendor-contract.test.ts +++ b/packages/cli/test/ast-grep-vendor-contract.test.ts @@ -26,7 +26,7 @@ import { * ast-grep's observable behaviour, pinned. * * Everything here is a property of a **vendored third-party binary**, exact- - * pinned at `0.45.3` in `packages/cli/package.json`. Our own tests assert that + * pinned at `0.51.1` in `packages/cli/package.json`. Our own tests assert that * our code behaves correctly *given* these; this file asserts the givens, so an * ast-grep bump that changes one fails here — naming the assumption and the * code that rests on it — instead of surfacing downstream. From 06935ee7eeb7d2c78ce77aa372b1e7e9db4ca7e0 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 11 Nov 2026 20:01:38 -0800 Subject: [PATCH 3/3] refactor(schema): check the regex/kind pairing with zod's public API 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. --- packages/cli/src/schemas/ast-grep-rule.ts | 74 +++++++++---------- .../cli/test/ast-grep-rule-schema.test.ts | 68 +++++++++++++++-- 2 files changed, 95 insertions(+), 47 deletions(-) diff --git a/packages/cli/src/schemas/ast-grep-rule.ts b/packages/cli/src/schemas/ast-grep-rule.ts index b410e78c4..af82e4ed6 100644 --- a/packages/cli/src/schemas/ast-grep-rule.ts +++ b/packages/cli/src/schemas/ast-grep-rule.ts @@ -1,5 +1,4 @@ import { z } from "zod"; -import * as core from "zod/v4/core"; import upstreamSchema from "../generated/ast-grep-rule-schema.json"; import type { RuleConstraintId } from "../rules/constraints"; @@ -61,7 +60,7 @@ function isObject(value: unknown): value is JsonObject { * * zod cannot consume either keyword (`z.fromJSONSchema` throws on both), so * {@link astGrepRuleSchema} is built from the upstream schema and mirrors this - * with a refinement. {@link findRegexWithoutKind} walks the data against this + * with a check. {@link findRegexWithoutKind} walks the data against this * same schema, so the two report the same paths by construction. */ function withRegexNeedsKind(schema: JsonObject): JsonObject { @@ -232,49 +231,44 @@ export function findRegexWithoutKind(ruleFile: unknown): RegexWithoutKind[] { return [...found.values()]; } +/** ast-grep's rule schema as zod reads it, without the regex/kind pairing. */ +const upstreamRuleSchema = z + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + .fromJSONSchema(upstreamSchema as any); + /** - * The zod half of the regex/kind pairing: one issue per path + * Zod schema for ast-grep rule validation: the official ast-grep JSON Schema + * via `z.fromJSONSchema()`, plus the regex/kind pairing + * {@link astGrepRuleJsonSchema} adds. zod cannot read the `if`/`then` that + * states it there, so the pairing is checked here by hand. + * + * The two run side by side rather than as `upstream.superRefine(pairing)`, + * because zod skips a refinement once the schema it is attached to has + * failed: a rule with a mistyped `severity` would hide its regex problem + * until the next round, which is the round this check exists to save. So the + * outer schema accepts any input, and its one check reports the upstream + * schema's issues first, then one issue per path * {@link findRegexWithoutKind} reports, each carrying `params.constraintId` * so `validate` can attribute it to `sg-regex-needs-kind` without matching on * wording. - * - * Built with `when: () => true` because a plain `.superRefine()` is skipped - * whenever the base schema already failed: a rule with a mistyped `severity` - * would hide its regex problem until the next round, which is the round this - * check exists to save. - * - * `core._check` is a leading-underscore export of `zod/v4/core`, so zod does - * not promise it across minor releases. It is used on purpose: the public - * `z.check()` (zod 4.3.6) builds a check with no params, so it cannot carry - * `when`. If a zod bump reshapes `_check`, the "still reports the pairing - * when another field fails the schema" case in - * `test/ast-grep-rule-schema.test.ts` fails rather than letting this go quiet. */ -const regexNeedsKindCheck = core._check( - (payload) => { - for (const { path, message } of findRegexWithoutKind(payload.value)) { - payload.issues.push({ - code: "custom", - path, - message, - input: payload.value, - params: { constraintId: REGEX_NEEDS_KIND }, - }); - } - }, - { when: () => true } -); - -/** - * Zod schema for ast-grep rule validation, derived from the official - * ast-grep JSON Schema via `z.fromJSONSchema()`, plus the check that mirrors - * the regex/kind pairing {@link astGrepRuleJsonSchema} adds. zod cannot read - * the `if`/`then` that states it there, so the upstream schema is the base. - */ -export const astGrepRuleSchema = z - // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument - .fromJSONSchema(upstreamSchema as any) - .check(regexNeedsKindCheck); +export const astGrepRuleSchema = z.unknown().superRefine((value, context) => { + const upstream = upstreamRuleSchema.safeParse(value); + for (const issue of upstream.error?.issues ?? []) { + // `continue: false` keeps zod's default for a schema's own issues, which + // `addIssue` would otherwise flip to `true`. + context.addIssue({ ...issue, continue: false }); + } + for (const { path, message } of findRegexWithoutKind(value)) { + context.addIssue({ + code: "custom", + path, + message, + continue: false, + params: { constraintId: REGEX_NEEDS_KIND }, + }); + } +}); /** * Taskless-specific required fields, beyond what ast-grep requires. diff --git a/packages/cli/test/ast-grep-rule-schema.test.ts b/packages/cli/test/ast-grep-rule-schema.test.ts index 1b38f3c8c..9737cfdde 100644 --- a/packages/cli/test/ast-grep-rule-schema.test.ts +++ b/packages/cli/test/ast-grep-rule-schema.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { z } from "zod"; import upstreamSchema from "../src/generated/ast-grep-rule-schema.json"; import { @@ -204,19 +205,72 @@ describe("astGrepRuleSchema", () => { } }); - // A plain `.superRefine()` would be skipped here, hiding the regex problem - // behind an unrelated one for a whole extra round. + // zod skips a refinement attached to a schema that already failed, so the + // pairing must not hang off the upstream schema: a rule with an unrelated + // mistake would hide its regex problem for a whole extra round. it("still reports the pairing when another field fails the schema", () => { const result = astGrepRuleSchema.safeParse({ ...BASE, severity: 3, rule: { regex: "a" }, }); - const paths = (result.error?.issues ?? []).map((issue) => - issue.path.join(".") - ); - expect(paths).toContain("severity"); - expect(paths).toContain("rule"); + expect(result.success).toBe(false); + const issues = result.error?.issues ?? []; + expect( + issues.map((issue) => [ + issue.path.join("."), + issue.code, + issue.code === "custom" ? issue.params : undefined, + ]) + ).toEqual([ + ["severity", "invalid_union", undefined], + ["rule", "custom", { constraintId: "sg-regex-needs-kind" }], + ]); + }); + + it("still reports the pairing when the rule is missing a required field", () => { + const { language: _language, ...withoutLanguage } = BASE; + const result = astGrepRuleSchema.safeParse({ + ...withoutLanguage, + rule: { regex: "a" }, + }); + const issues = result.error?.issues ?? []; + expect(issues.map((issue) => issue.path.join("."))).toEqual([ + "language", + "rule", + ]); + const last = issues.at(-1); + expect(last?.code === "custom" && last.params).toEqual({ + constraintId: "sg-regex-needs-kind", + }); + }); + + it("reports upstream schema failures unchanged when no regex is involved", () => { + const upstream = z + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + .fromJSONSchema(upstreamSchema as any); + for (const input of [ + { ...BASE, severity: 3, rule: { kind: "identifier" } }, + { ...BASE, rule: { kind: 3 } }, + { id: "probe", rule: { kind: "identifier" } }, + ]) { + const result = astGrepRuleSchema.safeParse(input); + expect(result.success).toBe(false); + expect(result.error?.issues).toEqual( + upstream.safeParse(input).error?.issues + ); + } + }); + + it("refuses input that is not a rule object at all", () => { + for (const input of [undefined, null, "rule", []]) { + expect(astGrepRuleSchema.safeParse(input).success).toBe(false); + } + }); + + it("returns the rule unchanged when it passes", () => { + const rule = { ...BASE, rule: { kind: "identifier", regex: "^a$" } }; + expect(astGrepRuleSchema.parse(rule)).toEqual(rule); }); it("accepts a regex anchored by a sibling `any` of kinds", () => {