From 83b0c92ba1c43a3e188f2b1b2f67272eb5beebea Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 23 Sep 2026 14:47:22 -0700 Subject: [PATCH 1/3] feat(test): report the findings an ast-grep rule's fixtures produced `test --json` carried real `findings` for Vale and runtime rules and an empty array for ast-grep. That was truthful and it left the defect #386 exists for unfixed: a rule whose `message` interpolates its metavariables can name them in the wrong order, fire on every `invalid:` snippet, stay quiet on every `valid:` one, and be reported green. `ast-grep test` never renders the message, so nothing could see it. Each fixture snippet is replayed through `ast-grep scan -r --stdin --json=stream`. Three alternatives were measured and rejected: - `sg test` cannot carry findings at all. On the vendored 0.45.3 binary `test --help` offers no `--json` and no output-format flag. - `check` over the rule's `.tests` directory returns no results, because the fixtures are inline YAML scalars rather than documents, so the workaround that exists for Vale does not transfer. - Materialising each snippet as a file needs a `language:` to extension mapping the CLI does not own. `language:` takes ast-grep's own spelling and the set belongs to the binary, so a drifting map would silently scan a snippet as the wrong language. `--stdin` takes the language from the rule's own `language:` key, so there is no mapping to keep in step and no temp file to clean up. Two properties were measured rather than assumed: `files:` globs do not suppress a stdin scan, which a path-scoped rule would otherwise have been silently invisible to; and `-r` isolates a rule ast-grep cannot parse to that rule alone, rather than aborting a whole config. A finding names the test YAML that declares the snippet, at the snippet's real line and column there, so the author can open it. Findings are gathered after `sg test` has decided the verdict and cannot change it. --- .changeset/sg-fixture-findings.md | 20 ++ packages/cli/src/rules/inspect.ts | 16 +- packages/cli/src/rules/sg-fixture-findings.ts | 325 ++++++++++++++++++ .../cli/test/test-fixture-findings.test.ts | 276 ++++++++++++++- 4 files changed, 618 insertions(+), 19 deletions(-) create mode 100644 .changeset/sg-fixture-findings.md create mode 100644 packages/cli/src/rules/sg-fixture-findings.ts diff --git a/.changeset/sg-fixture-findings.md b/.changeset/sg-fixture-findings.md new file mode 100644 index 00000000..154574ba --- /dev/null +++ b/.changeset/sg-fixture-findings.md @@ -0,0 +1,20 @@ +--- +"@taskless/cli": patch +--- + +`test` now reports the findings an ast-grep rule's fixtures produced, with the +message as ast-grep rendered it. Previously only Vale and runtime rules carried +`findings`; an ast-grep rule reported an empty array, so a rule whose message +interpolated its metavariables in the wrong order fired in exactly the right +places and was reported green. + +Each snippet is replayed through `ast-grep scan --stdin`, so the language comes +from the rule's own `language:` key and no temporary file is written. A finding +names the test YAML that declares the snippet, at the snippet's real line and +column in that file. + +Additive, and `patch` under the pre-1.0 rule: the `findings` array was already +present on every rule result and documented as possibly empty, so nothing a +consumer reads changes shape. Vale and runtime behaviour is untouched, and the +verdict `ast-grep test` decides is unchanged — findings are gathered after it +and cannot alter it. diff --git a/packages/cli/src/rules/inspect.ts b/packages/cli/src/rules/inspect.ts index cdd9e4cd..cf779d52 100644 --- a/packages/cli/src/rules/inspect.ts +++ b/packages/cli/src/rules/inspect.ts @@ -9,6 +9,7 @@ import { ruleFilePath, } from "./engines"; import { describeCoverageShortfall, type FixtureFinding } from "./fixtures"; +import { collectSgFixtureFindings } from "./sg-fixture-findings"; import { type EngineName } from "./layout"; import { assessCaptureDirectory, @@ -498,14 +499,13 @@ export async function testOneRule( violations, ran: true, notices: verification.notices, - // Empty, and true. `sg test` reports `test result: ok. N passed; N - // failed;` and nothing else — the vendored binary has no `--json` and no - // output-format flag — and its fixtures are inline YAML scalars rather - // than files, so there is no document to attribute a finding to. - // Surfacing them needs a different mechanism than reading what the - // engine already handed us, which is the whole of what this does for the - // other two engines. - findings: [], + // `sg test` still decides the verdict above and still cannot produce a + // finding — the vendored binary has no `--json` and no output-format + // flag. So unlike the other two engines, these are not read back out of + // what the run already handed us: each fixture snippet is replayed + // through `scan --stdin`, which renders the message the author needs to + // see. Gathered AFTER the verdict, and never able to change it. + findings: await collectSgFixtureFindings(cwd, ruleId), }; } diff --git a/packages/cli/src/rules/sg-fixture-findings.ts b/packages/cli/src/rules/sg-fixture-findings.ts new file mode 100644 index 00000000..6aaebbb0 --- /dev/null +++ b/packages/cli/src/rules/sg-fixture-findings.ts @@ -0,0 +1,325 @@ +import { spawn } from "node:child_process"; +import { readFile } from "node:fs/promises"; +import process from "node:process"; +import { createInterface } from "node:readline"; +import { join, posix, relative, sep } from "node:path"; + +import { parseDocument } from "yaml"; +import type { Scalar, YAMLSeq } from "yaml"; + +import { ruleFilePath, ruleTestsDirectory } from "./engines"; +import { + bucketEntries, + type FixtureBucket, + type FixtureFinding, +} from "./fixtures"; +import { buildPath, findSgBinary } from "./scan"; +import { toCheckResult, type AstGrepMatch } from "../types/check"; + +/** + * The findings an ast-grep rule's own fixtures produce, with their rendered + * messages. + * + * `test` could previously only say whether the `invalid:` cases fired and the + * `valid:` ones stayed quiet. A rule whose `message` interpolates its + * metavariables can have the slots in the wrong order, fire in exactly the + * right places, and be reported as a rule that passed — the rendered message is + * the only evidence otherwise, and it was being thrown away. + * + * ## Why this does not go through `sg test` + * + * `sg test` is what decides the verdict, and it cannot produce a finding. + * Measured against the vendored 0.45.3 binary, `ast-grep test --help` offers + * `--filter`, `--skip-snapshot-tests`, `--update-all`, `--interactive`, + * `--include-off`, `--color` and nothing else: there is no `--json` and no + * output-format flag of any kind. Its only machine-readable output is the + * `test result: ok. N passed; N failed;` line `parseTestSummary` already reads. + * + * ## Why this does not write temp files + * + * The obvious route was to materialise each snippet as a file and scan it, + * which needs a `language:` → file-extension mapping the CLI does not own and + * could not source reliably — `language:` takes ast-grep's own spelling + * (`Yaml`, not `yaml`), the set is the binary's rather than ours, and a mapping + * that drifted would silently scan a snippet as the wrong language. + * + * `scan --stdin` removes the whole question. Measured on 0.45.3: + * + * ``` + * printf 'console.log("hi");' | ast-grep scan -r rule.yml --stdin --json=stream + * {"text":"console.log(\"hi\")",…,"file":"STDIN","language":"TypeScript", + * "message":"avoid log on console in ",…} + * ``` + * + * The language comes from the rule's own `language:` key, parsed by ast-grep + * itself, so there is no mapping to keep in step and no extension to guess. No + * file is written, so nothing can be left behind in the project tree, nothing + * needs cleaning up on a failure, and no later `check` can pick up a stray. + * + * Two further properties were measured rather than assumed: + * + * - **`files:` globs do not suppress a stdin scan.** `ci-uses-workspace-cli` + * restricts itself to `.github/workflows/*.yml`, and the rule fires on the + * same snippet identically with and without that key — `files:` filters the + * file WALK, which stdin bypasses. A rule scoped to paths would otherwise + * have reported no findings at all, since the stdin document is named + * `STDIN`. + * - **`-r` isolates a malformed rule.** A `language:` ast-grep does not + * recognise makes it exit 8 with `Fail to parse yaml as RuleConfig` and emit + * no JSON. Because `-r` loads exactly one rule file rather than the + * assembled config, that failure cannot take down any other rule's report — + * which is precisely the silent, config-wide abort the assembled-config path + * is vulnerable to. Here it degrades to no findings for the one rule. + */ + +/** + * ast-grep's exit code for a rule file it could not parse. + * + * Distinguished from `0`/`1` (clean scan / error-severity matches found) so an + * unparseable rule degrades to no findings rather than being mistaken for a + * rule that simply matched nothing. + */ +const SG_RULE_PARSE_FAILURE = 8; + +/** Compare and report paths in the shape the other engines report them. */ +function toRelativePosix(cwd: string, absolute: string): string { + return relative(cwd, absolute).split(sep).join(posix.sep); +} + +/** + * Where a fixture snippet's text begins in the file that declares it, and how + * far it is indented there. + * + * A snippet reaches ast-grep dedented — `yaml` strips the block scalar's + * indentation — so a finding's position is relative to the snippet, not to the + * document the author has open. These two numbers are what turn one into the + * other. + */ +interface SnippetAnchor { + /** Zero-based line in the test file where the snippet's first line sits. */ + line: number; + /** Columns of block-scalar indentation stripped from every snippet line. */ + indent: number; + /** Whether a per-line mapping is sound at all; see {@link anchorOf}. */ + perLine: boolean; +} + +/** + * Locate a fixture snippet inside the YAML that declares it. + * + * Only a LITERAL block scalar (`|`) gets a per-line mapping. It is the spelling + * ast-grep's own test files use and the only one where snippet line N is file + * line `start + N`: a FOLDED scalar (`>`) joins lines, and a plain or quoted + * scalar can carry escapes, so in both the snippet's line numbering is not the + * file's. Rather than report a confidently wrong line, those anchor to the + * snippet's first line with no column offset — still somewhere the author can + * open, and honest about the precision available. + */ +function anchorOf(source: string, item: Scalar): SnippetAnchor { + const start = item.range?.[0] ?? 0; + const lineOf = (offset: number): number => + source.slice(0, offset).split("\n").length - 1; + + if (item.type !== "BLOCK_LITERAL") { + return { line: lineOf(start), indent: 0, perLine: false }; + } + + // A block scalar's content starts on the line after its `|` header, and every + // content line carries the same indentation, which `yaml` has already + // stripped from the value handed to ast-grep. + const newline = source.indexOf("\n", start); + if (newline === -1) return { line: lineOf(start), indent: 0, perLine: false }; + const contentStart = newline + 1; + const nextNewline = source.indexOf("\n", contentStart); + const firstLine = source.slice( + contentStart, + nextNewline === -1 ? undefined : nextNewline + ); + return { + line: lineOf(contentStart), + indent: firstLine.length - firstLine.trimStart().length, + perLine: true, + }; +} + +/** Move a finding's position from snippet coordinates into file coordinates. */ +function reanchor( + range: AstGrepMatch["range"], + anchor: SnippetAnchor +): { + start: { line: number; column: number }; + end: { line: number; column: number }; +} { + if (!anchor.perLine) { + return { + start: { line: anchor.line, column: 0 }, + end: { line: anchor.line, column: 0 }, + }; + } + return { + start: { + line: anchor.line + range.start.line, + column: range.start.column + anchor.indent, + }, + end: { + line: anchor.line + range.end.line, + column: range.end.column + anchor.indent, + }, + }; +} + +/** + * Scan one snippet with one rule, over stdin. + * + * Resolves to the matches ast-grep reported, or to an empty list when it could + * not run the rule at all. A rule it cannot parse is not an exception here: the + * caller's job is to report findings, and a rule that produces none — because + * its `language:` is one ast-grep does not know — is reported as a rule with no + * findings, exactly as the schema's "present and empty" contract requires. + */ +async function scanSnippet( + cwd: string, + ruleFile: string, + snippet: string +): Promise { + const sgBinary = findSgBinary(); + return new Promise((resolve) => { + const child = spawn( + sgBinary, + ["scan", "-r", ruleFile, "--stdin", "--json=stream"], + { + cwd, + stdio: ["pipe", "pipe", "pipe"], + env: { ...process.env, PATH: buildPath() }, + } + ); + + const matches: AstGrepMatch[] = []; + // `node:readline` over stdout, which handles character boundaries itself — + // the same treatment `runAstGrepScan` gives the identical stream. + const rl = createInterface({ input: child.stdout }); + rl.on("line", (line) => { + const trimmed = line.trim(); + if (trimmed === "") return; + try { + matches.push(JSON.parse(trimmed) as AstGrepMatch); + } catch { + // Non-JSON status lines are not findings. + } + }); + + // Drained rather than ignored: ast-grep writes its rule-parse diagnostics + // here, and a full pipe would block the child instead of letting it exit. + child.stderr.resume(); + + child.on("error", () => { + resolve([]); + }); + child.on("close", (code) => { + resolve(code === SG_RULE_PARSE_FAILURE ? [] : matches); + }); + + child.stdin.on("error", () => { + // A rule ast-grep refuses closes stdin before the snippet is written. + // That is the parse failure above, reported by exit code, not a crash. + }); + child.stdin.end(snippet); + }); +} + +/** The bucket keys of an ast-grep test document, in the vocabulary it uses. */ +const BUCKETS: { key: "valid" | "invalid"; bucket: FixtureBucket }[] = [ + { key: "valid", bucket: "pass" }, + { key: "invalid", bucket: "fail" }, +]; + +/** + * Every finding this rule's fixtures produced, tagged with its bucket. + * + * `file` is the TEST FILE that declares the snippet, cwd-relative and POSIX — + * the shape the other engines report, and the one place the author can + * actually go and edit. The temp path a materialising implementation would + * have reported, and ast-grep's own `STDIN`, are both useless to them. + * + * Returns an empty list rather than throwing for every way this can come up + * short — no rule file, no test directory, a rule ast-grep cannot parse. The + * findings are evidence ABOUT a verdict that has already been decided + * elsewhere by `sg test`; failing to gather them must not change it. + */ +export async function collectSgFixtureFindings( + cwd: string, + ruleId: string +): Promise { + const ruleFile = ruleFilePath(cwd, "sg", ruleId); + const testsDirectory = ruleTestsDirectory(cwd, "sg", ruleId); + const entries = await bucketEntries(testsDirectory); + const testFiles = entries + .filter( + (entry) => + entry.isFile() && + (entry.name.endsWith(".yml") || entry.name.endsWith(".yaml")) + ) + .map((entry) => join(testsDirectory, entry.name)) + .toSorted(); + + const findings: FixtureFinding[] = []; + + for (const testFile of testFiles) { + let source: string; + try { + source = await readFile(testFile, "utf8"); + } catch { + continue; + } + + let document; + try { + document = parseDocument(source); + } catch { + // A malformed test file is `verify`'s finding to report, not this one's. + continue; + } + + // The same exclusion `countFixtures` applies: a file carrying another + // rule's `id:` is not this rule's fixture set, and ast-grep would not run + // it under this rule either. + if (document.get("id") !== ruleId) continue; + + const reportedFile = toRelativePosix(cwd, testFile); + + for (const { key, bucket } of BUCKETS) { + const sequence = document.get(key) as YAMLSeq | undefined; + if (sequence === undefined || !Array.isArray(sequence.items)) continue; + + for (const item of sequence.items as Scalar[]) { + if (typeof item?.value !== "string") continue; + const snippet = item.value; + const anchor = anchorOf(source, item); + const matches = await scanSnippet(cwd, ruleFile, snippet); + + for (const match of matches) { + const result = toCheckResult(match); + findings.push({ + ...result, + // ast-grep writes `"note": null` for a rule without one, and + // `toCheckResult` passes it through — harmless for `check`, which + // does not validate, but the `test` payload is parsed by a schema + // where `note` is an optional STRING. Normalised to absent here + // rather than in `toCheckResult`, which would change the shape + // `check --json` has been emitting. + note: result.note ?? undefined, + // ast-grep reports `STDIN`; the author needs the file that declares + // the snippet. Overridden unconditionally rather than only when it + // reads `STDIN`, so a future ast-grep that named the stream + // differently could not leak an unopenable path. + file: reportedFile, + range: reanchor(match.range, anchor), + bucket, + }); + } + } + } + } + + return findings; +} diff --git a/packages/cli/test/test-fixture-findings.test.ts b/packages/cli/test/test-fixture-findings.test.ts index c1f8d384..44b50cce 100644 --- a/packages/cli/test/test-fixture-findings.test.ts +++ b/packages/cli/test/test-fixture-findings.test.ts @@ -1,5 +1,5 @@ import { execFile } from "node:child_process"; -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readdir, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { promisify } from "node:util"; @@ -33,9 +33,11 @@ const withVale = findValeBinary().path === undefined ? describe.skip : describe; let cwd: string; -async function runCli(args: string[]) { +async function runCli(args: string[], env?: NodeJS.ProcessEnv) { try { - const { stdout, stderr } = await execFileAsync("node", [binPath, ...args]); + const { stdout, stderr } = await execFileAsync("node", [binPath, ...args], { + env: env === undefined ? process.env : { ...process.env, ...env }, + }); return { stdout, stderr, exitCode: 0 }; } catch (error) { return cliRejectionToResult(error, [binPath, ...args]); @@ -389,6 +391,246 @@ describe("a runtime rule's fixture findings", () => { }); }); +/* -------------------------------------------------------------------------- */ +/* ast-grep */ +/* -------------------------------------------------------------------------- */ + +const SG_RULE = "swap-args"; + +/** + * A rule whose message interpolates TWO metavariables, chosen for the same + * reason Vale's `substitution` was. + * + * `swap($FIRST, $SECOND)` with a message naming them in the other order is the + * ast-grep spelling of the defect this file exists for: the rule fires on every + * `invalid:` snippet, stays quiet on every `valid:` one, and `sg test` reports + * it green whichever order the message names them in. Only the rendered string + * tells the two apart. + */ +const SG_RULE_YAML = [ + `id: ${SG_RULE}`, + "language: TypeScript", + "severity: warning", + "message: replace $SECOND with $FIRST", + "rule:", + " pattern: swap($FIRST, $SECOND)", + "", +].join("\n"); + +/** What the rule above renders for `swap(alpha, beta)`. */ +const SG_RENDERED = "replace beta with alpha"; + +/** The message a rule with the slots the other way round would render. */ +const SG_REVERSED = "replace alpha with beta"; + +/** + * Literal block scalars (`|`), which is what ast-grep's own test files use and + * the spelling a per-line position mapping is sound for. The exact line and + * column asserted below are read off this layout, so it is load-bearing: + * + * ``` + * 1 id: swap-args + * 2 valid: + * 3 - | + * 4 + * 5 invalid: + * 6 - | + * 7 swap(alpha, beta); + * ``` + */ +function sgTestYaml(validSnippet: string): string { + return [ + `id: ${SG_RULE}`, + "valid:", + " - |", + ` ${validSnippet}`, + "invalid:", + " - |", + " swap(alpha, beta);", + "", + ].join("\n"); +} + +async function writeSwapRule(options: { validFires?: boolean } = {}) { + const directory = join(cwd, ".taskless", "rules", "sg", SG_RULE); + await mkdir(join(directory, ".tests"), { recursive: true }); + await writeFile(join(directory, `${SG_RULE}.yml`), SG_RULE_YAML); + await writeFile( + join(directory, ".tests", `${SG_RULE}-test.yml`), + sgTestYaml( + (options.validFires ?? false) ? "swap(one, two);" : "const fine = 1;" + ) + ); +} + +/** The fixture file a finding must point back at, cwd-relative and POSIX. */ +const SG_TEST_FILE = `.taskless/rules/sg/${SG_RULE}/.tests/${SG_RULE}-test.yml`; + +async function testSg(...extra: string[]) { + return runCli(["test", `.taskless/rules/sg/${SG_RULE}`, "-d", cwd, ...extra]); +} + +/** As {@link testSg}, but with the CLI's whole temp directory redirected. */ +async function testSgWithTemporary(temporary: string, ...extra: string[]) { + return runCli( + ["test", `.taskless/rules/sg/${SG_RULE}`, "-d", cwd, ...extra], + { + TMPDIR: temporary, + } + ); +} + +describe("an ast-grep rule's fixture findings", () => { + it("reports the rendered message, so swapped metavariables cannot pass", async () => { + // THE test, and the ast-grep half of what #386 asked for. Both orderings + // fire in exactly the same places and both are reported green by + // `sg test`; this string is the only thing that distinguishes them. + await writeSwapRule(); + + const { stdout, exitCode } = await testSg("--json"); + const report = JSON.parse(stdout) as Report; + + expect(exitCode).toBe(0); + expect(report.ok).toBe(true); + const messages = findingsOf(report).map((finding) => finding.message); + expect(messages).toEqual([SG_RENDERED]); + expect(messages).not.toContain(SG_REVERSED); + }); + + it("carries the whole check finding, pointing back into the fixture file", async () => { + // `file` is the test YAML, not ast-grep's `STDIN` and not a temp path. + // Both of those are unopenable, and the author's next move after reading a + // wrong message is to go and edit the snippet. + await writeSwapRule(); + + const { stdout } = await testSg("--json"); + const report = JSON.parse(stdout) as Report; + const finding = findingsOf(report)[0]; + + expect(finding).toBeDefined(); + expect(finding?.source).toBe("ast-grep"); + expect(finding?.ruleId).toBe(SG_RULE); + expect(finding?.severity).toBe("warning"); + expect(finding?.matchedText).toBe("swap(alpha, beta)"); + expect(finding?.file).toBe(SG_TEST_FILE); + // Line 7 of the layout above, zero-based, and column 4 for the block + // scalar's indentation — the real coordinates of `swap(alpha, beta);` in + // the fixture file, not the snippet-relative 0:0 ast-grep reports. + expect(finding?.range.start.line).toBe(6); + expect(finding?.range.start.column).toBe(4); + }); + + it("reports the invalid bucket as `fail` on a run that passed", async () => { + await writeSwapRule(); + + const { stdout } = await testSg("--json"); + const report = JSON.parse(stdout) as Report; + + expect(report.rules[0]?.ok).toBe(true); + expect(findingsOf(report, "fail")).toHaveLength(1); + expect(findingsOf(report, "pass")).toHaveLength(0); + }); + + it("tags a wrongly-fired valid snippet as the pass bucket", async () => { + await writeSwapRule({ validFires: true }); + + const { stdout, exitCode } = await testSg("--json"); + const report = JSON.parse(stdout) as Report; + + expect(exitCode).toBe(1); + expect(report.ok).toBe(false); + const passFindings = findingsOf(report, "pass"); + expect(passFindings).toHaveLength(1); + expect(passFindings[0]?.file).toBe(SG_TEST_FILE); + expect(passFindings[0]?.message).toBe("replace two with one"); + // Line 4 of the layout: the `valid:` snippet, three lines above the + // `invalid:` one, so the two buckets cannot be reporting the same position. + expect(passFindings[0]?.range.start.line).toBe(3); + }); + + it("prints one line and no findings when the rule passed", async () => { + await writeSwapRule(); + + const { stdout } = await testSg(); + + expect(stdout).toContain(`✓ sg/${SG_RULE}`); + expect(stdout).not.toContain(SG_RENDERED); + expect(stdout).not.toContain("fixture findings"); + }); + + it("prints what matched under a failing rule, through check's renderer", async () => { + await writeSwapRule({ validFires: true }); + + const { stdout } = await testSg(); + + expect(stdout).toContain(`✗ sg/${SG_RULE}`); + expect(stdout).toContain("pass fixture findings:"); + expect(stdout).toContain("replace two with one"); + // `check`'s own renderer, so a finding does not read two ways depending on + // which command surfaced it — and the fixture file with a line a reader can + // go to. + expect(stdout).toContain(`warning[${SG_RULE}] replace two with one`); + expect(stdout).toContain(`${SG_TEST_FILE}:4:5`); + }); + + it("degrades to no findings when ast-grep cannot parse the rule's language", async () => { + // A `language:` ast-grep does not recognise makes it refuse the rule file + // outright. Collecting findings must not turn that into a crash, and + // because the collector loads this ONE rule with `-r`, it cannot take any + // other rule's report down with it either. + const directory = join(cwd, ".taskless", "rules", "sg", SG_RULE); + await mkdir(join(directory, ".tests"), { recursive: true }); + await writeFile( + join(directory, `${SG_RULE}.yml`), + SG_RULE_YAML.replace("language: TypeScript", "language: Cobolesque") + ); + await writeFile( + join(directory, ".tests", `${SG_RULE}-test.yml`), + sgTestYaml("const fine = 1;") + ); + + const { stdout } = await testSg("--json"); + const report = JSON.parse(stdout) as Report; + + // Reported as a rule with no findings rather than an absent key, and the + // command still produced a parseable report at all. + expect(report.rules[0]?.findings).toEqual([]); + }); + + it("leaves no temp files behind, on a passing run or a failing one", async () => { + // The route deliberately writes nothing: each snippet is streamed to + // `ast-grep scan --stdin`. This pins that, since the alternative design + // (materialise each snippet as a file) is the one that could strand + // artifacts and be picked up by a later `check`. + // + // The CLI's whole temp directory is redirected to a private one rather than + // diffing the shared `tmpdir()`, which other suites are writing to + // concurrently — a diff there would be measuring the rest of the run. + const temporary = await mkdtemp(join(tmpdir(), "tskl-sg-tmp-")); + + await writeSwapRule(); + const passing = await testSgWithTemporary(temporary, "--json"); + expect(passing.exitCode).toBe(0); + expect(await readdir(temporary)).toEqual([]); + + await writeSwapRule({ validFires: true }); + const failing = await testSgWithTemporary(temporary, "--json"); + expect(failing.exitCode).toBe(1); + expect(await readdir(temporary)).toEqual([]); + + await rm(temporary, { recursive: true, force: true }); + + // And nothing stray inside the rule directory either — only the rule file + // and its `.tests/`. + const ruleDirectory = join(cwd, ".taskless", "rules", "sg", SG_RULE); + const ruleEntries = await readdir(ruleDirectory); + expect(ruleEntries.toSorted()).toEqual([".tests", `${SG_RULE}.yml`]); + expect(await readdir(join(ruleDirectory, ".tests"))).toEqual([ + `${SG_RULE}-test.yml`, + ]); + }); +}); + /* -------------------------------------------------------------------------- */ /* Always present */ /* -------------------------------------------------------------------------- */ @@ -408,13 +650,27 @@ async function writeSgRule() { } describe("the findings array is present on every rule result", () => { - it("is empty rather than absent for an ast-grep rule", async () => { - // `sg test` reports a count and nothing else, and its fixtures are inline - // YAML scalars rather than files. Empty is the true answer here, and it has - // to be stated rather than left to an absent key. - await writeSgRule(); + it("is empty rather than absent for an ast-grep rule that matched nothing", async () => { + // An `invalid:` snippet the rule does not match is a rule whose fixtures + // produced nothing. Empty is the true answer, and it has to be stated + // rather than left to an absent key. + // + // `sg test` fails the rule for it, which is the point: the verdict and the + // findings are decided separately, and an empty `findings` is not the same + // claim as a passing rule. + const directory = join(cwd, ".taskless", "rules", "sg", "no-eval-sg"); + await mkdir(join(directory, ".tests"), { recursive: true }); + await writeFile( + join(directory, "no-eval-sg.yml"), + "id: no-eval-sg\nlanguage: TypeScript\nseverity: error\n" + + "message: no eval\nrule:\n pattern: eval($ARG)\n" + ); + await writeFile( + join(directory, ".tests", "no-eval-sg-test.yml"), + "id: no-eval-sg\nvalid:\n - const a = 1;\ninvalid:\n - const b = 2;\n" + ); - const { stdout, exitCode } = await runCli([ + const { stdout } = await runCli([ "test", ".taskless/rules/sg/no-eval-sg", "-d", @@ -423,8 +679,6 @@ describe("the findings array is present on every rule result", () => { ]); const report = JSON.parse(stdout) as Report; - expect(exitCode).toBe(0); - expect(report.rules[0]?.ok).toBe(true); expect(report.rules[0]?.findings).toEqual([]); }); From 883dcea7c4958f9fc2b90a4660dd5aab7ff31251 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 23 Sep 2026 14:47:31 -0700 Subject: [PATCH 2/3] docs(openspec): archive sg-fixture-findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds "Test reports the findings an ast-grep rule's fixtures produced" to `cli-rule-validation` as an ADDED requirement, so nothing standing is restated and nothing can be dropped on archive. ADDED rather than MODIFIED deliberately. The standing scenario "The findings array is present and empty rather than absent" lists four WHENs, one of them "an engine that does not surface fixture findings". After this change no engine is in that state, so the disjunct is inert — but it is not false, and the other three are still live and still tested. Restating a ten-scenario requirement to delete one clause is the operation that silently drops scenarios, and the clause costs nothing standing. Archive dry-run compared requirement and scenario title sets before and after: 9 requirements and 48 scenarios before, 10 and 54 after, with nothing in the before-only set either time. --- .../.openspec.yaml | 2 + .../proposal.md | 64 +++++++++++++++++++ .../specs/cli-rule-validation/spec.md | 52 +++++++++++++++ .../2026-09-23-sg-fixture-findings/tasks.md | 61 ++++++++++++++++++ openspec/specs/cli-rule-validation/spec.md | 51 +++++++++++++++ 5 files changed, 230 insertions(+) create mode 100644 openspec/changes/archive/2026-09-23-sg-fixture-findings/.openspec.yaml create mode 100644 openspec/changes/archive/2026-09-23-sg-fixture-findings/proposal.md create mode 100644 openspec/changes/archive/2026-09-23-sg-fixture-findings/specs/cli-rule-validation/spec.md create mode 100644 openspec/changes/archive/2026-09-23-sg-fixture-findings/tasks.md diff --git a/openspec/changes/archive/2026-09-23-sg-fixture-findings/.openspec.yaml b/openspec/changes/archive/2026-09-23-sg-fixture-findings/.openspec.yaml new file mode 100644 index 00000000..265da3d9 --- /dev/null +++ b/openspec/changes/archive/2026-09-23-sg-fixture-findings/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-23 diff --git a/openspec/changes/archive/2026-09-23-sg-fixture-findings/proposal.md b/openspec/changes/archive/2026-09-23-sg-fixture-findings/proposal.md new file mode 100644 index 00000000..bc5568d5 --- /dev/null +++ b/openspec/changes/archive/2026-09-23-sg-fixture-findings/proposal.md @@ -0,0 +1,64 @@ +## Why + +`test` reports the findings a rule's fixtures produced for Vale and for +runtime, and reports an empty list for ast-grep. That was truthful and it was +the half of #386 left undone: an ast-grep rule whose `message` interpolates its +metavariables can have the slots in the wrong order, fire on every `invalid:` +snippet, stay quiet on every `valid:` one, and be reported green. The rendered +message is the only evidence otherwise, and `sg test` never renders it. + +Two things made this harder than the other two engines, and neither is true any +more: + +- **`sg test` cannot produce a finding.** Measured against the vendored 0.45.3 + binary: `ast-grep test --help` offers `--filter`, `--skip-snapshot-tests`, + `--update-all`, `--interactive`, `--include-off` and `--color`, and no + `--json` or output-format flag of any kind. Its only machine-readable output + is the summary line `parseTestSummary` already reads. +- **ast-grep fixtures are not files**, so the `check ` workaround + that exists for Vale does not exist here. Measured: + `pnpm cli check .taskless/rules/sg/ci-uses-workspace-cli/.tests --json` + returns `{"success":true,"results":[]}` — the snippets are inline YAML + scalars under `valid:`/`invalid:`, and there is no document to walk. + +The route that was expected to be needed — materialise each snippet as a file +and scan it — requires a `language:` → file-extension mapping the CLI does not +own and cannot source reliably. `language:` takes ast-grep's own spelling +(`Yaml`, not `yaml`; the repo's own `ci-uses-workspace-cli` uses the +capitalised form), the set belongs to the binary, and a mapping that drifted +would silently scan a snippet as the wrong language. + +`ast-grep scan --stdin` removes that question entirely. The language comes from +the rule's own `language:` key, parsed by ast-grep, so there is no mapping to +keep in step, no extension to guess, and no temp file to clean up. + +## What Changes + +- **`cli-rule-validation`** gains one requirement: the ast-grep engine reports + the findings its fixtures produced, with rendered messages and positions that + point back into the fixture file. +- `test --json` and the human render for an ast-grep rule now carry real + `findings`, in the shape Vale and runtime already produce. Nothing about + either of those engines changes, and the verdict `sg test` decides is + untouched — findings are gathered after it and cannot alter it. + +## Delivery shape + +**Single PR.** The spec delta, the collector, the tests and the archive are one +reviewable diff of roughly 500 lines, well inside the ~1200-line guidance, and +there is no intermediate state worth landing on its own. + +## Why ADDED rather than MODIFIED + +The standing scenario "The findings array is present and empty rather than +absent" lists four WHENs, one of which is "whose engine does not surface fixture +findings". After this change no engine is in that state, so the disjunct is +inert — but it is not false, and the other three (a rule that produced nothing, +verification that failed first, a refused run) are all still live and still +tested. + +Restating a ten-scenario requirement to delete one clause of one WHEN is the +operation that silently drops scenarios on archive, and the clause costs +nothing standing. The new requirement states positively that ast-grep surfaces +its findings, which is what a reader needs; the inert disjunct is left for a +change that has reason to touch that requirement for its own sake. diff --git a/openspec/changes/archive/2026-09-23-sg-fixture-findings/specs/cli-rule-validation/spec.md b/openspec/changes/archive/2026-09-23-sg-fixture-findings/specs/cli-rule-validation/spec.md new file mode 100644 index 00000000..05b7acec --- /dev/null +++ b/openspec/changes/archive/2026-09-23-sg-fixture-findings/specs/cli-rule-validation/spec.md @@ -0,0 +1,52 @@ +## ADDED Requirements + +### Requirement: Test reports the findings an ast-grep rule's fixtures produced + +`test` SHALL report, for an ast-grep rule whose fixtures produced matches, the findings those fixtures produced, in the same shape the other engines report theirs. A consumer SHALL NOT be able to tell which engine produced a finding except by its `source`. + +The rendered `message` is what this exists for. `ast-grep test` decides only whether each `invalid:` snippet fired and each `valid:` one stayed quiet, and a rule whose message interpolates its metavariables can have the slots in the wrong order while satisfying both — so the verdict cannot see the defect and the message is the only thing that can. + +The findings SHALL be gathered independently of the verdict and SHALL NOT change it. `ast-grep test` remains what decides whether the rule passed; a failure to gather findings SHALL be reported as a rule with no findings rather than as a rule that failed. + +A finding SHALL name the fixture FILE that declares the snippet, as a project-relative path, not the temporary or synthetic name any scanning mechanism used internally. Where the snippet is a literal block scalar, the position SHALL be the snippet's real position in that file, so the author can open it directly. + +#### Scenario: The rendered message is reported, not just the verdict + +- **WHEN** `test --json` runs against an ast-grep rule whose `invalid:` snippet matched +- **THEN** the rule result SHALL carry a finding for that snippet +- **AND** the finding's `message` SHALL be the message as ast-grep rendered it, with the rule's metavariables already interpolated +- **AND** a rule whose message names the same metavariables in the other order SHALL produce a different `message`, so the two cannot both pass + +#### Scenario: A finding points back into the fixture file + +- **WHEN** an ast-grep fixture snippet produces a finding +- **THEN** the finding's `file` SHALL be the project-relative path of the test YAML declaring the snippet +- **AND** SHALL NOT be a temporary path or ast-grep's name for a stream +- **AND** where the snippet is a literal block scalar, the finding's line and column SHALL be its position in that file rather than its position within the snippet + +#### Scenario: Both buckets are reported, and named in ast-grep's vocabulary + +- **WHEN** `test --json` reports an ast-grep rule's findings +- **THEN** a finding from an `invalid:` snippet SHALL carry the `fail` bucket +- **AND** a finding from a `valid:` snippet SHALL carry the `pass` bucket +- **AND** the `fail` bucket SHALL be reported even when the rule passed + +#### Scenario: A valid snippet that wrongly fired is printed without --json + +- **WHEN** `test` runs without `--json` and an ast-grep rule fails because a `valid:` snippet fired +- **THEN** the offending findings SHALL be printed under that rule, labelled by bucket +- **AND** they SHALL be rendered the way `check` renders a finding +- **AND** a rule that passed SHALL still print one line and no findings + +#### Scenario: A language ast-grep cannot parse degrades to no findings + +- **WHEN** an ast-grep rule declares a `language:` the vendored binary does not recognise +- **THEN** `test` SHALL report that rule as carrying an empty `findings` array +- **AND** SHALL NOT fail to produce a report +- **AND** the failure SHALL NOT suppress any other rule's findings + +#### Scenario: Collecting findings writes nothing to disk + +- **WHEN** `test` collects an ast-grep rule's fixture findings, whether the rule passes or fails +- **THEN** no file SHALL be left behind in the project tree or the temporary directory +- **AND** nothing SHALL be written that a later `check` could report against diff --git a/openspec/changes/archive/2026-09-23-sg-fixture-findings/tasks.md b/openspec/changes/archive/2026-09-23-sg-fixture-findings/tasks.md new file mode 100644 index 00000000..50ce1989 --- /dev/null +++ b/openspec/changes/archive/2026-09-23-sg-fixture-findings/tasks.md @@ -0,0 +1,61 @@ +## 1. Measure the routes before choosing one + +- [x] 1.1 Confirm `ast-grep test --help` on the vendored 0.45.3 binary offers no + `--json` and no output-format flag, so the verdict path cannot carry + findings. +- [x] 1.2 Confirm `check` over an sg rule's `.tests` directory returns no + results, so the Vale workaround does not transfer. +- [x] 1.3 Measure `scan --stdin --json=stream`: it renders the message with + metavariables interpolated and names the document `STDIN`. +- [x] 1.4 Measure that `files:` globs do NOT suppress a stdin scan, since a + path-scoped rule would otherwise silently report nothing. +- [x] 1.5 Measure that `-r` isolates an unparseable rule (exit 8, no JSON), + rather than aborting a whole config the way the assembled path would. + +## 2. Spec + +- [x] 2.1 Confirm the standing "present and empty" scenario's carve-out becomes + inert rather than false, so ADDED is available and MODIFIED is not forced. +- [x] 2.2 Write the requirement as an ADDED block, restating nothing standing. +- [x] 2.3 Dry-run `openspec archive` and compare requirement and scenario title + sets in `cli-rule-validation` before and after. + +## 3. Collector + +- [x] 3.1 Read each `.tests/*.yml`, applying the same `id:` exclusion + `countFixtures` applies. +- [x] 3.2 Stream each `valid:`/`invalid:` snippet to + `ast-grep scan -r --stdin --json=stream`. +- [x] 3.3 Map positions from snippet coordinates back into the fixture file for + literal block scalars; anchor to the snippet's first line otherwise, + rather than reporting a confidently wrong line. +- [x] 3.4 Report `file` as the cwd-relative POSIX path of the test YAML. +- [x] 3.5 Normalise ast-grep's `"note": null` to absent, which the `test` + payload schema requires and `check` never validated. +- [x] 3.6 Return an empty list for every shortfall — no rule file, no tests, a + rule ast-grep refuses — so gathering findings can never change a verdict. + +## 4. Wire in + +- [x] 4.1 Populate `findings` on the sg branch of `testOneRule`, after the + verdict is decided. +- [x] 4.2 Leave Vale, runtime and the shared human renderer untouched. + +## 5. Tests + +- [x] 5.1 Message-order regression: a two-metavariable message, asserted on the + rendered string. +- [x] 5.2 Mutation-check it by swapping the message's slots; confirm the + assertions go red. +- [x] 5.3 Both buckets: an `invalid:` snippet that fires, and a `valid:` one + that wrongly fires and makes the run fail. +- [x] 5.4 Exact line and column, proving the mapping back into the fixture file. +- [x] 5.5 A `language:` the binary does not know degrades to empty findings. +- [x] 5.6 No temp files left behind, on a passing run and a failing one, with + the CLI's temp directory redirected so the assertion is not measuring + other suites. +- [x] 5.7 Mutation-check 5.6 by making the collector write a temp file; confirm + it goes red. +- [x] 5.8 Replace the standing "empty for an ast-grep rule" case with one that + is still true — a rule whose fixtures matched nothing — without weakening + it. diff --git a/openspec/specs/cli-rule-validation/spec.md b/openspec/specs/cli-rule-validation/spec.md index c19b07ee..bf29ce98 100644 --- a/openspec/specs/cli-rule-validation/spec.md +++ b/openspec/specs/cli-rule-validation/spec.md @@ -419,3 +419,54 @@ A notice SHALL be reported on a rule that passed as well as on one that failed, - **WHEN** `verify --json` reports a rule that drew no advisory - **THEN** that rule's `notices` SHALL be present and empty + +### Requirement: Test reports the findings an ast-grep rule's fixtures produced + +`test` SHALL report, for an ast-grep rule whose fixtures produced matches, the findings those fixtures produced, in the same shape the other engines report theirs. A consumer SHALL NOT be able to tell which engine produced a finding except by its `source`. + +The rendered `message` is what this exists for. `ast-grep test` decides only whether each `invalid:` snippet fired and each `valid:` one stayed quiet, and a rule whose message interpolates its metavariables can have the slots in the wrong order while satisfying both — so the verdict cannot see the defect and the message is the only thing that can. + +The findings SHALL be gathered independently of the verdict and SHALL NOT change it. `ast-grep test` remains what decides whether the rule passed; a failure to gather findings SHALL be reported as a rule with no findings rather than as a rule that failed. + +A finding SHALL name the fixture FILE that declares the snippet, as a project-relative path, not the temporary or synthetic name any scanning mechanism used internally. Where the snippet is a literal block scalar, the position SHALL be the snippet's real position in that file, so the author can open it directly. + +#### Scenario: The rendered message is reported, not just the verdict + +- **WHEN** `test --json` runs against an ast-grep rule whose `invalid:` snippet matched +- **THEN** the rule result SHALL carry a finding for that snippet +- **AND** the finding's `message` SHALL be the message as ast-grep rendered it, with the rule's metavariables already interpolated +- **AND** a rule whose message names the same metavariables in the other order SHALL produce a different `message`, so the two cannot both pass + +#### Scenario: A finding points back into the fixture file + +- **WHEN** an ast-grep fixture snippet produces a finding +- **THEN** the finding's `file` SHALL be the project-relative path of the test YAML declaring the snippet +- **AND** SHALL NOT be a temporary path or ast-grep's name for a stream +- **AND** where the snippet is a literal block scalar, the finding's line and column SHALL be its position in that file rather than its position within the snippet + +#### Scenario: Both buckets are reported, and named in ast-grep's vocabulary + +- **WHEN** `test --json` reports an ast-grep rule's findings +- **THEN** a finding from an `invalid:` snippet SHALL carry the `fail` bucket +- **AND** a finding from a `valid:` snippet SHALL carry the `pass` bucket +- **AND** the `fail` bucket SHALL be reported even when the rule passed + +#### Scenario: A valid snippet that wrongly fired is printed without --json + +- **WHEN** `test` runs without `--json` and an ast-grep rule fails because a `valid:` snippet fired +- **THEN** the offending findings SHALL be printed under that rule, labelled by bucket +- **AND** they SHALL be rendered the way `check` renders a finding +- **AND** a rule that passed SHALL still print one line and no findings + +#### Scenario: A language ast-grep cannot parse degrades to no findings + +- **WHEN** an ast-grep rule declares a `language:` the vendored binary does not recognise +- **THEN** `test` SHALL report that rule as carrying an empty `findings` array +- **AND** SHALL NOT fail to produce a report +- **AND** the failure SHALL NOT suppress any other rule's findings + +#### Scenario: Collecting findings writes nothing to disk + +- **WHEN** `test` collects an ast-grep rule's fixture findings, whether the rule passes or fails +- **THEN** no file SHALL be left behind in the project tree or the temporary directory +- **AND** nothing SHALL be written that a later `check` could report against From 794ac087e5c65828e9413de2995c5a3c91dd2336 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Wed, 23 Sep 2026 15:37:37 -0700 Subject: [PATCH 3/3] fix(sg): compare a fixture file's id as a string, and measure block indent from the first non-empty line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A test file's unquoted `id: 123` resolves to the JS number 123, which never equals the string rule id, so the file was excluded from its own rule's fixtures: findings read as empty while the rule still passed. Reachable when the rule file quotes its id and the test file does not; an unquoted numeric id in the rule file is rejected by the schema before fixtures run. The same comparison in `fixtureCoverage` carried the same gap and is fixed with it. YAML detects a block scalar's indentation from its first non-empty line. The anchor read the literal first line instead, so a snippet written with a leading blank reported indent 0 and shifted every column left by the real indent — wrong rather than imprecise, since that path still maps per line. --- packages/cli/src/rules/sg-fixture-findings.ts | 38 +++++--- packages/cli/src/rules/verify.ts | 4 +- .../cli/test/test-fixture-findings.test.ts | 95 +++++++++++++++++++ 3 files changed, 125 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/rules/sg-fixture-findings.ts b/packages/cli/src/rules/sg-fixture-findings.ts index 6aaebbb0..bd66cf84 100644 --- a/packages/cli/src/rules/sg-fixture-findings.ts +++ b/packages/cli/src/rules/sg-fixture-findings.ts @@ -130,16 +130,28 @@ function anchorOf(source: string, item: Scalar): SnippetAnchor { const newline = source.indexOf("\n", start); if (newline === -1) return { line: lineOf(start), indent: 0, perLine: false }; const contentStart = newline + 1; - const nextNewline = source.indexOf("\n", contentStart); - const firstLine = source.slice( - contentStart, - nextNewline === -1 ? undefined : nextNewline - ); - return { - line: lineOf(contentStart), - indent: firstLine.length - firstLine.trimStart().length, - perLine: true, - }; + // YAML detects a block scalar's indentation from its first NON-EMPTY line: + // leading blank lines carry no indentation and must not be measured. Reading + // the literal first line instead reports indent 0 for a snippet written with + // a leading blank line, which shifts every column left by the real indent — + // wrong rather than merely imprecise, since `perLine` stays true here. + // `line` still refers to the first content line, blank or not, because the + // value handed to ast-grep keeps those blanks and its line numbers count them. + let indent = 0; + for ( + let lineStart = contentStart; + lineStart < source.length; + lineStart = source.indexOf("\n", lineStart) + 1 + ) { + const lineEnd = source.indexOf("\n", lineStart); + const text = source.slice(lineStart, lineEnd === -1 ? undefined : lineEnd); + if (text.trim() !== "") { + indent = text.length - text.trimStart().length; + break; + } + if (lineEnd === -1) break; + } + return { line: lineOf(contentStart), indent, perLine: true }; } /** Move a finding's position from snippet coordinates into file coordinates. */ @@ -283,7 +295,11 @@ export async function collectSgFixtureFindings( // The same exclusion `countFixtures` applies: a file carrying another // rule's `id:` is not this rule's fixture set, and ast-grep would not run // it under this rule either. - if (document.get("id") !== ruleId) continue; + // Compared as a string: an unquoted numeric `id:` (`id: 123`) resolves to + // the JS number 123, which never equals the string ruleId, so the file + // would be excluded from its own rule's fixtures and the findings would + // silently read as empty. + if (String(document.get("id")) !== ruleId) continue; const reportedFile = toRelativePosix(cwd, testFile); diff --git a/packages/cli/src/rules/verify.ts b/packages/cli/src/rules/verify.ts index 8caeaef4..d20e68a9 100644 --- a/packages/cli/src/rules/verify.ts +++ b/packages/cli/src/rules/verify.ts @@ -506,7 +506,9 @@ async function fixtureCoverage( // author has written nothing, the other has written a file that is sitting // right there and silently not counted. Only the second is // `sg-fixture-id-matches-rule`, so only the second may be attributed to it. - if (buckets.id !== ruleId) { + // Compared as a string: an unquoted numeric `id:` resolves to a JS number, + // which never equals the string ruleId. + if (String(buckets.id) !== ruleId) { excludedById = true; continue; } diff --git a/packages/cli/test/test-fixture-findings.test.ts b/packages/cli/test/test-fixture-findings.test.ts index 44b50cce..ae61ebd3 100644 --- a/packages/cli/test/test-fixture-findings.test.ts +++ b/packages/cli/test/test-fixture-findings.test.ts @@ -629,6 +629,101 @@ describe("an ast-grep rule's fixture findings", () => { `${SG_RULE}-test.yml`, ]); }); + + it("measures block-scalar indent from the first non-empty line", async () => { + // YAML detects a block scalar's indentation from its first NON-EMPTY line. + // Reading the literal first line instead reports indent 0 for a snippet + // written with a leading blank, shifting every column left by the real + // indent — silently wrong, since this path still maps per-line. + // + // 1 id: swap-args + // 2 valid: + // 3 - | + // 4 const fine = 1; + // 5 invalid: + // 6 - | + // 7 (blank) + // 8 swap(alpha, beta); + const directory = join(cwd, ".taskless", "rules", "sg", SG_RULE); + await mkdir(join(directory, ".tests"), { recursive: true }); + await writeFile(join(directory, `${SG_RULE}.yml`), SG_RULE_YAML); + await writeFile( + join(directory, ".tests", `${SG_RULE}-test.yml`), + [ + `id: ${SG_RULE}`, + "valid:", + " - |", + " const fine = 1;", + "invalid:", + " - |", + "", + " swap(alpha, beta);", + "", + ].join("\n") + ); + + const { stdout } = await testSg("--json"); + const report = JSON.parse(stdout) as Report; + const finding = findingsOf(report)[0]; + + expect(finding?.message).toBe(SG_RENDERED); + // Column 4 is the snippet's real indentation. Measuring the blank first + // line instead yields 0. + expect(finding?.range.start.column).toBe(4); + }); + + it("matches a numeric fixture-file id against its rule", async () => { + // A test file's unquoted `id: 123` resolves to the JS number 123, which + // never equals the string "123", so the file would be excluded from its + // own rule's fixtures and the findings would silently read as empty while + // the rule still passed. + // + // The rule file's own id has to be quoted for this to be reachable: an + // unquoted numeric id there is rejected by the rule schema ("id: Invalid + // input") before any fixture runs, so that variant never gets this far. + const numericId = "123"; + const directory = join(cwd, ".taskless", "rules", "sg", numericId); + await mkdir(join(directory, ".tests"), { recursive: true }); + await writeFile( + join(directory, `${numericId}.yml`), + [ + `id: "${numericId}"`, + "language: TypeScript", + "severity: warning", + "message: replace $SECOND with $FIRST", + "rule:", + " pattern: swap($FIRST, $SECOND)", + "", + ].join("\n") + ); + await writeFile( + join(directory, ".tests", `${numericId}-test.yml`), + [ + `id: ${numericId}`, + "valid:", + " - |", + " const fine = 1;", + "invalid:", + " - |", + " swap(alpha, beta);", + "", + ].join("\n") + ); + + const { stdout } = await runCli([ + "test", + `.taskless/rules/sg/${numericId}`, + "-d", + cwd, + "--json", + ]); + const report = JSON.parse(stdout) as Report; + + expect(report.rules[0]?.ok).toBe(true); + expect(findingsOf(report).map((finding) => finding.message)).toEqual([ + SG_RENDERED, + ]); + }); }); /* -------------------------------------------------------------------------- */