ref(bash): codify dynamic-scope safety for outvar helpers

.claude/rules/bash-style.md

@@ -49,6 +49,94 @@ Constants -> Globals -> Private functions -> Public functions
 
 Source deps relative to script: `"$(dirname "${BASH_SOURCE[0]}")/dep.sh"`
 
+## Returning values from a helper (dynamic-scope safety)
+
+Bash `local` is **dynamically scoped**, not lexically scoped. A `local` inside a callee
+shadows the caller's same-named variable for the duration of the call. Same trap fires
+with `${!name}` reads and `eval "$name=..."`/`printf -v "$name"`/`export "$name"=...`
+writes — they all resolve against the dynamic scope.
+
+We need **all three** properties on the hot path:
+
+1. **No subshell fork.** Returning via stdout and capturing with `$(...)` costs a fork
+  per call, which dominates per-test cost.
+2. **No collision with caller locals.** A helper that silently overwrites or reads
+  the wrong variable is the worst kind of bug — no error, just stale data.
+3. **Bash 3.0+ portable.** Rules out `declare -n` (4.3+) and `printf -v` (3.1+, and
+  it has the same dynamic-scope shadowing as `eval "$name=..."` anyway).
+
+### Preferred: dedicated global return slot
+
+```bash
+_BASHUNIT_PKG_THING_OUT=""
+
+# Writes the result into _BASHUNIT_PKG_THING_OUT.
+# Arguments: $1 input
+function bashunit::pkg::do_thing() {
+  local input=$1                       # natural name, no prefix needed
+  local val="${input%%]*}"
+  val="${val#[}"
+  _BASHUNIT_PKG_THING_OUT=$val
+}
+
+# Caller
+bashunit::pkg::do_thing "$payload"
+local thing=$_BASHUNIT_PKG_THING_OUT
+```
+
+- Zero forks per call.
+- Helper has natural local names; no `__bu_` noise.
+- Caller cannot accidentally shadow the slot because the `_BASHUNIT_*` namespace
+  is reserved for the framework.
+- A dedicated slot per helper (rather than one shared `_BASHUNIT_OUT`) means
+  adjacent or nested calls can't clobber each other. Cheap: globals are free.
+
+Examples in tree: `src/runner.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`,
+`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`),
+`src/coverage.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`).
+
+### When the helper builds dynamic variable names (mock/spy state)
+
+Use a clear namespace prefix on the **constructed** name, not on the helper's locals:
+
+```bash
+# Spy state lives in _BASHUNIT_SPY_${variable}_TIMES_FILE, not ${variable}_times_file.
+# A caller doing `local foo_times_file=...` is therefore harmless: the helper
+# resolves a different global.
+export "_BASHUNIT_SPY_${variable}_TIMES_FILE"="$times_file"
+```
+
+Example in tree: `src/test_doubles.sh` (`_BASHUNIT_SPY_*`).
+
+### Last-resort fallback: outvar by name + `__bu_` prefix on locals
+
+Only use this when neither a fixed return slot nor a namespaced constructed name is
+practical (e.g. a generic helper called from many call sites with different output
+variables and no shared global is appropriate):
+
+```bash
+function bashunit::pkg::do_thing() {
+  local __bu_out=$1
+  local __bu_in=$2
+  local __bu_val="${__bu_in%%]*}"
+  eval "$__bu_out=\$__bu_val"
+}
+```
+
+- All internal locals MUST be `__bu_`-prefixed; otherwise dynamic-scope shadowing
+  silently breaks the caller's outvar (see PR #672 — that exact bug caused 12 parallel
+  test failures in #662).
+- Include a regression test that calls the helper with one of its own documented
+  internal names as the outvar.
+
+### Intentional dynamic-scope mutation is a separate pattern
+
+The coverage branch helpers in `src/coverage.sh` (`_branch_push_if` and friends) deliberately
+mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). That is
+documented inline at `src/coverage.sh:818-821` and is **not** the outvar pattern — the
+helper has no `$1`-named outvar argument; the caller agrees to share state by convention.
+Don't introduce new instances of this pattern without an inline justification comment.
+
 ## ShellCheck
 
 All code must pass `make sa`. Use directives sparingly with reason:

CHANGELOG.md

@@ -2,12 +2,15 @@
 
 ## Unreleased
 
+### Internal
+- Codify global-slot return pattern for hot-path helpers; namespace mock/spy state under `_BASHUNIT_SPY_*` (#674)
+
 ### Performance
-- Faster runtime-error detection: single `case` glob instead of 23-iteration loop in `detect_runtime_error` (#668)
-- Hot-path coverage flag now cached in `_BASHUNIT_COVERAGE_ON`, removing a function dispatch per call (#664)
-- Parallel runner blocks on `wait -n` on Bash 4.3+ instead of polling `jobs -r`, removing sleep-induced slot-release latency (#667)
-- Hot-path result helpers (`extract_encoded_field`, `extract_subshell_type`, `format_subshell_output`, `compute_total_assertions`) use outvar pattern, dropping a fork per call per test (#662)
-- `generate_id` and `normalize_variable_name` drop `grep` and `random_str` forks via pure-bash globbing/inlining (#663)
+- Faster runtime-error detection: single `case` glob (#668)
+- Cache coverage-enabled flag in hot path (#664)
+- Parallel runner uses `wait -n` on Bash 4.3+ instead of polling (#667)
+- Outvar pattern in hot-path result helpers, dropping a fork per call (#662)
+- Drop `grep`/`random_str` forks in `generate_id` and `normalize_variable_name` (#663)
 
 ## [0.36.0](https://github.com/TypedDevs/bashunit/compare/0.35.0...0.36.0) - 2026-05-07
 

src/runner.sh

@@ -57,71 +57,76 @@ function bashunit::runner::apply_interpolated_title() {
   printf '%s' "$interpolated"
 }
 
-# All four helpers below use the outvar pattern (first argument is the name of
-# the variable to assign into) so callers can avoid the per-test $(...) subshell
-# capture in the hot path. Internal locals use a `__bu_` prefix to avoid name
-# collisions with caller variables passed by name.
-
-# Writes the value of an encoded field (##KEY=value##) into the named outvar.
-# Arguments: $1 outvar name, $2 test_execution_result, $3 key
+# Hot-path result helpers below return their value via a dedicated global slot
+# (`_BASHUNIT_RUNNER_*_OUT`) instead of stdout. This avoids the per-test
+# `$(...)` subshell capture that dominated the result-parsing hot path. Callers
+# invoke the helper and immediately read the slot:
+#
+#   bashunit::runner::extract_subshell_type "$subshell_output"
+#   type=$_BASHUNIT_RUNNER_TYPE_OUT
+#
+# A dedicated slot per helper (rather than one shared slot) means nested or
+# adjacent calls cannot clobber each other and callers don't need to copy out
+# before every other helper runs.
+_BASHUNIT_RUNNER_FIELD_OUT=""
+_BASHUNIT_RUNNER_TOTAL_OUT=""
+_BASHUNIT_RUNNER_TYPE_OUT=""
+_BASHUNIT_RUNNER_OUTPUT_OUT=""
+
+# Writes the value of an encoded field (##KEY=value##) into _BASHUNIT_RUNNER_FIELD_OUT.
+# Arguments: $1 test_execution_result, $2 key
 function bashunit::runner::extract_encoded_field() {
-  local __bu_out=$1
-  local __bu_in=$2
-  local __bu_key=$3
-  local __bu_marker="##${__bu_key}="
-  local __bu_val=""
-  case "$__bu_in" in
-  *"$__bu_marker"*)
-    local __bu_rest="${__bu_in#*"$__bu_marker"}"
-    __bu_val="${__bu_rest%%##*}"
+  local test_execution_result=$1
+  local key=$2
+  local marker="##${key}="
+  case "$test_execution_result" in
+  *"$marker"*)
+    local rest="${test_execution_result#*"$marker"}"
+    _BASHUNIT_RUNNER_FIELD_OUT="${rest%%##*}"
     ;;
+  *) _BASHUNIT_RUNNER_FIELD_OUT="" ;;
   esac
-  eval "$__bu_out=\$__bu_val"
 }
 
-# Writes the sum of all ASSERTIONS_* counters into the named outvar.
-# Arguments: $1 outvar name, $2 test_execution_result
+# Writes the sum of all ASSERTIONS_* counters into _BASHUNIT_RUNNER_TOTAL_OUT.
+# Arguments: $1 test_execution_result
 function bashunit::runner::compute_total_assertions() {
-  local __bu_out=$1
-  local __bu_in=$2
-  local __bu_failed __bu_passed __bu_skipped __bu_incomplete __bu_snapshot
-  __bu_failed="${__bu_in##*##ASSERTIONS_FAILED=}"
-  __bu_failed="${__bu_failed%%##*}"
-  __bu_passed="${__bu_in##*##ASSERTIONS_PASSED=}"
-  __bu_passed="${__bu_passed%%##*}"
-  __bu_skipped="${__bu_in##*##ASSERTIONS_SKIPPED=}"
-  __bu_skipped="${__bu_skipped%%##*}"
-  __bu_incomplete="${__bu_in##*##ASSERTIONS_INCOMPLETE=}"
-  __bu_incomplete="${__bu_incomplete%%##*}"
-  __bu_snapshot="${__bu_in##*##ASSERTIONS_SNAPSHOT=}"
-  __bu_snapshot="${__bu_snapshot%%##*}"
-  local __bu_val
-  __bu_val=$((${__bu_failed:-0} + ${__bu_passed:-0} + ${__bu_skipped:-0}))
-  __bu_val=$((__bu_val + ${__bu_incomplete:-0} + ${__bu_snapshot:-0}))
-  eval "$__bu_out=\$__bu_val"
+  local test_execution_result=$1
+  local failed passed skipped incomplete snapshot
+  failed="${test_execution_result##*##ASSERTIONS_FAILED=}"
+  failed="${failed%%##*}"
+  passed="${test_execution_result##*##ASSERTIONS_PASSED=}"
+  passed="${passed%%##*}"
+  skipped="${test_execution_result##*##ASSERTIONS_SKIPPED=}"
+  skipped="${skipped%%##*}"
+  incomplete="${test_execution_result##*##ASSERTIONS_INCOMPLETE=}"
+  incomplete="${incomplete%%##*}"
+  snapshot="${test_execution_result##*##ASSERTIONS_SNAPSHOT=}"
+  snapshot="${snapshot%%##*}"
+  local total
+  total=$((${failed:-0} + ${passed:-0} + ${skipped:-0}))
+  total=$((total + ${incomplete:-0} + ${snapshot:-0}))
+  _BASHUNIT_RUNNER_TOTAL_OUT=$total
 }
 
-# Writes the subshell type marker (text inside leading [...]) into the named outvar.
-# Arguments: $1 outvar name, $2 subshell_output
+# Writes the subshell type marker (text inside leading [...]) into _BASHUNIT_RUNNER_TYPE_OUT.
+# Arguments: $1 subshell_output
 function bashunit::runner::extract_subshell_type() {
-  local __bu_out=$1
-  local __bu_in=$2
-  local __bu_val="${__bu_in%%]*}"
-  __bu_val="${__bu_val#[}"
-  eval "$__bu_out=\$__bu_val"
+  local subshell_output=$1
+  local type="${subshell_output%%]*}"
+  _BASHUNIT_RUNNER_TYPE_OUT="${type#[}"
 }
 
 # Writes the subshell output (minus the leading [type] marker, with embedded
-# status markers replaced by newlines) into the named outvar.
-# Arguments: $1 outvar name, $2 subshell_output
+# status markers replaced by newlines) into _BASHUNIT_RUNNER_OUTPUT_OUT.
+# Arguments: $1 subshell_output
 function bashunit::runner::format_subshell_output() {
-  local __bu_out=$1
-  local __bu_in=$2
-  local __bu_val="${__bu_in#*]}"
-  __bu_val=${__bu_val//\[failed\]/$'\n'}
-  __bu_val=${__bu_val//\[skipped\]/$'\n'}
-  __bu_val=${__bu_val//\[incomplete\]/$'\n'}
-  eval "$__bu_out=\$__bu_val"
+  local subshell_output=$1
+  local line="${subshell_output#*]}"
+  line=${line//\[failed\]/$'\n'}
+  line=${line//\[skipped\]/$'\n'}
+  line=${line//\[incomplete\]/$'\n'}
+  _BASHUNIT_RUNNER_OUTPUT_OUT=$line
 }
 
 function bashunit::runner::detect_runtime_error() {
@@ -837,9 +842,10 @@ function bashunit::runner::run_test() {
   local subshell_output=$(bashunit::runner::decode_subshell_output "$test_execution_result")
 
   if [ -n "$subshell_output" ]; then
-    local type
-    bashunit::runner::extract_subshell_type type "$subshell_output"
-    bashunit::runner::format_subshell_output subshell_output "$subshell_output"
+    bashunit::runner::extract_subshell_type "$subshell_output"
+    local type=$_BASHUNIT_RUNNER_TYPE_OUT
+    bashunit::runner::format_subshell_output "$subshell_output"
+    subshell_output=$_BASHUNIT_RUNNER_OUTPUT_OUT
     if ! bashunit::env::is_failures_only_enabled; then
       bashunit::state::print_line "$type" "$subshell_output"
     fi
@@ -854,13 +860,15 @@ function bashunit::runner::run_test() {
 
   local test_exit_code="$_BASHUNIT_TEST_EXIT_CODE"
 
-  local total_assertions
-  bashunit::runner::compute_total_assertions total_assertions "$test_execution_result"
+  bashunit::runner::compute_total_assertions "$test_execution_result"
+  local total_assertions=$_BASHUNIT_RUNNER_TOTAL_OUT
 
-  local encoded_test_title hook_failure encoded_hook_message
-  bashunit::runner::extract_encoded_field encoded_test_title "$test_execution_result" "TEST_TITLE"
-  bashunit::runner::extract_encoded_field hook_failure "$test_execution_result" "TEST_HOOK_FAILURE"
-  bashunit::runner::extract_encoded_field encoded_hook_message "$test_execution_result" "TEST_HOOK_MESSAGE"
+  bashunit::runner::extract_encoded_field "$test_execution_result" "TEST_TITLE"
+  local encoded_test_title=$_BASHUNIT_RUNNER_FIELD_OUT
+  bashunit::runner::extract_encoded_field "$test_execution_result" "TEST_HOOK_FAILURE"
+  local hook_failure=$_BASHUNIT_RUNNER_FIELD_OUT
+  bashunit::runner::extract_encoded_field "$test_execution_result" "TEST_HOOK_MESSAGE"
+  local encoded_hook_message=$_BASHUNIT_RUNNER_FIELD_OUT
 
   local test_title=""
   [ -n "$encoded_test_title" ] && test_title="$(bashunit::helper::decode_base64 "$encoded_test_title")"

src/test_doubles.sh

@@ -2,6 +2,11 @@
 
 declare -a _BASHUNIT_MOCKED_FUNCTIONS=()
 
+# Spy/mock state is keyed by the sanitized command name and stored in globals
+# prefixed with `_BASHUNIT_SPY_` so they cannot collide with caller locals.
+# A test that does `local foo_times_file=...` is harmless because the helper
+# resolves `_BASHUNIT_SPY_foo_TIMES_FILE` instead.
+
 function bashunit::unmock() {
   local command=$1
 
@@ -16,8 +21,8 @@ function bashunit::unmock() {
       unset -f "$command"
       local variable
       variable="$(bashunit::helper::normalize_variable_name "$command")"
-      local times_file_var="${variable}_times_file"
-      local params_file_var="${variable}_params_file"
+      local times_file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE"
+      local params_file_var="_BASHUNIT_SPY_${variable}_PARAMS_FILE"
       [ -f "${!times_file_var-}" ] && rm -f "${!times_file_var}"
       [ -f "${!params_file_var-}" ] && rm -f "${!params_file_var}"
       unset "$times_file_var"
@@ -54,8 +59,8 @@ function bashunit::spy() {
   params_file=$(bashunit::temp_file "${test_id}_${variable}_params")
   echo 0 >"$times_file"
   : >"$params_file"
-  export "${variable}_times_file"="$times_file"
-  export "${variable}_params_file"="$params_file"
+  export "_BASHUNIT_SPY_${variable}_TIMES_FILE"="$times_file"
+  export "_BASHUNIT_SPY_${variable}_PARAMS_FILE"="$params_file"
 
   local body_suffix=""
   if [[ "$exit_code_or_impl" =~ ^[0-9]+$ ]]; then
@@ -89,7 +94,7 @@ function assert_have_been_called() {
   local command=$1
   local variable
   variable="$(bashunit::helper::normalize_variable_name "$command")"
-  local file_var="${variable}_times_file"
+  local file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE"
   local times=0
   if [ -f "${!file_var-}" ]; then
     times=$(cat "${!file_var}" 2>/dev/null || builtin echo 0)
@@ -119,7 +124,7 @@ function assert_have_been_called_with() {
 
   local variable
   variable="$(bashunit::helper::normalize_variable_name "$command")"
-  local file_var="${variable}_params_file"
+  local file_var="_BASHUNIT_SPY_${variable}_PARAMS_FILE"
   local line=""
   if [ -f "${!file_var-}" ]; then
     if [ -n "$index" ]; then
@@ -147,7 +152,7 @@ function assert_have_been_called_times() {
   local command=$2
   local variable
   variable="$(bashunit::helper::normalize_variable_name "$command")"
-  local file_var="${variable}_times_file"
+  local file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE"
   local times=0
   if [ -f "${!file_var-}" ]; then
     times=$(cat "${!file_var}" 2>/dev/null || builtin echo 0)
@@ -172,8 +177,8 @@ function assert_have_been_called_nth_with() {
 
   local variable
   variable="$(bashunit::helper::normalize_variable_name "$command")"
-  local times_file_var="${variable}_times_file"
-  local file_var="${variable}_params_file"
+  local times_file_var="_BASHUNIT_SPY_${variable}_TIMES_FILE"
+  local file_var="_BASHUNIT_SPY_${variable}_PARAMS_FILE"
   local label
   label="$(bashunit::helper::normalize_test_function_name "${FUNCNAME[1]}")"