From 9814c75836945f726e48d26c79e7ea1177a3eb6f Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Wed, 11 Nov 2026 22:31:09 -0500 Subject: [PATCH] fix(lint): close three gates that passed what they exist to refuse arch-lint dropped the bare relative specifiers. Its filter kept only a specifier with a slash, so an import of "." or ".." (a directory's index, which tsc and bun both resolve) drew no edge, and a forbidden cross-layer import passed. The filter now keeps every relative specifier. The rule-gate plugin guarded .refine() and .superRefine(), which src/ does not write, and not .check(), which it does. zod skips a function written inline in .check() once a nested value failed, exactly as it skips a bare refinement, so the plugin now refuses that form with the message the existing patterns give. A check passed by name or built by a helper is not followed. build:check never saw a generated region that no generator registers. Each generator re-renders only the regions in its own registry, so a region dropped from one kept its markers and went stale with the gate green. Each "regions" row of the generated-output table now carries the region names its generator registers, read from the registry, and the runner refuses a BEGIN marker in a registered page that names none of them, naming the page and the region, before any generator runs. The check sits in the runner because docs/reference/inputs.md is written by two generators, and only the table sees both registries. The marker scan now returns the names it finds, so the table test and the runner read one scan. _typos.toml loses the allow entry for DELETEing, a word no tracked file holds, and its look-alikes comment no longer names a file that does not hold the word. --- .github/scripts/arch-lint.ts | 7 ++- .github/scripts/generated.ts | 54 +++++++++++++---- .github/scripts/lib/generated-regions.ts | 11 +++- _typos.toml | 4 +- lint/rule-gate.grit | 18 ++++-- test/lint/rule-gate.test.ts | 9 ++- test/scripts/arch-lint.test.ts | 40 ++++++++++++- test/scripts/generated.test.ts | 74 ++++++++++++++++-------- 8 files changed, 166 insertions(+), 51 deletions(-) diff --git a/.github/scripts/arch-lint.ts b/.github/scripts/arch-lint.ts index fb3ba195..e59dfd44 100644 --- a/.github/scripts/arch-lint.ts +++ b/.github/scripts/arch-lint.ts @@ -202,8 +202,9 @@ function literalModuleRequest(node: Node): string | undefined { return undefined; } -/** Every relative specifier `text` loads, each once. A file that does not parse or loads a module through a - * computed specifier throws naming the line: a dropped edge would let a forbidden import pass. */ +/** Every relative specifier `text` loads, each once, the bare `.` and `..` (a directory's index) included. A file + * that does not parse or loads a module through a computed specifier throws naming the line: a dropped edge would + * let a forbidden import pass. */ export function importSpecifiers(text: string, file: string): string[] { const { program, errors } = parseSync(file, text); const lineOf = (offset: number): number => text.slice(0, offset).split("\n").length; @@ -226,7 +227,7 @@ export function importSpecifiers(text: string, file: string): string[] { } specifiers.add(specifier); }); - return [...specifiers].filter((specifier) => /^\.\.?\//.test(specifier)); + return [...specifiers].filter((specifier) => /^\.\.?(?:\/|$)/.test(specifier)); } /** Source spells the emitted `.js`, hence the `.ts` and `/index.ts` candidates. Nothing found throws: a dangling diff --git a/.github/scripts/generated.ts b/.github/scripts/generated.ts index 92180600..07ff9143 100644 --- a/.github/scripts/generated.ts +++ b/.github/scripts/generated.ts @@ -1,35 +1,46 @@ /** * The one table of committed generated output, derived from the generators' own registries, and the drift check - * behind `bun run build:check`: every generator script runs, then every registered path must be tracked and unchanged. + * behind `bun run build:check`: every marker in a registered page must name a registered region, every generator + * script runs, then every registered path must be tracked and unchanged. * * The table is in dependency order, each artifact before its readers, or a new gap file would leave a run stale: * the descriptor-derived files -> imported through src/ by everything below * the gaps index -> imported through src/ by the docs and action.yml generators */ +import { readFileSync } from "node:fs"; import { join } from "node:path"; import { FILES } from "./gen-action-docs.js"; import { PAGES } from "./gen-docs.js"; import { INDEX_PATH } from "./gen-gaps-index.js"; -import { INPUTS_PAGE_PATH } from "./gen-inputs-table.js"; +import { INPUTS_PAGES } from "./gen-inputs-table.js"; import { OUTPUTS as OPENAPI_OUTPUTS } from "./gen-openapi.js"; import { runMain } from "./lib/entry.js"; +import { markerSyntaxFor, regionNames } from "./lib/generated-regions.js"; +import type { GeneratedFiles } from "./lib/region-driver.js"; const ROOT = join(import.meta.dir, "..", ".."); -export interface GeneratedOutput { +/** What a row of either kind carries. */ +interface OutputRow { /** The committed output, repo-relative. */ readonly path: string; /** The package.json script that writes it; `bun run ` regenerates it in place. */ readonly generator: string; - - /** Marker-delimited regions inside an authored file (lib/generated-regions.ts), or the whole file. */ - readonly kind: "regions" | "file"; } -function regions(generator: string, paths: readonly string[]): GeneratedOutput[] { - return paths.map((path) => ({ path, generator, kind: "regions" })); +/** The whole file, or marker-delimited regions inside an authored file (lib/generated-regions.ts) under the names `generator` registers there. */ +export type GeneratedOutput = OutputRow & + ({ readonly kind: "file" } | { readonly kind: "regions"; readonly regions: readonly string[] }); + +function regions(generator: string, files: GeneratedFiles): GeneratedOutput[] { + return Object.entries(files).map(([path, file]) => ({ + path, + generator, + kind: "regions", + regions: file.regions.map((region) => region.name), + })); } /** In the module doc's dependency order; a page two generators write into (docs/reference/inputs.md) has one row per generator. */ @@ -38,9 +49,9 @@ export const GENERATED_OUTPUTS: readonly GeneratedOutput[] = [ ({ path }): GeneratedOutput => ({ path, generator: "build:openapi", kind: "file" }), ), { path: INDEX_PATH, generator: "build:gaps-index", kind: "file" }, - ...regions("build:docs", Object.keys(PAGES)), - ...regions("build:action-docs", Object.keys(FILES)), - { path: INPUTS_PAGE_PATH, generator: "build:inputs-table", kind: "regions" }, + ...regions("build:docs", PAGES), + ...regions("build:action-docs", FILES), + ...regions("build:inputs-table", INPUTS_PAGES), ]; /** The distinct paths, in table order. */ @@ -59,6 +70,22 @@ export function generatorEntryPoint(script: string): string | null { return /^bun (\S+\.ts)$/.exec(script)?.[1] ?? null; } +/** One line per BEGIN marker in a registered page that names no region a generator registers there: nothing + * regenerates such a region, so its body would go stale unseen. */ +function unregisteredRegions(): string[] { + const pages = GENERATED_OUTPUTS.filter((output) => output.kind === "regions"); + return [...Map.groupBy(pages, (output) => output.path)].flatMap(([path, rows]) => { + const registered = new Set(rows.flatMap((row) => row.regions)); + const marked = regionNames(readFileSync(join(ROOT, path), "utf8"), markerSyntaxFor(path)); + return marked + .filter((name) => !registered.has(name)) + .map( + (name) => + `${path} marks a generated region "${name}" that no generator registers; register it with a generator, or remove its GENERATED markers`, + ); + }); +} + /** Runs `argv` at the repository root on the terminal's stdio; the exit code is the verdict. */ function run(argv: string[]): number { return Bun.spawnSync(argv, { cwd: ROOT, stdout: "inherit", stderr: "inherit" }).exitCode; @@ -66,6 +93,11 @@ function run(argv: string[]): number { if (import.meta.main) { await runMain("generated", () => { + const unregistered = unregisteredRegions(); + if (unregistered.length > 0) { + console.error(`build:check: ${unregistered.join("\nbuild:check: ")}`); + return 1; + } for (const generator of generatorScripts()) { if (run([process.execPath, "run", generator]) !== 0) { console.error(`build:check: bun run ${generator} failed`); diff --git a/.github/scripts/lib/generated-regions.ts b/.github/scripts/lib/generated-regions.ts index b10f047a..365e8389 100644 --- a/.github/scripts/lib/generated-regions.ts +++ b/.github/scripts/lib/generated-regions.ts @@ -40,6 +40,9 @@ const REGION_NAME_CLASS = "[a-z0-9-]+"; // A whole name, as regionBounds() admits it. const REGION_NAME = new RegExp(`^${REGION_NAME_CLASS}$`); +// The name inside a marker's own text. +const MARKED_NAME = new RegExp(` GENERATED: (${REGION_NAME_CLASS})`); + /** A docs path for biome's markdown formatter: it picks the language and the config's overrides from the path, which need not exist. */ const MARKDOWN_PATH = "docs/generated-region.md"; @@ -108,9 +111,11 @@ function markerSpans( }); } -/** True when `text` carries a BEGIN marker of any region in `syntax`: the scan the generated-output table is pinned against. */ -export function hasGeneratedRegion(text: string, syntax: MarkerSyntax): boolean { - return markerSpans(text, "BEGIN", REGION_NAME_CLASS, syntax).length > 0; +/** The region name of every BEGIN marker `text` carries in `syntax`, in file order: the scan the generated-output table and the build:check runner read. */ +export function regionNames(text: string, syntax: MarkerSyntax): string[] { + return markerSpans(text, "BEGIN", REGION_NAME_CLASS, syntax).flatMap( + ([start, end]) => MARKED_NAME.exec(text.slice(start, end))?.[1] ?? [], + ); } /** Region `name`'s marker spans: exactly one BEGIN and one END, in that order, else a throw. */ diff --git a/_typos.toml b/_typos.toml index 7b34508b..68f0a03f 100644 --- a/_typos.toml +++ b/_typos.toml @@ -27,7 +27,6 @@ extend-ignore-re = [ PATCHed = "PATCHed" DELETEd = "DELETEd" DELETEs = "DELETEs" -DELETEing = "DELETEing" # the class age-encryption exports, and the variable that holds one. Encrypter = "Encrypter" encrypter = "encrypter" @@ -39,8 +38,7 @@ R_kgDOnotOurs = "R_kgDOnotOurs" # valid spelling (the dictionary prefers "unparsable"); the harness's # UNPARSEABLE_YAML fixture pool and comments in src and test use it. unparseable = "unparseable" -# "look-alikes": the near-miss fixtures in test/scripts/generated-regions.test.ts -# and test/docs/guides.test.ts. +# "look-alikes": the near-miss fixtures in test/scripts/generated-regions.test.ts. alikes = "alikes" # Deliberate misspellings that are negative test input, allowed only where the diff --git a/lint/rule-gate.grit b/lint/rule-gate.grit index 8917a40a..014e6fa9 100644 --- a/lint/rule-gate.grit +++ b/lint/rule-gate.grit @@ -1,14 +1,24 @@ // Every refinement under src/ is authored through rule() (src/sections/shared/schema-helpers.ts): zod skips a bare -// .refine() or .superRefine() once a nested value failed, so a section's own rule would go silent exactly when the -// document has a second mistake. Reach and the one exemption are the plugin's `overrides` entry in biome.local.jsonc (see -// never-throw.grit for why): +// one once a nested value failed, so a section's own rule would go silent exactly when the document has a second +// mistake. +// .refine(fn), .superRefine(fn) -> flagged +// .check() given a function written inline -> flagged +// .check() given a name or a helper's call -> not followed +// +// Reach and the one exemption are the plugin's `overrides` entry in biome.local.jsonc (see never-throw.grit for why): // src/sections/shared/schema-helpers.ts -> rule() itself is the one superRefine or { `$_.superRefine($_)`, `$_.superRefine<$_>($_)`, `$_.refine($_)`, - `$_.refine<$_>($_)` + `$_.refine<$_>($_)`, + `$_.check($checks)` where { + $checks <: some or { + JsArrowFunctionExpression(), + JsFunctionExpression() + } + } } as $call where { register_diagnostic(span=$call, message="bare refinement: zod skips it once a nested value failed; wrap it in rule() so it runs beside nested failures") } diff --git a/test/lint/rule-gate.test.ts b/test/lint/rule-gate.test.ts index 7d121cda..6f62b27b 100644 --- a/test/lint/rule-gate.test.ts +++ b/test/lint/rule-gate.test.ts @@ -18,10 +18,10 @@ const MESSAGE = /** The file the plugin exempts: rule() itself is a bare refinement. */ const EXEMPT_FILE = "src/sections/shared/schema-helpers.ts"; -/** A line ending in `// bare` is one the plugin must flag; every other refinement here is authored through rule(). */ +/** A line ending in `// bare` is one the plugin must flag; every other line passes its check through rule(), through a helper, or by name. */ const BARE = /\/\/ bare$/; -/** A src/ module of refinements, every one through rule() but the lines marked bare. */ +/** A src/ module of refinements: the forms zod skips after a nested failure, each marked bare, beside the forms the tree writes. */ const REFINEMENTS = ` import { z } from "zod"; import { maxLength, rule } from "./sections/shared/schema-helpers.js"; @@ -33,8 +33,13 @@ export const sup = z.object({ n: z.string() }).superRefine(fn); // bare export const supParams = z.object({ n: z.string() }).superRefine(fn, { when: () => true }); // bare export const supTyped = z.object({ n: z.string() }).superRefine<{ n: string }>(fn); // bare export const composed = z.object({ n: z.string() }).check(z.superRefine(fn)); // bare +export const inline = z.object({ n: z.string() }).check((ctx) => { ctx.issues.push({ code: "custom", message: "x", input: ctx.value }); }); // bare +export const inlineFunction = z.object({ n: z.string() }).check(function whole(ctx) { ctx.issues.push({ code: "custom", message: "x", input: ctx.value }); }); // bare +export const inlineBesideHelper = z.array(z.string()).check(maxLength(1, "x"), (ctx) => { ctx.issues.push({ code: "custom", message: "x", input: ctx.value }); }); // bare export const gated = z.object({ n: z.string() }).check(rule(fn)); +export const gatedInline = z.object({ n: z.string() }).check(rule((value, ctx) => { ctx.addIssue({ code: "custom", message: String(value) }); })); export const bounded = z.array(z.string()).check(maxLength(1, "x")); +export const named = z.string().check(fn); `.trimStart(); describe("the rule-gate plugin", () => { diff --git a/test/scripts/arch-lint.test.ts b/test/scripts/arch-lint.test.ts index a135079e..6b7c8159 100644 --- a/test/scripts/arch-lint.test.ts +++ b/test/scripts/arch-lint.test.ts @@ -4,8 +4,8 @@ */ import { describe, expect, test } from "bun:test"; -import { writeFileSync } from "node:fs"; -import { join } from "node:path"; +import { mkdirSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; import { err } from "neverthrow"; import { ARCHITECTURE_PATH, @@ -58,6 +58,42 @@ describe("architecture.yml against src/", () => { }); }); +describe("lintArchitecture over a scratch tree", () => { + const DECLARATION = [ + "layers:", + " library: [src/index.ts]", + " cli: [src/cli.ts]", + " sections: [src/sections/]", + "exclude: []", + "edges: {}", + ].join("\n"); + + test.each<[form: string, importer: string, specifier: string, layer: string]>([ + ['a bare ".."', "src/sections/s.ts", "..", "sections"], + ['a bare "."', "src/cli.ts", ".", "cli"], + ["the same file spelled out (control)", "src/sections/s.ts", "../index.js", "sections"], + ])( + "a directory's index imported through %s draws its edge", + (_form, importer, specifier, layer) => + withTempDir("arch-lint-tree-", (dir) => { + // tsc and bun both resolve a bare "." or ".." to the directory's index.ts, so a scan that keeps only the + // specifiers with a slash would let this forbidden import pass. + const files = { + [ARCHITECTURE_PATH]: `${DECLARATION}\n`, + "src/index.ts": "export const run = 1;\n", + [importer]: `import type { run } from "${specifier}";\nexport type Run = typeof run;\n`, + }; + for (const [path, text] of Object.entries(files)) { + mkdirSync(dirname(join(dir, path)), { recursive: true }); + writeFileSync(join(dir, path), text); + } + expect(lintArchitecture(dir)).toEqual([ + `forbidden import ${layer} -> library: ${importer} -> src/index.ts; move it or declare the edge`, + ]); + }), + ); +}); + describe("importSpecifiers", () => { test.each<[string, string, string[]]>([ ["a runtime import", 'import { a } from "./a.js";', ["./a.js"]], diff --git a/test/scripts/generated.test.ts b/test/scripts/generated.test.ts index 6aec42e8..19f10d9f 100644 --- a/test/scripts/generated.test.ts +++ b/test/scripts/generated.test.ts @@ -9,8 +9,8 @@ import { generatorScripts, } from "../../.github/scripts/generated.js"; import { - hasGeneratedRegion, markerSyntaxFor, + regionNames, SYNTAX_BY_EXTENSION, } from "../../.github/scripts/lib/generated-regions.js"; import { ROOT } from "../root.js"; @@ -45,7 +45,7 @@ describe("the generated-output table", () => { const texts = new Map(tracked.map((path) => [path, readFileSync(join(ROOT, path), "utf8")])); const carrying = tracked.filter( (path) => - regionFile(path) && hasGeneratedRegion(texts.get(path) ?? "", markerSyntaxFor(path)), + regionFile(path) && regionNames(texts.get(path) ?? "", markerSyntaxFor(path)).length > 0, ); const wholeFiles = tracked.filter((path) => carriesWholeFileHeader(texts.get(path) ?? "")); expect( @@ -83,18 +83,13 @@ describe("the generated-output table", () => { test("the scan reads the shared marker grammar: an inline HTML marker counts, a marker-shaped scalar does not", () => { // A page whose only marker sits mid-line (inputs.md's outputs-list is the tree's inline case) would slip a line-anchored scan. expect( - hasGeneratedRegion( - "- `result`: x", - "html", - ), - ).toBe(true); - expect(hasGeneratedRegion("the words BEGIN GENERATED: a outside a comment\n", "html")).toBe( - false, - ); - expect( - hasGeneratedRegion("inputs:\n # BEGIN GENERATED: a\n # END GENERATED: a\n", "yaml"), - ).toBe(true); - expect(hasGeneratedRegion('d: "one\n # BEGIN GENERATED: a\n two"\n', "yaml")).toBe(false); + regionNames("- `result`: x", "html"), + ).toEqual(["a"]); + expect(regionNames("the words BEGIN GENERATED: a outside a comment\n", "html")).toEqual([]); + expect(regionNames("inputs:\n # BEGIN GENERATED: a\n # END GENERATED: a\n", "yaml")).toEqual([ + "a", + ]); + expect(regionNames('d: "one\n # BEGIN GENERATED: a\n two"\n', "yaml")).toEqual([]); }); }); @@ -113,19 +108,21 @@ describe("the build:check runner", () => { return { status: run.exitCode, printed: run.stdout.toString() + run.stderr.toString() }; } + /** HEAD's tree in `dir` with its own index, sharing the object store and node_modules; the runner, the generators, + * and the script table they run through are copied from the working tree, so the code under test is the code + * being edited. */ + function cloneHead(dir: string): void { + git(ROOT, "clone", "--quiet", "--shared", ROOT, dir); + symlinkSync(join(ROOT, "node_modules"), join(dir, "node_modules")); + cpSync(join(ROOT, "package.json"), join(dir, "package.json")); + cpSync(join(ROOT, ".github", "scripts"), join(dir, ".github", "scripts"), { recursive: true }); + } + test( "a clean clone passes; a staged stale byte in a page, in action.yml, and in a whole file fails naming each", () => withTempDir("build-check-", (dir) => { - // HEAD's tree with its own index, sharing the object store and node_modules; the runner, the generators, - // and the script table they run through are copied from the working tree, so the code under test is the - // code being edited. - git(ROOT, "clone", "--quiet", "--shared", ROOT, dir); - symlinkSync(join(ROOT, "node_modules"), join(dir, "node_modules")); - cpSync(join(ROOT, "package.json"), join(dir, "package.json")); - cpSync(join(ROOT, ".github", "scripts"), join(dir, ".github", "scripts"), { - recursive: true, - }); + cloneHead(dir); const untouched = [ ".", ":(exclude)node_modules", @@ -184,4 +181,35 @@ describe("the build:check runner", () => { }), 120_000, ); + + test( + "a marker no generator registers fails naming its page and region, in either marker syntax, before any generator runs", + () => + withTempDir("build-check-", (dir) => { + // A generator re-renders only the regions in its own registry, so a region dropped from one keeps its + // markers and its last body. Staged, the two plants below leave the drift check nothing to diff. + // inputs.md is the page two generators share: neither one's region may read as unregistered to the other. + cloneHead(dir); + const page = join(dir, "docs/reference/inputs.md"); + const retired = + "\n| grant |\n"; + writeFileSync(page, `${readFileSync(page, "utf8")}\n${retired}\n`); + const manifest = join(dir, "action.yml"); + const retiredKey = "# BEGIN GENERATED: retired-inputs\n# END GENERATED: retired-inputs"; + writeFileSync(manifest, `${readFileSync(manifest, "utf8")}${retiredKey}\n`); + git(dir, "add", "docs/reference/inputs.md", "action.yml"); + + const fix = + "that no generator registers; register it with a generator, or remove its GENERATED markers"; + expect(runner(dir)).toEqual({ + status: 1, + printed: [ + `build:check: docs/reference/inputs.md marks a generated region "retired-grant-table" ${fix}`, + `build:check: action.yml marks a generated region "retired-inputs" ${fix}`, + "", + ].join("\n"), + }); + }), + 120_000, + ); });