fix(test): notice when an sg rule's globs reach no file

.changeset/sg-empty-scope-notice.md

@@ -0,0 +1,5 @@
+---
+"@taskless/cli": patch
+---
+
+`taskless test` now says when an ast-grep rule's `files:`/`ignores:` globs match no file in the project. A fixture snippet has no path, so the globs never apply to it, and a rule scoped to `lib/**` in a project that keeps its code in `src/` used to pass `test` while `check` never ran it. The rule still passes; the result carries a notice naming the globs, on the human output and in `notices` under `--json`. Rules scoped by `language:` alone are not checked, since a project with no code in that language is honestly clean.

packages/cli/src/rules/inspect.ts

@@ -10,6 +10,7 @@ import {
 } from "./engines";
 import { describeCoverageShortfall, type FixtureFinding } from "./fixtures";
 import { collectSgFixtureFindings } from "./sg-fixture-findings";
+import { describeEmptySgScope } from "./sg-scope";
 import { type EngineName } from "./layout";
 import {
   assessCaptureDirectory,
@@ -498,7 +499,13 @@ export async function testOneRule(
       errors,
       violations,
       ran: true,
-      notices: verification.notices,
+      // The fixtures above have no path, so nothing they proved says the
+      // rule's globs reach a real file. Asked here, on a rule that verified,
+      // and only ever as a notice; see `sg-scope.ts`.
+      notices: collectNotices([
+        ...verification.notices,
+        await describeEmptySgScope(cwd, ruleId),
+      ]),
       // `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

packages/cli/src/rules/sg-scope.ts

@@ -0,0 +1,161 @@
+import { spawn } from "node:child_process";
+import { readFile } from "node:fs/promises";
+import process from "node:process";
+import { createInterface } from "node:readline";
+
+import { parse } from "yaml";
+
+import { ruleFilePath } from "./engines";
+import { buildPath, findSgBinary, sgWalkArgv } from "./scan";
+
+/**
+ * Whether an ast-grep rule's `files:`/`ignores:` globs reach any file in the
+ * project.
+ *
+ * `test` cannot answer this from the fixtures. An sg fixture is a `valid:` or
+ * `invalid:` snippet with no path, so the globs never apply to it, and a rule
+ * whose `files:` names `lib/**` in a project that keeps its code in `src/`
+ * passes every test and is then never run by `check` (#486). A typo in a glob
+ * and a clean codebase look exactly the same from either command.
+ *
+ * ## Why ast-grep answers, not a glob library
+ *
+ * The question is what ast-grep's walker would hand this rule, which depends on
+ * its glob dialect, the `ignore` crate's `.gitignore` handling, and the walk
+ * flags `check` passes. Re-deriving that with a second matcher is a second
+ * opinion that can disagree with the first. `--inspect entity` reports it
+ * directly, per file, on stderr. Measured against the vendored 0.45.3:
+ *
+ * ```
+ * sg: entity|file|src/app.ts: language=TypeScript,appliedRuleCount=0
+ * ```
+ *
+ * `appliedRuleCount` is the number of loaded rules whose language, `files:` and
+ * `ignores:` all admit the file. With `-r` exactly one rule is loaded, so any
+ * line above zero means the rule is in scope somewhere.
+ *
+ * `-r` rather than the assembled config for the reason
+ * `sg-fixture-findings.ts` gives: one malformed sibling rule aborts parsing of
+ * the assembled config, and the scope of THIS rule would then read as zero.
+ * With no project config, ast-grep resolves the globs against the cwd, which is
+ * the project root `check` scans from. `sgWalkArgv([])` is the whole-project
+ * walk `check` performs, so the files considered here are the files `check`
+ * considers.
+ */
+
+/** One `--inspect entity` file line, as ast-grep 0.45.3 writes it. */
+const FILE_ENTITY = /^sg: entity\|file\|.*,appliedRuleCount=(\d+)$/;
+
+/**
+ * Which scoping keys the rule declares, or none when it declares neither.
+ *
+ * A rule with neither key is scoped only by `language:`, and reaching no file
+ * then means the project has no code in that language, which `check` reporting
+ * nothing is an honest account of. The defect is a glob that selects nothing,
+ * so only a rule that has a glob is measured, and only that rule pays for the
+ * walk.
+ */
+async function scopingKeys(ruleFile: string): Promise<string[]> {
+  let rule: unknown;
+  try {
+    rule = parse(await readFile(ruleFile, "utf8")) as unknown;
+  } catch {
+    // An unreadable or malformed rule is `verify`'s to report.
+    return [];
+  }
+  if (typeof rule !== "object" || rule === null) return [];
+  return ["files", "ignores"].filter((key) => key in rule);
+}
+
+/**
+ * Whether ast-grep applies the rule to at least one file in the project.
+ *
+ * Three answers, because a walk that did not finish is neither: `unknown` is
+ * what lets the caller stay silent rather than report an empty scope it never
+ * measured.
+ *
+ * Stops at the first file in scope: the answer is a yes/no, and the rest of the
+ * walk, including running the rule over every file, would only confirm it.
+ */
+function reachesAnyFile(
+  cwd: string,
+  ruleFile: string
+): Promise<"reached" | "empty" | "unknown"> {
+  const sgBinary = findSgBinary();
+  return new Promise((resolve) => {
+    const child = spawn(
+      sgBinary,
+      [
+        "scan",
+        "-r",
+        ruleFile,
+        "--json=stream",
+        "--inspect",
+        "entity",
+        ...sgWalkArgv([]),
+      ],
+      {
+        cwd,
+        stdio: ["ignore", "pipe", "pipe"],
+        env: { ...process.env, PATH: buildPath() },
+      }
+    );
+
+    let reached = false;
+    // Findings arrive on stdout and are not this question. Drained so a full
+    // pipe cannot block the child before it reaches the line we are waiting
+    // for.
+    child.stdout.resume();
+    const rl = createInterface({ input: child.stderr });
+    rl.on("line", (line) => {
+      const match = FILE_ENTITY.exec(line);
+      if (match !== null && Number(match[1]) > 0 && !reached) {
+        reached = true;
+        child.kill();
+      }
+    });
+
+    child.on("error", () => {
+      resolve("unknown");
+    });
+    child.on("close", (code) => {
+      if (reached) {
+        resolve("reached");
+        return;
+      }
+      // 0 is a clean scan and 1 is error-severity findings; anything else is a
+      // walk that did not finish, and "no file seen" from it is not evidence
+      // of an empty scope.
+      resolve(code === 0 || code === 1 ? "empty" : "unknown");
+    });
+  });
+}
+
+/**
+ * A notice when the rule's globs select no file in the project, or `undefined`.
+ *
+ * A notice rather than a failure. A glob can be narrow on purpose, written
+ * ahead of the code it targets, and nothing about the rule itself is invalid.
+ * What must not happen is the rule reading as tested and working while `check`
+ * never runs it, and a notice is printed beside the tick, which is where that
+ * reading forms.
+ *
+ * Silent when ast-grep could not finish the walk: saying the scope is empty on
+ * the strength of a failed scan would be the same false clean this exists to
+ * remove, in the other direction.
+ */
+export async function describeEmptySgScope(
+  cwd: string,
+  ruleId: string
+): Promise<string | undefined> {
+  const ruleFile = ruleFilePath(cwd, "sg", ruleId);
+  const keys = await scopingKeys(ruleFile);
+  if (keys.length === 0) return undefined;
+  if ((await reachesAnyFile(cwd, ruleFile)) !== "empty") return undefined;
+  const globs = keys.map((key) => `${key}:`).join("/");
+  return (
+    `${ruleId}'s ${globs} globs match no file in this project, so \`check\` ` +
+    `will never run it. The globs are matched against paths relative to the ` +
+    `project root; the fixtures passing does not exercise them.`
+  );
+}

packages/cli/test/verify-test-commands.test.ts

@@ -601,3 +601,102 @@ describe("a rejection names the constraint it violated", () => {
     );
   });
 });
+
+/**
+ * An sg rule with both fixture buckets populated, so `test` passes it, and
+ * `scope` written verbatim between `message:` and `rule:`.
+ */
+async function scopedSgRule(id: string, scope = "") {
+  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: no log\n` +
+      `${scope}rule:\n  pattern: console.log($$$)\n`
+  );
+  await writeFile(
+    join(directory, ".tests", `${id}-test.yml`),
+    `id: ${id}\nvalid:\n  - console.error(x);\ninvalid:\n  - console.log(x);\n`
+  );
+  return `.taskless/rules/sg/${id}`;
+}
+
+/** The notices `test --json` attached to the one rule, asserting it passed. */
+async function noticesFor(path: string): Promise<string[] | undefined> {
+  const result = await runCli(["test", path, "-d", cwd, "--json"]);
+  // A notice, never a failure: a narrow glob can be deliberate.
+  expect(result.exitCode).toBe(0);
+  const rule = (JSON.parse(result.stdout) as Report).rules[0];
+  expect(rule?.ok).toBe(true);
+  return rule?.notices;
+}
+
+// #486. An sg fixture has no path, so `files:` never applies to it, and a glob
+// that selects nothing passed `test` while `check` never ran the rule.
+//
+// Every case here is one a glob matcher of our own would get wrong in at least
+// one direction, which is why the scope is asked of ast-grep's walk instead.
+describe("test says when an sg rule's globs reach no file", () => {
+  beforeEach(async () => {
+    await mkdir(join(cwd, "src"), { recursive: true });
+    await writeFile(join(cwd, "src", "app.ts"), "console.log('hi');\n");
+  });
+
+  it("notices a files: glob that matches nothing", async () => {
+    const path = await scopedSgRule("no-log", `files:\n  - "lib/**/*.ts"\n`);
+    const notices = await noticesFor(path);
+    expect(notices).toHaveLength(1);
+    expect(notices?.[0]).toContain("no-log's files: globs match no file");
+  });
+
+  it("is silent when the glob reaches a file", async () => {
+    const path = await scopedSgRule("no-log", `files:\n  - "src/**/*.ts"\n`);
+    expect(await noticesFor(path)).toEqual([]);
+  });
+
+  // The glob matches a real file, but not one in the rule's language, so
+  // ast-grep never applies the rule to it. Matching the glob alone would call
+  // this in scope.
+  it("notices a glob that only reaches files in another language", async () => {
+    await writeFile(join(cwd, "src", "notes.md"), "console.log('hi')\n");
+    const path = await scopedSgRule("no-log", `files:\n  - "src/**/*.md"\n`);
+    expect(await noticesFor(path)).toHaveLength(1);
+  });
+
+  // An `ignores:` that swallows every file empties the scope as surely as a
+  // mistyped `files:`, and the notice has to name the key that did it.
+  it("notices an ignores: glob that excludes every file", async () => {
+    const path = await scopedSgRule("no-log", `ignores:\n  - "**/*.ts"\n`);
+    const notices = await noticesFor(path);
+    expect(notices).toHaveLength(1);
+    expect(notices?.[0]).toContain("no-log's ignores: globs match no file");
+  });
+
+  // `check` honours `.gitignore`, so a glob reaching only ignored files is a
+  // rule `check` never runs, however many files the glob matches on disk.
+  it("does not count a gitignored file as in scope", async () => {
+    await execFileAsync("git", ["init", "-q"], { cwd });
+    await mkdir(join(cwd, "dist"), { recursive: true });
+    await writeFile(join(cwd, "dist", "app.ts"), "console.log('hi');\n");
+    await writeFile(join(cwd, ".gitignore"), "dist/\n");
+    const path = await scopedSgRule("no-log", `files:\n  - "dist/**/*.ts"\n`);
+    expect(await noticesFor(path)).toHaveLength(1);
+  });
+
+  // `check` never walks into `.taskless/`, so a file there must not count
+  // either, even though the glob and the language both admit it.
+  it("does not count the CLI's own directory as in scope", async () => {
+    await rm(join(cwd, "src"), { recursive: true });
+    await writeFile(join(cwd, ".taskless", "probe.ts"), "console.log(1);\n");
+    const path = await scopedSgRule("no-log", `files:\n  - "**/*.ts"\n`);
+    expect(await noticesFor(path)).toHaveLength(1);
+  });
+
+  // No glob means the scope is the language alone, and a project with no code
+  // in it is honestly clean rather than mis-scoped.
+  it("is silent for a rule with no files: or ignores:", async () => {
+    await rm(join(cwd, "src"), { recursive: true });
+    const path = await scopedSgRule("no-log");
+    expect(await noticesFor(path)).toEqual([]);
+  });
+});