From 14ed8ce38a537a3ce14cb8957d6418ef9a460bae Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 25 Sep 2026 18:37:16 +0200 Subject: [PATCH] fix(coverage): one line counter for a source buffer (#1967) Three places in internal/cbm/cbm.c counted the lines of the same source buffer with two different conventions. cbm_count_lines (parse_unusable 80% threshold) ignored a final newline, while the inline counter that sizes the preprocessed-line map (orig_lines) and cbm_line_offsets (the recovery-gap walker) counted every newline, so on any file ending in a newline -- nearly every file -- they disagreed by one. Nothing produced a wrong answer: every consumer of the larger count either clamps to it or sees the phantom last line as blank. The risk was the next reader picking up one convention without knowing the other existed. Add cbm_source_line_count() as the single helper, with the convention documented beside it: a '\n' terminates a line and opens no new one; an empty buffer counts as 1 line. All three call sites use it; the old static cbm_count_lines is gone. Test: parse_coverage::source_line_count_one_convention pins the convention (trailing newline, no final newline, CRLF, empty buffer, blank last line, src_len bound). Signed-off-by: Martin Vogel --- internal/cbm/cbm.c | 48 ++++++++++++++++++------------------- internal/cbm/cbm.h | 5 ++++ tests/test_parse_coverage.c | 18 ++++++++++++++ 3 files changed, 46 insertions(+), 25 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index 4d01d0c69..aae9d64ba 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -1433,6 +1433,26 @@ static void cbm_mark_pp_error_rows(TSNode n, uint8_t *rows, uint32_t row_count, } } +/* The ONE line counter for a source buffer (#1967). Every caller that asks + * "how many lines does this file have" uses it, so the coverage report never + * holds two answers for the same buffer. + * + * Convention: a '\n' TERMINATES a line; it does not open a new one. So the + * count is the number of '\n' that have at least one byte after them, plus + * one. "a\nb" and "a\nb\n" both have 2 lines (the trailing newline adds no + * phantom empty line, matching what an editor shows); "a\nb" without a final + * newline still counts its last line. An empty buffer counts as 1 line, because + * tree-sitter still reports row 0 for it and 1-based line maps index line 1. */ +uint32_t cbm_source_line_count(const char *src, int src_len) { + uint32_t n = 1; + for (int i = 0; i + 1 < src_len; i++) { + if (src[i] == '\n') { + n++; + } + } + return n; +} + /* Recovery subtraction (#963): tree-sitter error recovery plus the * ERROR-descending def walker often still extract constructs INSIDE a failed * region (verified: a function in an #ifdef-split ERROR region and even a @@ -1450,12 +1470,7 @@ static void cbm_mark_pp_error_rows(TSNode n, uint8_t *rows, uint32_t row_count, * Now the uncovered gaps are reported instead, and a gap holding only blank, * comment or preprocessor lines is not a miss at all. */ static uint32_t *cbm_line_offsets(const char *src, int src_len, uint32_t *out_lines) { - uint32_t lines = 1; - for (int i = 0; i < src_len; i++) { - if (src[i] == '\n') { - lines++; - } - } + uint32_t lines = cbm_source_line_count(src, src_len); uint32_t *offs = (uint32_t *)cbm_alloc(CBM_MEM_CLASS_EXTRACT, (size_t)(lines + 1) * sizeof(uint32_t)); if (!offs) { @@ -2060,18 +2075,6 @@ static void cbm_refine_regions_with_pp_lines(cbm_error_regions_t *regs, const ui * this repo covers 25.5% of its file, and the next widest 3.9%. */ #define CBM_UNUSABLE_PCT 80 -/* Number of 1-based lines in `src`. A file that does not end with a newline - * still has a last line, so the count is separators plus one. */ -static uint32_t cbm_count_lines(const char *src, int src_len) { - uint32_t n = 1; - for (int i = 0; i < src_len; i++) { - if (src[i] == '\n' && i + 1 < src_len) { - n++; - } - } - return n; -} - /* Serialize collected regions as "start-end,start-end,...", with a trailing * ",+" when the cap threw N ranges away. * @@ -2745,12 +2748,7 @@ static CBMFileResult *extract_file_ex_body(const char *source, int source_len, C * total loss (root is ERROR), because then it vouches for * nothing and there is no refinement to make. */ if (strcmp(ts_node_type(pp_root), "ERROR") != 0) { - uint32_t orig_lines = 1; - for (int ci = 0; ci < source_len; ci++) { - if (source[ci] == '\n') { - orig_lines++; - } - } + uint32_t orig_lines = cbm_source_line_count(source, source_len); uint8_t *map = (uint8_t *)cbm_arena_alloc(a, (size_t)orig_lines + 2); int exp_lines = preprocessed->expanded_line_count; uint8_t *bad_rows = @@ -2946,7 +2944,7 @@ static CBMFileResult *extract_file_ex_body(const char *source, int source_len, C * so the report can say "read the source" instead. See * parse_unusable in cbm.h for which files land here and why. */ if (regs.count == 1 && regs.dropped == 0) { - uint32_t total = cbm_count_lines(source, source_len); + uint32_t total = cbm_source_line_count(source, source_len); uint32_t span = regs.ends[0] - regs.starts[0] + 1; if (total > 0 && span * 100 >= total * CBM_UNUSABLE_PCT) { result->parse_unusable = true; diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index dc89a6b2d..7f1aad43e 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -920,6 +920,11 @@ uint64_t cbm_usage_field_lookup_test_work(void); uint64_t cbm_usage_slow_parent_fallback_test_count(void); #endif +// Number of 1-based lines in a source buffer. The single line-count convention +// for coverage reporting (#1967): a trailing '\n' ends the last line and opens +// no new one; an empty buffer counts as 1 line. See the definition in cbm.c. +uint32_t cbm_source_line_count(const char *src, int src_len); + // Toggle C/C++ preprocessor Macro-node extraction (#375). The pipeline enables // it only for full/advanced index modes (it dominates extraction on macro-dense // codebases). Default ON. Set before extraction; read-only during. diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 09caeaa82..5dbc38e4a 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -1615,7 +1615,25 @@ TEST(cs_malformed_conditional_remains_partial_issue1748) { PASS(); } +/* #1967: every line counter over one source buffer answers the same + * question the same way. A trailing newline ends the last line; it does not + * open a new one. */ +TEST(source_line_count_one_convention) { + ASSERT_EQ(cbm_source_line_count("", 0), 1u); + ASSERT_EQ(cbm_source_line_count("a", 1), 1u); + ASSERT_EQ(cbm_source_line_count("a\n", 2), 1u); + ASSERT_EQ(cbm_source_line_count("\n", 1), 1u); + ASSERT_EQ(cbm_source_line_count("a\nb", 3), 2u); + ASSERT_EQ(cbm_source_line_count("a\nb\n", 4), 2u); + ASSERT_EQ(cbm_source_line_count("a\n\n", 3), 2u); + ASSERT_EQ(cbm_source_line_count("a\r\nb\r\n", 6), 2u); + /* src_len bounds the count, not a NUL terminator. */ + ASSERT_EQ(cbm_source_line_count("a\nb\nc", 2), 1u); + PASS(); +} + SUITE(parse_coverage) { + RUN_TEST(source_line_count_one_convention); RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete); RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted); RUN_TEST(c_error_range_points_at_failed_region);