Skip to content

Commit f89f01e

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 b04f545 commit f89f01e

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
@@ -794,12 +794,47 @@ typedef struct {
794794
} cbm_error_regions_t;
795795

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

‎tests/test_parse_coverage.c‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,51 @@ TEST(c_thread_local_grammar_limit_is_pinned_issue963) {
834834
PASS();
835835
}
836836

837+
/* Two error nodes can sit on ONE line. Line 113 of scripts/setup-windows.ps1
838+
* does exactly that, and the report used to read "113-113,113-113" — the same
839+
* line named twice. A line range says nothing new the second time, so repeated
840+
* or overlapping regions must collapse into one. */
841+
static const char *PS_TWO_ERRORS_ONE_LINE = "Write-Host \"start\"\n" /* 1 */
842+
"wsl.exe -- bash -c $Command 2>&1\n" /* 2 */
843+
"Write-Host \"end\"\n"; /* 3 */
844+
845+
/* An error region that runs to the end of the file stops just after the last
846+
* newline. Tree-sitter calls that position row N, column 0 — a row that holds
847+
* no text. Reading it as a line number named a line past the end of the file:
848+
* scripts/setup-windows.ps1 has 326 lines and the report said "245-327". */
849+
static const char *PS_ERROR_TO_EOF = "} else {\n" /* 1 */
850+
" if ($a) {\n" /* 2 */
851+
" Write-Host x\n" /* 3 */
852+
"}\n"; /* 4 */
853+
854+
TEST(coverage_repeated_error_line_reports_one_range_issue963) {
855+
CBMFileResult *r =
856+
cbm_extract_file(PS_TWO_ERRORS_ONE_LINE, (int)strlen(PS_TWO_ERRORS_ONE_LINE),
857+
CBM_LANG_POWERSHELL, "covproj", "two_errors.ps1", 0, NULL, NULL);
858+
ASSERT_NOT_NULL(r);
859+
ASSERT_TRUE(r->parse_incomplete);
860+
ASSERT_NOT_NULL(r->error_ranges);
861+
/* Line 2 carries two separate error nodes. It must be named once. */
862+
ASSERT_STR_EQ(r->error_ranges, "2-2");
863+
ASSERT_EQ(r->error_region_count, 1);
864+
cbm_free_result(r);
865+
PASS();
866+
}
867+
868+
TEST(coverage_range_never_ends_past_the_last_line_issue963) {
869+
int len = (int)strlen(PS_ERROR_TO_EOF);
870+
CBMFileResult *r = cbm_extract_file(PS_ERROR_TO_EOF, len, CBM_LANG_POWERSHELL, "covproj",
871+
"error_to_eof.ps1", 0, NULL, NULL);
872+
ASSERT_NOT_NULL(r);
873+
ASSERT_TRUE(r->parse_incomplete);
874+
ASSERT_NOT_NULL(r->error_ranges);
875+
/* The file has four lines and ends with a newline. Line 5 does not exist. */
876+
ASSERT_STR_EQ(r->error_ranges, "1-4");
877+
ASSERT_NULL(strstr(r->error_ranges, "5"));
878+
cbm_free_result(r);
879+
PASS();
880+
}
881+
837882
SUITE(parse_coverage) {
838883
RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete);
839884
RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted);
@@ -870,4 +915,6 @@ SUITE(parse_coverage) {
870915
RUN_TEST(real_error_before_eof_still_flagged_with_trailing_blank_issue1746);
871916
RUN_TEST(width_bearing_error_at_eof_still_flagged_with_trailing_blank_issue1746);
872917
RUN_TEST(c_thread_local_grammar_limit_is_pinned_issue963);
918+
RUN_TEST(coverage_repeated_error_line_reports_one_range_issue963);
919+
RUN_TEST(coverage_range_never_ends_past_the_last_line_issue963);
873920
}

0 commit comments

Comments
 (0)