diff --git a/.changeset/ast-grep-0-45-3.md b/.changeset/ast-grep-0-45-3.md index b9dcdb4b..bec6ec87 100644 --- a/.changeset/ast-grep-0-45-3.md +++ b/.changeset/ast-grep-0-45-3.md @@ -3,3 +3,9 @@ --- Update the bundled ast-grep to 0.45.3 (from 0.45.2). + +What a rule author sees in `taskless check` on ast-grep rules: + +- An inline `ast-grep-ignore` comment in scanned code now takes effect only when it is the comment's first alphabetic text. A comment that merely mentioned the directive as prose above a flagged line used to suppress the finding; it no longer does, so findings can appear that were hidden before. Move `ast-grep-ignore` to the start of the comment if the suppression was meant. +- The same prose mentions no longer produce `unused-suppression` hints. +- Rule files, `taskless verify`, and the language list are unchanged. Nothing installed under `.taskless/` needs migrating. diff --git a/packages/cli/src/generated/ast-grep-rule-schema.json b/packages/cli/src/generated/ast-grep-rule-schema.json index d5278fb7..30159355 100644 --- a/packages/cli/src/generated/ast-grep-rule-schema.json +++ b/packages/cli/src/generated/ast-grep-rule-schema.json @@ -661,6 +661,11 @@ }, "Severity": { "oneOf": [ + { + "description": "Turns off the rule.", + "type": "string", + "const": "off" + }, { "description": "A kind reminder for code with potential improvement.", "type": "string", @@ -680,11 +685,6 @@ "description": "An error that code produces bugs or has logic errors.", "type": "string", "const": "error" - }, - { - "description": "Turns off the rule.", - "type": "string", - "const": "off" } ] }, @@ -743,5 +743,5 @@ "additionalProperties": true } }, - "$comment": "Generated by fetch-ast-grep-schema.ts at 2026-08-26T03:59:55.871Z from ast-grep v0.45.2 (https://raw.githubusercontent.com/ast-grep/ast-grep/0.45.2/schemas/rule.json)" + "$comment": "Generated by fetch-ast-grep-schema.ts at 2026-09-15T23:43:40.361Z from ast-grep v0.45.3 (https://raw.githubusercontent.com/ast-grep/ast-grep/0.45.3/schemas/rule.json)" } diff --git a/packages/cli/src/rules/capabilities.ts b/packages/cli/src/rules/capabilities.ts index 4c9ebbae..d64a2bbd 100644 --- a/packages/cli/src/rules/capabilities.ts +++ b/packages/cli/src/rules/capabilities.ts @@ -39,7 +39,7 @@ * Pinned against the binary by `test/ast-grep-vendor-contract.test.ts` * ("engine capabilities" → "reports the pinned version"). */ -export const AST_GREP_VERSION = "0.45.2"; +export const AST_GREP_VERSION = "0.45.3"; /** * Every language ast-grep can parse, verbatim from diff --git a/packages/cli/test/ast-grep-vendor-contract.test.ts b/packages/cli/test/ast-grep-vendor-contract.test.ts index b30ef195..040c47ad 100644 --- a/packages/cli/test/ast-grep-vendor-contract.test.ts +++ b/packages/cli/test/ast-grep-vendor-contract.test.ts @@ -25,7 +25,7 @@ import { escapeRegExp } from "../src/util/regex"; * ast-grep's observable behaviour, pinned. * * Everything here is a property of a **vendored third-party binary**, exact- - * pinned at `0.45.2` in `packages/cli/package.json`. Our own tests assert that + * pinned at `0.45.3` in `packages/cli/package.json`. Our own tests assert that * our code behaves correctly *given* these; this file asserts the givens, so an * ast-grep bump that changes one fails here — naming the assumption and the * code that rests on it — instead of surfacing downstream. @@ -326,6 +326,30 @@ const semanticsRule = (body: string) => "", ].join("\n"); +/** + * Every finding `rule("no-eval")` produces over one source file, in stream + * order — including the built-in `unused-suppression` rule's, which is why + * `ruleId` and `severity` are read rather than assumed. + */ +const findingsFor = (source: string) => + scan( + project({ + rules: { "no-eval": rule("no-eval") }, + sources: { "src/a.ts": source }, + }) + ) + .stdout.split("\n") + .filter((line) => line !== "") + .map( + (line) => + JSON.parse(line) as { + ruleId: string; + severity: string; + text: string; + note: unknown; + } + ); + /** A Markdown rule whose `rule:` body is given verbatim, already indented. */ const markdownRule = (body: string) => [ @@ -489,13 +513,22 @@ withSg("ast-grep vendor contract", () => { // `off` is accepted in a rule but disables it, so it can never appear in // output (asserted below). A rule at any other severity fails the parse // rather than reaching us, which is what keeps the four-value union safe. + // + // ORDER CHANGED AT 0.45.3, vocabulary unchanged. `off` moved from last + // to first, here and in the vendored schema's `Severity` enum alike: + // `--min-severity` (ast-grep/ast-grep#2917) compares severities as an + // ordered type, and `off` has to sort lowest for "off means no minimum" + // to fall out of that comparison. Nothing of ours reads the order — + // `verify` validates against the schema's `const` values, not their + // position — so the exact text is pinned to make the next reorder + // visible, not because anything depends on it. const cwd = project({ rules: { "no-eval": rule("no-eval", "catastrophe") }, sources: evalSource, }); const result = scan(cwd); expect(result.stderr).toContain( - "unknown variant `catastrophe`, expected one of `hint`, `info`, `warning`, `error`, `off`" + "unknown variant `catastrophe`, expected one of `off`, `hint`, `info`, `warning`, `error`" ); }); @@ -1086,6 +1119,132 @@ withSg("ast-grep vendor contract", () => { }); }); + /** + * Inline `ast-grep-ignore` comments in SCANNED code, and where the directive + * has to sit to count. CHANGED AT 0.45.3 (ast-grep/ast-grep#2909). + * + * Nothing of ours writes these or documents them, but `check` scans whatever + * source a project has, and ast-grep honours the comment wherever it finds + * one — so a user's code carries this behaviour into `taskless check` + * whether or not they wrote the comment for us. Through 0.45.2 the check was + * a substring search: any comment CONTAINING `ast-grep-ignore` was a live + * directive, so prose describing the mechanism above a flagged line + * suppressed the finding, and prose with nothing to suppress was reported as + * an unused directive. At 0.45.3 the directive must be the comment's first + * alphabetic text. + * + * Both halves of that are the quiet kind: a finding appears that did not + * before, or a hint stops appearing, and nothing says why. Measured against + * both binaries, so each case below records which side of the bump it is on. + */ + describe("inline ast-grep-ignore comments", () => { + it("suppresses the next line, bare or naming the rule", () => { + // The mechanism itself, unchanged across the bump and pinned so the + // cases that follow are read against a working baseline rather than a + // directive that stopped applying altogether. + expect( + findingsFor( + [ + "// ast-grep-ignore", + 'const a = eval("1");', + "// ast-grep-ignore: no-eval", + 'const b = eval("2");', + "", + ].join("\n") + ) + ).toEqual([]); + }); + + it("no longer suppresses from a comment that mentions the directive as prose", () => { + // CHANGED AT 0.45.3. At 0.45.2 this scan reported NOTHING: the + // substring match read the prose as a directive and swallowed the + // finding. A codebase whose comments discuss suppression now reports + // the findings those comments sat on, which a rule author sees as new + // errors from an unchanged rule. + const findings = findingsFor( + [ + "// see ast-grep-ignore: no-eval for how to suppress this", + 'const c = eval("3");', + "", + ].join("\n") + ); + expect(findings.map((finding) => finding.ruleId)).toEqual(["no-eval"]); + expect(findings[0]?.text).toBe('eval("3")'); + }); + + it("no longer reports a prose mention as an unused directive", () => { + // The other half of the same change, in the other direction: at 0.45.2 + // this scan produced an `unused-suppression` hint on the comment line. + // A project that had been carrying that hint sees it disappear. + expect( + findingsFor( + [ + "// This comment mentions ast-grep-ignore as prose", + "const g = 1;", + "", + ].join("\n") + ) + ).toEqual([]); + }); + + it("anchors on the first ALPHABETIC character, not the first character", () => { + // The exact boundary upstream chose: everything before the first letter + // is skipped, so the comment marker, extra whitespace, a block-comment + // opener and even a leading list number are not prose. A directive + // behind `// 1.` therefore still suppresses, which is the case an + // author would guess wrong about from "must be first". (This comment + // is itself scanned by the repo's own `check`, so no line of it may + // start with the token — a wrapped one did, and was reported.) + expect( + findingsFor( + [ + "/* ast-grep-ignore */", + 'const d = eval("4");', + "// ast-grep-ignore", + 'const e = eval("5");', + "// 1. ast-grep-ignore", + 'const f = eval("6");', + "", + ].join("\n") + ) + ).toEqual([]); + // And one letter before it is enough to make it prose again. + expect( + findingsFor( + ["// NOTE ast-grep-ignore", 'const h = eval("7");', ""].join("\n") + ).map((finding) => finding.text) + ).toEqual(['eval("7")']); + }); + + it("reports a genuinely unused directive on the stream as a hint", () => { + // How a user sees any of this at all. `unused-suppression` is a built-in + // rule, not one of ours, and it arrives on `--json=stream` like any + // finding: `ruleId` is the built-in's name, `severity` is `hint` (inside + // AstGrepMatch's union, so it renders), and `note` is `null` rather than + // absent — `check.md` documents that shape, and `format.ts` tests + // truthiness, so the null is tolerated rather than typed. A finding + // that scoped its rule to something else counts as unused too, and the + // finding it did not cover is reported beside it. + const findings = findingsFor( + [ + "// ast-grep-ignore", + "const h = 1;", + "// ast-grep-ignore: other-rule", + 'const i = eval("9");', + "", + ].join("\n") + ); + expect( + findings.map((finding) => [finding.ruleId, finding.severity]) + ).toEqual([ + ["no-eval", "error"], + ["unused-suppression", "hint"], + ["unused-suppression", "hint"], + ]); + expect(findings[1]?.note).toBeNull(); + }); + }); + describe("the `sg` alias prints a deprecation banner on stderr", () => { // DEPRECATED AT 0.45.0. `AST_GREP_BINARY.binaryNames` puts `ast-grep` // first, but the resolver reverses that list at its link-based tiers, so diff --git a/packages/cli/test/engine-version-consistency.test.ts b/packages/cli/test/engine-version-consistency.test.ts index 5a3fb29f..45832546 100644 --- a/packages/cli/test/engine-version-consistency.test.ts +++ b/packages/cli/test/engine-version-consistency.test.ts @@ -153,7 +153,7 @@ describe("a resolution says which tier answered", () => { /** * The third link: a pinned package contains the version its NAME claims. * - * The pin says `@ast-grep/cli-…: 0.45.2`, and nothing forced the file inside it + * The pin says `@ast-grep/cli-…: 0.45.3`, and nothing forced the file inside it * to be that. A mispublished or substituted package satisfies the pin, installs * cleanly, and answers `--version` with something else. * diff --git a/packages/cli/test/reconcile-marker.test.ts b/packages/cli/test/reconcile-marker.test.ts index 9938fbf4..30c6f819 100644 --- a/packages/cli/test/reconcile-marker.test.ts +++ b/packages/cli/test/reconcile-marker.test.ts @@ -86,7 +86,7 @@ describe("recording a rules reconciliation", () => { expect(rules?.reconciledTo).toBe(version); // Engine versions are the input a later differential needs. Recorded here // and nowhere else, so an upgrade cannot silently refresh them. - expect(rules?.engines).toEqual({ sg: "0.45.2", vale: "3.20.0" }); + expect(rules?.engines).toEqual({ sg: "0.45.3", vale: "3.20.0" }); }); it("reports the marker through info", async () => {