From d03f97129595966ac17da75f7fa152bd0bf699ec Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 2 Aug 2026 11:06:28 +0200 Subject: [PATCH] chore(sa): remove 11 dead ShellCheck disable directives A directive for a warning that no longer fires is dead weight, and worse than that it is misleading -- it tells the next reader that the line below is doing something ShellCheck objects to, when it is not. This project has found stale ones twice before, in #928 and #947, both times by accident. Detected mechanically rather than by eye: strip every disable from a file, run ShellCheck, and see which declared codes actually come back. Two details made the difference between a correct list and a wrong one. The scan has to run in place. `.shellcheckrc` sets `source-path=src` with `external-sources=true`, so copying a file to a temp dir breaks source resolution and changes what SC2154 and SC2034 report -- exactly the codes under test. And it has to run under both gates. `make sa` uses `-xC` (follows `source`), CI's ludeeus/action-shellcheck does not, and the two see different things for cross-file symbols. A directive only counts as dead if it fires under neither. That distinction turned out not to change the list here, but assuming it would have been luck rather than method. Kept: the disables for SC1091, SC2155 and SC2016. Those never fire because both gates already exclude those codes globally, which is not the same as the warning being obsolete -- they still do their job for anyone running plain `shellcheck`. Removed (9 files, 11 lines): SC2153, SC2034 x3, SC2154 x3, SC2254, SC2030, SC2031, SC2295. Explanatory comments that sat above or beside a removed directive are kept where they still say something true. Every changed line is a comment; verified by filtering the diff. Suites match baseline exactly: 1630 sequential, 1589 parallel/simple/strict. --- src/config/parallel.sh | 1 - src/console/line.sh | 1 - src/coverage/engine.sh | 1 - src/coverage/paths.sh | 1 - src/runner/exec.sh | 1 - src/runner/hooks.sh | 3 --- src/runner/payload.sh | 1 - src/state/payload.sh | 1 - src/system/check_os.sh | 1 - 9 files changed, 11 deletions(-) diff --git a/src/config/parallel.sh b/src/config/parallel.sh index 5351c357..190af4bd 100755 --- a/src/config/parallel.sh +++ b/src/config/parallel.sh @@ -9,7 +9,6 @@ function bashunit::parallel::must_stop_on_failure() { } function bashunit::parallel::cleanup() { - # shellcheck disable=SC2153 local target="$TEMP_DIR_PARALLEL_TEST_SUITE" case "$target" in */bashunit/parallel/*) diff --git a/src/console/line.sh b/src/console/line.sh index ee121632..5c6d5bb8 100644 --- a/src/console/line.sh +++ b/src/console/line.sh @@ -14,7 +14,6 @@ _BASHUNIT_TOTAL_TESTS_COUNT=0 # Arguments: $1 - test type, $2 - already formatted line ## function bashunit::console_results::print_line() { - # shellcheck disable=SC2034 local type=$1 local line=$2 diff --git a/src/coverage/engine.sh b/src/coverage/engine.sh index 2bc1d3d4..ab4d4f95 100644 --- a/src/coverage/engine.sh +++ b/src/coverage/engine.sh @@ -51,7 +51,6 @@ function bashunit::coverage::enable_trap() { # Set DEBUG trap to record line execution # Use ${VAR:-} to handle unset variables when set -u is active (in subshells) - # shellcheck disable=SC2154 trap 'bashunit::coverage::record_line "${BASH_SOURCE[0]:-}" "${LINENO:-}"' DEBUG } diff --git a/src/coverage/paths.sh b/src/coverage/paths.sh index 748c5f96..ae4746cb 100644 --- a/src/coverage/paths.sh +++ b/src/coverage/paths.sh @@ -63,7 +63,6 @@ function bashunit::coverage::should_track() { IFS=',' local pattern for pattern in $BASHUNIT_COVERAGE_EXCLUDE; do - # shellcheck disable=SC2254 case "$normalized_file" in *$pattern*) IFS="$old_ifs" diff --git a/src/runner/exec.sh b/src/runner/exec.sh index 3ebcd458..a8e562e5 100644 --- a/src/runner/exec.sh +++ b/src/runner/exec.sh @@ -165,7 +165,6 @@ function bashunit::runner::execute_test_body() { fi # Run set_up and capture exit code without || to preserve errexit behavior - # shellcheck disable=SC2030 _BASHUNIT_SETUP_COMPLETED=false local setup_exit_code=0 bashunit::runner::run_set_up "$test_file" diff --git a/src/runner/hooks.sh b/src/runner/hooks.sh index d72e3f15..7fb44ca0 100644 --- a/src/runner/hooks.sh +++ b/src/runner/hooks.sh @@ -22,7 +22,6 @@ function bashunit::runner::cleanup_on_exit() { # Additionally, the stdout redirect from execute_test_hook leaks into the # EXIT trap. Restore stdout from saved FD 5 so export_subshell_context # output reaches test_execution_result. - # shellcheck disable=SC2031 if [ "${_BASHUNIT_SETUP_COMPLETED:-true}" != "true" ]; then exec 1>&5 if [ "$exit_code" -eq 0 ]; then @@ -103,7 +102,6 @@ function bashunit::runner::execute_file_hook() { # NOT this executor: on Bash >= 4 the trap also fires HERE when the hook call # itself returns non-zero, and an unconditional return skipped # record_file_hook_failure entirely (silent failures, off-by-one counts, #836). - # shellcheck disable=SC2154 trap '_BASHUNIT_HOOK_ERR_STATUS=$? if [ "${FUNCNAME[0]:-}" != "bashunit::runner::execute_file_hook" ]; then set +Eu +o pipefail @@ -224,7 +222,6 @@ function bashunit::runner::execute_test_hook() { # See the twin comment in execute_file_hook: conditional return keeps the # early-exit semantics for intermediate failures without silently returning # from THIS executor when the trap re-fires here on Bash >= 4 (#836). - # shellcheck disable=SC2154 trap '_BASHUNIT_HOOK_ERR_STATUS=$? if [ "${FUNCNAME[0]:-}" != "bashunit::runner::execute_test_hook" ]; then set +Eu +o pipefail diff --git a/src/runner/payload.sh b/src/runner/payload.sh index 2cc6eed8..e6b053af 100644 --- a/src/runner/payload.sh +++ b/src/runner/payload.sh @@ -184,7 +184,6 @@ function bashunit::runner::extract_assertion_runtime_output() { echo "$runtime_output" } -# shellcheck disable=SC2295 ## # Parses the encoded per-test result's last line into the counts out-slots # (_BASHUNIT_RUNNER_COUNTS_*_OUT). Pure read: never mutates the cumulative diff --git a/src/state/payload.sh b/src/state/payload.sh index de96369c..3ed85fe3 100644 --- a/src/state/payload.sh +++ b/src/state/payload.sh @@ -16,7 +16,6 @@ unset _bashunit_base64_help # in line parsing, so encode_base64 emits this token and both decode sites map it # back to "". Single source of truth keeps the encode (helpers.sh) and decode # (helpers.sh, runner/payload.sh) sides byte-identical. -# shellcheck disable=SC2034 # read cross-file in helpers.sh and runner/payload.sh _BASHUNIT_BASE64_EMPTY_SENTINEL="_BASHUNIT_EMPTY_" function bashunit::state::encode_field() { diff --git a/src/system/check_os.sh b/src/system/check_os.sh index 2ca74e7b..d2caa401 100644 --- a/src/system/check_os.sh +++ b/src/system/check_os.sh @@ -1,6 +1,5 @@ #!/usr/bin/env bash -# shellcheck disable=SC2034 _BASHUNIT_OS="Unknown" _BASHUNIT_DISTRO="Unknown"