From e2920830059ee308bf12f875467acb3568cd9150 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 12 Oct 2026 16:18:08 -0700 Subject: [PATCH 1/4] feat(cli): validate an sg rule with ast-grep, and verify it alone The second half of #489. `validate` now decides more of what a rule's files alone can decide, and `verify` stops letting one rule fail another. - `validate` has ast-grep load each sg rule on its own, with `scan -r --stdin` and nothing on stdin. A `kind` the grammar does not have (`readonly`, a Markdown `link`) passed the schema and then broke every sg rule's report; it is now rejected under its own rule, in ast-grep's words, with no constraint id. Measured on 0.45.3: every config error exits 8, a valid rule exits 0, about 3 ms a call. - A top-level `strictness:` is rejected under the new constraint `sg-no-top-level-strictness`. ast-grep accepts the key there and ignores it, so the pattern stayed narrowed. The corpus regenerates with it. - Three silent traps from `create-sg-rule`'s Quirks section become notices, decided over the parsed rule: `$$$` beside a comma, a sibling relation with no `stopBy`, and a TypeScript/Tsx callee `regex` with no `await_expression` branch. The recipe's own workarounds are tests that must stay quiet. - `verify` runs an sg rule's fixtures under a one-rule config written to a temporary directory and removed afterwards. `--filter` narrows which tests run, not which rules ast-grep parses, so under the project config a refused rule failed every other rule's run under the wrong name. `create-sg-rule` (topic v9) and `validate-rule` (topic v6) say which quirks `validate` now reports, and the changeset grows to cover it. --- .changeset/rename-rule-checks.md | 8 + openspec/changes/rename-rule-checks/tasks.md | 20 +- packages/cli/assets/reference.json | 7 + packages/cli/src/agent/create-sg-rule.md | 19 +- packages/cli/src/agent/validate-rule.md | 8 +- packages/cli/src/rules/constraints.ts | 9 + packages/cli/src/rules/sg-parse.ts | 129 ++++++++++++ packages/cli/src/rules/sg-traps.ts | 163 +++++++++++++++ packages/cli/src/rules/verify.ts | 83 ++++++-- packages/cli/test/constraints.test.ts | 9 + packages/cli/test/sg-engine-parse.test.ts | 165 +++++++++++++++ packages/cli/test/sg-traps.test.ts | 209 +++++++++++++++++++ 12 files changed, 790 insertions(+), 39 deletions(-) create mode 100644 packages/cli/src/rules/sg-parse.ts create mode 100644 packages/cli/src/rules/sg-traps.ts create mode 100644 packages/cli/test/sg-engine-parse.test.ts create mode 100644 packages/cli/test/sg-traps.test.ts diff --git a/.changeset/rename-rule-checks.md b/.changeset/rename-rule-checks.md index 9db72b5f..e626de5f 100644 --- a/.changeset/rename-rule-checks.md +++ b/.changeset/rename-rule-checks.md @@ -18,4 +18,12 @@ What to change: - **The `./schemas` export `verifyTestOutputSchema`** is renamed `ruleCheckOutputSchema`, with no alias. The schema itself is unchanged. - **`@taskless/cli/reference.json`** moves to `version` 3. Its shape is unchanged, but `enforcedBy` now reads `"validate"` or `"verify"`, so each value names a different command than it did in version 2. The corpus's own `version` is the signal a consumer should refuse on, not the package version. +`validate` also catches more on an ast-grep rule: + +- It has ast-grep load the rule on its own, so a `kind` the grammar does not have (a keyword such as `readonly`, a Markdown inline construct such as `link`) is rejected under that rule's name, in ast-grep's own words. Before, it passed and then broke every `sg` rule's report in `check`. +- A top-level `strictness:` is rejected under the new constraint `sg-no-top-level-strictness`. ast-grep ignores the key there, so the pattern stayed narrowed while the rule read as fixed. +- It reports three silent authoring traps from `create-sg-rule` as notices, which never fail a rule: `$$$` beside a comma, `follows`/`precedes` without `stopBy: end`, and a callee `regex` that misses awaited generic calls in TypeScript and Tsx. + +`verify` runs an ast-grep rule's fixtures against a config holding only that rule, so another rule ast-grep refuses no longer fails it under the wrong name. + Installed skills pin their CLI version, so a project keeps the old names until it upgrades, and the upgrade rewrites the skill with the new ones. diff --git a/openspec/changes/rename-rule-checks/tasks.md b/openspec/changes/rename-rule-checks/tasks.md index d6d5bceb..7489e667 100644 --- a/openspec/changes/rename-rule-checks/tasks.md +++ b/openspec/changes/rename-rule-checks/tasks.md @@ -31,24 +31,24 @@ PR 1 is groups 1–4 (bottom of the stack, targets `main`). PR 2 is groups 5–8 ## 5. `validate`: the ast-grep engine parse -- [ ] 5.1 Measure against ast-grep 0.45.3 that `sg scan -r --stdin` with empty input exits 8 with the message for an invalid `kind` (`readonly`, Markdown `link`) and an unknown `language:`, and exits 0 for a valid rule. Time it over this repository's rules. Record the measurements in the module's header comment. Verify: the comment states each case and its exit code. -- [ ] 5.2 Add the parse as a layer in `validate`, run only after the schema layer passes, reporting ast-grep's message in `errors` with no violation. Verify: tests for "A kind the grammar does not have is rejected", "An engine config error is reported under the rule that has it", and "An engine refusal carries no constraint id". +- [x] 5.1 Measure against ast-grep 0.45.3 that `sg scan -r --stdin` with empty input exits 8 with the message for an invalid `kind` (`readonly`, Markdown `link`) and an unknown `language:`, and exits 0 for a valid rule. Time it over this repository's rules. Record the measurements in the module's header comment. Verify: the comment states each case and its exit code. +- [x] 5.2 Add the parse as a layer in `validate`, run only after the schema layer passes, reporting ast-grep's message in `errors` with no violation. Verify: tests for "A kind the grammar does not have is rejected", "An engine config error is reported under the rule that has it", and "An engine refusal carries no constraint id". ## 6. `validate`: top-level `strictness` -- [ ] 6.1 Add `sg-no-top-level-strictness` to `RULE_CONSTRAINTS` (`enforcedBy: "validate"`) and enforce it, with a message naming the pattern object as where the key belongs. Regenerate `assets/reference.json`. Verify: a test for "A top-level strictness is rejected", plus a passing `test/reference.test.ts`. +- [x] 6.1 Add `sg-no-top-level-strictness` to `RULE_CONSTRAINTS` (`enforcedBy: "validate"`) and enforce it, with a message naming the pattern object as where the key belongs. Regenerate `assets/reference.json`. Verify: a test for "A top-level strictness is rejected", plus a passing `test/reference.test.ts`. ## 7. `validate`: trap notices -- [ ] 7.1 Add the `$$$`-beside-a-comma detector over pattern strings and object-pattern `context`. Verify: tests for "A comma beside $$$ draws a notice" and "A pattern that already uses strictness ast draws no notice". -- [ ] 7.2 Add the `follows`/`precedes`-without-`stopBy` detector. Verify: a test for "A sibling relation without stopBy draws a notice", including the `stopBy: end` silent case. -- [ ] 7.3 Add the awaited-generic-callee detector for `TypeScript`/`Tsx`. Verify: a test for "A callee regex under TypeScript draws a notice", plus "A method callee regex under TypeScript draws a notice", including the `await_expression` branch and other-language silent cases. Use the recipe's own workaround YAML (topic v7 and later) as the silent fixture, so the detector can't flag the fix the recipe recommends. -- [ ] 7.4 Point `create-sg-rule`'s Quirks section at the notices: say which quirks `validate` now reports, and that the keyword-`kind` and Markdown-inline config errors are now caught by `validate` under the rule's own name. Bump the topic version. Verify: `pnpm cli agent create-sg-rule` shows the change after `pnpm build`. -- [ ] 7.5 Verify that a trap notice doesn't fail `verify`. Verify: a test for "A trap notice does not fail verify". +- [x] 7.1 Add the `$$$`-beside-a-comma detector over pattern strings and object-pattern `context`. Verify: tests for "A comma beside $$$ draws a notice" and "A pattern that already uses strictness ast draws no notice". +- [x] 7.2 Add the `follows`/`precedes`-without-`stopBy` detector. Verify: a test for "A sibling relation without stopBy draws a notice", including the `stopBy: end` silent case. +- [x] 7.3 Add the awaited-generic-callee detector for `TypeScript`/`Tsx`. Verify: a test for "A callee regex under TypeScript draws a notice", plus "A method callee regex under TypeScript draws a notice", including the `await_expression` branch and other-language silent cases. Use the recipe's own workaround YAML (topic v7 and later) as the silent fixture, so the detector can't flag the fix the recipe recommends. +- [x] 7.4 Point `create-sg-rule`'s Quirks section at the notices: say which quirks `validate` now reports, and that the keyword-`kind` and Markdown-inline config errors are now caught by `validate` under the rule's own name. Bump the topic version. Verify: `pnpm cli agent create-sg-rule` shows the change after `pnpm build`. +- [x] 7.5 Verify that a trap notice doesn't fail `verify`. Verify: a test for "A trap notice does not fail verify". ## 8. `verify`: isolated fixture run, then PR 2 -- [ ] 8.1 Build a temporary one-rule `sgconfig.yml` for `runTests` and remove it afterwards. Verify: a test for "Another rule's config error does not fail this rule's fixtures", which reproduces gap 2 with a `kw` rule holding `kind: readonly`. Also check that no file is left behind. -- [ ] 8.2 Extend the changeset with the new `validate` checks. Verify: one changeset file, one release note. +- [x] 8.1 Build a temporary one-rule `sgconfig.yml` for `runTests` and remove it afterwards. Verify: a test for "Another rule's config error does not fail this rule's fixtures", which reproduces gap 2 with a `kw` rule holding `kind: readonly`. Also check that no file is left behind. +- [x] 8.2 Extend the changeset with the new `validate` checks. Verify: one changeset file, one release note. - [ ] 8.3 Run `pnpm typecheck`, `pnpm lint` and `pnpm test`, then archive the change with `pnpm openspec archive rename-rule-checks -y`. Verify: everything passes, and the archived specs keep every scenario (repeat the 4.4 comparison). - [ ] 8.4 Open PR 2 on top of PR 1, request the review, address the findings, and confirm with the user before merging. Verify: review posted and findings resolved. diff --git a/packages/cli/assets/reference.json b/packages/cli/assets/reference.json index 41935b20..f97590f0 100644 --- a/packages/cli/assets/reference.json +++ b/packages/cli/assets/reference.json @@ -61,6 +61,13 @@ "summary": "`files:` globs must not name `.tsx` under TypeScript, or `.ts` under Tsx.", "rationale": "A glob naming an extension the language cannot parse matches nothing, so the rule reports a clean codebase rather than an error. Only the TypeScript/Tsx pair is checked, and deliberately so: they are separate parsers rather than aliases, which is the one language/extension mismatch decidable from the rule file alone. No other extension is compared against `language`, so this is narrower than it first reads." }, + { + "id": "sg-no-top-level-strictness", + "engine": "sg", + "enforcedBy": "validate", + "summary": "`strictness:` belongs inside a pattern object, never at the top level of the rule file.", + "rationale": "ast-grep accepts a top-level `strictness:` and ignores it, so the pattern keeps its default `smart` strictness and goes on narrowing exactly as it did before the author tried to widen it. Inside `rule:` the same key is a schema error, which is loud; at the top level it is silent, so only this check can say so." + }, { "id": "sg-required-fields", "engine": "sg", diff --git a/packages/cli/src/agent/create-sg-rule.md b/packages/cli/src/agent/create-sg-rule.md index c38405a4..4bd2d575 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 v8) +# Topic: create-sg-rule (CLI v%(CLI_VERSION)s / topic v9) ## You are here This is `create-sg-rule`. It helps you write an ast-grep rule: a check @@ -188,7 +188,8 @@ 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 - goes unreported until it is fixed. + goes unreported until it is fixed. `validate` has ast-grep load each + rule on its own, so it names the rule that has the error. - **Silent.** The rule parses, validates, runs, and exits 0. What it misses goes unreported, which reads exactly like a clean codebase. @@ -196,16 +197,18 @@ When a rule matches less than it should, check this list before doubting the rest of the rule. Each entry was measured against the ast-grep this CLI ships (v%(AST_GREP_VERSION)s). The last column says whether `validate` reports it; where it says no, only you will notice. +A notice never fails the rule, because each of those shapes is +sometimes what the author meant. Read it and decide. -| quirk | symptom | `validate` reports it | +| quirk | symptom | `validate` reports it | |------------------------------------------------|-------------------------|-----------------------------| | `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 | no | -| `$$$` next to a comma | silent | no | -| a keyword named as a `kind` | config error | no | -| a callee rule over `await f(...)` | silent | no | -| `follows` or `precedes` without `stopBy: end` | silent | no | +| an inline construct named in a `Markdown` rule | config error, or silent | the `kind:` form only | +| `$$$` next to a comma | silent | as a notice | +| a keyword named as a `kind` | config error | yes | +| a callee rule over `await f(...)` | silent | as a notice | +| `follows` or `precedes` without `stopBy: end` | silent | as a notice | | a `not` containing a bare metavariable | silent | no | ### `language:` copied from `detect --json` diff --git a/packages/cli/src/agent/validate-rule.md b/packages/cli/src/agent/validate-rule.md index 9bdf3146..b9d7ef4a 100644 --- a/packages/cli/src/agent/validate-rule.md +++ b/packages/cli/src/agent/validate-rule.md @@ -1,4 +1,4 @@ -# Topic: validate-rule (CLI v%(CLI_VERSION)s / topic v5) +# Topic: validate-rule (CLI v%(CLI_VERSION)s / topic v6) ## Goal Check that a rule is well-formed (`validate`), then run its tests @@ -41,7 +41,7 @@ What each engine is checked for: | engine | `validate` checks | `verify` runs | |-----------|--------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------| -| `sg` | ast-grep schema, plus `id`/`language`/`severity`/`message`/`rule`, and `regex` accompanied by `kind` | the `valid`/`invalid` cases in `.tests/` | +| `sg` | ast-grep schema, plus `id`/`language`/`severity`/`message`/`rule`, `regex` accompanied by `kind`, no top-level `strictness`, then ast-grep loads the rule on its own | the `valid`/`invalid` cases in `.tests/` | | `vale` | style parses, `extends` and `message` present, `level` in vocabulary, and the rule's `.vale.ini` enables `.` under a matcher | the `.tests/pass/` and `.tests/fail/` buckets | | `runtime` | `check.ts` present, at least one capture rule in `captures/` | each directory under `.tests/pass/` and `.tests/fail/`, as the check's `root` | @@ -97,7 +97,9 @@ Both commands answer in the same shape, one entry per rule: it is reported on a pass as well. For a Vale rule it carries what Vale wrote to stderr on a run that still exited zero, such as the `W101` warning about an assignment placed above any matcher: the rule - validates, Vale ignores it, and only this field says so. Read it + validates, Vale ignores it, and only this field says so. For an `sg` + rule it names the silent traps from `create-sg-rule`'s Quirks section + that the rule's shape matches, such as `$$$` beside a comma. Read it before `ok`, the same as `ran`. A green Vale `verify` also assumes Vale read every fixture. A fixture diff --git a/packages/cli/src/rules/constraints.ts b/packages/cli/src/rules/constraints.ts index 24e9364d..6a77e76d 100644 --- a/packages/cli/src/rules/constraints.ts +++ b/packages/cli/src/rules/constraints.ts @@ -104,6 +104,15 @@ export const RULE_CONSTRAINTS = [ rationale: "A glob naming an extension the language cannot parse matches nothing, so the rule reports a clean codebase rather than an error. Only the TypeScript/Tsx pair is checked, and deliberately so: they are separate parsers rather than aliases, which is the one language/extension mismatch decidable from the rule file alone. No other extension is compared against `language`, so this is narrower than it first reads.", }, + { + id: "sg-no-top-level-strictness", + engine: "sg", + enforcedBy: "validate", + summary: + "`strictness:` belongs inside a pattern object, never at the top level of the rule file.", + rationale: + "ast-grep accepts a top-level `strictness:` and ignores it, so the pattern keeps its default `smart` strictness and goes on narrowing exactly as it did before the author tried to widen it. Inside `rule:` the same key is a schema error, which is loud; at the top level it is silent, so only this check can say so.", + }, { id: "sg-required-fields", engine: "sg", diff --git a/packages/cli/src/rules/sg-parse.ts b/packages/cli/src/rules/sg-parse.ts new file mode 100644 index 00000000..a062547a --- /dev/null +++ b/packages/cli/src/rules/sg-parse.ts @@ -0,0 +1,129 @@ +import { spawn } from "node:child_process"; +import process from "node:process"; +import { StringDecoder } from "node:string_decoder"; +import { stripVTControlCharacters } from "node:util"; + +import { ruleFilePath } from "./engines"; +import { buildPath, findSgBinary } from "./scan"; + +/** + * Whether the vendored ast-grep will load one `sg` rule, asked of that rule + * alone. + * + * The schema cannot decide everything the engine refuses. Which `kind` names + * are valid depends on the grammar, and the binary has no way to list them: a + * keyword token (`readonly`) or a Markdown inline construct (`link`) passes the + * vendored JSON Schema and is then refused at load. Only the engine can say, so + * this asks it. + * + * `scan -r --stdin` with nothing on stdin loads exactly one rule and + * scans nothing. No project config is assembled, so no other rule is parsed and + * the error can only belong to this one. Measured on ast-grep 0.45.3: + * + * | rule | exit | last line of the cause chain | + * | -------------------------------------- | ---- | ----------------------------------------------------- | + * | valid | 0 | | + * | `kind: readonly` (TypeScript) | 8 | Kind `readonly` is invalid. | + * | `kind: link` (Markdown) | 8 | Kind `link` is invalid. | + * | `language: Klingon` | 8 | data did not match any variant of untagged enum SgLang | + * | `pattern: "foo("` | 8 | Rule must specify a set of AST kinds to match. ... | + * | `regex: "(["` | 8 | regex parse error: (then, on later lines) unclosed character class | + * | top-level `strictness: ast` | 0 | (accepted and ignored; see `sg-no-top-level-strictness`) | + * + * Ten calls take about 30 ms together, so one per rule costs nothing a person + * would notice. + * + * Resolves to `undefined` when the rule loads, and otherwise to the message to + * report under it. + */ +export async function parseSgRuleAlone( + cwd: string, + ruleId: string +): Promise { + const sgBinary = findSgBinary(); + return new Promise((resolve) => { + const child = spawn( + sgBinary, + [ + "scan", + "-r", + ruleFilePath(cwd, "sg", ruleId), + "--stdin", + "--json=stream", + ], + { + cwd, + stdio: ["pipe", "ignore", "pipe"], + env: { ...process.env, PATH: buildPath() }, + } + ); + + // One decoder for the stream, for the reason `runTests` gives: a + // multi-byte sequence split across chunks must not become two U+FFFDs. + const decoder = new StringDecoder("utf8"); + const chunks: string[] = []; + child.stderr.on("data", (chunk: Buffer) => { + chunks.push(decoder.write(chunk)); + }); + + child.on("error", () => { + resolve("ast-grep (sg) binary not found. Is @ast-grep/cli installed?"); + }); + child.on("close", (code) => { + chunks.push(decoder.end()); + const refusal = + code === 0 ? undefined : describeRefusal(chunks.join(""), code); + resolve(refusal); + }); + + child.stdin.on("error", () => { + // A rule ast-grep refuses can close stdin before it is ended. The exit + // code above is the verdict; this is not a second one. + }); + child.stdin.end(); + }); +} + +/** + * The message for a rule ast-grep would not load. + * + * ast-grep prints a generic header ("Cannot parse rule ") and then a + * `Caused by` chain from the general to the specific. The last link is the one + * an author can act on, so it leads; the rest follow as context. The header is + * dropped because it names the file by the path this module passed, which says + * nothing the rule's own entry in the report does not already say. + */ +function describeRefusal(stderr: string, code: number | null): string { + // A link can run on past its own line: a regex error prints the pattern, a + // caret, and `error: ` beneath `regex parse error:`. The last + // of those lines is the specific part, so it is folded into its link. + const causes: string[] = []; + for (const raw of stripVTControlCharacters(stderr).split("\n")) { + const line = raw.trim(); + if (line.startsWith("╰▻")) { + causes.push(line.replace(/^╰▻\s*/, "")); + } else if (causes.length > 0 && line.startsWith("error:")) { + const last = causes.length - 1; + causes[last] = `${causes[last] ?? ""} ${line.replace(/^error:\s*/, "")}`; + } + } + + const consequence = + "`check` loads every sg rule into one config, so until this is fixed it reports no sg findings for the project at all."; + + if (causes.length === 0) { + const firstLine = stripVTControlCharacters(stderr) + .split("\n") + .map((line) => line.trim()) + .find((line) => line !== ""); + return `ast-grep refuses this rule (exit ${String(code)})${firstLine === undefined ? "" : `: ${firstLine}`}. ${consequence}`; + } + + const specific = causes.at(-1) ?? ""; + const context = causes.slice(0, -1); + return ( + `ast-grep refuses this rule: ${specific}` + + (context.length > 0 ? ` (${context.join(" › ")})` : "") + + ` ${consequence}` + ); +} diff --git a/packages/cli/src/rules/sg-traps.ts b/packages/cli/src/rules/sg-traps.ts new file mode 100644 index 00000000..889b8819 --- /dev/null +++ b/packages/cli/src/rules/sg-traps.ts @@ -0,0 +1,163 @@ +import { resolveAstGrepLanguage } from "./capabilities"; + +/** + * Notices for the ast-grep authoring traps `create-sg-rule` lists as SILENT: + * the rule parses, validates, runs and exits zero while missing what its + * author meant it to match. + * + * Notices and never errors. Each shape has a legitimate use — a sibling + * relation meant to stop at the neighbour, a pattern meant to require at least + * one argument — and `validate` cannot tell which the author meant. A notice is + * something they can decide about; an error is something they cannot write + * past. + * + * Decided over the parsed rule object rather than by a second engine pass. + * Every trap here is a fact about the rule's own structure, so the structure is + * the place to read it, and it costs no subprocess. + * + * A trap ast-grep refuses outright (a keyword named as a `kind`, a Markdown + * inline construct) is not here: `parseSgRuleAlone` rejects those. + */ +export function findSgTraps(rule: Record): string[] { + const notices: string[] = []; + const language = + typeof rule.language === "string" + ? resolveAstGrepLanguage(rule.language) + : undefined; + const typescriptFamily = language === "TypeScript" || language === "Tsx"; + + for (const key of ["rule", "constraints", "utils"] as const) { + const container = rule[key]; + if (container === undefined) continue; + walk(container, key, (node, path) => { + notices.push( + ...separatorTraps(node, path), + ...siblingTraps(node, path), + ...(typescriptFamily ? calleeTraps(node, path) : []) + ); + }); + } + + return notices; +} + +type Visit = (node: Record, path: string) => void; + +/** Every object in a rule tree, with a dotted path for the message. */ +function walk(value: unknown, path: string, visit: Visit): void { + if (Array.isArray(value)) { + for (const [index, item] of value.entries()) { + walk(item, `${path}[${String(index)}]`, visit); + } + return; + } + if (value === null || typeof value !== "object") return; + const node = value as Record; + visit(node, path); + for (const [key, child] of Object.entries(node)) { + walk(child, `${path}.${key}`, visit); + } +} + +/** A comma directly after a `$$$` metavariable: the leading form. */ +const LEADING_ELLIPSIS = /\$\$\$[A-Z0-9_]*\s*,/; +/** A comma directly before a `$$$` metavariable: the trailing form. */ +const TRAILING_ELLIPSIS = /,\s*\$\$\$/; + +/** + * `$$$` beside a comma. Under the default `smart` strictness the `,` is a node + * the target must also have, so `foo($A, $$$)` never matches `foo(1)`. + * + * The trailing form is rescued by an object pattern with `strictness: ast`, so + * one that already declares it is left alone. The leading form is not rescued + * by anything but alternatives, so it is reported either way. + */ +function separatorTraps(node: Record, path: string): string[] { + const pattern = node.pattern; + let text: string | undefined; + let astStrict = false; + if (typeof pattern === "string") { + text = pattern; + } else if ( + pattern !== null && + typeof pattern === "object" && + !Array.isArray(pattern) + ) { + const object = pattern as Record; + if (typeof object.context === "string") text = object.context; + astStrict = object.strictness === "ast"; + } + if (text === undefined) return []; + + if (LEADING_ELLIPSIS.test(text)) { + return [ + `${path}.pattern \`${text}\` puts \`$$$\` before a \`,\`, so it never matches a call where nothing precedes the last item: the comma is a node the target must also have. \`strictness: ast\` does not rescue this form. Match each arity as its own branch of an \`any\` instead.`, + ]; + } + if (TRAILING_ELLIPSIS.test(text) && !astStrict) { + return [ + `${path}.pattern \`${text}\` puts a \`,\` before \`$$$\`, so under the default \`smart\` strictness it never matches zero trailing items (\`foo($A, $$$)\` misses \`foo(1)\`). Write it as a pattern object with \`context\`, \`selector\` and \`strictness: ast\`, which ignores the separator.`, + ]; + } + return []; +} + +/** + * `follows` or `precedes` with no `stopBy`. The default walks only the + * immediate sibling, and separators are siblings too, so the previous element + * of `[10, 10]` is the `,` rather than the `10`. + */ +function siblingTraps(node: Record, path: string): string[] { + const notices: string[] = []; + for (const relation of ["follows", "precedes"] as const) { + const value = node[relation]; + if (value === null || typeof value !== "object" || Array.isArray(value)) { + continue; + } + if ("stopBy" in value) continue; + notices.push( + `${path}.${relation} declares no \`stopBy\`, so it checks only the immediately adjacent sibling, and punctuation such as \`,\` counts as one: in \`[10, 10]\` the second \`10\` does not follow a number. Add \`stopBy: end\` inside \`${relation}\` unless stopping at the neighbour is what you mean.` + ); + } + return notices; +} + +/** + * A callee matched by `regex` under `field: function`, in TypeScript or Tsx, + * with no branch for the awaited form. The grammar parses + * `await rpc()` as a call OF the await expression, so the callee is an + * `await_expression` and the regex never sees `rpc`. + * + * Any `await_expression` inside the relation counts as handling it, which is + * how both of the recipe's workarounds (function and method) are written. + */ +function calleeTraps(node: Record, path: string): string[] { + if (node.field !== "function") return []; + if ( + !subtreeHas(node, (key, value) => key === "regex" && value !== undefined) + ) { + return []; + } + if ( + subtreeHas( + node, + (key, value) => key === "kind" && value === "await_expression" + ) + ) { + return []; + } + return [ + `${path} matches the callee by \`regex\` under \`field: function\`, which misses every awaited generic call: TypeScript parses \`await rpc()\` as a call of the \`await_expression\`, so the callee is never \`rpc\`. Add an \`any\` branch with \`kind: await_expression\` that matches the same callee inside it.`, + ]; +} + +function subtreeHas( + value: unknown, + test: (key: string, value: unknown) => boolean +): boolean { + if (Array.isArray(value)) return value.some((item) => subtreeHas(item, test)); + if (value === null || typeof value !== "object") return false; + return Object.entries(value as Record).some( + ([key, child]) => test(key, child) || subtreeHas(child, test) + ); +} diff --git a/packages/cli/src/rules/verify.ts b/packages/cli/src/rules/verify.ts index 07bc1f16..329410e8 100644 --- a/packages/cli/src/rules/verify.ts +++ b/packages/cli/src/rules/verify.ts @@ -1,5 +1,6 @@ -import { readFile, readdir } from "node:fs/promises"; -import { join } from "node:path"; +import { mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, posix, sep } from "node:path"; import { spawn } from "node:child_process"; import process from "node:process"; import { StringDecoder } from "node:string_decoder"; @@ -23,14 +24,15 @@ import { type AstGrepLanguage, } from "./capabilities"; import { ensureTasklessDirectory } from "../filesystem/directory"; -import { assembleSgConfig } from "./assemble"; -import { ruleFilePath, ruleTestsDirectory } from "./engines"; +import { ruleDirectory, ruleFilePath, ruleTestsDirectory } from "./engines"; import { RULE_TESTS_DIRECTORY, RULES_DIRECTORY } from "./layout"; import { findSgBinary, buildPath, stripSgDeprecationBanner } from "./scan"; 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"; +import { parseSgRuleAlone } from "./sg-parse"; +import { findSgTraps } from "./sg-traps"; import { escapeRegExp } from "../util/regex"; // --- Types --- @@ -370,6 +372,17 @@ async function validateRequirements( } } + // `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. + if ("strictness" in ruleData) { + violate( + collected, + "sg-no-top-level-strictness", + "strictness: at the top level of the rule file is ignored by ast-grep, so the pattern keeps its default `smart` strictness. Move it into the pattern object it is meant for (`pattern: { context: ..., selector: ..., strictness: ast }`)." + ); + } + // The rule's own `id:` must be the directory it lives in. // // The directory name IS the rule id: it is what `check` and `verify` address, @@ -577,19 +590,43 @@ async function runTests( cwd: string, ruleId: string ): Promise> { - // Assembly names every rule's `.tests/` as its own `testConfigs` entry, so - // the filter below selects a rule whose tests ast-grep already knows how to - // find. - const configPath = await assembleSgConfig(cwd); - if (configPath === undefined) { - return { - valid: false, - errors: ["No ast-grep rules are present, so no tests could be run."], - passed: 0, - failed: 0, - }; + // A config naming this rule and nothing else, rather than the project-wide + // one `check` assembles. `--filter` narrows which TESTS run, not which rules + // ast-grep parses, so under the project config one rule ast-grep refuses + // fails every other rule's run, reported under the wrong rule's name. + // + // Written outside the project and removed afterwards, so a run leaves + // nothing behind that a later `check` could read. Absolute paths, because + // ast-grep resolves `ruleDirs` and `testDir` against the config's own + // directory, which is no longer the project's. Measured on 0.45.3: the + // rule's `.tests/` sits inside `ruleDirs` and is not parsed as a rule, the + // same as under the project config. + const scratch = await mkdtemp(join(tmpdir(), "taskless-sg-verify-")); + try { + const configPath = join(scratch, "sgconfig.yml"); + await writeFile( + configPath, + [ + "ruleDirs:", + ` - ${JSON.stringify(ruleDirectory(cwd, "sg", ruleId).split(sep).join(posix.sep))}`, + "testConfigs:", + ` - testDir: ${JSON.stringify(ruleTestsDirectory(cwd, "sg", ruleId).split(sep).join(posix.sep))}`, + "", + ].join("\n"), + "utf8" + ); + return await runSgTest(cwd, configPath, ruleId); + } finally { + await rm(scratch, { recursive: true, force: true }); } +} +/** `sg test` for one rule, under the given config. */ +async function runSgTest( + cwd: string, + configPath: string, + ruleId: string +): Promise> { const sgBinary = findSgBinary(); return new Promise((resolve) => { @@ -826,11 +863,21 @@ export async function verifyRule( // Layer 1, plus the `language:` field the vendored JSON Schema cannot type. const parsed = validateSchema(ruleData); const language = validateLanguage(ruleRecord); + const schemaErrors = [...parsed.errors, ...language.errors]; + // Then the engine itself, over this rule alone, for what no schema can + // decide (a `kind` the grammar does not have). Only once the schema passes: + // a rule it already rejects would draw a second, vaguer message for the + // same defect. Unattributed, because the refusal is the engine's own rule + // and not one of ours. + if (schemaErrors.length === 0) { + const refusal = await parseSgRuleAlone(cwd, ruleId); + if (refusal !== undefined) schemaErrors.push(refusal); + } const schemaResult: SchemaLayerResult = { - valid: parsed.valid && language.errors.length === 0, - errors: [...parsed.errors, ...language.errors], + valid: schemaErrors.length === 0, + errors: schemaErrors, violations: language.violations, - notices: language.notices, + notices: [...language.notices, ...findSgTraps(ruleRecord)], }; // Layer 2 diff --git a/packages/cli/test/constraints.test.ts b/packages/cli/test/constraints.test.ts index ced3030c..34691e54 100644 --- a/packages/cli/test/constraints.test.ts +++ b/packages/cli/test/constraints.test.ts @@ -200,6 +200,15 @@ const VIOLATIONS: Record = { }), names: /every glob names \.tsx, but language is TypeScript/, }, + "sg-no-top-level-strictness": { + run: () => + verifyWritten({ + rule: { ...VALID_RULE, strictness: "ast" }, + fixture: VALID_FIXTURE, + }), + names: + /strictness: at the top level of the rule file is ignored by ast-grep/, + }, "sg-required-fields": { run: () => verifyWritten({ diff --git a/packages/cli/test/sg-engine-parse.test.ts b/packages/cli/test/sg-engine-parse.test.ts new file mode 100644 index 00000000..6f05c24b --- /dev/null +++ b/packages/cli/test/sg-engine-parse.test.ts @@ -0,0 +1,165 @@ +import { execFile } from "node:child_process"; +import { mkdir, mkdtemp, readdir, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import process from "node:process"; +import { promisify } from "node:util"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { cliRejectionToResult } from "./support/spawn-cli"; +import { builtCli } from "./support/built-cli"; + +const execFileAsync = promisify(execFile); +const binPath = builtCli(); + +let cwd: string; + +async function runCli(args: string[], env?: NodeJS.ProcessEnv) { + try { + const { stdout, stderr } = await execFileAsync("node", [binPath, ...args], { + env: { ...process.env, ...env }, + }); + return { stdout, stderr, exitCode: 0 }; + } catch (error) { + return cliRejectionToResult(error, [binPath, ...args]); + } +} + +interface Report { + ok: boolean; + rules: { + ruleId: string; + ok: boolean; + errors: string[]; + violations: { constraintId: string; message: string }[]; + notices: string[]; + }[]; +} + +beforeEach(async () => { + cwd = await mkdtemp(join(tmpdir(), "tskl-sg-parse-")); + await runCli(["init", "-d", cwd]); +}); + +afterEach(async () => { + await rm(cwd, { recursive: true, force: true }); +}); + +/** An sg rule with both fixture buckets, `body` written verbatim. */ +async function sgRule(id: string, body: string) { + const directory = join(cwd, ".taskless", "rules", "sg", id); + await mkdir(join(directory, ".tests"), { recursive: true }); + await writeFile( + join(directory, `${id}.yml`), + `id: ${id}\nlanguage: TypeScript\nseverity: error\nmessage: m\n${body}` + ); + await writeFile( + join(directory, ".tests", `${id}-test.yml`), + `id: ${id}\nvalid:\n - foo(x);\ninvalid:\n - eval(x);\n` + ); +} + +const GOOD = + 'rule:\n kind: call_expression\n has: { field: function, kind: identifier, regex: "^eval$" }\n'; +/** Passes the schema, refused by ast-grep: `readonly` is a keyword token. */ +const KEYWORD_KIND = "rule:\n kind: readonly\n"; + +async function report(command: string, path: string): Promise { + const result = await runCli([command, path, "-d", cwd, "--json"]); + return JSON.parse(result.stdout) as Report; +} + +describe("validate parses each sg rule with ast-grep, alone", () => { + it("rejects a kind the grammar does not have, in ast-grep's words", async () => { + await sgRule("kw", KEYWORD_KIND); + const { rules } = await report("validate", ".taskless/rules/sg/kw"); + expect(rules[0]?.ok).toBe(false); + expect(rules[0]?.errors.join("\n")).toContain("Kind `readonly` is invalid"); + }); + + it("attaches no constraint id to the engine's refusal", async () => { + await sgRule("kw", KEYWORD_KIND); + const { rules } = await report("validate", ".taskless/rules/sg/kw"); + expect(rules[0]?.violations).toEqual([]); + }); + + it("reports the error under the rule that has it, and only there", async () => { + await sgRule("kw", KEYWORD_KIND); + await sgRule("good", GOOD); + const { rules } = await report("validate", ".taskless/rules/sg"); + const byId = Object.fromEntries(rules.map((rule) => [rule.ruleId, rule])); + expect(byId.kw?.ok).toBe(false); + expect(byId.good?.ok).toBe(true); + expect(byId.good?.errors).toEqual([]); + }); +}); + +describe("validate refuses a top-level strictness", () => { + it("names the constraint and where the key belongs", async () => { + await sgRule("loose", `strictness: ast\n${GOOD}`); + const { rules } = await report("validate", ".taskless/rules/sg/loose"); + expect(rules[0]?.ok).toBe(false); + expect(rules[0]?.violations.map((v) => v.constraintId)).toEqual([ + "sg-no-top-level-strictness", + ]); + expect(rules[0]?.errors.join("\n")).toContain("pattern object"); + }); +}); + +describe("verify judges an sg rule on its own fixtures", () => { + // #489, gap 2. Under the project-wide config `kw` failed `good`'s fixture + // run with "Cannot parse rule .../kw/kw.yml", reported under `good`. + it("is not failed by another rule ast-grep refuses", async () => { + await sgRule("kw", KEYWORD_KIND); + await sgRule("good", GOOD); + const { rules } = await report("verify", ".taskless/rules/sg/good"); + expect(rules[0]?.ok).toBe(true); + expect(rules[0]?.errors).toEqual([]); + }); + + // Its own temporary directory, so a `verify` another test file runs in + // parallel cannot put a directory there between the run and the listing. + it("leaves no config behind, in the project or the temporary directory", async () => { + await sgRule("good", GOOD); + const scratch = await mkdtemp(join(tmpdir(), "tskl-sg-tmp-")); + try { + const result = await runCli( + ["verify", ".taskless/rules/sg/good", "-d", cwd, "--json"], + { TMPDIR: scratch, TMP: scratch, TEMP: scratch } + ); + expect(result.exitCode).toBe(0); + expect(await readdir(scratch)).toEqual([]); + } finally { + await rm(scratch, { recursive: true, force: true }); + } + const ruleDirectory = await readdir( + join(cwd, ".taskless", "rules", "sg", "good") + ); + expect(ruleDirectory.toSorted()).toEqual([".tests", "good.yml"]); + }); + + it("passes a rule whose only findings from validate are trap notices", async () => { + // A statement calling eval that follows another statement. `follows` has + // no `stopBy`, which draws the sibling notice (and the callee regex under + // TypeScript draws the awaited-call one); neither is a failure, so the + // fixtures decide. Statements are adjacent siblings with no punctuation + // between them, so this rule does fire where the invalid case says. + await sgRule( + "sib", + "rule:\n kind: expression_statement\n has:\n kind: call_expression\n" + + ' has: { field: function, kind: identifier, regex: "^eval$" }\n' + + " follows: { kind: expression_statement }\n" + ); + await writeFile( + join(cwd, ".taskless", "rules", "sg", "sib", ".tests", "sib-test.yml"), + "id: sib\nvalid:\n - eval(x);\ninvalid:\n - |\n a();\n eval(x);\n" + ); + const { rules } = await report("verify", ".taskless/rules/sg/sib"); + expect(rules[0]?.errors).toEqual([]); + expect(rules[0]?.ok).toBe(true); + expect(rules[0]?.notices.some((notice) => notice.includes("stopBy"))).toBe( + true + ); + }); +}); diff --git a/packages/cli/test/sg-traps.test.ts b/packages/cli/test/sg-traps.test.ts new file mode 100644 index 00000000..0393e072 --- /dev/null +++ b/packages/cli/test/sg-traps.test.ts @@ -0,0 +1,209 @@ +import { describe, expect, it } from "vitest"; + +import { findSgTraps } from "../src/rules/sg-traps"; + +/** A TypeScript rule around the given `rule:` body. */ +function typescript(rule: unknown, language = "TypeScript") { + return { + id: "probe", + language, + severity: "error", + message: "m", + rule, + }; +} + +describe("$$$ beside a comma", () => { + it("notices the trailing form in a plain pattern", () => { + const notices = findSgTraps(typescript({ pattern: "foo($A, $$$)" })); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("foo($A, $$$)"); + expect(notices[0]).toContain("strictness: ast"); + }); + + it("is silent when the pattern object already declares strictness: ast", () => { + const notices = findSgTraps( + typescript({ + pattern: { + context: "foo($A, $$$)", + selector: "call_expression", + strictness: "ast", + }, + }) + ); + expect(notices).toEqual([]); + }); + + // The mirror case: the recipe says `strictness: ast` does not rescue it, so + // declaring it must not silence the notice. + it("notices the leading form even under strictness: ast", () => { + const notices = findSgTraps( + typescript({ + pattern: { + context: "foo($$$REST, $LAST)", + selector: "call_expression", + strictness: "ast", + }, + }) + ); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("any"); + }); + + it("is silent for a standalone $$$", () => { + expect(findSgTraps(typescript({ pattern: "foo($$$)" }))).toEqual([]); + }); + + it("finds a pattern nested inside a relation", () => { + const notices = findSgTraps( + typescript({ + kind: "expression_statement", + has: { pattern: "foo($A, $$$)" }, + }) + ); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("rule.has.pattern"); + }); +}); + +describe("a sibling relation without stopBy", () => { + it("notices follows with no stopBy", () => { + const notices = findSgTraps( + typescript({ kind: "number", follows: { kind: "number" } }) + ); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("follows"); + expect(notices[0]).toContain("stopBy: end"); + }); + + it("notices precedes the same way", () => { + const notices = findSgTraps( + typescript({ kind: "number", precedes: { kind: "number" } }) + ); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("precedes"); + }); + + it("is silent once stopBy is declared", () => { + expect( + findSgTraps( + typescript({ + kind: "number", + follows: { kind: "number", stopBy: "end" }, + }) + ) + ).toEqual([]); + }); +}); + +describe("an awaited generic call hides its callee", () => { + const plain = { + kind: "call_expression", + has: { field: "function", kind: "identifier", regex: "^rpc$" }, + }; + + it("notices a callee regex under TypeScript", () => { + const notices = findSgTraps(typescript(plain)); + expect(notices).toHaveLength(1); + expect(notices[0]).toContain("await_expression"); + }); + + it("notices it under Tsx", () => { + expect(findSgTraps(typescript(plain, "Tsx"))).toHaveLength(1); + }); + + it("is silent under any other language", () => { + expect(findSgTraps(typescript(plain, "JavaScript"))).toEqual([]); + }); + + it("notices a method callee", () => { + const notices = findSgTraps( + typescript({ + kind: "call_expression", + has: { + field: "function", + kind: "member_expression", + has: { + field: "property", + kind: "property_identifier", + regex: "^rpc$", + }, + }, + }) + ); + expect(notices).toHaveLength(1); + }); + + // The two workarounds `create-sg-rule` recommends, verbatim. A detector that + // flagged the recipe's own fix would teach authors to ignore it. + it("is silent for the recipe's function workaround", () => { + expect( + findSgTraps( + typescript({ + kind: "call_expression", + has: { + field: "function", + any: [ + { kind: "identifier", regex: "^rpc$" }, + { + kind: "await_expression", + has: { kind: "identifier", regex: "^rpc$" }, + }, + ], + }, + }) + ) + ).toEqual([]); + }); + + it("is silent for the recipe's method workaround", () => { + expect( + findSgTraps( + typescript({ + kind: "call_expression", + has: { + field: "function", + any: [ + { + kind: "member_expression", + has: { + field: "property", + kind: "property_identifier", + regex: "^rpc$", + }, + }, + { + kind: "await_expression", + has: { + kind: "member_expression", + has: { + field: "property", + kind: "property_identifier", + regex: "^rpc$", + }, + }, + }, + ], + }, + }) + ) + ).toEqual([]); + }); + + it("is silent when the callee is matched without a regex", () => { + expect( + findSgTraps( + typescript({ + kind: "call_expression", + has: { field: "function", pattern: "rpc" }, + }) + ) + ).toEqual([]); + }); +}); + +describe("a rule with none of the traps", () => { + it("carries no notices", () => { + expect(findSgTraps(typescript({ pattern: "eval($ARG)" }))).toEqual([]); + }); +}); From 15a3bf0a92c00c6b1a843195a9d2a2736bd91a3b Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 12 Oct 2026 16:28:21 -0700 Subject: [PATCH 2/4] test(cli): assert the specific cause for each refusal shape ast-grep reports Only `kind: readonly` was asserted, so a reformat of the regex or pattern diagnostics could have degraded the message to the generic exit-code fallback without failing anything. The regex case is the only one that reaches the continuation-line fold; removing the fold fails it. Found in review of #493. --- packages/cli/test/sg-engine-parse.test.ts | 42 +++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/cli/test/sg-engine-parse.test.ts b/packages/cli/test/sg-engine-parse.test.ts index 6f05c24b..7406ac98 100644 --- a/packages/cli/test/sg-engine-parse.test.ts +++ b/packages/cli/test/sg-engine-parse.test.ts @@ -47,12 +47,12 @@ afterEach(async () => { }); /** An sg rule with both fixture buckets, `body` written verbatim. */ -async function sgRule(id: string, body: string) { +async function sgRule(id: string, body: string, language = "TypeScript") { const directory = join(cwd, ".taskless", "rules", "sg", id); await mkdir(join(directory, ".tests"), { recursive: true }); await writeFile( join(directory, `${id}.yml`), - `id: ${id}\nlanguage: TypeScript\nseverity: error\nmessage: m\n${body}` + `id: ${id}\nlanguage: ${language}\nseverity: error\nmessage: m\n${body}` ); await writeFile( join(directory, ".tests", `${id}-test.yml`), @@ -78,6 +78,44 @@ describe("validate parses each sg rule with ast-grep, alone", () => { expect(rules[0]?.errors.join("\n")).toContain("Kind `readonly` is invalid"); }); + // Each shape reaches a different part of the cause-chain parsing, and each + // was measured on 0.45.3. The specific cause has to survive: a reformat that + // dropped it would still fail the rule, but with the generic exit-code + // message, and only these assertions would notice. + it.each([ + { + shape: "a Markdown inline kind", + body: "rule:\n kind: link\n", + language: "Markdown", + expected: "Kind `link` is invalid", + }, + { + // A one-link chain, with no context to put in parentheses. + shape: "a pattern with no AST kind", + body: 'rule:\n pattern: "foo("\n', + language: "TypeScript", + expected: "Rule must specify a set of AST kinds to match", + }, + { + // The specific part arrives on a later line, after the pattern and a + // caret, as `error: ...`; it is folded into the last link. + shape: "a regex that does not compile", + body: 'rule:\n kind: identifier\n regex: "(["\n', + language: "TypeScript", + expected: "regex parse error: unclosed character class", + }, + ])( + "names the specific cause for $shape", + async ({ body, language, expected }) => { + await sgRule("refused", body, language); + const { rules } = await report("validate", ".taskless/rules/sg/refused"); + const errors = rules[0]?.errors.join("\n") ?? ""; + expect(rules[0]?.ok).toBe(false); + expect(errors).toContain(`ast-grep refuses this rule: ${expected}`); + expect(errors).not.toContain("(exit "); + } + ); + it("attaches no constraint id to the engine's refusal", async () => { await sgRule("kw", KEYWORD_KIND); const { rules } = await report("validate", ".taskless/rules/sg/kw"); From 6265e683bcf89ec55e632594efb3adf12d6d4829 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 12 Oct 2026 16:44:06 -0700 Subject: [PATCH 3/4] fix(cli): report both `$$$` traps when one pattern holds both `separatorTraps` returned after the leading form, so `foo($$$HEAD, $A, $$$TAIL)` reported only it. The two forms have different remedies (alternatives for the leading one, `strictness: ast` for the trailing one), so an author who fixed the first had no word of the second. Found in the second review of #493. --- packages/cli/src/rules/sg-traps.ts | 20 ++++++++++++-------- packages/cli/test/sg-traps.test.ts | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/rules/sg-traps.ts b/packages/cli/src/rules/sg-traps.ts index 889b8819..650f3c98 100644 --- a/packages/cli/src/rules/sg-traps.ts +++ b/packages/cli/src/rules/sg-traps.ts @@ -70,7 +70,10 @@ const TRAILING_ELLIPSIS = /,\s*\$\$\$/; * * The trailing form is rescued by an object pattern with `strictness: ast`, so * one that already declares it is left alone. The leading form is not rescued - * by anything but alternatives, so it is reported either way. + * by anything but alternatives, so it is reported either way. One pattern can + * hold both (`foo($$$HEAD, $A, $$$TAIL)`), and they have different remedies, + * so each is reported on its own: an author who fixes one must still hear + * about the other. */ function separatorTraps(node: Record, path: string): string[] { const pattern = node.pattern; @@ -89,17 +92,18 @@ function separatorTraps(node: Record, path: string): string[] { } if (text === undefined) return []; + const notices: string[] = []; if (LEADING_ELLIPSIS.test(text)) { - return [ - `${path}.pattern \`${text}\` puts \`$$$\` before a \`,\`, so it never matches a call where nothing precedes the last item: the comma is a node the target must also have. \`strictness: ast\` does not rescue this form. Match each arity as its own branch of an \`any\` instead.`, - ]; + notices.push( + `${path}.pattern \`${text}\` puts \`$$$\` before a \`,\`, so it never matches a call where nothing precedes the last item: the comma is a node the target must also have. \`strictness: ast\` does not rescue this form. Match each arity as its own branch of an \`any\` instead.` + ); } if (TRAILING_ELLIPSIS.test(text) && !astStrict) { - return [ - `${path}.pattern \`${text}\` puts a \`,\` before \`$$$\`, so under the default \`smart\` strictness it never matches zero trailing items (\`foo($A, $$$)\` misses \`foo(1)\`). Write it as a pattern object with \`context\`, \`selector\` and \`strictness: ast\`, which ignores the separator.`, - ]; + notices.push( + `${path}.pattern \`${text}\` puts a \`,\` before \`$$$\`, so under the default \`smart\` strictness it never matches zero trailing items (\`foo($A, $$$)\` misses \`foo(1)\`). Write it as a pattern object with \`context\`, \`selector\` and \`strictness: ast\`, which ignores the separator.` + ); } - return []; + return notices; } /** diff --git a/packages/cli/test/sg-traps.test.ts b/packages/cli/test/sg-traps.test.ts index 0393e072..4f19376c 100644 --- a/packages/cli/test/sg-traps.test.ts +++ b/packages/cli/test/sg-traps.test.ts @@ -50,6 +50,20 @@ describe("$$$ beside a comma", () => { expect(notices[0]).toContain("any"); }); + // Two traps with different remedies in one pattern. Returning after the + // first would leave an author who fixes the leading form with no word of the + // trailing one. + it("notices both forms when one pattern holds both", () => { + const notices = findSgTraps( + typescript({ pattern: "foo($$$HEAD, $A, $$$TAIL)" }) + ); + expect(notices).toHaveLength(2); + expect(notices.some((notice) => notice.includes("any"))).toBe(true); + expect(notices.some((notice) => notice.includes("strictness: ast"))).toBe( + true + ); + }); + it("is silent for a standalone $$$", () => { expect(findSgTraps(typescript({ pattern: "foo($$$)" }))).toEqual([]); }); From 6557fc6ccd4538a86dd19e9f978d07022c48c2b9 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Mon, 12 Oct 2026 16:44:19 -0700 Subject: [PATCH 4/4] docs(openspec): archive rename-rule-checks Checked before committing: every scenario in the 8 affected specs survives, and the only titles that change are the five the REMOVED reasons name. --- .../.openspec.yaml | 0 .../2026-10-12-rename-rule-checks}/design.md | 0 .../proposal.md | 0 .../specs/cli-agent-authoring/spec.md | 0 .../specs/cli-conformance-reference/spec.md | 0 .../specs/cli-rule-format/spec.md | 0 .../specs/cli-rule-routing/spec.md | 0 .../specs/cli-rule-validation/spec.md | 0 .../specs/cli-rules/spec.md | 0 .../specs/cli-runtime-rule-execution/spec.md | 0 .../specs/cli-taskless-bootstrap/spec.md | 0 .../specs/cli-vale-rule-engine/spec.md | 0 .../2026-10-12-rename-rule-checks}/tasks.md | 2 +- openspec/specs/cli-agent-authoring/spec.md | 12 +- .../specs/cli-conformance-reference/spec.md | 12 +- openspec/specs/cli-rule-format/spec.md | 6 +- openspec/specs/cli-rule-routing/spec.md | 2 +- openspec/specs/cli-rule-validation/spec.md | 580 ++++++++++-------- openspec/specs/cli-rules/spec.md | 8 +- .../specs/cli-runtime-rule-execution/spec.md | 4 +- openspec/specs/cli-taskless-bootstrap/spec.md | 4 +- openspec/specs/cli-vale-rule-engine/spec.md | 28 +- 22 files changed, 375 insertions(+), 283 deletions(-) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/.openspec.yaml (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/design.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/proposal.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-agent-authoring/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-conformance-reference/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-rule-format/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-rule-routing/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-rule-validation/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-rules/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-runtime-rule-execution/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-taskless-bootstrap/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/specs/cli-vale-rule-engine/spec.md (100%) rename openspec/changes/{rename-rule-checks => archive/2026-10-12-rename-rule-checks}/tasks.md (99%) diff --git a/openspec/changes/rename-rule-checks/.openspec.yaml b/openspec/changes/archive/2026-10-12-rename-rule-checks/.openspec.yaml similarity index 100% rename from openspec/changes/rename-rule-checks/.openspec.yaml rename to openspec/changes/archive/2026-10-12-rename-rule-checks/.openspec.yaml diff --git a/openspec/changes/rename-rule-checks/design.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/design.md similarity index 100% rename from openspec/changes/rename-rule-checks/design.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/design.md diff --git a/openspec/changes/rename-rule-checks/proposal.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/proposal.md similarity index 100% rename from openspec/changes/rename-rule-checks/proposal.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/proposal.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-agent-authoring/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-agent-authoring/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-agent-authoring/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-agent-authoring/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-conformance-reference/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-conformance-reference/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-conformance-reference/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-conformance-reference/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-rule-format/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rule-format/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-rule-format/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rule-format/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-rule-routing/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rule-routing/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-rule-routing/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rule-routing/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-rule-validation/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rule-validation/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-rule-validation/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rule-validation/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-rules/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rules/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-rules/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-rules/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-runtime-rule-execution/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-runtime-rule-execution/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-runtime-rule-execution/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-runtime-rule-execution/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-taskless-bootstrap/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-taskless-bootstrap/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-taskless-bootstrap/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-taskless-bootstrap/spec.md diff --git a/openspec/changes/rename-rule-checks/specs/cli-vale-rule-engine/spec.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-vale-rule-engine/spec.md similarity index 100% rename from openspec/changes/rename-rule-checks/specs/cli-vale-rule-engine/spec.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/specs/cli-vale-rule-engine/spec.md diff --git a/openspec/changes/rename-rule-checks/tasks.md b/openspec/changes/archive/2026-10-12-rename-rule-checks/tasks.md similarity index 99% rename from openspec/changes/rename-rule-checks/tasks.md rename to openspec/changes/archive/2026-10-12-rename-rule-checks/tasks.md index 7489e667..d0e534d5 100644 --- a/openspec/changes/rename-rule-checks/tasks.md +++ b/openspec/changes/archive/2026-10-12-rename-rule-checks/tasks.md @@ -50,5 +50,5 @@ PR 1 is groups 1–4 (bottom of the stack, targets `main`). PR 2 is groups 5–8 - [x] 8.1 Build a temporary one-rule `sgconfig.yml` for `runTests` and remove it afterwards. Verify: a test for "Another rule's config error does not fail this rule's fixtures", which reproduces gap 2 with a `kw` rule holding `kind: readonly`. Also check that no file is left behind. - [x] 8.2 Extend the changeset with the new `validate` checks. Verify: one changeset file, one release note. -- [ ] 8.3 Run `pnpm typecheck`, `pnpm lint` and `pnpm test`, then archive the change with `pnpm openspec archive rename-rule-checks -y`. Verify: everything passes, and the archived specs keep every scenario (repeat the 4.4 comparison). +- [x] 8.3 Run `pnpm typecheck`, `pnpm lint` and `pnpm test`, then archive the change with `pnpm openspec archive rename-rule-checks -y`. Verify: everything passes, and the archived specs keep every scenario (repeat the 4.4 comparison). - [ ] 8.4 Open PR 2 on top of PR 1, request the review, address the findings, and confirm with the user before merging. Verify: review posted and findings resolved. diff --git a/openspec/specs/cli-agent-authoring/spec.md b/openspec/specs/cli-agent-authoring/spec.md index a087616b..6ca787f5 100644 --- a/openspec/specs/cli-agent-authoring/spec.md +++ b/openspec/specs/cli-agent-authoring/spec.md @@ -32,11 +32,11 @@ The `create-vale-rule` recipe SHALL instruct the agent to produce three artifact The recipe SHALL state that scope is declared in the rule's own config, that no shared file is edited, and that the project-wide config is assembled rather than authored. -It SHALL direct the agent to check its work by running `verify` and then `test` against the rule's directory path. +It SHALL direct the agent to check its work by running `validate` and then `verify` against the rule's directory path. The previous version of this requirement taught the agent to add a matcher to a single project-wide `.vale.ini`. Executing that recipe against sandboxed agents found every one of its silent failures in that step and nowhere else — an assignment above the first matcher, a glob that missed the fixture extension, three names that had to agree with nothing reporting when they didn't. The layout change removes the step rather than documenting it further. -The recipe SHALL additionally carry the guidance below. Each item is a failure observed while authoring rules against this repository, and each produced a rule that passed `verify` and `test` while reporting nothing: +The recipe SHALL additionally carry the guidance below. Each item is a failure observed while authoring rules against this repository, and each produced a rule that passed `validate` and `verify` while reporting nothing: - **What each `scope` reaches**, as measured: `text` sees prose, `code` sees inline code spans, `[code, text]` sees both, and `raw` sees prose, inline code, and fenced blocks. `raw` subsumes the other two. - **That `scope` is per-rule.** Vale assembles one config per run, which invites the assumption that scopes interact. They do not. @@ -61,13 +61,13 @@ The recipe SHALL additionally carry the guidance below. Each item is a failure o #### Scenario: The recipe names the commands that check the work - **WHEN** the agent has written the three artifacts -- **THEN** the recipe SHALL direct it to run `verify` and `test` against the rule's path +- **THEN** the recipe SHALL direct it to run `validate` and `verify` against the rule's path #### Scenario: An unscoped rule is not silently accepted - **WHEN** the agent writes a style file without a matcher enabling it in the rule's own config - **THEN** the recipe SHALL identify this as incomplete -- **AND** `verify` SHALL report it +- **AND** `validate` SHALL report it #### Scenario: A rule about a command reaches fenced blocks @@ -147,7 +147,7 @@ The line SHALL orient, not classify: it states this recipe's own scope and SHALL An authoring recipe SHALL document the ways a rule of its engine can be well formed, enabled, green on its fixtures, and still report nothing. -A malformed rule is caught by `verify` and needs no recipe. A rule that is merely _wrong_ is caught by nothing, ships green, and is discovered only when someone notices it has never fired — so the recipe is the only place that failure can be prevented. +A malformed rule is caught by `validate` and needs no recipe. A rule that is merely _wrong_ is caught by nothing, ships green, and is discovered only when someone notices it has never fired — so the recipe is the only place that failure can be prevented. #### Scenario: The recipe names the silent failures for its engine @@ -157,7 +157,7 @@ A malformed rule is caught by `verify` and needs no recipe. A rule that is merel #### Scenario: Passing fixtures are not presented as proof of reach -- **WHEN** the recipe directs the agent to run `test` +- **WHEN** the recipe directs the agent to run `verify` - **THEN** it SHALL state that fixtures run under an isolating config - **AND** that a clean `check` SHALL be confirmed against a real file before the rule is believed to be working diff --git a/openspec/specs/cli-conformance-reference/spec.md b/openspec/specs/cli-conformance-reference/spec.md index 213115b3..a4025a34 100644 --- a/openspec/specs/cli-conformance-reference/spec.md +++ b/openspec/specs/cli-conformance-reference/spec.md @@ -10,7 +10,7 @@ The conformance corpus published as `@taskless/cli/reference.json`, so an extern The CLI SHALL publish `assets/reference.json` as the package export `@taskless/cli/reference.json`, carrying a `version`, the `protocol` the corpus -exists to support, the `constraints` `verify` and `test` enforce beyond an +exists to support, the `constraints` `validate` and `verify` enforce beyond an engine's own schema, and one entry per shipped demonstration rule. The corpus SHALL be built from the shipped rules rather than restating them, so @@ -110,7 +110,7 @@ CLI, which is the failure this corpus exists to remove. - **THEN** the published layout SHALL agree with the CLI's own engine layout table - **AND** each entry's resolved directory SHALL be the directory the CLI resolves for that engine and id -### Requirement: The corpus publishes what verify enforces beyond the engine +### Requirement: The corpus publishes what validate and verify enforce beyond the engine The corpus SHALL carry every entry in `RULE_CONSTRAINTS`, each with a stable `id`, the `engine` it applies to, which command enforces it, a summary, and the @@ -123,4 +123,10 @@ disagreement about the subject the rule addresses. #### Scenario: A constraint says which command enforces it - **WHEN** a consumer plans the order of an evaluation -- **THEN** each constraint SHALL state whether `verify` or `test` refuses the rule +- **THEN** each constraint SHALL state whether `validate` or `verify` refuses the rule + +#### Scenario: Renaming the enforcing commands bumps the corpus version + +- **WHEN** the commands an `enforcedBy` value names are renamed, so an existing value would mean a different command +- **THEN** the corpus `version` SHALL be incremented +- **AND** a consumer reading the new version SHALL find only the current command names among the `enforcedBy` values diff --git a/openspec/specs/cli-rule-format/spec.md b/openspec/specs/cli-rule-format/spec.md index 7ef45c87..552c09f3 100644 --- a/openspec/specs/cli-rule-format/spec.md +++ b/openspec/specs/cli-rule-format/spec.md @@ -144,13 +144,13 @@ The name is deliberate. "Matcher" denotes a Vale `[]` config section elsew - **THEN** it reads its capture rules from `captures/` - **AND** it reads `check.ts` from the rule directory root -### Requirement: A rule's canonical location is what verify and test address +### Requirement: A rule's canonical location is what validate and verify address -Each engine SHALL have one canonical on-disk location per rule — the rule directory — and that location SHALL be what `verify` and `test` accept as a path. A directory above it SHALL mean every rule beneath. +Each engine SHALL have one canonical on-disk location per rule — the rule directory — and that location SHALL be what `validate` and `verify` accept as a path. A directory above it SHALL mean every rule beneath. #### Scenario: One address per rule -- **WHEN** `verify` or `test` is given `.taskless/rules///` +- **WHEN** `validate` or `verify` is given `.taskless/rules///` - **THEN** it operates on exactly that rule - **AND** the engine is determined from the path without reading the rule diff --git a/openspec/specs/cli-rule-routing/spec.md b/openspec/specs/cli-rule-routing/spec.md index e64b6ecb..7ab219b8 100644 --- a/openspec/specs/cli-rule-routing/spec.md +++ b/openspec/specs/cli-rule-routing/spec.md @@ -178,7 +178,7 @@ The CLI SHALL provide a `create-sg-rule` help recipe that instructs the agent to local ast-grep rule on-device, without calling the Taskless service, and to verify it against the user's success and failure cases before reporting success. The recipe SHALL produce the canonical on-disk rule shape and paths used by remote -generation so that `check`, `improve`, and `verify` see a single dialect. +generation so that `check`, `improve`, and `validate` see a single dialect. The recipe SHALL be named for the artifact it produces rather than for a trust tier. "Static" describes when a rule runs, which is a different axis from which engine enforces it, and naming the ast-grep authoring path after the tier taught the conflation that engine selection exists to correct. diff --git a/openspec/specs/cli-rule-validation/spec.md b/openspec/specs/cli-rule-validation/spec.md index 5b46fcd1..08843827 100644 --- a/openspec/specs/cli-rule-validation/spec.md +++ b/openspec/specs/cli-rule-validation/spec.md @@ -6,17 +6,17 @@ How `validate` and `verify` judge a rule: addressed by path rather than by id, r ## Requirements -### Requirement: Rules are validated and tested by path, not by id +### Requirement: Rules are validated and verified by path, not by id -The CLI SHALL provide `verify ` and `test `. Both SHALL accept a path to a rule's canonical location or to any directory above it, and SHALL resolve the owning engine from the path's position under `.taskless/rules//` rather than by parsing the file. +The CLI SHALL provide `validate ` and `verify `. Both SHALL accept a path to a rule's canonical location or to any directory above it, and SHALL resolve the owning engine from the path's position under `.taskless/rules//` rather than by parsing the file. An id does not name one thing. The same id can exist under `sg` and under `vale`, so an id-addressed command has to either guess or report an ambiguity; a path has neither problem. Resolving the engine from position — never from content — is the same rule dispatch follows, so a rule cannot be validated by one engine and executed by another. -Addressing a rule by path is what removes the ambiguity from the COMMAND. It does not make the project's layout correct: the two rules still share one metadata sidecar, and `verify` reports that as a failure of each rule. The two are separate answers to separate questions, and neither replaces the other. +Addressing a rule by path is what removes the ambiguity from the COMMAND. It does not make the project's layout correct: the two rules still share one metadata sidecar, and `validate` reports that as a failure of each rule. The two are separate answers to separate questions, and neither replaces the other. #### Scenario: A rule path resolves to its engine -- **WHEN** `verify .taskless/rules/vale/no-simply` is run +- **WHEN** `validate .taskless/rules/vale/no-simply` is run - **THEN** the CLI SHALL validate it as a Vale rule #### Scenario: The same id under two engines is not ambiguous @@ -24,11 +24,11 @@ Addressing a rule by path is what removes the ambiguity from the COMMAND. It doe - **WHEN** `no-simply` exists under both `rules/sg/` and `rules/vale/` - **THEN** each is addressed by its own path - **AND** neither command SHALL require the user to disambiguate -- **AND** each rule SHALL still be reported as failing verification, because the id is held by two engines +- **AND** each rule SHALL still be reported as failing validation, because the id is held by two engines #### Scenario: A directory means everything beneath it -- **WHEN** `verify .taskless/` is run +- **WHEN** `validate .taskless/` is run - **THEN** every rule beneath it SHALL be validated, each against its own engine - **AND** the command SHALL report per-rule results rather than a single pass or fail @@ -37,198 +37,21 @@ Addressing a rule by path is what removes the ambiguity from the COMMAND. It doe - **WHEN** a path resolves to no engine - **THEN** the CLI SHALL exit non-zero naming the path, rather than guessing an engine -### Requirement: Verify checks a rule's required components +### Requirement: The generation loop runs validate and verify -`verify` SHALL check that a rule has the components its engine requires and that they are well formed, and SHALL NOT require fixtures or test cases to exist. - -The two commands split because they have different preconditions. An agent part-way through authoring has a rule and no fixtures yet, and needs to know the rule itself is valid before it can write a meaningful test for it. - -Per engine, `verify` SHALL check: - -| Engine | Components | -| --------- | ------------------------------------------------------------------------------------------------------------------------------ | -| `sg` | `.yml` against the ast-grep schema and the Taskless required fields | -| `vale` | `.yml` against the Vale rule schema and the Taskless required fields, and the rule's `.vale.ini` against the config schema | -| `runtime` | `check.ts` present, and at least one capture rule under `captures/` | - -The `vale` row previously read "against Vale's own validation." Measured against the pinned 3.18.0 binary, that covers less than it claims: `level: bananas` is reported, while `extends: nonsense` and `scope: fenced` both verify clean and produce a rule that matches nothing. Vale validates a rule when it _runs_ one, and it runs one field at a time — so a name it does not recognize is not an error, it is a check that never fires. Schema validation is therefore its own layer for `vale`, as it already is for `sg`. - -#### Scenario: A rule with no fixtures still verifies - -- **WHEN** `verify` runs against a rule whose fixture buckets are empty or absent -- **THEN** it SHALL report on the rule's components only -- **AND** the absence of fixtures SHALL NOT be a verify failure - -#### Scenario: A malformed rule reports its own error - -- **WHEN** a Vale style declares a `level` outside `suggestion`/`warning`/`error` -- **THEN** `verify` SHALL report that error, naming the field - -#### Scenario: An unrecognized extension point is rejected - -- **WHEN** a Vale style declares an `extends` that is not one of Vale's check types -- **THEN** `verify` SHALL report it, naming the field and the accepted values -- **AND** it SHALL NOT report the rule as valid - -#### Scenario: An unrecognized scope is rejected - -- **WHEN** a Vale style declares a `scope` that is not one of Vale's scope values -- **THEN** `verify` SHALL report it, naming the field -- **AND** a scope using the `~` negation or `&` chaining syntax over recognized values SHALL be accepted - -#### Scenario: A field belonging to another check type is rejected - -- **WHEN** a Vale style declares a field its `extends` does not accept, such as `tokens` on an `occurrence` check -- **THEN** `verify` SHALL report it before Vale is invoked - -The failure it prevents is not a local one: Vale reports this as `E201: has invalid keys` and reads one assembled config per run, so a single rule with a stray field suppresses every other Vale rule's findings. - -#### Scenario: A rule config that never enables the rule is rejected - -- **WHEN** a Vale rule's `.vale.ini` declares matchers but no `. = YES` -- **THEN** `verify` SHALL report that the rule is present but off, naming the file - -#### Scenario: A rule config that carries BasedOnStyles is rejected - -- **WHEN** a Vale rule's `.vale.ini` assigns `BasedOnStyles` in any matcher, with any value -- **THEN** `verify` SHALL report it under `vale-config-no-based-on-styles`, naming the line -- **AND** it SHALL NOT report the rule as valid - -#### Scenario: A rule config that assigns a foreign key is rejected - -- **WHEN** a Vale rule's `.vale.ini` assigns a `