diff --git a/docs/superpowers/evidence/array-literal-continuation-2026-08-22.json b/docs/superpowers/evidence/array-literal-continuation-2026-08-22.json new file mode 100644 index 00000000..5cd91232 --- /dev/null +++ b/docs/superpowers/evidence/array-literal-continuation-2026-08-22.json @@ -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." + ] +} diff --git a/meta/scripts/lib/shell_coverage_lines.py b/meta/scripts/lib/shell_coverage_lines.py index 7b7aa896..de78d6b3 100755 --- a/meta/scripts/lib/shell_coverage_lines.py +++ b/meta/scripts/lib/shell_coverage_lines.py @@ -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. diff --git a/meta/shell-coverage-allowlist.txt b/meta/shell-coverage-allowlist.txt index 3330404e..41eb564f 100644 --- a/meta/shell-coverage-allowlist.txt +++ b/meta/shell-coverage-allowlist.txt @@ -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 diff --git a/python_pkg/tests/test_shell_coverage_lines.py b/python_pkg/tests/test_shell_coverage_lines.py index db82628e..95a3b8c3 100644 --- a/python_pkg/tests/test_shell_coverage_lines.py +++ b/python_pkg/tests/test_shell_coverage_lines.py @@ -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")