Skip to content

Commit cbb456a

Browse files
committed
fix(extraction): treat trailing blanks at EOF as an absent newline, not a miss
A file whose final line lacks its newline leaves the grammar's mandatory terminator MISSING at EOF. #1610 suppressed that phantom parse_partial on the grounds that the node is ZERO-WIDTH: the parser consumed no source for it, so by construction nothing was dropped. That suppression tested `end == source_len` exactly. Trailing blanks are extras owned by no node, so `ENTRYPOINT ["a"] ` + EOF parks the zero-width terminator one byte short of source_len and the check missed it. This is why neither a trailing blank nor an absent final newline flagged on its own -- only the pair did, exactly as the reporter's byte-exact control matrix showed. Treat "at EOF" as EOF modulo a trailing blank run. Every blank except newline qualifies, deliberately not a hand-picked subset: a form feed is no more content than a space, and #1610 exists precisely because whether a file got flagged used to hinge on such incidentals. Newline stays excluded because a terminated final line produces no MISSING terminator at all. The rule remains ZERO-WIDTH ONLY. A width-bearing MISSING/ERROR at EOF is a genuine loss and is still flagged (a Makefile's unterminated final recipe really does vanish), as is any failure earlier in the file; both are pinned by guard tests alongside the existing #1610 guards. Verified end to end: the issue's own `cli index_repository` reproduction goes from parse_partial_count 1 to 0 with node count unchanged, so the phantom flag is gone without anything dropping out of the graph. Fixes #1746 Signed-off-by: Rares Popa <2606875+rarepops@users.noreply.github.com>
1 parent 31819c9 commit cbb456a

2 files changed

Lines changed: 119 additions & 8 deletions

File tree

‎internal/cbm/cbm.c‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -810,30 +810,52 @@ static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) {
810810
* Deliberately narrow — ZERO-WIDTH AT EOF ONLY. A MISSING or ERROR node with
811811
* WIDTH still counts even at EOF (a Makefile whose last recipe line is
812812
* unterminated really does lose the recipe), and anything before EOF is
813-
* untouched. */
814-
static bool cbm_is_eof_terminator_miss(TSNode n, int source_len) {
813+
* untouched.
814+
*
815+
* #1746: "at EOF" means EOF modulo trailing blanks. Blanks are extras owned by
816+
* no node, so `ENTRYPOINT ["a"] ` + EOF parks the terminator one byte short of
817+
* source_len and an exact end == source_len test missed it. Every blank except
818+
* newline counts: a file ending in \n has a terminated final line and produces
819+
* no MISSING terminator at all. */
820+
static bool cbm_is_blank_not_newline(char c) {
821+
return c == ' ' || c == '\t' || c == '\v' || c == '\f' || c == '\r';
822+
}
823+
824+
static bool cbm_is_eof_terminator_miss(TSNode n, const char *source, int source_len) {
815825
if (!ts_node_is_missing(n) || source_len < 0) {
816826
return false;
817827
}
818828
uint32_t start = ts_node_start_byte(n);
819829
uint32_t end = ts_node_end_byte(n);
820-
return start == end && end == (uint32_t)source_len;
830+
if (start != end || end > (uint32_t)source_len) {
831+
return false;
832+
}
833+
if (!source) {
834+
return end == (uint32_t)source_len;
835+
}
836+
for (uint32_t i = end; i < (uint32_t)source_len; i++) {
837+
if (!cbm_is_blank_not_newline(source[i])) {
838+
return false;
839+
}
840+
}
841+
return true;
821842
}
822843

823-
static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc, int source_len) {
844+
static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc, const char *source,
845+
int source_len) {
824846
if (acc->count >= CBM_MAX_ERROR_REGIONS) {
825847
return;
826848
}
827849
uint32_t k = ts_node_child_count(n);
828850
for (uint32_t i = 0; i < k && acc->count < CBM_MAX_ERROR_REGIONS; i++) {
829851
TSNode c = ts_node_child(n, i);
830852
if (ts_node_is_missing(c) || strcmp(ts_node_type(c), "ERROR") == 0) {
831-
if (cbm_is_eof_terminator_miss(c, source_len)) {
853+
if (cbm_is_eof_terminator_miss(c, source, source_len)) {
832854
continue; /* absent final newline only — nothing was dropped */
833855
}
834856
cbm_error_regions_push(acc, c); /* top-most region; do not descend */
835857
} else if (ts_node_has_error(c)) {
836-
cbm_collect_error_regions(c, acc, source_len);
858+
cbm_collect_error_regions(c, acc, source, source_len);
837859
}
838860
}
839861
}
@@ -1456,7 +1478,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
14561478
* already extract. */
14571479
if (ts_node_has_error(root)) {
14581480
cbm_error_regions_t raw_regs = {{0}, {0}, 0};
1459-
cbm_collect_error_regions(root, &raw_regs, source_len);
1481+
cbm_collect_error_regions(root, &raw_regs, source, source_len);
14601482
if (raw_regs.count > 0) {
14611483
int defs_before = result->defs.count;
14621484
cbm_extract_definitions(&pp_ctx);
@@ -1614,7 +1636,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
16141636
if (strcmp(ts_node_type(root), "ERROR") == 0) {
16151637
cbm_error_regions_push(&regs, root); /* whole file unparseable */
16161638
} else {
1617-
cbm_collect_error_regions(root, &regs, source_len);
1639+
cbm_collect_error_regions(root, &regs, source, source_len);
16181640
}
16191641
cbm_subtract_recovered_regions(&regs, &result->defs);
16201642
/* #1071: don't flag a benign function-like-macro call (defined in-file)

‎tests/test_parse_coverage.c‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,91 @@ TEST(width_bearing_error_at_eof_still_flagged_issue1610) {
369369
PASS();
370370
}
371371

372+
/* ── #1746: trailing blanks before EOF are still just an absent newline ───────
373+
*
374+
* #1610 suppressed the zero-width MISSING terminator but tested for it with
375+
* `end == source_len`. Trailing blanks are extras owned by no node, so
376+
* `ENTRYPOINT ["a"] ` + EOF parks it at [29,29) while source_len is 30.
377+
*
378+
* The reporter's byte-exact controls pin the trigger to the PAIR: `] ` + EOF
379+
* flags, `] ` + newline is clean, `]` + EOF is clean. */
380+
TEST(dockerfile_trailing_blank_at_eof_not_flagged_issue1746) {
381+
const char *cases[] = {
382+
"FROM scratch\nENTRYPOINT [\"a\"] ", /* space + EOF — the report */
383+
"FROM scratch\nENTRYPOINT [\"a\"]\t", /* tab + EOF */
384+
"FROM scratch\nENTRYPOINT [\"a\"]\v", /* vertical tab + EOF */
385+
"FROM scratch\nENTRYPOINT [\"a\"]\f", /* form feed + EOF */
386+
"FROM scratch\nENTRYPOINT [\"a\"] \t ", /* run of blanks + EOF */
387+
"FROM scratch\nENTRYPOINT [\"a\"] \r", /* CRLF file truncated to CR */
388+
};
389+
for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
390+
CBMFileResult *r = do_extract(cases[i], CBM_LANG_DOCKERFILE, "Dockerfile");
391+
ASSERT_NOT_NULL(r);
392+
bool flagged = r->parse_incomplete;
393+
if (flagged) {
394+
fprintf(stderr, " case %zu flagged: ranges=%s\n", i,
395+
r->error_ranges ? r->error_ranges : "(none)");
396+
}
397+
cbm_free_result(r);
398+
if (flagged) {
399+
FAIL("trailing blanks must not turn an absent final newline into parse_partial");
400+
}
401+
}
402+
PASS();
403+
}
404+
405+
/* The reporter's third control: blanks BEFORE a real newline were always clean
406+
* and must stay that way, pinning the pair-trigger from both sides. */
407+
TEST(dockerfile_trailing_blank_then_newline_still_clean_issue1746) {
408+
const char *src = "FROM scratch\nENTRYPOINT [\"a\"] \n";
409+
CBMFileResult *r = do_extract(src, CBM_LANG_DOCKERFILE, "Dockerfile");
410+
ASSERT_NOT_NULL(r);
411+
bool flagged = r->parse_incomplete;
412+
cbm_free_result(r);
413+
if (flagged) {
414+
FAIL("a terminated Dockerfile must not be parse_partial because of trailing blanks");
415+
}
416+
PASS();
417+
}
418+
419+
/* GUARD: widening the tail must not swallow a genuine mid-file failure just
420+
* because the file happens to end in blanks. */
421+
TEST(real_error_before_eof_still_flagged_with_trailing_blank_issue1746) {
422+
size_t n = strlen(C_IFDEF_SPLIT);
423+
char *buf = (char *)malloc(n + 1);
424+
ASSERT_NOT_NULL(buf);
425+
memcpy(buf, C_IFDEF_SPLIT, n + 1);
426+
buf[n - 1] = ' '; /* final newline becomes a blank */
427+
428+
CBMFileResult *r = do_extract(buf, CBM_LANG_C, "split.c");
429+
free(buf);
430+
ASSERT_NOT_NULL(r);
431+
bool flagged = r->parse_incomplete;
432+
bool has_ranges = r->error_ranges != NULL;
433+
cbm_free_result(r);
434+
if (!flagged) {
435+
FAIL("a real mid-file parse failure must still be reported when the file ends in blanks");
436+
}
437+
if (!has_ranges) {
438+
FAIL("a reported failure must still name its line range");
439+
}
440+
PASS();
441+
}
442+
443+
/* GUARD: a WIDTH-BEARING loss at EOF stays honest with a blank tail too — the
444+
* Makefile recipe really is dropped, and only zero-width nodes are excused. */
445+
TEST(width_bearing_error_at_eof_still_flagged_with_trailing_blank_issue1746) {
446+
const char *src = "all:\n\techo hi ";
447+
CBMFileResult *r = do_extract(src, CBM_LANG_MAKEFILE, "Makefile");
448+
ASSERT_NOT_NULL(r);
449+
bool flagged = r->parse_incomplete;
450+
cbm_free_result(r);
451+
if (!flagged) {
452+
FAIL("a width-bearing parse failure at EOF must still be reported when the file ends in blanks");
453+
}
454+
PASS();
455+
}
456+
372457
SUITE(parse_coverage) {
373458
RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete);
374459
RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted);
@@ -384,4 +469,8 @@ SUITE(parse_coverage) {
384469
RUN_TEST(missing_final_newline_not_flagged_across_grammars_issue1610);
385470
RUN_TEST(real_error_before_eof_still_flagged_without_final_newline_issue1610);
386471
RUN_TEST(width_bearing_error_at_eof_still_flagged_issue1610);
472+
RUN_TEST(dockerfile_trailing_blank_at_eof_not_flagged_issue1746);
473+
RUN_TEST(dockerfile_trailing_blank_then_newline_still_clean_issue1746);
474+
RUN_TEST(real_error_before_eof_still_flagged_with_trailing_blank_issue1746);
475+
RUN_TEST(width_bearing_error_at_eof_still_flagged_with_trailing_blank_issue1746);
387476
}

0 commit comments

Comments
 (0)