You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The case-pattern rule in is_executable_line (src/coverage/lines.sh:94-112) classifies real statements as non-executable, which removes them from the denominator and hides them from the uncovered list.
The rule treats "something, then ), then end of line or a comment" as a case arm pattern. A command substitution that closes at end of line matches it:
x=$(foo)# classified non-executable
x=$(printf '%s\n')# classified executable, because the backslash suppresses the match
The asymmetry is documented in place as a preserved quirk of the pre-#1005 regex: inside a POSIX bracket expression a backslash is a literal member of the set, so the old [^\)]+ failed to match whenever a backslash appeared before the ). #1005 reproduced the regex exactly, quirks included, which was the right call for a performance change. It left a classification bug in place.
Measured over git ls-files 'src/*.sh' on this repo, 18,751 lines:
lines excluded only by the case-pattern rule: 546
of those, lines containing $( or =, so almost certainly real statements: 236
236 executable lines of src/ silently vanish from every coverage denominator, roughly 2.5% of the 9,285 executable lines the classifier finds.
Proposal
Make the rule mean what it says: a case arm pattern, not any line ending in ).
The ) must close a case arm, so it must not be the closing paren of $( ), ( ), <( ) or an arithmetic (( )).
Drop the accidental backslash exclusion. A backslash anywhere earlier in the line must stop mattering.
A cheap improvement without full parsing: reject the line as a case pattern when the text before the ) contains an unbalanced (, and keep requiring end-of-line or a comment after it.
Decide and document what a real case pattern line should be. It is a decision point, so branch coverage already accounts for it, and counting it as non-executable is defensible. State the choice in the function's comment either way.
This changes reported percentages, in both directions per file. That is the fix, not a side effect. Do it before porting the rules to awk, so only one set of rules gets ported.
Where to change
src/coverage/lines.sh:94-112 the case-pattern rule
tests/unit/coverage/executable_test.sh the rule tests
Acceptance criteria
x=$(foo) classifies as executable
x=$(printf '%s\n') classifies as executable, unchanged
arr=(a b c) classifies as executable
if (( i > 0 )); then and a bare ) closing a subshell keep their current classification, asserted explicitly
A real case arm pattern (--option), *) # note, a|b)) keeps the documented classification, whichever is chosen
A differential run over every git ls-files '*.sh' line lists every line whose classification changed, and each change is reviewed as correct in the PR description
The src/ executable-line total moves by roughly the expected amount, stated in the PR description
Coverage percentages change, and the CHANGELOG says so
tests/unit/coverage/executable_test.sh gains a case per bullet above
Repo checklist (agent)
TDD: RED then GREEN then REFACTOR. Write the failing classification test first.
Bash 3.0+ only: no printf -v, no += append, no declare -A, no [[ ]], no ${var,,}, no &>>, no ${arr[-1]}.
Problem
The case-pattern rule in
is_executable_line(src/coverage/lines.sh:94-112) classifies real statements as non-executable, which removes them from the denominator and hides them from the uncovered list.The rule treats "something, then
), then end of line or a comment" as acasearm pattern. A command substitution that closes at end of line matches it:The asymmetry is documented in place as a preserved quirk of the pre-#1005 regex: inside a POSIX bracket expression a backslash is a literal member of the set, so the old
[^\)]+failed to match whenever a backslash appeared before the). #1005 reproduced the regex exactly, quirks included, which was the right call for a performance change. It left a classification bug in place.Measured over
git ls-files 'src/*.sh'on this repo, 18,751 lines:$(or=, so almost certainly real statements: 236236 executable lines of
src/silently vanish from every coverage denominator, roughly 2.5% of the 9,285 executable lines the classifier finds.Proposal
Make the rule mean what it says: a
casearm pattern, not any line ending in).)must close acasearm, so it must not be the closing paren of$( ),( ),<( )or an arithmetic(( )).)contains an unbalanced(, and keep requiring end-of-line or a comment after it.casepattern line should be. It is a decision point, so branch coverage already accounts for it, and counting it as non-executable is defensible. State the choice in the function's comment either way.This changes reported percentages, in both directions per file. That is the fix, not a side effect. Do it before porting the rules to awk, so only one set of rules gets ported.
Where to change
src/coverage/lines.sh:94-112the case-pattern ruletests/unit/coverage/executable_test.shthe rule testsAcceptance criteria
x=$(foo)classifies as executablex=$(printf '%s\n')classifies as executable, unchangedarr=(a b c)classifies as executableif (( i > 0 )); thenand a bare)closing a subshell keep their current classification, asserted explicitlycasearm pattern (--option),*) # note,a|b)) keeps the documented classification, whichever is chosengit ls-files '*.sh'line lists every line whose classification changed, and each change is reviewed as correct in the PR descriptionsrc/executable-line total moves by roughly the expected amount, stated in the PR descriptiontests/unit/coverage/executable_test.shgains a case per bullet aboveRepo checklist (agent)
printf -v, no+=append, nodeclare -A, no[[ ]], no${var,,}, no&>>, no${arr[-1]}.grepforks from it. A rewrite that reintroduces a per-line fork is a regression, guarded bytests/acceptance/bashunit_coverage_forks_test.sh.[^\)]excludes\, and[\{\}]matches a lone\.make sa,make lint,./bashunit tests/,./bashunit --parallel tests/. Never runshfmt -w.## Unreleased(Fixed), stating that percentages move.