diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 39822cfaaf5..d9a88694ad2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,6 +46,8 @@ See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/star Test scripts and helpers in `tests/` are plain bash too. `bin/fm-lint.sh` must pass: it is the single owner of the lint definition (the shellcheck file set, config, and pinned shellcheck version), and both CI and the no-mistakes pre-push gate run it, so local and CI can never diverge. It pins one exact shellcheck version and refuses to run under any other; print it with `bin/fm-lint.sh --required-version` and install that build locally. + It shards the file set and caches clean results, so an unchanged tree costs well under a second while a fresh checkout costs about half of what the old single command did. + If you change how it plans, shards, or caches, run `bin/fm-lint.sh --verify-parity`: it runs the canonical single-process command (`bin/fm-lint.sh --whole-set`) against the fast path and fails on any difference in findings. - Changes to harness adapters (detection in `bin/fm-harness.sh`, launch and hook mechanics in `bin/fm-spawn.sh`, busy signatures in `bin/fm-watch.sh` and `bin/fm-tmux-lib.sh`, cleanup in `bin/fm-teardown.sh`, and facts in `.agents/skills/harness-adapters/SKILL.md`) must be verified empirically against the real harness, never written from documentation alone. - Changes to runtime session backends (`bin/fm-backend.sh`, `bin/backends/`, and the scripts that dispatch through them) need empirical adapter notes in the relevant backend guide: `docs/tmux-backend.md`, `docs/herdr-backend.md`, `docs/zellij-backend.md`, `docs/orca-backend.md`, `docs/cmux-backend.md`, or `docs/codex-app-backend.md` for blocked Codex App transport work. - In Markdown, put each full sentence on its own line. diff --git a/bin/fm-lint-plan.awk b/bin/fm-lint-plan.awk new file mode 100644 index 00000000000..93838cb83cf --- /dev/null +++ b/bin/fm-lint-plan.awk @@ -0,0 +1,235 @@ +# fm-lint-plan.awk - closure + shard planner for bin/fm-lint.sh. +# +# ShellCheck (without -x) follows a sourced file ONLY when the target is also +# given as an input file on the same command line, and it resolves that target +# two ways: from a `# shellcheck source=` directive, or from a plain +# `source ` / `. ` statement whose target is a variable-free +# literal path. That following is the entire reason the canonical whole-set run +# is slow: every importer gets its sourced files inlined and re-analysed, and +# ShellCheck's analysis is superlinear in the size of the script it ends up +# analysing. +# +# The parity invariant this planner rests on is that a file's findings depend +# only on its own bytes plus the transitively sourced files present as input. +# Any resolution mechanism the planner fails to model breaks that invariant +# SILENTLY, and a silently-wrong lint gate is far worse than a slow one. This +# planner therefore does NOT parse source statements out of shell code itself: +# successive review rounds proved that chasing shell grammar in awk keeps +# leaking (line starts, separators, `then`/subshell positions, `function` +# bodies, redirection prefixes). Instead, bin/fm-lint.sh runs a discovery pass +# that asks ShellCheck itself: each file is checked ALONE, and every SC1091 +# "was not specified as input" note names exactly one followable literal +# source target, in every syntactic position, with no parsing on our side. +# Those discovered edges arrive here via EDGES. This file still reads each +# script, but only its `# shellcheck` directive lines: a `source=` key +# is an edge (it can name the target of a variable-path statement, which +# discovery also surfaces but the directive states outright), and any +# directive key or disable= item that could change resolution in a way the +# discovery pass cannot see makes this planner exit non-zero so bin/fm-lint.sh +# falls back to the whole-set reference implementation instead of guessing. +# +# The findings ShellCheck reports for a file therefore depend on exactly two +# things: the file's own contents, and the contents of the files it transitively +# sources that are present in the input list. Nothing else on the command line +# can change them. So for any input list S that is CLOSED under the transitive +# source relation, every file in S yields precisely the findings it yields in +# the whole-set run. +# +# This planner emits such closed shards. Reading: +# FILES - newline-delimited canonical file list (repo-relative, cwd = repo root) +# EDGES - pre-discovered source edges, one per line: "", +# the target exactly as ShellCheck named it in the SC1091 note. +# Unset or empty means "no discovered edges". +# WORK - newline-delimited subset that still needs linting (cache misses). +# Empty or unset means "all of FILES". +# JOBS - how many shards to emit +# MODE - "closures" emits one record per file, " ", and +# skips sharding entirely. Anything else shards as described below. +# Do not emulate this by passing a huge JOBS: the packer scans every +# shard slot per file, so JOBS=999999 spent 13s doing nothing. +# Emitting one record per shard on stdout: +# +# "owned" is the balanced partition of the work set; "full shard" is that +# partition plus its transitive source closure, which is what ShellCheck runs on. + +function trim(s) { + sub(/^[ \t]+/, "", s); sub(/[ \t]+$/, "", s); return s +} + +# Record a source edge from -> to. Only targets that are themselves canonical +# inputs are followed by ShellCheck; anything else stays unresolved in both +# modes alike, so it contributes no edge. +function addedge(from, to, key) { + if (to == "" || to == from || !(to in inset)) return + key = from SUBSEP to + if (!(key in edge)) { edge[key] = 1; deps[from] = deps[from] " " to } +} + +function tripwire(f, ln, line, what) { + printf "%s at %s:%d: %s\n", what, f, ln, trim(line) > "/dev/stderr" + exit 3 +} + +# Cost model for bin packing. ShellCheck's runtime grows faster than the line +# count (measured on this repo: 76 lines = 0.016s, 2287 lines = 2.56s), so +# packing by raw line count badly underweights the big files that actually +# dominate a shard's wall time. The exponent only has to rank shards sensibly. +function cost(lines) { return (lines / 100.0) ^ 1.6 } + +BEGIN { + if (JOBS < 1) JOBS = 1 + + # --- read the canonical file list ----------------------------------------- + n = 0 + while ((getline line < FILES) > 0) { + line = trim(line) + if (line == "") continue + files[++n] = line + inset[line] = 1 + } + close(FILES) + if (n == 0) exit 0 + + # --- read the work set (cache misses) -------------------------------------- + # An unset WORK means "own everything". An explicitly supplied but empty WORK + # means every file was a cache hit, so the planner must emit no shards at all; + # those two cases must not collapse into each other. + if (WORK == "") { + for (i = 1; i <= n; i++) work[files[i]] = 1 + } else { + while ((getline line < WORK) > 0) { + line = trim(line) + if (line == "" || !(line in inset)) continue + work[line] = 1 + } + close(WORK) + } + + # --- read the discovered source edges -------------------------------------- + # bin/fm-lint.sh ran every canonical file through ShellCheck alone and + # harvested the target named by each SC1091 "was not specified as input" + # note. A target counts as an edge when it names a canonical input, tried + # both as ShellCheck spelled it and script-dir-relative; over-inclusion is + # safe under the parity invariant, a missed edge is not. + if (EDGES != "") { + while ((getline line < EDGES) > 0) { + ti = index(line, "\t") + if (ti == 0) continue + from = substr(line, 1, ti - 1) + tgt = trim(substr(line, ti + 1)) + if (!(from in inset) || tgt == "") continue + sub(/^(\.\/)+/, "", tgt) + fdir = "" + if (from ~ /\//) { fdir = from; sub(/\/[^\/]*$/, "/", fdir) } + addedge(from, tgt) + addedge(from, fdir tgt) + } + close(EDGES) + } + + # --- scan each file for directive edges and count its lines ---------------- + # Only `# shellcheck` directive lines are inspected; statement edges come + # pre-discovered via EDGES. A `source=` key is an edge. Directive keys this + # planner does not model (e.g. source-path=, which changes how a later + # source statement resolves) are tripwires, so no unmodelled resolution + # mechanism can be silently guessed around; keys that cannot affect + # resolution (disable=, shell=, enable=, external-sources=) pass. One + # nuance makes disable= safe to pass: an in-file disable of SC1091 would + # blind the discovery pass, so bin/fm-lint.sh neutralizes SC1091 inside + # directive lines before each discovery run and the note still surfaces. + # That neutralization only recognises plain SCnnnn items, so a disable= + # item in any other spelling ("all", SCnnnn-SCnnnn ranges, or anything + # unrecognised) could still suppress SC1091 unseen - those tripwire here. + for (i = 1; i <= n; i++) { + f = files[i] + lc = 0 + while ((getline line < f) > 0) { + lc++ + if (line !~ /^[ \t]*#[ \t]*shellcheck[ \t]/) continue + rest = line + sub(/^[ \t]*#[ \t]*shellcheck[ \t]+/, "", rest) + nt = split(rest, toks, /[ \t]+/) + for (t = 1; t <= nt; t++) { + if (toks[t] ~ /^#/) break + if (toks[t] !~ /=/) continue + dkey = toks[t] + sub(/=.*$/, "", dkey) + if (dkey == "source") { + tail = toks[t] + sub(/^source=/, "", tail) + addedge(f, tail) + } else if (dkey == "disable") { + tail = toks[t] + sub(/^disable=/, "", tail) + nv = split(tail, items, ",") + for (v = 1; v <= nv; v++) + if (items[v] !~ /^SC[0-9]+$/) + tripwire(f, lc, line, "unclassifiable disable= item") + } else if (dkey != "shell" && dkey != "enable" && dkey != "external-sources") + tripwire(f, lc, line, "unmodelled shellcheck directive") + } + } + close(f) + lines[f] = lc + } + + # --- transitive closure per file ------------------------------------------ + for (i = 1; i <= n; i++) { + f = files[i] + delete seen + stackn = 0 + cnt = split(deps[f], d, " ") + for (j = 1; j <= cnt; j++) if (d[j] != "") stack[++stackn] = d[j] + while (stackn > 0) { + cur = stack[stackn--] + if (cur in seen || cur == f) continue + seen[cur] = 1 + cnt2 = split(deps[cur], d2, " ") + for (j = 1; j <= cnt2; j++) if (d2[j] != "") stack[++stackn] = d2[j] + } + cl = ""; tot = lines[f] + for (k in seen) { cl = cl " " k; tot += lines[k] } + closure[f] = cl + weight[f] = cost(tot) + } + + if (MODE == "closures") { + for (i = 1; i <= n; i++) printf "%s%s\n", files[i], closure[files[i]] + exit 0 + } + + # --- balance the work set across JOBS shards (longest-processing-time) ----- + # Sort work files by descending weight, then repeatedly assign the next + # heaviest to the currently lightest shard. Cheap and close to optimal for + # this shape of workload. + m = 0 + for (i = 1; i <= n; i++) { f = files[i]; if (f in work) ord[++m] = f } + for (i = 1; i < m; i++) + for (j = 1; j <= m - i; j++) + if (weight[ord[j]] < weight[ord[j + 1]]) { t = ord[j]; ord[j] = ord[j + 1]; ord[j + 1] = t } + + for (s = 1; s <= JOBS; s++) load[s] = 0 + for (i = 1; i <= m; i++) { + best = 1 + for (s = 2; s <= JOBS; s++) if (load[s] < load[best]) best = s + load[best] += weight[ord[i]] + owned[best] = owned[best] " " ord[i] + members[best] = members[best] " " ord[i] closure[ord[i]] + } + + # --- emit ------------------------------------------------------------------ + for (s = 1; s <= JOBS; s++) { + if (trim(owned[s]) == "") continue + # De-duplicate shard members: a shared library pulled in by several owned + # files must be listed once. + delete uniq + full = "" + cnt = split(members[s], mm, " ") + for (j = 1; j <= cnt; j++) { + if (mm[j] == "" || mm[j] in uniq) continue + uniq[mm[j]] = 1 + full = full " " mm[j] + } + printf "%d\t%s\t%s\n", s, trim(owned[s]), trim(full) + } +} diff --git a/bin/fm-lint.sh b/bin/fm-lint.sh index 3e950455317..05d17530916 100755 --- a/bin/fm-lint.sh +++ b/bin/fm-lint.sh @@ -21,17 +21,67 @@ # so CI and local run the identical rule set. This is not a CI relaxation: it # adopts one upstream release consistently; the only difference from the old # floating CI is dropping the upstream-retired, false-positive-prone SC2015. -# No severity downgrade and no blanket exclude of checks - every still-supported -# finding at default severity is enforced. +# No severity downgrade and no blanket suppression of checks - every +# still-supported finding at default severity is enforced. # The local == CI parity contract is asserted by tests/fm-lint.test.sh. # +# WHY THIS IS NOT ONE BIG SHELLCHECK CALL ANY MORE +# ------------------------------------------------ +# The canonical whole-set command took 3m45s-5m23s on a clean tree, which made +# the pre-push gate slow enough to be worth routing around - the worst possible +# property for a gate. +# +# Measured cause on this repo at ShellCheck 0.11.0: it is NOT file length and +# NOT process startup. Without -x, ShellCheck follows a `# shellcheck +# source=` directive only when the target is ALSO an input file on the +# same command line. The whole-set command supplies all 152 files at once, so +# all 184 source edges resolve and every importer is analysed with its sourced +# libraries inlined. ShellCheck's analysis is superlinear in the size of the +# script it ends up analysing, so that inlining, not the file count, dominates. +# Splitting bin/*.sh into four arbitrary chunks - which silently drops most +# source edges - cut 216s to 53s, a 4x swing in pure analysis work with the file +# set unchanged. +# +# That measurement also names the invariant that makes this safe to shard. A +# file's findings depend on exactly two inputs: its own contents, and the +# contents of the files it transitively sources that are present in the input +# list. Nothing else on the command line can affect them. Therefore, for any +# input list CLOSED under the transitive source relation, every file in that +# list yields precisely the findings the whole-set run gives it. +# +# So this script: +# 1. discovers the source graph by checking each file ALONE with ShellCheck +# and harvesting the target named by every SC1091 "was not specified as +# input" note, unioned with the `source=` directive edges parsed by +# bin/fm-lint-plan.awk, +# 2. skips files whose own contents AND whose entire transitive source closure +# are byte-identical to a previously recorded clean result under this exact +# ShellCheck version and flags, +# 3. packs the rest into closed shards and runs those shards in parallel. +# None of those steps can change a finding: steps 1 and 3 preserve closure, and +# step 2 only ever skips work whose every byte of input is unchanged. +# +# This is a speed change ONLY. The file set, the severity, the config and the +# version are untouched, and `--verify-parity` re-derives that claim on demand +# by running the canonical whole-set command against the fast path and diffing +# the findings. +# # Usage: # fm-lint.sh lint the canonical file set (what both gates run) # fm-lint.sh ... lint only the given paths with the same config # (developer convenience; the gates never pass args) +# fm-lint.sh --whole-set the canonical single-process command, unsharded +# and uncached; the reference implementation +# fm-lint.sh --verify-parity run --whole-set and the fast path, diff findings # fm-lint.sh --required-version print the pinned ShellCheck version and exit # (CI reads this to install the exact same one) # +# Environment: +# FM_LINT_JOBS shard count (default: nproc, capped at 8) +# FM_LINT_CACHE_DIR where clean results and discovered source edges are +# recorded (default: .git/fm-lint-cache) +# FM_LINT_NO_CACHE=1 read and write no cache entries +# # Exit status is ShellCheck's own on a lint run, so a caller (CI or the gate) # fails exactly when ShellCheck reports a finding; a version mismatch or a # missing ShellCheck fails before linting with a distinct message. @@ -41,6 +91,17 @@ set -eu # automatically via `--required-version`; the test suite reads it the same way. REQUIRED_SHELLCHECK=0.11.0 +# The behavior-affecting ShellCheck flags. The cache key embeds this exact +# string, so the sharded invocations below must take their flags from here and +# nowhere else: a flag added to an invocation but not to the key would serve +# stale clean results recorded under a different effective lint. The canonical +# --whole-set command spells the same flags literally because the test suite +# pins that command string. --verify-parity cannot detect drift between this +# variable and those literal exec lines (it re-spells the file set with +# $LINT_FLAGS and never executes the literal command); the guard is +# tests/fm-lint.test.sh, which asserts the exec lines' flags equal LINT_FLAGS. +LINT_FLAGS='--norc' + ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" cd "$ROOT" || exit 1 @@ -67,10 +128,426 @@ if [ "$resolved" != "$REQUIRED_SHELLCHECK" ]; then exit 1 fi -if [ "$#" -gt 0 ]; then +# Developer convenience: explicit paths bypass planning entirely and run the +# same ShellCheck with the same config. +if [ "$#" -gt 0 ] && [ "${1#--}" = "$1" ]; then exec shellcheck --norc "$@" fi -# Canonical file set: the ONE authoritative definition. Callers reference this -# script; they never re-spell these globs. -exec shellcheck --norc bin/*.sh bin/backends/*.sh tests/*.sh +MODE=default +case "${1:-}" in + '') ;; + --whole-set) MODE=whole-set ;; + --verify-parity) MODE=verify-parity ;; + *) + printf 'fm-lint.sh: unknown option %s\n' "$1" >&2 + exit 2 + ;; +esac + +if [ "$MODE" = whole-set ]; then + # The canonical file set: the ONE authoritative definition of what gets + # linted, in a single process, uncached and unsharded. This is the reference + # implementation every other path must agree with, and --verify-parity keeps + # it honest. Callers reference this script; they never re-spell these globs. + exec shellcheck --norc bin/*.sh bin/backends/*.sh tests/*.sh +fi + +TMP=$(mktemp -d "${TMPDIR:-/tmp}/fm-lint.XXXXXX") +trap 'rm -rf "$TMP"' EXIT INT TERM + +# The fast path plans over exactly the canonical set above, so the two modes +# cannot cover different files. +ls -1 bin/*.sh bin/backends/*.sh tests/*.sh >"$TMP/files" + +if [ "$MODE" = verify-parity ]; then + printf 'fm-lint.sh: running the canonical whole-set command (this is the slow one)...\n' >&2 + # shellcheck disable=SC2046,SC2086 + shellcheck $LINT_FLAGS --format=gcc $(cat "$TMP/files") 2>/dev/null | sort -u >"$TMP/ref" || true + printf 'fm-lint.sh: running the sharded fast path...\n' >&2 + # The inner run's status matters as much as its findings: a planner tripwire + # execs the whole-set fallback, which never writes FM_LINT_EMIT_FINDINGS, and + # a fatal shard error leaves the findings incomplete. Substituting an empty + # findings file in either case would let a clean tree report PARITY OK with + # the fast path never having run, which is exactly the vacuous confirmation + # a developer changing the planner must not receive. + fast_rc=0 + FM_LINT_NO_CACHE=1 FM_LINT_EMIT_FINDINGS="$TMP/fast" "$ROOT/bin/fm-lint.sh" \ + >/dev/null 2>"$TMP/fast.err" || fast_rc=$? + if grep -q 'falling back to the canonical command' "$TMP/fast.err"; then + printf 'fm-lint.sh: PARITY BROKEN - the fast path fell back to the whole-set command instead of sharding:\n' >&2 + cat "$TMP/fast.err" >&2 + exit 1 + fi + if [ "$fast_rc" -gt 1 ]; then + printf 'fm-lint.sh: PARITY BROKEN - the fast path exited %s instead of reaching a verdict:\n' "$fast_rc" >&2 + cat "$TMP/fast.err" >&2 + exit 1 + fi + if [ ! -f "$TMP/fast" ]; then + printf 'fm-lint.sh: PARITY BROKEN - the fast path never emitted its findings, so there is nothing to compare:\n' >&2 + cat "$TMP/fast.err" >&2 + exit 1 + fi + if diff -u "$TMP/ref" "$TMP/fast" >"$TMP/diff"; then + printf 'fm-lint.sh: PARITY OK - %s finding(s), identical in both modes.\n' \ + "$(wc -l <"$TMP/ref" | tr -d ' ')" >&2 + exit 0 + fi + printf 'fm-lint.sh: PARITY BROKEN - the fast path disagrees with the canonical command:\n' >&2 + cat "$TMP/diff" >&2 + exit 1 +fi + +# --- content hashing -------------------------------------------------------- +# Used only to decide whether a file's inputs are byte-identical to a recorded +# clean result. With no hasher available the cache is skipped, never guessed at. +HASHER= +if command -v sha256sum >/dev/null 2>&1; then + HASHER=sha256sum +elif command -v shasum >/dev/null 2>&1; then + HASHER='shasum -a 256' +fi + +CACHE_DIR="${FM_LINT_CACHE_DIR:-}" +if [ -z "$CACHE_DIR" ]; then + # .git is never tracked and never shipped, so a stale or hostile cache cannot + # ride into the repo or into CI, which always starts cold. + git_dir=$(git rev-parse --git-dir 2>/dev/null || true) + CACHE_DIR="${git_dir:-$ROOT/.fm-lint}/fm-lint-cache" +fi +USE_CACHE=1 +[ -n "$HASHER" ] || USE_CACHE=0 +[ "${FM_LINT_NO_CACHE:-0}" = 1 ] && USE_CACHE=0 + +JOBS="${FM_LINT_JOBS:-}" +if [ -z "$JOBS" ]; then + JOBS=$(nproc 2>/dev/null || sysctl -n hw.physicalcpu 2>/dev/null || echo 4) + # Past this, added shards contend for memory bandwidth instead of adding + # throughput (measured on this repo: 8 concurrent ShellCheck processes + # returned about 3x, 16 returned about 5x), while every extra shard + # re-analyses the shared libraries its owned files pull in. + [ "$JOBS" -gt 8 ] && JOBS=8 +fi +[ "$JOBS" -ge 1 ] || JOBS=1 + +# --- source-edge discovery -------------------------------------------------- +# The planner does not parse source statements out of shell code; ShellCheck +# itself is asked. Checking a file ALONE (no other inputs) makes every +# followable literal source emit an SC1091 "was not specified as input" note +# that names the resolved target, in every syntactic position - the resolver +# enumerates its own edges, so no shell grammar is re-implemented here. Two +# properties matter: +# - A file's discovery result depends only on its own bytes, because no +# other input is supplied, so it caches on the single-file digest and an +# unchanged tree pays nothing for this pass. +# - An in-file disable of SC1091 would blind this pass, so SC1091 is +# rewritten to an inert code inside ShellCheck directive lines on the +# stream fed to each discovery run; the note then still surfaces. Only +# the discovery input is rewritten - the shard and whole-set runs always +# see the original bytes, and only SC1091 notes are harvested, so +# disabling a different code on those lines changes nothing. disable= +# items the rewrite cannot recognise (anything but plain SCnnnn) are a +# planner tripwire instead. +DISC_CACHE="$CACHE_DIR/discovery" +: >"$TMP/edges" +: >"$TMP/disc.keep" +if [ "$USE_CACHE" = 1 ]; then + mkdir -p "$CACHE_DIR" + # shellcheck disable=SC2046 + $HASHER $(cat "$TMP/files") >"$TMP/hashes" 2>/dev/null || : >"$TMP/hashes" + [ -f "$DISC_CACHE" ] || : >"$DISC_CACHE" + awk -v VER="$REQUIRED_SHELLCHECK" -v FLAGS="$LINT_FLAGS" -v HASHES="$TMP/hashes" \ + -v CACHE="$DISC_CACHE" -v EDGES="$TMP/edges" -v KEEP="$TMP/disc.keep" ' + BEGIN { + while ((getline hl < HASHES) > 0) { split(hl, hf, " "); digest[hf[2]] = hf[1] } + close(HASHES) + while ((getline cl < CACHE) > 0) { + ti = index(cl, "\t") + if (ti == 0) continue + ck = substr(cl, 1, ti - 1) + seen[ck] = 1 + val[ck] = substr(cl, ti + 1) + } + close(CACHE) + } + { + d = digest[$0] + ck = VER " " FLAGS " " d + if (d != "" && (ck in seen)) { + m = split(val[ck], ee, " ") + for (e = 1; e <= m; e++) if (ee[e] != "") printf "%s\t%s\n", $0, ee[e] >> EDGES + printf "%s\t%s\n", ck, val[ck] >> KEEP + next + } + print + } + ' "$TMP/files" >"$TMP/disc.todo" +else + cp "$TMP/files" "$TMP/disc.todo" +fi + +if [ -s "$TMP/disc.todo" ]; then + djobs="$JOBS" + dn=$(wc -l <"$TMP/disc.todo" | tr -d ' ') + [ "$djobs" -le "$dn" ] || djobs="$dn" + awk -v N="$djobs" -v PFX="$TMP/disc.chunk." '{ print > (PFX (((NR - 1) % N) + 1)) }' "$TMP/disc.todo" + dpids= + ci=0 + while [ "$ci" -lt "$djobs" ]; do + ci=$((ci + 1)) + ( + : >"$TMP/disc.out.$ci" + while IFS= read -r df; do + drc=0 + # shellcheck disable=SC2086 + sed '/^[[:space:]]*#[[:space:]]*shellcheck[[:space:]]/ s/SC1091/SC2317/g' "$df" \ + | shellcheck $LINT_FLAGS --format=gcc - \ + >"$TMP/disc.raw.$ci" 2>>"$TMP/disc.err.$ci" || drc=$? + if [ "$drc" -gt 1 ]; then + printf '%s %s\n' "$drc" "$df" >"$TMP/disc.rc.$ci" + exit 0 + fi + awk -v F="$df" '/ \[SC1091\]$/ && match($0, / note: Not following: .* was not specified as input/) { + printf "%s\t%s\n", F, substr($0, RSTART + 22, RLENGTH - 49) + }' "$TMP/disc.raw.$ci" >>"$TMP/disc.out.$ci" + done <"$TMP/disc.chunk.$ci" + printf '0 -\n' >"$TMP/disc.rc.$ci" + ) & + dpids="$dpids $!" + done + for p in $dpids; do + wait "$p" || true + done + ci=0 + while [ "$ci" -lt "$djobs" ]; do + ci=$((ci + 1)) + if [ -f "$TMP/disc.rc.$ci" ]; then + read -r drc dfile <"$TMP/disc.rc.$ci" || { drc=1; dfile='(unreadable)'; } + else + drc=1; dfile='(worker died)' + fi + if [ "$drc" != 0 ]; then + printf 'fm-lint.sh: could not discover source edges (ShellCheck exited %s on %s); falling back to the canonical command.\n' \ + "$drc" "$dfile" >&2 + rm -rf "$TMP" + exec "$ROOT/bin/fm-lint.sh" --whole-set + fi + done + cat "$TMP"/disc.out.* >>"$TMP/edges" +fi + +# Discovery results are a pure function of a file's own bytes, so unlike the +# clean-run manifest they are recorded even when the tree has findings. The +# rewrite below keeps only entries for the current digests, so the cache +# cannot grow without bound. +if [ "$USE_CACHE" = 1 ]; then + awk -v VER="$REQUIRED_SHELLCHECK" -v FLAGS="$LINT_FLAGS" -v HASHES="$TMP/hashes" \ + -v TODO="$TMP/disc.todo" -v KEEP="$TMP/disc.keep" ' + BEGIN { + while ((getline hl < HASHES) > 0) { split(hl, hf, " "); digest[hf[2]] = hf[1] } + close(HASHES) + nn = 0 + while ((getline tf < TODO) > 0) if (tf != "") { order[++nn] = tf; disc[tf] = "" } + close(TODO) + while ((getline kl < KEEP) > 0) print kl + close(KEEP) + } + { + ti = index($0, "\t") + if (ti == 0) next + f = substr($0, 1, ti - 1) + if (!(f in disc)) next + t = substr($0, ti + 1) + disc[f] = disc[f] == "" ? t : disc[f] " " t + } + END { + for (e = 1; e <= nn; e++) { + f = order[e] + if (digest[f] == "") continue + printf "%s %s %s\t%s\n", VER, FLAGS, digest[f], disc[f] + } + } + ' "$TMP/edges" >"$DISC_CACHE.new" && mv "$DISC_CACHE.new" "$DISC_CACHE" +fi + +# --- closures --------------------------------------------------------------- +# One shard per file gives each file's own transitive closure, which is both the +# exact input set ShellCheck reads for it and therefore the exact thing its +# cache key has to cover. +if ! awk -v FILES="$TMP/files" -v EDGES="$TMP/edges" -v WORK="" -v JOBS=1 -v MODE=closures \ + -f "$ROOT/bin/fm-lint-plan.awk" >"$TMP/closures" 2>"$TMP/plan.err"; then + # Never silently lint less than the canonical set: if planning fails for any + # reason, fall back to the reference implementation rather than guessing. + printf 'fm-lint.sh: could not plan the lint (%s); falling back to the canonical command.\n' \ + "$(tr -d '\n' <"$TMP/plan.err")" >&2 + rm -rf "$TMP" + exec "$ROOT/bin/fm-lint.sh" --whole-set +fi + +# --- cache lookup ----------------------------------------------------------- +# Each file's cache line is the pinned version, the flags, and the digest of +# every file ShellCheck will actually read for it (itself plus its closure), in +# a stable order. Any byte change anywhere in that set changes the line, so a +# stale result can never be served. The whole thing is two awk passes and one +# hash pass: doing it per file in the shell cost 12s of process spawns on a +# run that had no linting to do at all. +: >"$TMP/work" +: >"$TMP/material" +MANIFEST="$CACHE_DIR/manifest" + +if [ "$USE_CACHE" = 1 ]; then + # Both passes below load their lookup table in BEGIN rather than with the + # usual NR==FNR two-file idiom. That idiom silently inverts when the first + # file is empty - every record of the SECOND file is then read as a lookup + # entry and nothing is emitted. On a cold cache the manifest is empty, which + # made the work set come out empty, which reported all 152 files clean + # without running ShellCheck at all. A lint gate that passes everything is + # far worse than a slow one, so neither pass may depend on that idiom. + awk -v VER="$REQUIRED_SHELLCHECK" -v FLAGS="$LINT_FLAGS" -v HASHES="$TMP/hashes" ' + BEGIN { + while ((getline hl < HASHES) > 0) { + split(hl, hf, " ") + digest[hf[2]] = hf[1] + } + close(HASHES) + } + { + n = split($0, m, " ") + if (n == 0) next + # Sort the digests so shard ordering can never change the line. + for (i = 1; i <= n; i++) { + d = digest[m[i]] + if (d == "") { bad = 1 } + h[i] = d + } + for (i = 2; i <= n; i++) { v = h[i]; j = i - 1 + while (j >= 1 && h[j] > v) { h[j+1] = h[j]; j-- } + h[j+1] = v } + line = VER " " FLAGS + for (i = 1; i <= n; i++) line = line " " h[i] + # A file whose digest is missing is never treated as cacheable. + if (bad) { bad = 0; next } + printf "%s\t%s\n", m[1], line + } + ' "$TMP/closures" >"$TMP/material" +fi + +if [ "$USE_CACHE" != 1 ]; then + cut -d' ' -f1 "$TMP/closures" >"$TMP/work" +else + [ -f "$MANIFEST" ] || : >"$MANIFEST" + # A file is a hit only when its entire line is byte-identical to the line + # recorded by a previous clean run. + awk -F'\t' -v MAN="$MANIFEST" ' + BEGIN { while ((getline ml < MAN) > 0) seen[ml] = 1; close(MAN) } + !($0 in seen) { print $1 } + ' "$TMP/material" >"$TMP/work" + # Anything the material pass could not key (a missing digest) must still be + # linted rather than silently dropped from the run. + cut -d' ' -f1 "$TMP/closures" | sort -u >"$TMP/all-owners" + cut -f1 "$TMP/material" | sort -u >"$TMP/keyed" + comm -23 "$TMP/all-owners" "$TMP/keyed" >>"$TMP/work" + sort -u "$TMP/work" -o "$TMP/work" +fi + +total=$(wc -l <"$TMP/files" | tr -d ' ') +todo=$(wc -l <"$TMP/work" | tr -d ' ') +cached=$((total - todo)) + +if [ "$todo" -eq 0 ]; then + [ -n "${FM_LINT_EMIT_FINDINGS:-}" ] && : >"$FM_LINT_EMIT_FINDINGS" + printf 'fm-lint.sh: clean - all %s files unchanged since their last clean lint.\n' \ + "$total" >&2 + exit 0 +fi + +[ "$JOBS" -le "$todo" ] || JOBS="$todo" +printf 'fm-lint.sh: linting %s of %s files in %s shards (%s cached clean).\n' \ + "$todo" "$total" "$JOBS" "$cached" >&2 + +if ! awk -v FILES="$TMP/files" -v EDGES="$TMP/edges" -v WORK="$TMP/work" -v JOBS="$JOBS" \ + -f "$ROOT/bin/fm-lint-plan.awk" >"$TMP/plan" 2>"$TMP/plan.err"; then + printf 'fm-lint.sh: could not plan the lint (%s); falling back to the canonical command.\n' \ + "$(tr -d '\n' <"$TMP/plan.err")" >&2 + rm -rf "$TMP" + exec "$ROOT/bin/fm-lint.sh" --whole-set +fi + +# --- run the shards in parallel --------------------------------------------- +# --format=gcc emits one finding per line, which is what makes shard outputs +# mergeable and comparable. It selects a rendering; it cannot suppress or +# reclassify a finding. +TAB=$(printf '\t') +pids= +idx=0 +while IFS="$TAB" read -r sid owned members; do + [ -n "$members" ] || continue + idx=$((idx + 1)) + : "$sid" "$owned" + ( + rc=0 + # shellcheck disable=SC2086 + shellcheck $LINT_FLAGS --format=gcc $members >"$TMP/out.$idx" 2>"$TMP/err.$idx" || rc=$? + printf '%s\n' "$rc" >"$TMP/rc.$idx" + ) & + pids="$pids $!" +done <"$TMP/plan" + +for p in $pids; do + wait "$p" || true +done + +# --- collect ---------------------------------------------------------------- +fatal=0 +i=0 +while [ "$i" -lt "$idx" ]; do + i=$((i + 1)) + [ -f "$TMP/rc.$i" ] || { fatal=1; printf 'fm-lint.sh: shard %s produced no exit status.\n' "$i" >&2; continue; } + rc=$(cat "$TMP/rc.$i") + # ShellCheck exits 1 for findings; anything above that is a fatal or usage + # error and must never be reported as a clean lint. + if [ "$rc" -gt 1 ]; then + fatal=1 + printf 'fm-lint.sh: ShellCheck exited %s on shard %s:\n' "$rc" "$i" >&2 + cat "$TMP/err.$i" >&2 2>/dev/null || true + fi +done + +cat "$TMP"/out.* 2>/dev/null | sed '/^$/d' | sort -u >"$TMP/findings" +[ -n "${FM_LINT_EMIT_FINDINGS:-}" ] && cp "$TMP/findings" "$FM_LINT_EMIT_FINDINGS" + +if [ "$fatal" = 1 ]; then + printf 'fm-lint.sh: aborting without a verdict; re-run with --whole-set to reproduce.\n' >&2 + exit 2 +fi + +if [ -s "$TMP/findings" ]; then + # Re-render the affected files through ShellCheck's normal output so a failure + # reads the way a developer expects, giving each file the same closure that + # produced its findings. + cut -d: -f1 "$TMP/findings" | sort -u >"$TMP/failed" + while read -r shard; do + owner=${shard%% *} + grep -Fxq "$owner" "$TMP/failed" || continue + for m in $shard; do printf '%s\n' "$m"; done + done <"$TMP/closures" | sort -u >"$TMP/replay" + # shellcheck disable=SC2046,SC2086 + shellcheck $LINT_FLAGS $(cat "$TMP/replay") || true + printf 'fm-lint.sh: %s finding(s) across %s file(s).\n' \ + "$(wc -l <"$TMP/findings" | tr -d ' ')" "$(wc -l <"$TMP/failed" | tr -d ' ')" >&2 + exit 1 +fi + +# --- record the clean result ------------------------------------------------ +# Every canonical file was either a cache hit or a member of some closed shard, +# and every shard member is analysed against its full closure. So reaching here +# with no findings means every file is clean, and the whole material file is a +# valid manifest. Written atomically so an interrupted run cannot leave a +# half-manifest that would be read as a set of hits. +if [ "$USE_CACHE" = 1 ]; then + cp "$TMP/material" "$MANIFEST.new" && mv "$MANIFEST.new" "$MANIFEST" +fi + +printf 'fm-lint.sh: clean (%s file(s) checked, %s cached).\n' "$todo" "$cached" >&2 +exit 0 diff --git a/tests/fm-lint.test.sh b/tests/fm-lint.test.sh index 81fe0736d40..22bbd7b2d2e 100755 --- a/tests/fm-lint.test.sh +++ b/tests/fm-lint.test.sh @@ -12,6 +12,15 @@ # ShellCheck floated with the runner image and still emitted SC2015, which # ShellCheck retired in 0.11.0. fm-lint.sh now pins one exact version and both # gates resolve it, so command, file set, config, AND version all match. +# +# Second contract, added when fm-lint.sh stopped being one big ShellCheck call: +# the sharded, cached fast path must report EXACTLY the findings the canonical +# whole-set command reports. Speed work on a gate is only safe while that holds, +# so the fixture tests below assert it directly rather than trusting the +# argument for it. One of them pins a real regression caught during that work: +# an empty cache manifest inverted an awk NR==FNR lookup, the work set came out +# empty, and the gate reported all 152 files clean without running ShellCheck at +# all. A lint gate that silently passes everything is worse than a slow one. set -u # shellcheck source=tests/lib.sh @@ -33,6 +42,434 @@ pinned_ready() { [ "$(shellcheck --version | awk '/^version:/ {print $2; exit}')" = "$REQUIRED" ] } +# fm_lint_fixture : build a miniature repo with the canonical layout +# (bin/*.sh, bin/backends/*.sh, tests/*.sh) and a REAL source graph, so the +# sharding and closure logic is exercised rather than mocked. fm-lint.sh +# resolves its root from its own location, so the fixture gets its own copy. +fm_lint_fixture() { + local root=$1 + mkdir -p "$root/bin/backends" "$root/tests" + cp "$LINT" "$root/bin/fm-lint.sh" + cp "$ROOT/bin/fm-lint-plan.awk" "$root/bin/fm-lint-plan.awk" + chmod +x "$root/bin/fm-lint.sh" + cat > "$root/bin/lib-core.sh" <<'SH' +#!/usr/bin/env bash +CORE_READY=1 +core_ready() { printf '%s\n' "$CORE_READY"; } +SH + # Sources a library through a variable path, so ShellCheck can only follow it + # via the source= directive AND only when the target is also an input. + cat > "$root/bin/app.sh" <<'SH' +#!/usr/bin/env bash +set -eu +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=bin/lib-core.sh +. "$SCRIPT_DIR/lib-core.sh" +core_ready +SH + cat > "$root/bin/backends/be.sh" <<'SH' +#!/usr/bin/env bash +be_name() { printf 'be\n'; } +SH + # Exported so the library is clean on its own: an unexported, locally unused + # assignment is itself an SC2034 finding, which would make the "clean" + # fixture dirty for a reason that has nothing to do with what is being tested. + cat > "$root/tests/lib.sh" <<'SH' +#!/usr/bin/env bash +TEST_LIB=1 +export TEST_LIB +SH + cat > "$root/tests/a.test.sh" <<'SH' +#!/usr/bin/env bash +set -eu +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +printf '%s\n' "$TEST_LIB" +SH +} + +# A genuine default-severity finding (SC1007), appended to any fixture file. +fm_lint_plant_defect() { + cat >> "$1" <<'SH' + +planted() { + local x= y= + echo "$x$y" +} +planted +SH +} + +test_fast_path_matches_the_canonical_command() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): fast-path parity check" + return + fi + # The whole justification for sharding is that a file's findings depend only + # on itself plus the transitively sourced files present as input. Assert that + # on a tree that actually HAS findings, in files with different closure + # shapes: a leaf, a sourced library, and an importer of one. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-parity) + fx="$tmp/repo" + fm_lint_fixture "$fx" + fm_lint_plant_defect "$fx/bin/backends/be.sh" + fm_lint_plant_defect "$fx/bin/lib-core.sh" + fm_lint_plant_defect "$fx/tests/a.test.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" --verify-parity 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "sharded fast path disagreed with the canonical whole-set command"$'\n'"$out" + assert_contains "$out" "PARITY OK" "--verify-parity did not confirm parity" + pass "sharded fast path reports exactly the canonical command's findings" +} + +test_cold_cache_never_reports_a_false_clean() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): cold-cache false-clean check" + return + fi + # Regression: with no manifest yet, the cache lookup absorbed every file as a + # cache hit, so the gate exited 0 having linted nothing. A cold cache must + # lint everything and must still fail on a real defect. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-cold) + fx="$tmp/repo" + fm_lint_fixture "$fx" + fm_lint_plant_defect "$fx/bin/app.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache-never-written" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 1 ] || fail "cold-cache run did not fail on a planted defect (exit $rc)"$'\n'"$out" + assert_contains "$out" "SC1007" "cold-cache run did not report the planted finding" + assert_not_contains "$out" "unchanged since their last clean lint" \ + "cold-cache run claimed cached results it could not have had" + pass "a cold cache lints the whole set instead of reporting a false clean" +} + +test_cache_hit_does_not_hide_a_later_defect() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): cache invalidation check" + return + fi + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-cache) + fx="$tmp/repo" + fm_lint_fixture "$fx" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "clean fixture failed its first lint (exit $rc)"$'\n'"$out" + # Second run must be served from the cache, proving the cache is real. + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "second clean run failed (exit $rc)"$'\n'"$out" + assert_contains "$out" "unchanged since their last clean lint" \ + "an unchanged tree was not served from the cache" + # A defect introduced after that clean result must still be caught. + fm_lint_plant_defect "$fx/bin/app.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 1 ] || fail "cached run hid a defect introduced after the cache was written (exit $rc)"$'\n'"$out" + assert_contains "$out" "SC1007" "cached run did not report the new finding" + pass "a cache hit never hides a defect introduced after it was recorded" +} + +test_cache_key_covers_the_source_closure() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): closure invalidation check" + return + fi + # A file is analysed with its sourced libraries inlined, so its cached result + # is only valid while those libraries are byte-identical too. Editing a + # library must put its importers back into the work set, not just itself. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-closure) + fx="$tmp/repo" + fm_lint_fixture "$fx" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "clean fixture failed its first lint (exit $rc)"$'\n'"$out" + printf '\nTEST_LIB_EXTRA=2\nexport TEST_LIB_EXTRA\n' >> "$fx/tests/lib.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "closure re-lint failed unexpectedly (exit $rc)"$'\n'"$out" + # tests/lib.sh itself plus its only importer, tests/a.test.sh. + assert_contains "$out" "linting 2 of" \ + "editing a sourced library did not invalidate its importer's cached result" + pass "a cached result is invalidated by a change anywhere in its source closure" +} + +test_literal_source_path_is_a_closure_edge() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): literal-source closure check" + return + fi + # ShellCheck (without -x) also follows a plain `source ` whose target + # is a variable-free literal path naming another input file - no source= + # directive involved. The planner must model that edge too, or the sharded + # path would emit an SC1091 the whole-set command does not, and a change to + # the sourced library would not invalidate its importer's cached result. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-litsrc) + fx="$tmp/repo" + fm_lint_fixture "$fx" + cat > "$fx/bin/lit-lib.sh" <<'SH' +#!/usr/bin/env bash +LIT_READY=1 +export LIT_READY +SH + cat > "$fx/bin/lit-app.sh" <<'SH' +#!/usr/bin/env bash +set -eu +source bin/lit-lib.sh +printf '%s\n' "$LIT_READY" +SH + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache-parity" "$fx/bin/fm-lint.sh" --verify-parity 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "a literal source path broke fast-path parity"$'\n'"$out" + assert_contains "$out" "PARITY OK" "--verify-parity did not confirm parity with a literal source edge" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "literal-source fixture failed its first lint (exit $rc)"$'\n'"$out" + printf '\nLIT_EXTRA=2\nexport LIT_EXTRA\n' >> "$fx/bin/lit-lib.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "literal-source re-lint failed unexpectedly (exit $rc)"$'\n'"$out" + # bin/lit-lib.sh itself plus its only importer, bin/lit-app.sh. + assert_contains "$out" "linting 2 of" \ + "editing a literal-sourced library did not invalidate its importer's cached result" + pass "a literal source path is a closure edge for parity and cache invalidation" +} + +test_guarded_literal_source_is_a_closure_edge() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): guarded-source closure check" + return + fi + # A literal source guarded behind a top-level separator ('[ -f x ] && . x') + # is still followed by ShellCheck, so it must still be a closure edge: parity + # must hold and editing the library must invalidate the importer. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-guardsrc) + fx="$tmp/repo" + fm_lint_fixture "$fx" + cat > "$fx/bin/guard-app.sh" <<'SH' +#!/usr/bin/env bash +set -eu +[ -f bin/lib-core.sh ] && . bin/lib-core.sh +core_ready +SH + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache-parity" "$fx/bin/fm-lint.sh" --verify-parity 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "a guarded literal source broke fast-path parity"$'\n'"$out" + assert_contains "$out" "PARITY OK" "--verify-parity did not confirm parity with a guarded source edge" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "guarded-source fixture failed its first lint (exit $rc)"$'\n'"$out" + printf '\nCORE_EXTRA=2\nexport CORE_EXTRA\n' >> "$fx/bin/lib-core.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "guarded-source re-lint failed unexpectedly (exit $rc)"$'\n'"$out" + # bin/lib-core.sh itself plus both importers: bin/app.sh (directive) and + # bin/guard-app.sh (guarded literal). + assert_contains "$out" "linting 3 of" \ + "editing a guarded-sourced library did not invalidate its importer's cached result" + pass "a guarded literal source is a closure edge for parity and cache invalidation" +} + +test_source_in_any_command_position_is_a_closure_edge() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): command-position source closure check" + return + fi + # ShellCheck follows a literal source wherever it sits in command position, + # not just at line start or after ';', '&&', '||'. Verified on the pinned + # version: 'then . lib', a subshell '( . lib', a 'function f { . lib; }' + # body and a redirection-prefixed '>file . lib' are all followed when the + # target is an input, so each must be a closure edge - otherwise a shard + # split emits an SC1091 the whole-set run does not, and editing the library + # never invalidates the importer's cached clean result. The discovery pass + # harvests these from ShellCheck itself, so no position list is maintained. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-anypos) + fx="$tmp/repo" + fm_lint_fixture "$fx" + cat > "$fx/bin/then-app.sh" <<'SH' +#!/usr/bin/env bash +set -eu +if true; then . bin/lib-core.sh; fi +core_ready +SH + cat > "$fx/bin/sub-app.sh" <<'SH' +#!/usr/bin/env bash +set -eu +( . bin/lib-core.sh; core_ready ) +SH + cat > "$fx/bin/fn-app.sh" <<'SH' +#!/usr/bin/env bash +set -eu +function fn_ready { . bin/lib-core.sh; } +fn_ready +SH + cat > "$fx/bin/redir-app.sh" <<'SH' +#!/usr/bin/env bash +set -eu +>/dev/null . bin/lib-core.sh +core_ready +SH + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache-parity" "$fx/bin/fm-lint.sh" --verify-parity 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "a keyword- or subshell-position source broke fast-path parity"$'\n'"$out" + assert_contains "$out" "PARITY OK" "--verify-parity did not confirm parity with command-position source edges" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "command-position source fixture failed its first lint (exit $rc)"$'\n'"$out" + printf '\nCORE_EXTRA=2\nexport CORE_EXTRA\n' >> "$fx/bin/lib-core.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "command-position source re-lint failed unexpectedly (exit $rc)"$'\n'"$out" + # bin/lib-core.sh itself plus all five importers: bin/app.sh (directive), + # bin/then-app.sh, bin/sub-app.sh, bin/fn-app.sh and bin/redir-app.sh + # (command-position literals). + assert_contains "$out" "linting 6 of" \ + "editing the library did not invalidate its command-position importers" + pass "a literal source in any command position is a closure edge" +} + +test_disabled_sc1091_source_is_still_a_closure_edge() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): suppressed-SC1091 closure check" + return + fi + # An in-file disable of SC1091 suppresses the very note the discovery pass + # reads. Discovery therefore rewrites SC1091 inside directive lines on its + # input stream, so the note still surfaces and the edge is still found: + # no whole-set fallback, and editing the library re-lints the suppressing + # importer. The directive is assembled at runtime so this test file never + # carries a file-wide suppression itself. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-dis1091) + fx="$tmp/repo" + fm_lint_fixture "$fx" + { + printf '#!/usr/bin/env bash\n' + printf '# shellcheck %s\n' 'disable=SC1091' + printf 'set -eu\n. bin/lib-core.sh\ncore_ready\n' + } > "$fx/bin/dis-app.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "suppressed-SC1091 fixture failed its first lint (exit $rc)"$'\n'"$out" + assert_not_contains "$out" "falling back to the canonical command" \ + "a plain disable=SC1091 must not force the whole-set fallback" + printf '\nCORE_EXTRA=2\nexport CORE_EXTRA\n' >> "$fx/bin/lib-core.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "suppressed-SC1091 re-lint failed unexpectedly (exit $rc)"$'\n'"$out" + # bin/lib-core.sh itself plus both importers: bin/app.sh (directive) and + # bin/dis-app.sh (literal source under a file-wide SC1091 suppression). + assert_contains "$out" "linting 3 of" \ + "a disable=SC1091 importer was not re-linted after its library changed" + pass "a source suppressed by disable=SC1091 is still a closure edge" +} + +test_unclassifiable_disable_falls_back_to_whole_set() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): unclassifiable-disable fallback check" + return + fi + # 'disable=all' (or an SCnnnn-SCnnnn range) can suppress SC1091 in a form + # the discovery rewrite does not recognise, which would hide edges silently; + # the planner must force the whole-set fallback instead. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-disall) + fx="$tmp/repo" + fm_lint_fixture "$fx" + { + printf '#!/usr/bin/env bash\nset -eu\n' + printf '# shellcheck %s\n' 'disable=all' + printf 'true\n' + } > "$fx/bin/da-app.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "whole-set fallback run failed on a clean fixture (exit $rc)"$'\n'"$out" + assert_contains "$out" "falling back to the canonical command" \ + "an unclassifiable disable= item did not trigger the whole-set fallback" + assert_contains "$out" "da-app.sh" "the fallback message did not name the offending file" + pass "an unclassifiable disable= item falls back to the whole-set command" +} + +test_verify_parity_fails_when_fast_path_falls_back() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): vacuous-parity regression check" + return + fi + # Regression: the inner fast-path run's output was discarded, so a planner + # tripwire made it fall back to the whole-set command, no findings file was + # emitted, and verify-parity compared the reference against a fabricated + # empty file - reporting PARITY OK without the fast path ever executing. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-vacuous) + fx="$tmp/repo" + fm_lint_fixture "$fx" + { + printf '#!/usr/bin/env bash\nset -eu\n' + printf '# shellcheck %s\n' 'source-path=bin' + printf 'true\n' + } > "$fx/bin/sp-app.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" --verify-parity 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "--verify-parity reported success without a sharded fast-path run"$'\n'"$out" + assert_contains "$out" "PARITY BROKEN" "--verify-parity did not report the vacuous run as a failure" + assert_contains "$out" "falling back to the canonical command" \ + "--verify-parity hid the inner run's stderr naming the cause" + assert_not_contains "$out" "PARITY OK" "--verify-parity claimed parity it never measured" + pass "--verify-parity fails when the fast path falls back instead of sharding" +} + +test_exec_lines_carry_exactly_lint_flags() { + # --verify-parity re-spells the file set with $LINT_FLAGS and never executes + # the literal `exec shellcheck` commands, so it cannot detect drift between + # the two. This assertion is the actual guard: every literal exec line must + # carry exactly the flags LINT_FLAGS holds. + local flags mismatch + flags=$(sed -n "s/^LINT_FLAGS='\(.*\)'$/\1/p" "$LINT") + [ -n "$flags" ] || fail "could not read LINT_FLAGS from bin/fm-lint.sh" + mismatch=$(awk -v want="$flags" ' + /exec shellcheck/ { + got = "" + for (i = 1; i <= NF; i++) if ($i ~ /^-/) got = got (got == "" ? "" : " ") $i + if (got != want) print FNR ": " $0 + } + ' "$LINT") + [ -z "$mismatch" ] || fail "exec shellcheck flags drifted from LINT_FLAGS ($flags):"$'\n'"$mismatch" + pass "the literal exec shellcheck commands carry exactly LINT_FLAGS" +} + +test_unmodelled_directive_falls_back_to_whole_set() { + if ! pinned_ready; then + pass "SKIP (ShellCheck $REQUIRED not resolved): unmodelled-directive fallback check" + return + fi + # The planner does not model ShellCheck's search-path resolution, so a + # directive key it does not understand must force the whole-set fallback + # rather than risk a silently wrong shard plan. The directive line is + # assembled at runtime so this test file never carries it verbatim. + local tmp fx out rc + tmp=$(fm_test_tmproot fm-lint-srcpath) + fx="$tmp/repo" + fm_lint_fixture "$fx" + { + printf '#!/usr/bin/env bash\nset -eu\n' + printf '# shellcheck %s\n' 'source-path=bin' + printf 'true\n' + } > "$fx/bin/sp-app.sh" + rc=0 + out=$(FM_LINT_CACHE_DIR="$tmp/cache" "$fx/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "whole-set fallback run failed on a clean fixture (exit $rc)"$'\n'"$out" + assert_contains "$out" "falling back to the canonical command" \ + "an unmodelled shellcheck directive did not trigger the whole-set fallback" + assert_contains "$out" "sp-app.sh" "the fallback message did not name the offending file" + pass "an unmodelled shellcheck directive falls back to the whole-set command" +} + test_owner_exists_and_executable() { assert_present "$LINT" "bin/fm-lint.sh is missing" [ -x "$LINT" ] || fail "bin/fm-lint.sh must be executable so CI/gate can run it directly" @@ -190,3 +627,15 @@ test_rejects_wrong_shellcheck_version test_catches_a_real_lint_defect test_ignores_ambient_shellcheck_opts test_clean_fixture_passes +test_fast_path_matches_the_canonical_command +test_cold_cache_never_reports_a_false_clean +test_cache_hit_does_not_hide_a_later_defect +test_cache_key_covers_the_source_closure +test_literal_source_path_is_a_closure_edge +test_guarded_literal_source_is_a_closure_edge +test_source_in_any_command_position_is_a_closure_edge +test_disabled_sc1091_source_is_still_a_closure_edge +test_unclassifiable_disable_falls_back_to_whole_set +test_verify_parity_fails_when_fast_path_falls_back +test_exec_lines_carry_exactly_lint_flags +test_unmodelled_directive_falls_back_to_whole_set