From 197296edba65a1381231d9559dfb2db6f91b6ed9 Mon Sep 17 00:00:00 2001 From: user Date: Fri, 4 Sep 2026 11:31:06 +0000 Subject: [PATCH] lint-once: follow every spelling of docker build's file flag The edge resolver matched `-f ` only. `docker build --file=X` fell through to the default `Dockerfile` edge, so a second prettier pass wired in that way was followed into the wrong file, counted nothing, and left the suite green -- the exact false green this test exists to prevent, reachable by writing the flag the long way. Mutation, adding one line to the `check` recipe, before this commit: @docker build -f Dockerfile.lint . 1 failed / 47 caught @docker build --file=Dockerfile.lint . 48 passed / 48 MISSED After, all four spellings fail with `expected 2 to be 1`: `-f X`, `-f=X`, `--file X`, `--file=X`. Docker takes the value either way for both the short and long flag, so the resolver now reads `(?:-f|--file)[=\s]+`, still searched anywhere in the invocation rather than at a fixed position. A leading \s keeps a longer flag ending in the same letters (`--force-rm`) from supplying the match. The header claimed `docker build -f ` coverage without qualification, which a reader could take to include the long form it did not follow; it now names all four forms and the fallback. The one limitation it asserts -- a bundled cluster like `-qf X` resolving to the default -- is pinned by a test, since an unpinned limitation is how the header drifts back into overclaiming. --- test/packaging/lint-once.test.ts | 80 ++++++++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/test/packaging/lint-once.test.ts b/test/packaging/lint-once.test.ts index cf4f26b..8972f5c 100644 --- a/test/packaging/lint-once.test.ts +++ b/test/packaging/lint-once.test.ts @@ -18,8 +18,8 @@ // `"$SCRIPT_DIR/"` and `script/` into other scripts, `make ` // and `$(MAKE) ` through the Makefile shims, the prerequisites of a // Makefile target, `yarn run ` through the `package.json` scripts, and -// `docker build -f ` into that Dockerfile's `RUN` steps, heredoc bodies -// included — and counts the prettier invocations it finds. +// `docker build` into the `RUN` steps of the Dockerfile it names, heredoc +// bodies included — and counts the prettier invocations it finds. // // What that covers is worth stating exactly rather than as "anywhere", because // a comment claiming more than the code delivers is the same defect as an @@ -38,8 +38,13 @@ // writes its recipes that way (`@$(YARN) tsc --watch`); the substitution is // one pass over literals, so a value that itself names another variable, a // value built by a make function (`$(shell ...)`, `$(addprefix ...)`), and the -// body of a `define`/`endef` block are not expanded and not followed. Within -// those edges, a prettier call is caught wherever it is added. +// body of a `define`/`endef` block are not expanded and not followed. A +// `docker build` resolves to the file named by `-f`, `-f=`, `--file` or +// `--file=`, wherever in the invocation that 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. // // 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. @@ -430,8 +435,18 @@ const edgesOf = (line: string): string[] => { // The container lint pass lives behind a `docker build`; without following // it the count would miss the one invocation that is supposed to survive. + // + // Every spelling of the file flag is read, not just `-f `. Docker + // accepts `--file `, `--file=` and `-f=` for the same + // thing, and matching `-f ` alone resolved all of them to the default + // `Dockerfile` edge — so `docker build --file=Dockerfile.lint .` in a + // recipe was followed into the wrong file, counted nothing, and reported + // green. The flag is searched for anywhere in the invocation, as `-f` + // already was, because a real build line wraps it in `--build-arg` and + // other flags. A leading `\s` is required so that a longer flag ending in + // the same letters cannot supply the match. if (/\bdocker\s+build\b/.test(line)) { - const file = /\s-f\s+(\S+)/.exec(line); + const file = /\s(?:-f|--file)[=\s]+(\S+)/.exec(line); edges.push(`docker:${file === null ? "Dockerfile" : file[1]}`); } @@ -982,6 +997,61 @@ describe("the resolver reads what the shell would run", () => { ); }); + // `-f` is not the only spelling of the flag. `--file=Dockerfile.lint` and + // `--file Dockerfile.lint` mean exactly what `-f Dockerfile.lint` means, + // and matching only `-f` sent both to the default `Dockerfile` edge + // instead. That is the false-green shape this whole file exists to + // prevent: a second prettier pass added as + // `docker build --file=Dockerfile.lint .` was followed into the wrong + // Dockerfile, counted nothing, and left every build green. The long form + // is an ordinary thing for a human to write, so it is followed rather than + // merely documented as unfollowed. + // + // The equality rather than containment matters: resolving to the right + // file is only half of it, the default edge must not also be emitted. + it("follows --file in all three spellings as it does -f", () => { + expect(edgesOf("docker build --file=Dockerfile.lint .")).toEqual([ + "docker:Dockerfile.lint", + ]); + expect(edgesOf("docker build --file Dockerfile.lint .")).toEqual([ + "docker:Dockerfile.lint", + ]); + expect(edgesOf("docker build -f=Dockerfile.lint .")).toEqual([ + "docker:Dockerfile.lint", + ]); + }); + + // The flag is found anywhere in the invocation rather than at a fixed + // position, matching how `-f` was already read, since a real build line + // carries `--build-arg` and friends around it. + it("finds the file flag after other flags and build args", () => { + expect( + edgesOf( + 'docker build --build-arg LINT_EPOCH="1" --file=Dockerfile.lint --progress=plain .', + ), + ).toEqual(["docker:Dockerfile.lint"]); + }); + + // A different flag that merely begins with the same letters is not the + // file flag: `--force-rm` must not read as one, or its next token would be + // resolved as a Dockerfile and thrown on as unreadable. + it("does not mistake --force-rm for the file flag", () => { + expect(edgesOf("docker build --force-rm .")).toEqual([ + "docker:Dockerfile", + ]); + }); + + // The header says a bundled short flag cluster resolves to the default + // rather than to the file it names, which is a limitation and so has to be + // pinned: an unpinned limitation is how the header starts overclaiming + // again. If someone teaches the resolver to split clusters, this test is + // the one that tells them to correct the header too. + it("resolves a bundled -qf cluster to the default Dockerfile", () => { + expect(edgesOf("docker build -qf Dockerfile.lint .")).toEqual([ + "docker:Dockerfile", + ]); + }); + // This exact-equality assertion is also the block-scalar guard, which is // not obvious from its name: a `run: |` step resolves to the bare `|`, // which reaches nothing, so the prettier count would stay 1 no matter what