Skip to content

Commit a5af586

Browse files
fix(coverage): stop reporting a duplicate range and a line past EOF (#963)
scripts/setup-windows.ps1 has 326 lines and its parse-coverage report read "113-113,113-113,245-327" — the same line named twice, and an end line that does not exist. Two separate faults, both in cbm_error_regions_push. Past-EOF end line. A tree-sitter node that ends at column 0 stopped right after the previous line's newline, so it holds no text on the row it points at. Adding 1 to that row named a line past the end of the file whenever the region ran to EOF. The node here is start=(244,2) end=(326,0). Clamp the end to the row above when the end column is 0 and the node spans more than one row. Duplicate range. Line 113 carries two separate ERROR nodes, at columns 25-29 and 31-32, and each pushed its own range. A line range says nothing new the second time. Drop a range that exactly repeats the one already open. The drop runs BEFORE the cap check, so a repeat is never miscounted as a range the cap threw away. Only an EXACT repeat is dropped, never a range that merely overlaps. Each range is judged separately afterwards by cbm_region_is_recovered, which asks whether definitions starting inside that range cover it. Two ranges holding the same numbers always get the same verdict, so dropping the repeat changes nothing. Two different ranges do not. Merging 3-3 into 2-3 hands the wider range's covering definition to an error that definition does not explain, and a real parse failure then vanishes from the report. That is not hypothetical. An earlier version of this commit merged on overlap and broke perl_malformed_source_remains_partial_issue1838, the test added with the Perl grammar refresh in 17b5a43. The malformed fixture produces two ERROR nodes, at lines 2-3 and 3-3. Merged, the 2-3 range looks fully covered by before_error and is removed, so parse_incomplete comes back false on a file that plainly does not parse. That test now pins this boundary. The real file reports "113-113,245-326". Two tests, both proved RED first with the exact expected text: coverage_repeated_error_line_reports_one_range_issue963 "2-2,2-2" != "2-2" coverage_range_never_ends_past_the_last_line_issue963 "1-5" != "1-4" Suites run on this change: parse_coverage 34, extraction 325, pipeline 264, mcp 246, index_resilience 7 — all passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
1 parent 35efca8 commit a5af586

2 files changed

Lines changed: 84 additions & 2 deletions

File tree

internal/cbm/cbm.c

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -790,12 +790,47 @@ typedef struct {
790790
} cbm_error_regions_t;
791791

792792
static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) {
793+
TSPoint start = ts_node_start_point(n);
794+
TSPoint end = ts_node_end_point(n);
795+
uint32_t start_line = start.row + 1;
796+
uint32_t end_line = end.row + 1;
797+
798+
/* A node that ends at column 0 stopped right after the previous line's
799+
* newline, so it holds no text on the row it points at. Counting that row
800+
* named a line past the end of the file whenever the region ran to EOF:
801+
* scripts/setup-windows.ps1 has 326 lines and reported "245-327". */
802+
if (end.column == 0 && end.row > start.row) {
803+
end_line = end.row;
804+
}
805+
806+
/* One line can carry several error nodes, and repeating the same line range
807+
* says nothing new. Line 113 of scripts/setup-windows.ps1 has two error
808+
* nodes, at columns 25-29 and 31-32, and the report read "113-113,113-113".
809+
* Drop the repeat.
810+
*
811+
* Only an EXACT repeat of the range already open is dropped. Do not merge
812+
* ranges that merely overlap. Each range is judged separately later by
813+
* cbm_region_is_recovered, which asks whether definitions starting inside
814+
* that range cover it. Two ranges with the same numbers always get the same
815+
* verdict, so collapsing them changes nothing. Two DIFFERENT ranges do not:
816+
* merging 3-3 into 2-3 hands the wider range's covering definition to an
817+
* error the definition does not explain, and a real parse failure then
818+
* disappears from the report. tests/test_parse_coverage.c pins that case in
819+
* perl_malformed_source_remains_partial_issue1838.
820+
*
821+
* This runs BEFORE the cap check, so a dropped repeat never counts as a
822+
* range the cap threw away. */
823+
if (acc->count > 0 && start_line == acc->starts[acc->count - 1] &&
824+
end_line == acc->ends[acc->count - 1]) {
825+
return;
826+
}
827+
793828
if (acc->count >= CBM_MAX_ERROR_REGIONS) {
794829
acc->dropped++;
795830
return;
796831
}
797-
acc->starts[acc->count] = ts_node_start_point(n).row + 1;
798-
acc->ends[acc->count] = ts_node_end_point(n).row + 1;
832+
acc->starts[acc->count] = start_line;
833+
acc->ends[acc->count] = end_line;
799834
acc->count++;
800835
}
801836

tests/test_parse_coverage.c

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,51 @@ TEST(c_thread_local_grammar_limit_is_pinned_issue963) {
763763
PASS();
764764
}
765765

766+
/* Two error nodes can sit on ONE line. Line 113 of scripts/setup-windows.ps1
767+
* does exactly that, and the report used to read "113-113,113-113" — the same
768+
* line named twice. A line range says nothing new the second time, so repeated
769+
* or overlapping regions must collapse into one. */
770+
static const char *PS_TWO_ERRORS_ONE_LINE = "Write-Host \"start\"\n" /* 1 */
771+
"wsl.exe -- bash -c $Command 2>&1\n" /* 2 */
772+
"Write-Host \"end\"\n"; /* 3 */
773+
774+
/* An error region that runs to the end of the file stops just after the last
775+
* newline. Tree-sitter calls that position row N, column 0 — a row that holds
776+
* no text. Reading it as a line number named a line past the end of the file:
777+
* scripts/setup-windows.ps1 has 326 lines and the report said "245-327". */
778+
static const char *PS_ERROR_TO_EOF = "} else {\n" /* 1 */
779+
" if ($a) {\n" /* 2 */
780+
" Write-Host x\n" /* 3 */
781+
"}\n"; /* 4 */
782+
783+
TEST(coverage_repeated_error_line_reports_one_range_issue963) {
784+
CBMFileResult *r =
785+
cbm_extract_file(PS_TWO_ERRORS_ONE_LINE, (int)strlen(PS_TWO_ERRORS_ONE_LINE),
786+
CBM_LANG_POWERSHELL, "covproj", "two_errors.ps1", 0, NULL, NULL);
787+
ASSERT_NOT_NULL(r);
788+
ASSERT_TRUE(r->parse_incomplete);
789+
ASSERT_NOT_NULL(r->error_ranges);
790+
/* Line 2 carries two separate error nodes. It must be named once. */
791+
ASSERT_STR_EQ(r->error_ranges, "2-2");
792+
ASSERT_EQ(r->error_region_count, 1);
793+
cbm_free_result(r);
794+
PASS();
795+
}
796+
797+
TEST(coverage_range_never_ends_past_the_last_line_issue963) {
798+
int len = (int)strlen(PS_ERROR_TO_EOF);
799+
CBMFileResult *r = cbm_extract_file(PS_ERROR_TO_EOF, len, CBM_LANG_POWERSHELL, "covproj",
800+
"error_to_eof.ps1", 0, NULL, NULL);
801+
ASSERT_NOT_NULL(r);
802+
ASSERT_TRUE(r->parse_incomplete);
803+
ASSERT_NOT_NULL(r->error_ranges);
804+
/* The file has four lines and ends with a newline. Line 5 does not exist. */
805+
ASSERT_STR_EQ(r->error_ranges, "1-4");
806+
ASSERT_NULL(strstr(r->error_ranges, "5"));
807+
cbm_free_result(r);
808+
PASS();
809+
}
810+
766811
SUITE(parse_coverage) {
767812
RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete);
768813
RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted);
@@ -796,4 +841,6 @@ SUITE(parse_coverage) {
796841
RUN_TEST(perl_format_followed_by_named_sub_is_complete_issue1838);
797842
RUN_TEST(perl_malformed_source_remains_partial_issue1838);
798843
RUN_TEST(c_thread_local_grammar_limit_is_pinned_issue963);
844+
RUN_TEST(coverage_repeated_error_line_reports_one_range_issue963);
845+
RUN_TEST(coverage_range_never_ends_past_the_last_line_issue963);
799846
}

0 commit comments

Comments
 (0)