Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions internal/cbm/cbm.c
Original file line number Diff line number Diff line change
Expand Up @@ -787,17 +787,53 @@ static void cbm_error_regions_push(cbm_error_regions_t *acc, TSNode n) {
acc->count++;
}

static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc) {
/* #1610: a file that does not end with a newline leaves the grammar's
* mandatory line terminator MISSING. That node is ZERO-WIDTH and sits at EOF.
*
* It is not a miss. The parser consumed no source for it — start_byte ==
* end_byte — so by construction nothing was dropped: no construct can live in
* a zero-byte span, and every real instruction above it parsed normally. This
* is a property of the grammar's terminator rule, not of the file.
*
* Flagging it made the verdict arbitrary. Grammars whose terminator token is
* VISIBLE (dockerfile, tcl, fish, gomod, hyprlang) reported parse_partial for
* a missing final newline; grammars whose terminator is HIDDEN (ini, fsharp,
* beancount, requirements, gitignore, sshconfig, kconfig) reported nothing for
* exactly the same omission, because a hidden node is invisible to
* ts_node_child(). Whether a user was told their file was partially parsed
* depended on a grammar-authoring accident.
*
* The cost was not cosmetic: a phantom parse_partial writes a
* "<project>::missed" shadow row, and until #1609 that row made the project
* fail cross-repo validation as both source and target.
*
* Deliberately narrow — ZERO-WIDTH AT EOF ONLY. A MISSING or ERROR node with
* WIDTH still counts even at EOF (a Makefile whose last recipe line is
* unterminated really does lose the recipe), and anything before EOF is
* untouched. */
static bool cbm_is_eof_terminator_miss(TSNode n, int source_len) {
if (!ts_node_is_missing(n) || source_len < 0) {
return false;
}
uint32_t start = ts_node_start_byte(n);
uint32_t end = ts_node_end_byte(n);
return start == end && end == (uint32_t)source_len;
}

static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc, int source_len) {
if (acc->count >= CBM_MAX_ERROR_REGIONS) {
return;
}
uint32_t k = ts_node_child_count(n);
for (uint32_t i = 0; i < k && acc->count < CBM_MAX_ERROR_REGIONS; i++) {
TSNode c = ts_node_child(n, i);
if (ts_node_is_missing(c) || strcmp(ts_node_type(c), "ERROR") == 0) {
if (cbm_is_eof_terminator_miss(c, source_len)) {
continue; /* absent final newline only — nothing was dropped */
}
cbm_error_regions_push(acc, c); /* top-most region; do not descend */
} else if (ts_node_has_error(c)) {
cbm_collect_error_regions(c, acc);
cbm_collect_error_regions(c, acc, source_len);
}
}
}
Expand Down Expand Up @@ -1413,7 +1449,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
* already extract. */
if (ts_node_has_error(root)) {
cbm_error_regions_t raw_regs = {{0}, {0}, 0};
cbm_collect_error_regions(root, &raw_regs);
cbm_collect_error_regions(root, &raw_regs, source_len);
if (raw_regs.count > 0) {
int defs_before = result->defs.count;
cbm_extract_definitions(&pp_ctx);
Expand Down Expand Up @@ -1571,7 +1607,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
if (strcmp(ts_node_type(root), "ERROR") == 0) {
cbm_error_regions_push(&regs, root); /* whole file unparseable */
} else {
cbm_collect_error_regions(root, &regs);
cbm_collect_error_regions(root, &regs, source_len);
}
cbm_subtract_recovered_regions(&regs, &result->defs);
/* #1071: don't flag a benign function-like-macro call (defined in-file)
Expand Down
127 changes: 127 additions & 0 deletions tests/test_parse_coverage.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,128 @@ TEST(c_trailing_recovered_defs_keep_flag) {

/* ── Suite ────────────────────────────────────────────────────────────────── */

/* ── #1610: a missing FINAL NEWLINE is not a parse failure ────────────────────
*
* A file that does not end with "\n" leaves the grammar's mandatory line
* terminator MISSING. That node is ZERO-WIDTH and sits at EOF: the parser
* consumed no source for it, so by construction nothing was dropped — no
* construct can live in a zero-byte span. Every instruction still parses.
*
* Reported on #1610 for Dockerfile, where a reporter proved with a byte-exact
* matrix that the trigger is independent of BOM, CRLF/LF, exec-form vs
* shell-form and file length — it is purely the absent final newline.
*
* It was never Dockerfile-specific: tcl, fish, gomod and hyprlang flag the same
* way, while ini, fsharp, beancount and others do NOT — only because those
* grammars declare the terminator token hidden rather than visible. Whether a
* user saw a phantom parse_partial came down to a grammar-authoring accident.
*
* The cost was not cosmetic: a phantom flag writes a "<project>::missed" shadow
* row, and until #1609 that row removed the whole project from cross-repo
* linking, as source AND as target. */
TEST(dockerfile_missing_final_newline_not_flagged_issue1610) {
const char *src = "FROM mcr.microsoft.com/dotnet/aspnet:8.0\n"
"ENTRYPOINT [\"dotnet\", \"App.dll\"]"; /* deliberately no \n */
CBMFileResult *r = do_extract(src, CBM_LANG_DOCKERFILE, "Dockerfile");
ASSERT_NOT_NULL(r);
bool flagged = r->parse_incomplete;
cbm_free_result(r);
if (flagged) {
FAIL("a Dockerfile lacking only its final newline must not be parse_partial");
}
PASS();
}

/* The same bytes WITH the newline must stay clean — pins the equivalence the
* reporter's matrix proved, so a future change cannot "fix" one by breaking the
* other. */
TEST(dockerfile_with_final_newline_still_clean_issue1610) {
const char *src = "FROM mcr.microsoft.com/dotnet/aspnet:8.0\n"
"ENTRYPOINT [\"dotnet\", \"App.dll\"]\n";
CBMFileResult *r = do_extract(src, CBM_LANG_DOCKERFILE, "Dockerfile");
ASSERT_NOT_NULL(r);
bool flagged = r->parse_incomplete;
cbm_free_result(r);
if (flagged) {
FAIL("a terminated Dockerfile must not be parse_partial");
}
PASS();
}

/* Language-general, not a Dockerfile patch: these four were each proven to flag
* on a stripped trailing newline. */
TEST(missing_final_newline_not_flagged_across_grammars_issue1610) {
struct {
const char *src;
CBMLanguage lang;
const char *path;
} cases[] = {
{"proc foo {} {}\nproc bar {} {}", CBM_LANG_TCL, "a.tcl"},
{"function foo\n echo hi\nend", CBM_LANG_FISH, "a.fish"},
{"module example.com/m\n\ngo 1.21", CBM_LANG_GOMOD, "go.mod"},
{"general {\n gaps_in = 5\n}", CBM_LANG_HYPRLANG, "hypr.conf"},
};
for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
CBMFileResult *r = do_extract(cases[i].src, cases[i].lang, cases[i].path);
ASSERT_NOT_NULL(r);
bool flagged = r->parse_incomplete;
if (flagged) {
fprintf(stderr, " %s flagged: ranges=%s\n", cases[i].path,
r->error_ranges ? r->error_ranges : "(none)");
}
cbm_free_result(r);
if (flagged) {
FAIL("an unterminated final line must not be parse_partial in any grammar");
}
}
PASS();
}

/* GUARD (the reason this suppression is safe rather than convenient): the rule
* is ZERO-WIDTH AT EOF only. A real failure earlier in the file must still be
* reported, and its range must name the broken line — not be swallowed along
* with the terminator. */
TEST(real_error_before_eof_still_flagged_without_final_newline_issue1610) {
/* Built from C_IFDEF_SPLIT, the fixture this suite already proves is
* flagged, with its trailing newline removed. Two conditions now hold at
* once: a genuine width-bearing ERROR mid-file, AND an unterminated last
* line. Suppressing the EOF terminator must not swallow the real one. */
size_t n = strlen(C_IFDEF_SPLIT);
char *unterminated = (char *)malloc(n + 1);
ASSERT_NOT_NULL(unterminated);
memcpy(unterminated, C_IFDEF_SPLIT, n);
unterminated[n - 1] = '\0'; /* drop the final newline */

CBMFileResult *r = do_extract(unterminated, CBM_LANG_C, "split.c");
free(unterminated);
ASSERT_NOT_NULL(r);
bool flagged = r->parse_incomplete;
bool has_ranges = r->error_ranges != NULL;
cbm_free_result(r);
if (!flagged) {
FAIL("a real mid-file parse failure must still be reported when the file also lacks its final newline");
}
if (!has_ranges) {
FAIL("a reported failure must still name its line range");
}
PASS();
}

/* GUARD: a MISSING/ERROR node WITH WIDTH at EOF is a genuine loss and must
* still be flagged. A Makefile whose final recipe line lacks its newline really
* does drop the recipe from the tree — cbm's flag is honest there. */
TEST(width_bearing_error_at_eof_still_flagged_issue1610) {
const char *src = "all:\n\techo hi"; /* no trailing newline; recipe is lost */
CBMFileResult *r = do_extract(src, CBM_LANG_MAKEFILE, "Makefile");
ASSERT_NOT_NULL(r);
bool flagged = r->parse_incomplete;
cbm_free_result(r);
if (!flagged) {
FAIL("a width-bearing parse failure at EOF must still be reported");
}
PASS();
}

SUITE(parse_coverage) {
RUN_TEST(c_ifdef_split_brace_sets_parse_incomplete);
RUN_TEST(c_ifdef_split_brace_neighbors_still_extracted);
Expand All @@ -257,4 +379,9 @@ SUITE(parse_coverage) {
RUN_TEST(py_clean_file_not_flagged);
RUN_TEST(error_region_cap_is_honored);
RUN_TEST(c_trailing_recovered_defs_keep_flag);
RUN_TEST(dockerfile_missing_final_newline_not_flagged_issue1610);
RUN_TEST(dockerfile_with_final_newline_still_clean_issue1610);
RUN_TEST(missing_final_newline_not_flagged_across_grammars_issue1610);
RUN_TEST(real_error_before_eof_still_flagged_without_final_newline_issue1610);
RUN_TEST(width_bearing_error_at_eof_still_flagged_issue1610);
}
Loading