From 88a5fcaa8724a7c2db93228802640e24041f7846 Mon Sep 17 00:00:00 2001 From: user Date: Fri, 4 Sep 2026 11:49:38 +0000 Subject: [PATCH] lint-once: bound the docker file flag to its own invocation edgesOf used a single test/exec for docker build, so a line with two builds produced one edge and the file flag was searched across the whole line. Follow every occurrence with matchAll and slice each one to the next shell separator before looking for its flag. Also correct the README's claim that script/fmt-check is the one formatting path left on the host. --- README.md | 10 ++++---- test/packaging/lint-once.test.ts | 39 +++++++++++++++++++++++++------- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index a4147ab..9c94308 100644 --- a/README.md +++ b/README.md @@ -127,10 +127,12 @@ in each. `test/packaging/lint-once.test.ts` asserts that count by walking the invocation graph, so a second pass cannot creep back in unnoticed. `script/fmt-check` remains as a standalone entrypoint for asking the formatting -question on its own, without docker and without the rest of lint. It is the one -formatting path that still runs on the host, and nothing reaches it: neither -`script/check` nor `script/precommit` calls it, so it never contributes to their -verdicts. Its own verdict cannot drift from the container's: prettier is pinned +question on its own, without docker and without the rest of lint. It is not the +last formatting path on the host — `script/fmt` writes with the host prettier, +and `package.json` exposes `fmt` and `fmt-check` as direct prettier calls — it is +the one of them that the check graph could plausibly have reached, and nothing +reaches it: neither `script/check` nor `script/precommit` calls it, so it never +contributes to their verdicts. Its own verdict cannot drift from the container's: prettier is pinned to an exact version, installed from `yarn.lock` under `--frozen-lockfile` in both places, and reads `.gitignore` as its default ignore file — which is why `.dockerignore` deliberately keeps `.gitignore` in the build context. diff --git a/test/packaging/lint-once.test.ts b/test/packaging/lint-once.test.ts index 895bb6b..7b2c4bf 100644 --- a/test/packaging/lint-once.test.ts +++ b/test/packaging/lint-once.test.ts @@ -40,12 +40,18 @@ // value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the // body of a `define`/`endef` block are not expanded and not followed. A // `docker build` resolves to the file named by `-f`, `-f=`, `-f` with -// the value attached to the flag, `--file` or `--file=`, wherever in the -// invocation that flag sits, and to `Dockerfile` when it names none; a +// the value attached to the flag, `--file` or `--file=`, wherever in that +// invocation the flag sits, and to `Dockerfile` when it names none; a // Dockerfile chosen some other way — a bundled short flag cluster // (`-qf `), a value this walk cannot expand to a literal — resolves to -// the default rather than to the real file. Within those edges, a -// prettier call is caught wherever it is added. +// the default rather than to the real file. Each `docker build` on a line is +// followed separately, and the flag search for one is bounded to the slice +// running from that command to the next `&&`, `||`, `;` or `|`, so a second +// build on the same line is not swallowed by the first and a later command's +// `-f` is not read as the build's. That bounding is a split on those four +// separators, not a shell parse: a separator appearing inside quotes or a +// command substitution still ends the slice. Within those edges, a prettier +// call is caught wherever it is added. // // Two entrypoints are walked, because they cover different graphs: `make check` // is what a developer runs, and `.gitea/workflows/check.yml` is what CI runs. @@ -58,8 +64,9 @@ // Undercounting is the failure mode that would make this test worthless. Three // things guard against it: the walk is asserted to have reached the nodes that // matter, an unresolvable or empty node is a thrown error rather than a quiet -// zero, and prettier is counted per occurrence rather than per line, so two -// invocations chained with `&&` cannot read as one. +// zero, and both the counting and the edge-following are per occurrence rather +// than per line, so two prettier calls or two `docker build`s chained with `&&` +// on one line cannot read as one. import { describe, expect, it } from "vitest"; import { readFileSync } from "node:fs"; import { fileURLToPath } from "node:url"; @@ -449,8 +456,24 @@ const edgesOf = (line: string): string[] => { // the same letters cannot supply the match, and the attached form is // allowed only for the short flag, so that `--force-rm` — a long flag that // merely starts with the same letters — still does not read as one. - if (/\bdocker\s+build\b/.test(line)) { - const file = /\s(?:--file[=\s]+|-f=?\s*)(\S+)/.exec(line); + // + // Every `docker build` on the line produces an edge, and each one looks for + // its file flag only within its own invocation — the slice from the command + // to the next `&&`, `||`, `;` or `|`. One `test` and one `exec` over the + // whole line got both halves of that wrong: `docker build -f Dockerfile.lint + // . && docker build -f Dockerfile.extra .` followed the first file and + // dropped the second, so a tree running prettier twice reported green, and a + // bare `docker build . && cp -f Dockerfile.lint /tmp/x` read `cp`'s `-f` as + // the build's, counting an invocation that never happens and losing the + // default `Dockerfile` edge. The bounding is a split on those four + // separators and nothing more: a separator inside quotes or a `$(...)` + // substitution ends the slice anyway, and a newline-separated command list + // is already one line per invocation by the time it gets here. + for (const match of line.matchAll(/\bdocker\s+build\b/g)) { + const invocation = line + .slice(match.index) + .split(/&&|\|\||;|\|/)[0] as string; + const file = /\s(?:--file[=\s]+|-f=?\s*)(\S+)/.exec(invocation); edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`); }