lint-once: bound the docker file flag to its own invocation
Some checks failed
check / check (push) Failing after 42s
Some checks failed
check / check (push) Failing after 42s
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.
This commit is contained in:
10
README.md
10
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.
|
invocation graph, so a second pass cannot creep back in unnoticed.
|
||||||
|
|
||||||
`script/fmt-check` remains as a standalone entrypoint for asking the formatting
|
`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
|
question on its own, without docker and without the rest of lint. It is not the
|
||||||
formatting path that still runs on the host, and nothing reaches it: neither
|
last formatting path on the host — `script/fmt` writes with the host prettier,
|
||||||
`script/check` nor `script/precommit` calls it, so it never contributes to their
|
and `package.json` exposes `fmt` and `fmt-check` as direct prettier calls — it is
|
||||||
verdicts. Its own verdict cannot drift from the container's: prettier is pinned
|
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
|
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
|
both places, and reads `.gitignore` as its default ignore file — which is why
|
||||||
`.dockerignore` deliberately keeps `.gitignore` in the build context.
|
`.dockerignore` deliberately keeps `.gitignore` in the build context.
|
||||||
|
|||||||
@@ -40,12 +40,18 @@
|
|||||||
// value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the
|
// value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the
|
||||||
// body of a `define`/`endef` block are not expanded and not followed. A
|
// body of a `define`/`endef` block are not expanded and not followed. A
|
||||||
// `docker build` resolves to the file named by `-f`, `-f=`, `-f<file>` with
|
// `docker build` resolves to the file named by `-f`, `-f=`, `-f<file>` with
|
||||||
// the value attached to the flag, `--file` or `--file=`, wherever in the
|
// the value attached to the flag, `--file` or `--file=`, wherever in that
|
||||||
// invocation that flag sits, and to `Dockerfile` when it names none; a
|
// invocation the flag sits, and to `Dockerfile` when it names none; a
|
||||||
// Dockerfile chosen some other way — a bundled short flag cluster
|
// Dockerfile chosen some other way — a bundled short flag cluster
|
||||||
// (`-qf <file>`), a value this walk cannot expand to a literal — resolves to
|
// (`-qf <file>`), a value this walk cannot expand to a literal — resolves to
|
||||||
// the default rather than to the real file. Within those edges, a
|
// the default rather than to the real file. Each `docker build` on a line is
|
||||||
// prettier call is caught wherever it is added.
|
// 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`
|
// 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.
|
// 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
|
// 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
|
// 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
|
// 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
|
// zero, and both the counting and the edge-following are per occurrence rather
|
||||||
// invocations chained with `&&` cannot read as one.
|
// 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 { describe, expect, it } from "vitest";
|
||||||
import { readFileSync } from "node:fs";
|
import { readFileSync } from "node:fs";
|
||||||
import { fileURLToPath } from "node:url";
|
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
|
// 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
|
// 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.
|
// 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]}`);
|
edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user