diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c7da29b..3680f491 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ - Performance: hit data is grouped once per run by one awk pass, instead of `grep | cut | sort | uniq -c` per file — 1294ms to 203ms at 121 files and 10,890 records (#1057) - Performance: the DEBUG trap rejects a line from an untracked file before calling the recorder — a run matching no coverage path went from 2609ms to 497ms, against a 480ms no-coverage baseline (#1060) - Performance: the LCOV emitter classifies and writes each file in one awk pass instead of a Bash loop per line — 8520ms to 6632ms for 40 files of 752 lines. The awk rules are diffed against the Bash reference line by line over every shell file in the repo (#1059) +- Performance: function declarations are scanned in one awk pass instead of a Bash loop counting braces with pattern substitution — 2238ms to 399ms for 128 files, and a `--coverage` run over `src` from 9.23s to 6.81s (#1084) ### Fixed - Coverage reports every file under `--coverage-paths`, not only the ones a test executed: an untouched file shows as `0/N (0%)` and `--coverage-min` gates on that denominator. This repo reported 11 of its own 121 files. **Percentages drop, because the old ones were measured over the files that ran** (#1053) diff --git a/src/coverage/functions.sh b/src/coverage/functions.sh index e064192f..7a415ee2 100644 --- a/src/coverage/functions.sh +++ b/src/coverage/functions.sh @@ -2,105 +2,101 @@ # Locating function definitions and their line spans, for the reports. -# Extract function definitions from a bash file -# Output format: function_name:start_line:end_line (one per function) -function bashunit::coverage::extract_functions() { - local file="$1" - - local lineno=0 - local in_function=0 - local brace_count=0 - local current_fn="" - local fn_start=0 - local line - - while IFS= read -r line || [ -n "$line" ]; do - ((++lineno)) - - # Check for function definition patterns +# The declaration scanner, as awk source. +# +# It was a Bash `while read` loop with two `${line//[^\{]/}` substitutions per +# line to count braces. Bash 3.2 pattern substitution over a whole file is the +# single most expensive thing in the report phase: 17.5 ms per file against +# awk's 3.1 ms, and the report calls this once per file per renderer. One pass +# in awk instead (#1084). +# +# The rules are unchanged, quirks included -- notably that braces are counted +# without regard for strings or comments, so `echo "{"` inside a body extends +# the span. Changing that is a numbers change, not a perf change. +# +# It lives in a shell string rather than a .awk file because the build flattens +# *.sh into one artifact (ADR-011); a separate file would not ship. +# shellcheck disable=SC2016 # the $0 in here is awk's, not the shell's +_BASHUNIT_COVERAGE_AWK_FUNCTIONS=' +{ + line = $0 + if (in_function == 0) { # Pattern 1: function name() { or function name { # Pattern 2: name() { or name () { - if [ "$in_function" -eq 0 ]; then - local fn_name="" - - # Extract function name using pure Bash string operations (avoids sed subshell) - local stripped="${line#"${line%%[![:space:]]*}"}" - - # Strip "function " prefix if present - case "$stripped" in - function[\ \ ]*) - stripped="${stripped#function}" - stripped="${stripped#"${stripped%%[![:space:]]*}"}" - ;; - esac + stripped = line + sub(/^[ \t]+/, "", stripped) + if (stripped ~ /^function[ \t]/) { + sub(/^function/, "", stripped) + sub(/^[ \t]+/, "", stripped) + } - # Extract first word as candidate function name - fn_name="${stripped%%[[:space:]\(\{]*}" + # The candidate name is the first word, ending at whitespace, `(` or `{`. + name = stripped + sub(/[ \t({].*$/, "", name) - # Validate: must BE an identifier, and rest must have () or { - if [ -n "$fn_name" ]; then - case "$fn_name" in - # A candidate holding anything outside the identifier alphabet is not a - # function name. Cutting at the first `{` means `VAR="x${Y}"` yields - # `VAR="x$`, whose trailing `{Y}"` then looks like a body opener — every - # such assignment became a phantom FN record, and one containing the - # record separator `|` shifted the fields and crashed report_lcov's - # arithmetic (#936). Checking only the first character let all of that - # through. - *[!a-zA-Z0-9_:]*) fn_name="" ;; - [a-zA-Z_]*) - local after_name="${stripped#"$fn_name"}" - after_name="${after_name#"${after_name%%[![:space:]]*}"}" - case "$after_name" in - '()'* | '{'*) ;; - *) fn_name="" ;; - esac - ;; - *) fn_name="" ;; - esac - fi + if (name != "") { + ok = 1 + # A candidate holding anything outside the identifier alphabet is not a + # function name. Cutting at the first `{` means `VAR="x${Y}"` yields + # `VAR="x$`, whose trailing `{Y}"` then looks like a body opener -- every + # such assignment became a phantom FN record, and one containing the + # record separator `|` shifted the fields and crashed the arithmetic in + # report_lcov (#936). Checking only the first character let all of that + # through. + if (name ~ /[^a-zA-Z0-9_:]/) { + ok = 0 + } else if (name !~ /^[a-zA-Z_]/) { + ok = 0 + } else { + # A declaration continues with `()` or `{`; a call does not. + after = substr(stripped, length(name) + 1) + sub(/^[ \t]+/, "", after) + if (substr(after, 1, 2) != "()" && substr(after, 1, 1) != "{") { ok = 0 } + } - if [ -n "$fn_name" ]; then - in_function=1 - current_fn="$fn_name" - fn_start=$lineno - brace_count=0 + if (ok) { + in_function = 1 + current_fn = name + fn_start = NR + tmp = line; nopen = gsub(/\{/, "{", tmp) + tmp = line; nclose = gsub(/\}/, "}", tmp) + brace_count = nopen - nclose + # Single-line function: braces balance on the same line, both present. + if (brace_count == 0 && nopen > 0 && nclose > 0) { + print current_fn "|" fn_start "|" NR + in_function = 0 + current_fn = "" + } + next + } + } + } - # Count opening braces on this line - local open_braces="${line//[^\{]/}" - local close_braces="${line//[^\}]/}" - local open_count=${#open_braces} - local close_count=${#close_braces} - brace_count=$((brace_count + open_count - close_count)) - - # Single-line function: braces balance on same line and both present - if [ "$brace_count" -eq 0 ] && [ "$open_count" -gt 0 ] && [ "$close_count" -gt 0 ]; then - echo "${current_fn}|${fn_start}|${lineno}" - in_function=0 - current_fn="" - fi - continue - fi - fi - - # Track braces inside function - if [ "$in_function" -eq 1 ]; then - local open_braces="${line//[^\{]/}" - local close_braces="${line//[^\}]/}" - brace_count=$((brace_count + ${#open_braces} - ${#close_braces})) + if (in_function == 1) { + tmp = line; nopen = gsub(/\{/, "{", tmp) + tmp = line; nclose = gsub(/\}/, "}", tmp) + brace_count = brace_count + nopen - nclose + if (brace_count <= 0) { + print current_fn "|" fn_start "|" NR + in_function = 0 + current_fn = "" + brace_count = 0 + } + } +} - # Function ended - if [ "$brace_count" -le 0 ]; then - echo "${current_fn}|${fn_start}|${lineno}" - in_function=0 - current_fn="" - brace_count=0 - fi - fi - done <"$file" +# An unclosed function (should not happen in valid code) still gets a record, +# ending at the last line, so a truncated file cannot drop one silently. +END { + if (in_function == 1 && current_fn != "") { print current_fn "|" fn_start "|" NR } +} +' - # Handle unclosed function (shouldn't happen in valid code) - if [ "$in_function" -eq 1 ] && [ -n "$current_fn" ]; then - echo "${current_fn}|${fn_start}|${lineno}" - fi +## +# Extract function definitions from a bash file. +# Output format: function_name|start_line|end_line (one per function) +# Arguments: $1 - source file +## +function bashunit::coverage::extract_functions() { + env LC_ALL=C "$AWK" "$_BASHUNIT_COVERAGE_AWK_FUNCTIONS" "$1" } diff --git a/tests/unit/coverage/helpers_test.sh b/tests/unit/coverage/helpers_test.sh index e103ddaa..b01e8e1b 100644 --- a/tests/unit/coverage/helpers_test.sh +++ b/tests/unit/coverage/helpers_test.sh @@ -265,6 +265,129 @@ EOF rm -f "$temp_file" } +# The span of a function is decided by brace balance, so a nested block inside +# the body must not close it early. +function test_coverage_extract_functions_spans_nested_braces() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +function outer() { + local map="${x:-fallback}" + if [ -n "$map" ]; then + echo "deep" + fi +} +function after() { + echo "after" +} +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "outer|2|7 +after|8|10" "$result" + + rm -f "$temp_file" +} + +# A body that opens and closes on the declaration line is one record whose start +# and end are the same line. +function test_coverage_extract_functions_reports_a_single_line_function() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +function one_liner() { echo "hi"; } +bare_liner() { echo "there"; } +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "one_liner|2|2 +bare_liner|3|3" "$result" + + rm -f "$temp_file" +} + +# The three declaration spellings bash accepts, plus an indented one: `name()`, +# `function name()` and `function name` with no parentheses at all. +function test_coverage_extract_functions_accepts_every_declaration_spelling() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +bare() { + echo "a" +} +function keyword() { + echo "b" +} +function no_parens { + echo "c" +} + indented() { + echo "d" + } +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "bare|2|4 +keyword|5|7 +no_parens|8|10 +indented|11|13" "$result" + + rm -f "$temp_file" +} + +# An unbalanced file still reports the function, ending at the last line, so a +# truncated or generated file cannot drop a record silently. +function test_coverage_extract_functions_closes_an_unclosed_function_at_eof() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +function unclosed() { + echo "no closing brace" +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "unclosed|2|3" "$result" + + rm -f "$temp_file" +} + +# A name is only a name inside [a-zA-Z0-9_:] and starting with a letter or `_`, +# and the declaration must continue with `()` or `{` -- a call is not a +# definition. +function test_coverage_extract_functions_rejects_non_declarations() { + local temp_file + temp_file=$(mktemp) + cat >"$temp_file" <<'FIXTURE' +#!/usr/bin/env bash +9lives() { + echo "starts with a digit" +} +call_me arg +real_fn() { + echo "yes" +} +FIXTURE + + local result + result=$(bashunit::coverage::extract_functions "$temp_file") + + assert_same "real_fn|6|8" "$result" + + rm -f "$temp_file" +} + # === Line hits tests === function test_coverage_get_all_line_hits_counts_per_line() {