Instrument: multi-line array elements are not statements; clear arch_perf_fixes

Fourth flavour of defect (a) found this session, after multi-line quotes,
`} |` and the backslash-continued assignment. bash reports a multi-line array
assignment at its OPENING line and never emits the element lines or the
closing paren, so kcov instrumenting them inflates the denominator:

    +L3 items=('one' 'two')      # opened line 3, closed line 6
                                 # lines 4, 5, 6: never emitted

The rule is therefore symmetric with the existing multi-line-quote case: the
opening line is the statement, everything up to the closing paren is data.
_closes_array() finds that paren by recomputing quote state per candidate, so
an element like "a(b)" does not end the literal early.

Measured effect through fixes/lib/tests/run_all.sh:

  arch_perf_fixes.sh  93.48% -> 100.00%  (43/43)
  bt_audio.sh        100.00% -> 100.00%  (127/127)
  arch_perf_report.sh 100.00% -> 100.00% (59/59)
  arch_perf_probes.sh 100.00% -> 100.00% (93/93)
  ubuntu_perf_more.sh 100.00% -> 100.00% (32/32)
  bt_report.sh       100.00% -> 100.00%  (37/37)
  bt_pairing.sh      100.00% -> 100.00%  (65/65)

One gained, six held, none went DOWN. Acceptance test unchanged:
dot_resolver_install.sh 39/39 and rpi_nc_ca.sh 73/73. arch_perf_fixes.sh
therefore comes off the allowlist in the same commit, confirmed by the real
gate with no exemption.

The fail-open check, since over-detection INFLATES coverage: the old and new
predicates were diffed over every allowlisted lib plus the six cleared ones,
printing each newly excluded line WITH its text. All 91 across 10 libs are
array elements or closing parens -- common.sh's tool tables, packages.sh's
associative arrays, arch_sysctl.sh's sysctl maps, thorium_repairs.sh's four
path lists, ubuntu_perf_fixes.sh's sssd units, and so on. No statement caught.

Tests cover both directions, including three negatives: a single-line
`x=(a b)`, an element containing a parenthesis, and `x=$(echo hi)` which is a
command substitution rather than an array. Module stays at 100% branch
coverage.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Krzysztof Rudnicki 2026-08-22 20:13:45 +02:00
parent 0c92a00f0f
commit 28f2925c30
4 changed files with 122 additions and 3 deletions

View File

@ -0,0 +1,56 @@
{
"intent": "Teach continuation_lines() that the element lines and closing paren of a multi-line array literal are not statements. This is defect (a) in docs/kcov-under-report.md, in the fourth flavour found this session, and it caps arch_perf_fixes.sh plus at least two of the libs still to be tested.",
"scope": [
"meta/scripts/lib/shell_coverage_lines.py (one regex, one helper, and array state in the main loop)",
"python_pkg/tests/test_shell_coverage_lines.py (four new cases: one positive, three negative)",
"Non-goal: the backslash-continued ASSIGNMENT flavour that caps bt_adapter.sh line 97, still deferred",
"meta/shell-coverage-allowlist.txt (arch_perf_fixes.sh removed, since the fix takes it to 100%)"
],
"changes": [
"Added _ARRAY_OPEN, matching an assignment whose value opens a multi-line array literal -- `local -a c=(`, `declare -A m=(`, `FILES=(`, `x+=(` -- with nothing but an optional comment after the paren. While open, every line is excluded until a `)` that is not itself inside quotes.",
"Added _closes_array(), which finds that closing paren by recomputing quote state from the start of the line for each candidate, so an element like \"a(b)\" does not end the literal early.",
"The rule is symmetric with the existing multi-line-quote case: bash reports the WHOLE assignment at its OPENING line, so that line stays a statement and everything up to the closing paren is data.",
"Added four tests: a multi-line array marks its elements and closing paren; a single-line `x=(a b)` does NOT; an element containing a parenthesis does not close the literal early; and `x=$(echo hi)` is a command substitution rather than an array literal.",
"Removed linux_configuration/scripts/single_use/fixes/lib/arch_perf_fixes.sh from meta/shell-coverage-allowlist.txt: the fix takes it to 100.00%, so it now meets the bar."
],
"verification": [
{
"command": "PS4='+L${LINENO} ' bash -x on a minimal multi-line array literal",
"result": "pass",
"evidence": "bash prints '+L3 items=(\\'one\\' \\'two\\')' for an array opened on line 3 and closed on line 6, and never emits lines 4, 5 or 6. The elements and closing paren are data; only the opening line is a statement."
},
{
"command": "python3 -m coverage run --branch --source=meta/scripts/lib -m pytest python_pkg/tests/test_shell_coverage_lines.py",
"result": "pass",
"evidence": "22 passed. shell_coverage_lines.py: 71 statements, 48 branches, 0 missed, 0 partial = 100%, so both arms of every new branch are covered as the repo's fail_under=100 branch gate requires."
},
{
"command": "Diff old vs new predicate over every allowlisted lib plus the six already cleared, printing each newly excluded line WITH its text",
"result": "pass",
"evidence": "91 newly excluded lines across 10 libs, and every one is an array element or a closing paren: common.sh's tool tables, packages.sh's associative arrays, arch_sysctl.sh's sysctl key/value maps, thorium_repairs.sh's four path lists, ubuntu_perf_fixes.sh's sssd unit list, clean_audio_filters.sh's model search paths, trace_prefix.sh, trace_stubs.sh and arch_perf_fixes.sh's candidate list. No ordinary statement was caught -- the fail-open direction this module's test header warns about."
},
{
"command": "Re-measure seven libs through fixes/lib/tests/run_all.sh after the change",
"result": "pass",
"evidence": "arch_perf_fixes.sh 93.48% -> 100.00% (43/43). bt_audio.sh 127/127, arch_perf_report.sh 59/59, arch_perf_probes.sh 93/93, ubuntu_perf_more.sh 32/32, bt_report.sh 37/37 and bt_pairing.sh 65/65 all unchanged at 100.00%. One gained, six held, none went DOWN."
},
{
"command": "The handoff's acceptance test: dot_resolver_install.sh and rpi_nc_ca.sh through features/lib/tests/run_all.sh with its jail_args",
"result": "pass",
"evidence": "dot_resolver_install.sh: 39/39 = 100.00% and rpi_nc_ca.sh: 73/73 = 100.00%, exactly the figures the handoff names as the instrument's acceptance criteria."
},
{
"command": "bash meta/scripts/check_shell_coverage.sh linux_configuration/scripts/single_use/fixes/lib/arch_perf_fixes.sh",
"result": "pass",
"evidence": "'check_shell_coverage.sh: every checked shell library meets the coverage bar', exit 0, with the allowlist line already removed so the lib had no exemption."
}
],
"risks": [
"This is the second denominator change of the session, so the same arithmetic bound applies: _measure computes covered_set = (executed|traced) & denominator, so a lib already at 100.00% cannot regress from excluding more lines. The six cleared libs were re-measured anyway and all held.",
"_ARRAY_OPEN deliberately requires the line to END at the open paren. `x=(a` followed by more elements on later lines would not match, and neither would a `for x in (` construct, which is not valid bash anyway. Narrower than necessary is the safe direction here, since over-detection inflates coverage."
],
"rollback": [
"git revert the commit; continuation_lines() loses array tracking and the four new tests go with it.",
"After rollback, re-measure arch_perf_fixes.sh and expect it back at 93.48%. It is not on the cleared list at that point, so no gate breaks -- but if the follow-up clearing commit has landed, revert that too."
]
}

View File

@ -75,6 +75,17 @@ def traced_paths(trace_dir: Path, subject: str) -> list[Path]:
# bare `}` closing a function carries no operator.
_CLOSING_BRACE_OP = re.compile(r"^\s*\}\s*(\||>>?|<|&>)")
# An assignment whose value opens a multi-line array literal, e.g.
# `local -a candidates=(` or `FILES=(`. bash reports the WHOLE assignment at
# this opening line and never emits the element lines or the closing paren, so
# those are data. Symmetric with the multi-line-quote case: the opening line
# is the statement, everything up to the closing paren is not.
_ARRAY_OPEN = re.compile(
r"^\s*(?:local\s+|declare\s+|readonly\s+|export\s+)*"
r"(?:-[a-zA-Z]+\s+)*"
r"[A-Za-z_][A-Za-z0-9_]*\+?=\(\s*(?:#.*)?$"
)
def continuation_lines(source: Path) -> set[int]:
"""Return lines that are *inside* a multi-line quoted argument.
@ -92,20 +103,41 @@ def continuation_lines(source: Path) -> set[int]:
opening line of such a construct is a statement.
* a line holding just a group's closing brace and an operator (``} |``,
``} >>file``). See ``_CLOSING_BRACE_OP``.
* the element lines and closing paren of a multi-line array literal.
See ``_ARRAY_OPEN``.
"""
inside: set[int] = set()
quote: str | None = None
in_array = False
for number, raw in enumerate(
source.read_text(encoding="utf-8", errors="replace").splitlines(), start=1
):
if quote is not None:
if quote is not None or in_array:
inside.add(number)
starts_open = quote is None and not in_array and bool(_ARRAY_OPEN.match(raw))
quote = _quote_after(raw, quote)
if quote is None and _CLOSING_BRACE_OP.match(raw):
if in_array and quote is None and _closes_array(raw):
in_array = False
elif starts_open:
in_array = True
if quote is None and not in_array and _CLOSING_BRACE_OP.match(raw):
inside.add(number)
return inside
def _closes_array(raw: str) -> bool:
"""Return True when ``raw`` closes a multi-line array literal.
Only a ``)`` that is not itself inside quotes counts, so an element like
``"a(b)"`` does not end the literal. Quote state is recomputed from the
start of the line for each candidate rather than pattern-matched.
"""
for index, char in enumerate(raw):
if char == ")" and _quote_after(raw[:index], None) is None:
return True
return False
def _quote_after(raw: str, quote: str | None) -> str | None:
"""Return the open-quote state at the end of ``raw``, given its state at the start.

View File

@ -75,7 +75,6 @@ linux_configuration/scripts/single_use/features/lib/wg_keys.sh
linux_configuration/scripts/single_use/features/lib/wg_sshd.sh
linux_configuration/scripts/single_use/fixes/lib/arch_cpu.sh
linux_configuration/scripts/single_use/fixes/lib/arch_hardware.sh
linux_configuration/scripts/single_use/fixes/lib/arch_perf_fixes.sh
linux_configuration/scripts/single_use/fixes/lib/arch_sysctl.sh
linux_configuration/scripts/single_use/fixes/lib/bt_adapter.sh
linux_configuration/scripts/single_use/fixes/lib/hosts_guard_migrate.sh

View File

@ -103,6 +103,38 @@ def test_closing_brace_with_redirect_is_a_continuation(
assert mod.continuation_lines(src) == {3}
def test_multiline_array_marks_elements_and_paren(
mod: ModuleType, tmp_path: Path
) -> None:
"""bash reports the whole assignment at the opening line; elements are data."""
src = _write(
tmp_path,
"a.sh",
'local -a c=(\n\t"one"\n\t"two"\n)\nfor i in "${c[@]}"; do :; done\n',
)
assert mod.continuation_lines(src) == {2, 3, 4}
def test_single_line_array_is_a_statement(mod: ModuleType, tmp_path: Path) -> None:
"""`x=(a b)` closes on its own line and stays a statement."""
src = _write(tmp_path, "a.sh", "x=(a b)\necho after\n")
assert mod.continuation_lines(src) == set()
def test_array_element_containing_a_paren_does_not_close_it(
mod: ModuleType, tmp_path: Path
) -> None:
"""A `)` inside quotes must not end the literal early."""
src = _write(tmp_path, "a.sh", 'x=(\n\t"a(b)"\n\t"c"\n)\necho after\n')
assert mod.continuation_lines(src) == {2, 3, 4}
def test_command_substitution_is_not_an_array(mod: ModuleType, tmp_path: Path) -> None:
"""`x=$(cmd` is a substitution, not an array literal, and must not match."""
src = _write(tmp_path, "a.sh", "x=$(echo hi)\necho after\n")
assert mod.continuation_lines(src) == set()
def test_bare_closing_brace_is_a_statement(mod: ModuleType, tmp_path: Path) -> None:
"""A `}` closing a function has no operator and must stay in the count."""
src = _write(tmp_path, "a.sh", "f() {\n\techo a\n}\nf\n")