Summary
bashunit::coverage::extract_functions misclassifies ordinary variable assignments as
single-line function definitions. The result is wrong function metrics in every LCOV report,
and — when the assigned value also contains a | — a raw bash arithmetic error printed to
stderr on every --coverage run.
Reproduces on main (be37b72). Long-standing: it also reproduces before the coverage
module split (#925/#928), which only changed the file the error names.
Reproduce
./bashunit --coverage --parallel tests/
./src/coverage/report_lcov.sh: line 38: ((: $: syntax error: operand expected (error token is "$")
Minimal case, in any user project:
# victim.sh
PS4_LIKE="x|${LINENO}"functionreal_fn() { echo hi; }extract_functions emits:
PS4_LIKE="x|$|4|4 <- not a function, and 4 fields instead of 3
real_fn|5|5
Root cause
Two independent defects that compound.
1. extract_functions accepts a variable assignment as a function definition.
It derives the candidate name by cutting at the first space, ( or {:
fn_name="${stripped%%[[:space:]\(\{]*}"For PS4_LIKE="x|${LINENO}" the first such character is the { of the ${LINENO}
expansion, so fn_name becomes PS4_LIKE="x|$ and the remainder {LINENO}" starts with
{ — which the validator accepts as a function body opener:
case"$after_name"in'()'* | '{'*) ;; # <- matches "{LINENO}\""Brace counting then balances on the same line ({ … } inside the expansion), so it is
emitted as a single-line function.
The trigger is any assignment with no whitespace before a ${...}, which is extremely
common: URL="https://${host}/api", PATH_="$HOME/${sub}", MSG="a${b}".
GREETING="hello ${USER}" is not affected — the space cuts the name first — which is why
this has gone unnoticed.
2. The record format is pipe-delimited with no escaping. A bogus name containing |
shifts every field. report_lcov then reads it with IFS='|' read -r fn_name fn_start fn_end
and does arithmetic on the result without validating it:
for((fln = fn_start; fln <= fn_end; fln++));do
fn_start is the literal $, hence ((: $: syntax error: operand expected.
In bashunit's own source the offending line is
src/coverage/engine.sh:18 (_BASHUNIT_COVERAGE_XTRACE_PS4='@|${BASH_SOURCE}'…).
Impact
- Silent and wide: any file containing
VAR="...${x}..." gets a phantom FN record, so
FNF (functions found) and FNH (functions hit) are wrong in the LCOV output consumed by
genhtml, Codecov and Coveralls. This affects user projects, not just bashunit. - Loud but narrow: when the value also contains
|, stderr gets a bash arithmetic error
per affected file. Cosmetic, but it makes --coverage output look broken.
Not caught by CI because --coverage is a nightly, non-gating workflow
(.github/workflows/coverage.yml) and the error goes to stderr without failing the run.
Proposal
Fix the detection, and make the consumer defensive:
- Reject assignments in
extract_functions. A candidate name containing =, ", '
or $ is not a function name. The cheapest correct guard is to require the candidate to
match an identifier ([a-zA-Z_][a-zA-Z0-9_:]*) in full, rather than only checking its
first character — a pure-bash case with no fork, consistent with the current fast path. - Make the record robust. Either reject any name that would contain the delimiter, or
have report_lcov validate fn_start/fn_end are numeric before the for ((...)) —
preferably both. A malformed record should never reach arithmetic.
Acceptance criteria
Do not
- Do not "fix" it by suppressing stderr — the silent wrong
FNF/FNH is the more serious half - Do not add a
grep/sed fork per line to extract_functions; the guard is a case - Do not run
shfmt -w; make lint is the format gate
Summary
bashunit::coverage::extract_functionsmisclassifies ordinary variable assignments assingle-line function definitions. The result is wrong function metrics in every LCOV report,
and — when the assigned value also contains a
|— a raw bash arithmetic error printed tostderr on every
--coveragerun.Reproduces on
main(be37b72). Long-standing: it also reproduces before the coveragemodule split (#925/#928), which only changed the file the error names.
Reproduce
Minimal case, in any user project:
extract_functionsemits:Root cause
Two independent defects that compound.
1.
extract_functionsaccepts a variable assignment as a function definition.It derives the candidate name by cutting at the first space,
(or{:fn_name="${stripped%%[[:space:]\(\{]*}"For
PS4_LIKE="x|${LINENO}"the first such character is the{of the${LINENO}expansion, so
fn_namebecomesPS4_LIKE="x|$and the remainder{LINENO}"starts with{— which the validator accepts as a function body opener:Brace counting then balances on the same line (
{…}inside the expansion), so it isemitted as a single-line function.
The trigger is any assignment with no whitespace before a
${...}, which is extremelycommon:
URL="https://${host}/api",PATH_="$HOME/${sub}",MSG="a${b}".GREETING="hello ${USER}"is not affected — the space cuts the name first — which is whythis has gone unnoticed.
2. The record format is pipe-delimited with no escaping. A bogus name containing
|shifts every field.
report_lcovthen reads it withIFS='|' read -r fn_name fn_start fn_endand does arithmetic on the result without validating it:
fn_startis the literal$, hence((: $: syntax error: operand expected.In bashunit's own source the offending line is
src/coverage/engine.sh:18(_BASHUNIT_COVERAGE_XTRACE_PS4='@|${BASH_SOURCE}'…).Impact
VAR="...${x}..."gets a phantomFNrecord, soFNF(functions found) andFNH(functions hit) are wrong in the LCOV output consumed bygenhtml, Codecov and Coveralls. This affects user projects, not just bashunit.
|, stderr gets a bash arithmetic errorper affected file. Cosmetic, but it makes
--coverageoutput look broken.Not caught by CI because
--coverageis a nightly, non-gating workflow(
.github/workflows/coverage.yml) and the error goes to stderr without failing the run.Proposal
Fix the detection, and make the consumer defensive:
extract_functions. A candidate name containing=,",'or
$is not a function name. The cheapest correct guard is to require the candidate tomatch an identifier (
[a-zA-Z_][a-zA-Z0-9_:]*) in full, rather than only checking itsfirst character — a pure-bash
casewith no fork, consistent with the current fast path.have
report_lcovvalidatefn_start/fn_endare numeric before thefor ((...))—preferably both. A malformed record should never reach arithmetic.
Acceptance criteria
extract_functionspinning thatVAR="x|${Y}",URL="https://${host}/api"and similar assignments yield no record, and that realsingle-line and multi-line functions still do
report_lcovemits no bash error for a file containing such an assignment./bashunit --coverage --parallel tests/produces no((:error on stderrFNF/FNHcounts in the generatedcoverage/lcov.infono longer include phantomentries (compare before/after on
src/coverage/engine.sh)./bashunit tests/·--parallel·--parallel --simple --strictgreenmake sa && make lintgreen(
.claude/rules/perf-fork-budget.md)Do not
FNF/FNHis the more serious halfgrep/sedfork per line toextract_functions; the guard is acaseshfmt -w;make lintis the format gate