From 31be062e2b365ed5b36d0bd34ce0283e0b3fdf29 Mon Sep 17 00:00:00 2001 From: fujibee Date: Tue, 8 Sep 2026 11:19:11 -0700 Subject: [PATCH 1/2] ci: run shellcheck over every tracked .sh, ratcheted against a recorded baseline No linter ran over the 97 tracked shell scripts in CI, so every "shellcheck clean" was a claim about an unknown local setup -- once made on a machine with no shellcheck at all, where no output read as no findings (#1068). The checker counts every finding, all severities, over `git ls-files '*.sh'` and compares with .github/shellcheck-baseline, which records the shellcheck version the count was measured with and the count: 0.10.0 and 159 on main (error 7, warning 39, note 113; -x changes nothing because the sourced paths are variables). Above the baseline is exit 1 with the findings listed; below it is exit 1 asking for the baseline to be lowered; at it is exit 0. It is exit 2, never 0, when shellcheck is missing, when the version that ran is not the one the baseline names (counts from different versions are not comparable), when no tracked .sh exists, or when the baseline cannot be read. The version and path of the tool that ran are the first line of every run. The workflow pins shellcheck 0.10.0 by version and by the tarball's sha256, and runs the checker's --positive-control before the verdict: a throwaway repository with one broken script and a zero baseline must produce exit 1 from the checker itself, with the same binary. A job that has never gone red is indistinguishable from one that never ran. The checker's own fixture writes a literal `$` and carries an inline disable with its reason, which is the policy for a rule broken on purpose; without it, tracking the script would have moved the count to 160. tests/test_check_shellcheck.bats pins the ratchet in all three directions, the version attribution, each exit-2 case, the positive control, the baseline file's shape, and the workflow's pin, digest and step order; the tests skip visibly where no binary is available. --- .github/scripts/check-shellcheck.sh | 137 +++++++++++++++++++++ .github/shellcheck-baseline | 2 + .github/workflows/shellcheck.yml | 59 +++++++++ tests/test_check_shellcheck.bats | 183 ++++++++++++++++++++++++++++ 4 files changed, 381 insertions(+) create mode 100755 .github/scripts/check-shellcheck.sh create mode 100644 .github/shellcheck-baseline create mode 100644 .github/workflows/shellcheck.yml create mode 100644 tests/test_check_shellcheck.bats diff --git a/.github/scripts/check-shellcheck.sh b/.github/scripts/check-shellcheck.sh new file mode 100755 index 000000000..8c54a8c0e --- /dev/null +++ b/.github/scripts/check-shellcheck.sh @@ -0,0 +1,137 @@ +#!/usr/bin/env bash +# +# Fail when shellcheck finds more in the tracked .sh files than the baseline. +# +# No linter ran over the shell scripts in CI (#1068), so every "shellcheck +# clean" ever claimed was made on an unknown local setup -- and once, on a +# machine with no shellcheck at all, where "no output" read as "no findings". +# This runs it in one known place, and the claim it makes is attributable: +# the shellcheck version that ran is printed first, and the baseline is only +# valid for the version it was measured with. +# +# Same shape as check-enforced-assertions: the baseline is a COUNT recorded in +# the repository, it may only go down, and lowering it is the burn-down. All +# severities count. Gating at `error` would hide every warning and note for +# good, and that is where the real bash bugs in this tree live (unquoted +# expansions, `$?` after a pipeline, `read` without `-r`). A rule broken on +# purpose gets an inline `# shellcheck disable=SCxxxx` with a reason, not a +# global exclusion. +# +# What it refuses to be green about (exit 2, never 0): +# - shellcheck is not there, or fails for a reason other than findings +# - the shellcheck that ran is not the version the baseline was measured +# with -- counts from different versions are not comparable +# - no tracked .sh files were found: scanning nothing is not a pass +# - the baseline file cannot be read +# +# Usage: +# check-shellcheck.sh check the tree this script lives in +# check-shellcheck.sh --positive-control prove the checker fires: build a +# throwaway repository with one +# broken script and a zero baseline, +# and require exit 1 from itself +# +# Environment: +# SHELLCHECK the binary to run (default: `shellcheck` on PATH) +# AGMSG_SHELLCHECK_ROOT the git tree to scan (default: this checkout) +# AGMSG_SHELLCHECK_BASELINE the baseline file (default: /.github/shellcheck-baseline) +# +# Baseline file: two lines -- +# +# + +set -u + +ME=check-shellcheck +SELF="$(cd "$(dirname "$0")" && pwd)/$(basename "$0")" +ROOT="${AGMSG_SHELLCHECK_ROOT:-$(cd "$(dirname "$0")/../.." && pwd)}" +BASELINE_FILE="${AGMSG_SHELLCHECK_BASELINE:-$ROOT/.github/shellcheck-baseline}" +SHELLCHECK="${SHELLCHECK:-shellcheck}" + +# --- the tool must demonstrably run ----------------------------------------------- +if ! version="$("$SHELLCHECK" --version 2>/dev/null | sed -n 's/^version: *//p')" || [ -z "$version" ]; then + echo "$ME: cannot run '$SHELLCHECK --version'; no shellcheck, no claim." >&2 + exit 2 +fi +echo "$ME: shellcheck $version ($SHELLCHECK)" + +# --- positive control --------------------------------------------------------------- +if [ "${1-}" = "--positive-control" ]; then + tmp="$(mktemp -d)" + git init -q "$tmp" + # The `$` must reach the fixture literally: it is the unquoted expansion the + # control expects shellcheck to flag (SC2086 at broken.sh:3:6). + # shellcheck disable=SC2016 + printf '#!/bin/bash\nfoo=$1\necho $foo\n' > "$tmp/broken.sh" + git -C "$tmp" add broken.sh + printf '%s\n0\n' "$version" > "$tmp/baseline" + out="$(AGMSG_SHELLCHECK_ROOT="$tmp" AGMSG_SHELLCHECK_BASELINE="$tmp/baseline" SHELLCHECK="$SHELLCHECK" "$SELF" 2>&1)" + rc=$? + rm -f "$tmp/broken.sh" "$tmp/baseline" + rm -rf "$tmp/.git" + rmdir "$tmp" 2>/dev/null || true + if [ "$rc" -eq 1 ] && printf '%s\n' "$out" | grep -q 'broken.sh:3:6: '; then + echo "$ME: positive control fired -- a broken script against a zero baseline is exit 1, naming the finding." + exit 0 + fi + echo "$ME: positive control did NOT fire (exit $rc); the checker cannot be trusted to go red." >&2 + printf '%s\n' "$out" | sed 's/^/ /' >&2 + exit 2 +fi + +# --- the baseline ------------------------------------------------------------------- +if [ ! -r "$BASELINE_FILE" ]; then + echo "$ME: no readable baseline at $BASELINE_FILE" >&2 + exit 2 +fi +baseline_version="$(sed -n '1p' "$BASELINE_FILE" | tr -d '[:space:]')" +baseline="$(sed -n '2p' "$BASELINE_FILE" | tr -d '[:space:]')" +case "$baseline" in + ''|*[!0-9]*) + echo "$ME: $BASELINE_FILE line 2 must be the finding count (got '$baseline')" >&2 + exit 2 ;; +esac +if [ "$baseline_version" != "$version" ]; then + echo "$ME: the baseline ($baseline findings) was measured with shellcheck $baseline_version, but $version ran." >&2 + echo "Counts from different versions are not comparable. Re-measure with $version and record both lines," >&2 + echo "or run the version the baseline names." >&2 + exit 2 +fi + +# --- the files ---------------------------------------------------------------------- +files=() +while IFS= read -r -d '' f; do files+=("$f"); done < <(git -C "$ROOT" ls-files -z -- '*.sh' 2>/dev/null) +if [ "${#files[@]}" -eq 0 ]; then + echo "$ME: no tracked .sh files under $ROOT; this is not a clean tree, it is an empty scan." >&2 + exit 2 +fi + +# --- the count ---------------------------------------------------------------------- +listing="$(cd "$ROOT" && "$SHELLCHECK" -f gcc "${files[@]}" 2>&1)" +rc=$? +if [ "$rc" -ne 0 ] && [ "$rc" -ne 1 ]; then + echo "$ME: shellcheck exited $rc, which is a tool failure, not a verdict:" >&2 + printf '%s\n' "$listing" | head -20 | sed 's/^/ /' >&2 + exit 2 +fi +findings="$(printf '%s\n' "$listing" | grep -cE '^[^:]+:[0-9]+:[0-9]+: (error|warning|note|style): ')" + +echo "$ME: ${#files[@]} tracked .sh files, $findings findings, baseline $baseline (shellcheck $version)." + +if [ "$findings" -gt "$baseline" ]; then + echo "$ME: $findings findings, above the baseline of $baseline." >&2 + echo >&2 + printf '%s\n' "$listing" | grep -E '^[^:]+:[0-9]+:[0-9]+: (error|warning|note|style): ' | sed 's/^/ /' >&2 + echo >&2 + echo "Fix the new findings, or disable the rule inline with a reason (# shellcheck disable=SCxxxx)." >&2 + echo "The baseline in $BASELINE_FILE only goes down." >&2 + exit 1 +fi + +if [ "$findings" -lt "$baseline" ]; then + echo "$ME: $findings findings, below the baseline of $baseline." + echo "Lower line 2 of $BASELINE_FILE to $findings so it cannot drift back up." + exit 1 +fi + +echo "$ME: at the baseline." diff --git a/.github/shellcheck-baseline b/.github/shellcheck-baseline new file mode 100644 index 000000000..6b5602d4e --- /dev/null +++ b/.github/shellcheck-baseline @@ -0,0 +1,2 @@ +0.10.0 +159 diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml new file mode 100644 index 000000000..d9590a53c --- /dev/null +++ b/.github/workflows/shellcheck.yml @@ -0,0 +1,59 @@ +# shellcheck over every tracked .sh, ratcheted against a recorded baseline. +# +# Until this job existed no linter ran over the shell scripts in CI, so every +# "shellcheck clean" was a claim about an unknown local setup -- once made on +# a machine that had no shellcheck at all (#1068). Here the tool is pinned by +# version and digest, its version is printed by the check itself, and before +# the tree is judged the job proves the checker fires on a deliberately broken +# script. A job that has never gone red is indistinguishable from one that +# never ran; this one goes red on purpose first. +# +# The baseline (.github/shellcheck-baseline: version, then count) only goes +# down. Lowering it is a separate PR per area; see the script's header. +# +# Same trigger set as verify-versions: main and every integration branch, +# on push and on pull_request. + +name: shellcheck + +on: + push: + branches: [main, 'integration/**'] + pull_request: + branches: [main, 'integration/**'] + +permissions: + contents: read + +env: + # Pinned three ways: the version is what the baseline was measured with, + # the URL is derived from it, and the digest is the measured sha256 of that + # exact tarball (2026-09-08). Bumping the version means re-measuring the + # baseline in the same change. + SHELLCHECK_VERSION: '0.10.0' + SHELLCHECK_SHA256: '6c881ab0698e4e6ea235245f22832860544f17ba386442fe7e9d629f8cbedf87' + +jobs: + check: + name: shellcheck at or below the baseline + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + + - name: Fetch the pinned shellcheck and verify its digest + run: | + set -eu + curl -sSL -o shellcheck.tar.xz \ + "https://github.com/koalaman/shellcheck/releases/download/v${SHELLCHECK_VERSION}/shellcheck-v${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" + echo "${SHELLCHECK_SHA256} shellcheck.tar.xz" | sha256sum -c - + tar -xJf shellcheck.tar.xz + echo "SHELLCHECK=${GITHUB_WORKSPACE}/shellcheck-v${SHELLCHECK_VERSION}/shellcheck" >> "$GITHUB_ENV" + + # The control runs BEFORE the verdict, with the same binary: a green + # verdict from a checker that was never shown to go red proves nothing. + - name: Prove the checker fires on a broken script + run: .github/scripts/check-shellcheck.sh --positive-control + + - name: No new shellcheck findings above the baseline + run: .github/scripts/check-shellcheck.sh diff --git a/tests/test_check_shellcheck.bats b/tests/test_check_shellcheck.bats new file mode 100644 index 000000000..dedb465d4 --- /dev/null +++ b/tests/test_check_shellcheck.bats @@ -0,0 +1,183 @@ +#!/usr/bin/env bats +# +# .github/scripts/check-shellcheck.sh: the tracked .sh files must not carry +# more shellcheck findings than the recorded baseline, and the checker must +# refuse to be green when the tool did not demonstrably run. Pinned here, each +# with a control the other way: +# +# - the RATCHET: above the baseline is red and names the findings, at it is +# green, below it is red and asks for the baseline to be lowered; +# - the ATTRIBUTION: the version that ran is printed, and a baseline measured +# with another version is exit 2, not a comparison; +# - the ZERO-TARGET answer: no shellcheck, no .sh files, no baseline -- exit 2; +# - the POSITIVE CONTROL: the checker can prove, on demand, that it fires. +# +# These need a shellcheck binary. Where there is none the tests skip -- visibly, +# as `ok # skip`, never as a silent green -- and the CI job that matters runs +# the checker with a pinned binary regardless (see .github/workflows/shellcheck.yml). + +setup() { + load 'test_helper' + REPO_ROOT="$(cd "$BATS_TEST_DIRNAME/.." && pwd)" + CHECK="$REPO_ROOT/.github/scripts/check-shellcheck.sh" + export SHELLCHECK="${SHELLCHECK:-shellcheck}" + if ! "$SHELLCHECK" --version >/dev/null 2>&1; then + SC_VERSION="" + else + SC_VERSION="$("$SHELLCHECK" --version | sed -n 's/^version: *//p')" + fi +} + +_need_shellcheck() { + [ -n "$SC_VERSION" ] || skip "no shellcheck binary (set SHELLCHECK=); the CI job pins one" +} + +# A throwaway git tree holding the given .sh files. Prints its path. +_tree() { # =... + local d="$BATS_TEST_TMPDIR/tree-$RANDOM" spec + git init -q "$d" + for spec in "$@"; do + printf '%b' "${spec#*=}" > "$d/${spec%%=*}" + git -C "$d" add "${spec%%=*}" + done + printf '%s' "$d" +} + +_baseline() { # -> path + local f="$BATS_TEST_TMPDIR/baseline-$RANDOM" + printf '%s\n%s\n' "$1" "$2" > "$f" + printf '%s' "$f" +} + +BROKEN='#!/bin/bash\nfoo=$1\necho $foo\n' +CLEAN='#!/bin/bash\nfoo="$1"\necho "$foo"\n' + +# --- the ratchet -------------------------------------------------------------------- + +@test "above the baseline is red and names the finding" { + _need_shellcheck + local d; d="$(_tree "broken.sh=$BROKEN")" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 0)" run bash "$CHECK" + [ "$status" -eq 1 ] + grep -q 'above the baseline' <<<"$output" + grep -q 'broken.sh:3:6: ' <<<"$output" +} + +@test "at the baseline is green, and says how many files and which version" { + _need_shellcheck + local d; d="$(_tree "broken.sh=$BROKEN" "clean.sh=$CLEAN")" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 1)" run bash "$CHECK" + [ "$status" -eq 0 ] + grep -q '2 tracked .sh files, 1 findings, baseline 1' <<<"$output" + grep -q "shellcheck $SC_VERSION" <<<"$output" + grep -q 'at the baseline' <<<"$output" +} + +@test "below the baseline is red and asks for the baseline to be lowered" { + _need_shellcheck + local d; d="$(_tree "clean.sh=$CLEAN")" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 3)" run bash "$CHECK" + [ "$status" -eq 1 ] + grep -q 'below the baseline' <<<"$output" + grep -q 'Lower line 2' <<<"$output" +} + +# --- attribution ---------------------------------------------------------------------- + +@test "a baseline measured with another shellcheck version is exit 2, not a comparison" { + # Differential pair: same tree, same count, only the version line differs. + _need_shellcheck + local d; d="$(_tree "clean.sh=$CLEAN")" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "0.0.1" 0)" run bash "$CHECK" + [ "$status" -eq 2 ] + grep -q 'not comparable' <<<"$output" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 0)" run bash "$CHECK" + [ "$status" -eq 0 ] +} + +@test "the version that ran is the first thing printed, in every outcome" { + _need_shellcheck + local d; d="$(_tree "clean.sh=$CLEAN")" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 0)" run bash "$CHECK" + [ "${lines[0]}" = "check-shellcheck: shellcheck $SC_VERSION ($SHELLCHECK)" ] + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 5)" run bash "$CHECK" + [ "${lines[0]}" = "check-shellcheck: shellcheck $SC_VERSION ($SHELLCHECK)" ] +} + +# --- zero targets are not a pass ------------------------------------------------------ + +@test "no shellcheck binary is exit 2 -- an absent tool is not a clean tree" { + # This is the failure that motivated the job: on a machine without the + # binary, 'no output' read as '0 findings'. + local d; d="$(_tree "broken.sh=$BROKEN")" + SHELLCHECK=/nonexistent/shellcheck AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "0.10.0" 0)" run bash "$CHECK" + [ "$status" -eq 2 ] + grep -q 'no shellcheck, no claim' <<<"$output" +} + +@test "a tree with no tracked .sh files is exit 2, not green" { + _need_shellcheck + local d; d="$(_tree "notes.txt=hello\n")" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 0)" run bash "$CHECK" + [ "$status" -eq 2 ] + grep -q 'empty scan' <<<"$output" +} + +@test "an untracked .sh file is not scanned: the scope is what git tracks" { + _need_shellcheck + local d; d="$(_tree "clean.sh=$CLEAN")" + printf '%b' "$BROKEN" > "$d/untracked.sh" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 0)" run bash "$CHECK" + [ "$status" -eq 0 ] +} + +@test "a missing or malformed baseline is exit 2" { + _need_shellcheck + local d; d="$(_tree "clean.sh=$CLEAN")" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$BATS_TEST_TMPDIR/missing" run bash "$CHECK" + [ "$status" -eq 2 ] + printf '%s\nmany\n' "$SC_VERSION" > "$BATS_TEST_TMPDIR/bad" + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$BATS_TEST_TMPDIR/bad" run bash "$CHECK" + [ "$status" -eq 2 ] +} + +# --- the positive control --------------------------------------------------------------- + +@test "--positive-control proves the checker fires, and fails loudly when it cannot" { + _need_shellcheck + run bash "$CHECK" --positive-control + [ "$status" -eq 0 ] + grep -q 'positive control fired' <<<"$output" + # Without a working tool the control must not report success either. + SHELLCHECK=/nonexistent/shellcheck run bash "$CHECK" --positive-control + [ "$status" -eq 2 ] +} + +# --- the repository's own baseline and workflow ------------------------------------------ + +@test "the repository baseline has the shape the checker reads: a version, then a count" { + local f="$REPO_ROOT/.github/shellcheck-baseline" + [ "$(wc -l < "$f" | tr -d ' ')" -eq 2 ] + sed -n '1p' "$f" | grep -Eq '^[0-9]+\.[0-9]+\.[0-9]+$' + sed -n '2p' "$f" | grep -Eq '^[0-9]+$' +} + +@test "the workflow pins the version the baseline names, verifies a digest, and runs the control before the verdict" { + local wf="$REPO_ROOT/.github/workflows/shellcheck.yml" pinned + pinned="$(sed -nE "s/^ *SHELLCHECK_VERSION: *'([^']+)'.*/\1/p" "$wf")" + [ "$pinned" = "$(sed -n '1p' "$REPO_ROOT/.github/shellcheck-baseline")" ] + grep -Eq "SHELLCHECK_SHA256: *'[0-9a-f]{64}'" "$wf" + grep -q 'sha256sum -c' "$wf" + # Order: the control step must come before the verdict step. + [ "$(grep -n 'check-shellcheck.sh --positive-control' "$wf" | cut -d: -f1)" -lt "$(grep -n 'run: .github/scripts/check-shellcheck.sh$' "$wf" | cut -d: -f1)" ] +} + +@test "the recorded baseline is what shellcheck measures on this tree (when the pinned version is available)" { + # The one test that reads the real tree. It only runs with the pinned + # version, since any other version's count is not comparable. + _need_shellcheck + [ "$SC_VERSION" = "$(sed -n '1p' "$REPO_ROOT/.github/shellcheck-baseline")" ] || skip "shellcheck $SC_VERSION is not the pinned version" + run bash "$CHECK" + [ "$status" -eq 0 ] + grep -q 'at the baseline' <<<"$output" +} From f369debae42e707da69e78ecb33d50b4af8cc0ef Mon Sep 17 00:00:00 2001 From: fujibee Date: Tue, 8 Sep 2026 12:53:24 -0700 Subject: [PATCH 2/2] fix(ci): count shellcheck findings from its structured output, not a list of severity words Review asked whether the finding regex -- error|warning|note|style -- had dropped `info`, and whether the baseline of 159 was therefore low. Measured with shellcheck 0.10.0 on main: the gcc text formatter prints exactly three level words (error 7, warning 39, note 113 -- it spells both info and style as `note`), so the regex matched all 159 lines, and `-f json1` reports the same 159 as error 7 / warning 39 / info 100 / style 13. The count was right and the baseline is unchanged; the way it was taken was a proxy, and a proxy can miss a level a formatter spells differently. The count is now the number of comments in `-f json1`, and the listing on a red is rendered from the same structure with the real level names, so an info finding is visible as `info`. The positive control requires three things from the checker: exit 1, the finding at its position, and its level spelled `info` -- the level a hand-listed set loses first. A new control runs a tree with one finding of each level and requires the checker's count to equal shellcheck's own json count for the same files. Calibrated: ignoring findings, or dropping the info level from the count, each turn the positive control into exit 2 and redden the tests. Also found by the tool itself: a comment line beginning with its name followed by a word is parsed as a directive, and a malformed one is a finding. Reworded. --- .github/scripts/check-shellcheck.sh | 45 ++++++++++++++++++++++++----- tests/test_check_shellcheck.bats | 27 +++++++++++++++-- 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/.github/scripts/check-shellcheck.sh b/.github/scripts/check-shellcheck.sh index 8c54a8c0e..5a05273e4 100755 --- a/.github/scripts/check-shellcheck.sh +++ b/.github/scripts/check-shellcheck.sh @@ -11,7 +11,8 @@ # # Same shape as check-enforced-assertions: the baseline is a COUNT recorded in # the repository, it may only go down, and lowering it is the burn-down. All -# severities count. Gating at `error` would hide every warning and note for +# levels count -- error, warning, info and style, as shellcheck's structured +# output names them (measured on main, 0.10.0: 7 / 39 / 100 / 13 = 159). Gating at `error` would hide every warning and note for # good, and that is where the real bash bugs in this tree live (unquoted # expansions, `$?` after a pipeline, `read` without `-r`). A rule broken on # purpose gets an inline `# shellcheck disable=SCxxxx` with a reason, not a @@ -70,8 +71,13 @@ if [ "${1-}" = "--positive-control" ]; then rm -f "$tmp/broken.sh" "$tmp/baseline" rm -rf "$tmp/.git" rmdir "$tmp" 2>/dev/null || true - if [ "$rc" -eq 1 ] && printf '%s\n' "$out" | grep -q 'broken.sh:3:6: '; then - echo "$ME: positive control fired -- a broken script against a zero baseline is exit 1, naming the finding." + # Three things, all required: the checker's own exit 1 (not merely a line + # of text), the finding named at its position, and its level spelled the + # way the tool spells it -- SC2086 is `info`, the level most easily lost. + # (A comment line must not begin with the tool's name followed by a word: + # that is parsed as a directive, and a malformed one is itself a finding.) + if [ "$rc" -eq 1 ] && printf '%s\n' "$out" | grep -q 'broken.sh:3:6: info: .*\[SC2086\]'; then + echo "$ME: positive control fired -- a broken script against a zero baseline is exit 1, naming the info-level finding." exit 0 fi echo "$ME: positive control did NOT fire (exit $rc); the checker cannot be trusted to go red." >&2 @@ -107,21 +113,46 @@ if [ "${#files[@]}" -eq 0 ]; then fi # --- the count ---------------------------------------------------------------------- -listing="$(cd "$ROOT" && "$SHELLCHECK" -f gcc "${files[@]}" 2>&1)" +# +# Counted from shellcheck's structured output, not from a regex over its text: +# `-f json1` lists one comment per finding with its real level. The text +# formatters rename levels (measured, 0.10.0: `-f gcc` prints both `info` and +# `style` as `note`), so a hand-listed alternation of severity words is a +# proxy that can silently miss a level the formatter spells differently. The +# listing shown to a human is rendered from the same structure, with the real +# level names, so an `info` finding is visible as `info`. +raw="$(cd "$ROOT" && "$SHELLCHECK" -f json1 "${files[@]}" 2>&1)" rc=$? if [ "$rc" -ne 0 ] && [ "$rc" -ne 1 ]; then echo "$ME: shellcheck exited $rc, which is a tool failure, not a verdict:" >&2 - printf '%s\n' "$listing" | head -20 | sed 's/^/ /' >&2 + printf '%s\n' "$raw" | head -20 | sed 's/^/ /' >&2 + exit 2 +fi +if ! listing="$(printf '%s' "$raw" | python3 -c ' +import json, sys +try: + comments = json.load(sys.stdin)["comments"] +except (ValueError, KeyError) as e: + sys.stderr.write("not shellcheck json1 output: %s\n" % e); sys.exit(2) +for c in comments: + print("%s:%d:%d: %s: %s [SC%d]" % (c["file"], c["line"], c["column"], c["level"], c["message"], c["code"])) +')"; then + echo "$ME: could not read shellcheck's json1 output; no count, no verdict." >&2 + printf '%s\n' "$raw" | head -5 | sed 's/^/ /' >&2 exit 2 fi -findings="$(printf '%s\n' "$listing" | grep -cE '^[^:]+:[0-9]+:[0-9]+: (error|warning|note|style): ')" +if [ -z "$listing" ]; then + findings=0 +else + findings="$(printf '%s\n' "$listing" | wc -l | tr -d '[:space:]')" +fi echo "$ME: ${#files[@]} tracked .sh files, $findings findings, baseline $baseline (shellcheck $version)." if [ "$findings" -gt "$baseline" ]; then echo "$ME: $findings findings, above the baseline of $baseline." >&2 echo >&2 - printf '%s\n' "$listing" | grep -E '^[^:]+:[0-9]+:[0-9]+: (error|warning|note|style): ' | sed 's/^/ /' >&2 + printf '%s\n' "$listing" | sed 's/^/ /' >&2 echo >&2 echo "Fix the new findings, or disable the rule inline with a reason (# shellcheck disable=SCxxxx)." >&2 echo "The baseline in $BASELINE_FILE only goes down." >&2 diff --git a/tests/test_check_shellcheck.bats b/tests/test_check_shellcheck.bats index dedb465d4..84a6dfc4e 100644 --- a/tests/test_check_shellcheck.bats +++ b/tests/test_check_shellcheck.bats @@ -54,13 +54,35 @@ CLEAN='#!/bin/bash\nfoo="$1"\necho "$foo"\n' # --- the ratchet -------------------------------------------------------------------- -@test "above the baseline is red and names the finding" { +@test "above the baseline is red and names the finding with its real level" { _need_shellcheck local d; d="$(_tree "broken.sh=$BROKEN")" AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 0)" run bash "$CHECK" [ "$status" -eq 1 ] grep -q 'above the baseline' <<<"$output" - grep -q 'broken.sh:3:6: ' <<<"$output" + # SC2086 is an `info` finding: the level the text formatters rename (gcc + # prints it as `note`), and therefore the one a severity list loses first. + grep -q 'broken.sh:3:6: info: .*\[SC2086\]' <<<"$output" +} + +@test "every level counts: the checker's count equals shellcheck's own json count over a tree with all four levels" { + # Derived, not listed: the expected count is what shellcheck's structured + # output reports for the same files, and the tree carries at least one + # finding of each level so a level dropped by the counter is a mismatch. + _need_shellcheck + local d expected got + d="$(_tree \ + "err.sh=#!/bin/bash\na=1\nb=2\nif [ \"\$a\" \\\\> \"\$b\" ]; then :; fi\n" \ + "warn.sh=#!/bin/bash\necho \"\$undefined_var\"\n" \ + "info.sh=$BROKEN" \ + "style.sh=#!/bin/bash\ncat file | grep x\n")" + expected="$(cd "$d" && "$SHELLCHECK" -f json1 err.sh warn.sh info.sh style.sh | python3 -c 'import json,sys; d=json.load(sys.stdin)["comments"]; print(len(d), " ".join(sorted(set(c["level"] for c in d))))')" + [ "${expected#* }" = "error info style warning" ] || { echo "fixture does not cover all four levels: $expected"; return 1; } + AGMSG_SHELLCHECK_ROOT="$d" AGMSG_SHELLCHECK_BASELINE="$(_baseline "$SC_VERSION" 0)" run bash "$CHECK" + [ "$status" -eq 1 ] + got="$(grep -oE '[0-9]+ findings, baseline' <<<"$output" | grep -oE '^[0-9]+')" + [ "$got" = "${expected%% *}" ] || { echo "checker counted $got, shellcheck json says ${expected%% *}"; echo "$output"; return 1; } + [ "$(grep -oE ': (error|warning|info|style): ' <<<"$output" | sort -u | wc -l | tr -d ' ')" -eq 4 ] } @test "at the baseline is green, and says how many files and which version" { @@ -148,6 +170,7 @@ CLEAN='#!/bin/bash\nfoo="$1"\necho "$foo"\n' run bash "$CHECK" --positive-control [ "$status" -eq 0 ] grep -q 'positive control fired' <<<"$output" + grep -q 'info-level finding' <<<"$output" # Without a working tool the control must not report success either. SHELLCHECK=/nonexistent/shellcheck run bash "$CHECK" --positive-control [ "$status" -eq 2 ]