Merge pull request #1971 from CaptainMittens/fix/coverage-range-duplicate-and-eof

fix(coverage): stop reporting a duplicate range and a line past EOF (#963)

Author: DeusData

internal/cbm/cbm.c

@@ -794,12 +794,47 @@ typedef struct {
 } cbm_error_regions_t;
 
 static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) {
+    TSPoint start = ts_node_start_point(n);
+    TSPoint end = ts_node_end_point(n);
+    uint32_t start_line = start.row + 1;
+    uint32_t end_line = end.row + 1;
+
+    /* A 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. Counting that row
+     * named a line past the end of the file whenever the region ran to EOF:
+     * scripts/setup-windows.ps1 has 326 lines and reported "245-327". */
+    if (end.column == 0 && end.row > start.row) {
+        end_line = end.row;
+    }
+
+    /* One line can carry several error nodes, and repeating the same line range
+     * says nothing new. Line 113 of scripts/setup-windows.ps1 has two error
+     * nodes, at columns 25-29 and 31-32, and the report read "113-113,113-113".
+     * Drop the repeat.
+     *
+     * Only an EXACT repeat of the range already open is dropped. Do not merge
+     * ranges that merely overlap. Each range is judged separately later by
+     * cbm_region_is_recovered, which asks whether definitions starting inside
+     * that range cover it. Two ranges with the same numbers always get the same
+     * verdict, so collapsing them changes nothing. Two DIFFERENT ranges do not:
+     * merging 3-3 into 2-3 hands the wider range's covering definition to an
+     * error the definition does not explain, and a real parse failure then
+     * disappears from the report. tests/test_parse_coverage.c pins that case in
+     * perl_malformed_source_remains_partial_issue1838.
+     *
+     * This runs BEFORE the cap check, so a dropped repeat never counts as a
+     * range the cap threw away. */
+    if (acc->count > 0 && start_line == acc->starts[acc->count - 1] &&
+        end_line == acc->ends[acc->count - 1]) {
+        return;
+    }
+
     if (acc->count >= CBM_MAX_ERROR_REGIONS) {
         acc->dropped++;
         return;
     }
-    acc->starts[acc->count] = ts_node_start_point(n).row + 1;
-    acc->ends[acc->count] = ts_node_end_point(n).row + 1;
+    acc->starts[acc->count] = start_line;
+    acc->ends[acc->count] = end_line;
     acc->count++;
 }
 

tests/test_parse_coverage.c

@@ -834,6 +834,51 @@ TEST(c_thread_local_grammar_limit_is_pinned_issue963) {
     PASS();
 }
 
+/* Two error nodes can sit on ONE line. Line 113 of scripts/setup-windows.ps1
+ * does exactly that, and the report used to read "113-113,113-113" — the same
+ * line named twice. A line range says nothing new the second time, so repeated
+ * or overlapping regions must collapse into one. */
+static const char *PS_TWO_ERRORS_ONE_LINE = "Write-Host \"start\"\n"             /* 1 */
+                                            "wsl.exe -- bash -c $Command 2>&1\n" /* 2 */
+                                            "Write-Host \"end\"\n";              /* 3 */
+
+/* An error region that runs to the end of the file stops just after the last
+ * newline. Tree-sitter calls that position row N, column 0 — a row that holds
+ * no text. Reading it as a line number named a line past the end of the file:
+ * scripts/setup-windows.ps1 has 326 lines and the report said "245-327". */
+static const char *PS_ERROR_TO_EOF = "} else {\n"             /* 1 */
+                                     "    if ($a) {\n"        /* 2 */
+                                     "        Write-Host x\n" /* 3 */
+                                     "}\n";                   /* 4 */
+
+TEST(coverage_repeated_error_line_reports_one_range_issue963) {
+    CBMFileResult *r =
+        cbm_extract_file(PS_TWO_ERRORS_ONE_LINE, (int)strlen(PS_TWO_ERRORS_ONE_LINE),
+                         CBM_LANG_POWERSHELL, "covproj", "two_errors.ps1", 0, NULL, NULL);
+    ASSERT_NOT_NULL(r);
+    ASSERT_TRUE(r->parse_incomplete);
+    ASSERT_NOT_NULL(r->error_ranges);
+    /* Line 2 carries two separate error nodes. It must be named once. */
+    ASSERT_STR_EQ(r->error_ranges, "2-2");
+    ASSERT_EQ(r->error_region_count, 1);
+    cbm_free_result(r);
+    PASS();
+}
+
+TEST(coverage_range_never_ends_past_the_last_line_issue963) {
+    int len = (int)strlen(PS_ERROR_TO_EOF);
+    CBMFileResult *r = cbm_extract_file(PS_ERROR_TO_EOF, len, CBM_LANG_POWERSHELL, "covproj",
+                                        "error_to_eof.ps1", 0, NULL, NULL);
+    ASSERT_NOT_NULL(r);
+    ASSERT_TRUE(r->parse_incomplete);
+    ASSERT_NOT_NULL(r->error_ranges);
+    /* The file has four lines and ends with a newline. Line 5 does not exist. */
+    ASSERT_STR_EQ(r->error_ranges, "1-4");
+    ASSERT_NULL(strstr(r->error_ranges, "5"));
+    cbm_free_result(r);
+    PASS();
+}
+
 SUITE(parse_coverage) {
     RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete);
     RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted);
@@ -870,4 +915,6 @@ SUITE(parse_coverage) {
     RUN_TEST(real_error_before_eof_still_flagged_with_trailing_blank_issue1746);
     RUN_TEST(width_bearing_error_at_eof_still_flagged_with_trailing_blank_issue1746);
     RUN_TEST(c_thread_local_grammar_limit_is_pinned_issue963);
+    RUN_TEST(coverage_repeated_error_line_reports_one_range_issue963);
+    RUN_TEST(coverage_range_never_ends_past_the_last_line_issue963);
 }