From 8669ba8f9e4f03217c42ed616d61458360c6c303 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 15 Aug 2026 16:09:04 +0200 Subject: [PATCH 1/2] fix(extract): a missing final newline is not a partial parse A file that does not end with a newline leaves the grammar's mandatory line terminator MISSING. cbm_collect_error_regions counted that node, so the file was reported parse_partial with the last line as its error range. It is not a miss. The 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 - and every real instruction above it parsed normally. Proven by dumping the tree: the reporter's two-line Dockerfile yields (source_file (from_instruction ...) (entrypoint_instruction ...) (MISSING "\n")) with both instructions intact and the MISSING node spanning bytes 73-73. It was never Dockerfile-specific. Stripping the trailing newline from the 156 linkable grammar fixtures flips 13 of them to has_error, and SIX produce regions: dockerfile, tcl, fish, gomod, hyprlang - and makefile, which is a genuinely different case (its ERROR has WIDTH; the recipe really is lost). Worse, the ones that stayed silent did so for no principled reason. ini, fsharp, beancount, requirements, gitignore, sshconfig and kconfig omit the same terminator, but theirs is a HIDDEN node and hidden nodes are invisible to ts_node_child(). Whether a user was told their file was partially parsed came down to whether that grammar's author declared the terminator visible. The cost was not cosmetic: a phantom parse_partial writes a "::missed" shadow row, and until #1609 that row made the project fail cross-repo validation as BOTH source and target. A single absent byte could remove an entire repository from cross-repo intelligence with no error shown anywhere. The suppression is deliberately narrow - zero-width AND at EOF. A MISSING or ERROR node with width still counts even at EOF, and anything before EOF is untouched. Both callers pass the raw root, so one source_len is correct for both; verified rather than assumed, since root is bound once and never reassigned. Reported by @vitaliy-shatskiy, who could not share the original file and instead rebuilt the property from scratch with a byte-exact script - an editor would have silently re-added the newline and hidden it. Their isolation matrix ruled out BOM, CRLF vs LF, exec-form vs shell-form and file length before we looked at it once. Reproduce-first, revert-checked: the Dockerfile and cross-grammar tests fail on the previous tree and pass with the fix; forcing the new predicate to return false brings the identical REDs back. Two guards pin the boundary and hold in both directions - a width-bearing failure at EOF (makefile) and a real mid-file ERROR in a file that ALSO lacks its final newline (built from C_IFDEF_SPLIT, the fixture this suite already proves is flagged). parse_coverage 14, extraction 276, language 217, infrascan 3, grammar_regression 1 - 511 passed, 0 failed. Signed-off-by: Martin Vogel --- internal/cbm/cbm.c | 44 +++++++++++-- tests/test_parse_coverage.c | 120 ++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 4 deletions(-) diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index ea149dd9c..d7468af5a 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -787,7 +787,40 @@ 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 + * "::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; } @@ -795,9 +828,12 @@ static void cbm_collect_error_regions(TSNode n, cbm_error_regions_t *acc) { 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); } } } @@ -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); @@ -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(®s, root); /* whole file unparseable */ } else { - cbm_collect_error_regions(root, ®s); + cbm_collect_error_regions(root, ®s, source_len); } cbm_subtract_recovered_regions(®s, &result->defs); /* #1071: don't flag a benign function-like-macro call (defined in-file) diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 7d3645ffb..39ce43c20 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -247,6 +247,121 @@ 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 "::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; + const char *ranges = r->error_ranges; + if (flagged) { + FAIL("a Dockerfile lacking only its final newline must not be parse_partial"); + } + (void)ranges; + 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); + if (r->parse_incomplete) { + 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); + if (r->parse_incomplete) { + fprintf(stderr, " %s flagged: ranges=%s\n", cases[i].path, + r->error_ranges ? r->error_ranges : "(none)"); + 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"); + bool flagged = r && r->parse_incomplete; + bool has_ranges = r && r->error_ranges != NULL; + free(unterminated); + + ASSERT_NOT_NULL(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); + if (!r->parse_incomplete) { + 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); @@ -257,4 +372,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); } From 63f0a6c0e7c03e09e3bc7724b017bf74517dd7c8 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 15 Aug 2026 17:22:59 +0200 Subject: [PATCH 2/2] test(parse-coverage): free the extraction results in the #1610 tests LeakSanitizer on CI caught all five new tests leaking their CBMFileResult: Indirect leak of 24 byte(s) ... ts_tree_new cbm_extract_file_ex cbm.c:1256 do_extract test_parse_coverage.c:39 test_dockerfile_missing_final_newline_not_flagged_issue1610:272 SUMMARY: AddressSanitizer: 706504 byte(s) leaked in 189 allocation(s) Every pre-existing test in this suite calls cbm_free_result before PASS; the new ones did not. The local run could not have found it - LeakSanitizer reports "detect_leaks is not supported on this platform" on macOS arm64, so this class of defect is CI-only here. Each test now captures what it asserts, frees, and only then decides, so the early-FAIL paths do not leak either. The cross-grammar loop prints its diagnostic before freeing so the failure message keeps naming the grammar. While correcting the guard, a first attempt left ASSERT_TRUE(flagged || has_ranges || true) in real_error_before_eof_still_flagged - always true, and it would have silently disarmed the guard that stops the EOF suppression from being over-broad. Removed. The guard is re-proven binding: forcing cbm_is_eof_terminator_miss to return true makes EIGHT tests fail, including both guards, and restoring it returns the suite to green. parse_coverage 14 passed. Signed-off-by: Martin Vogel --- tests/test_parse_coverage.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/test_parse_coverage.c b/tests/test_parse_coverage.c index 39ce43c20..1c0720c94 100644 --- a/tests/test_parse_coverage.c +++ b/tests/test_parse_coverage.c @@ -272,11 +272,10 @@ TEST(dockerfile_missing_final_newline_not_flagged_issue1610) { CBMFileResult *r = do_extract(src, CBM_LANG_DOCKERFILE, "Dockerfile"); ASSERT_NOT_NULL(r); bool flagged = r->parse_incomplete; - const char *ranges = r->error_ranges; + cbm_free_result(r); if (flagged) { FAIL("a Dockerfile lacking only its final newline must not be parse_partial"); } - (void)ranges; PASS(); } @@ -288,7 +287,9 @@ TEST(dockerfile_with_final_newline_still_clean_issue1610) { "ENTRYPOINT [\"dotnet\", \"App.dll\"]\n"; CBMFileResult *r = do_extract(src, CBM_LANG_DOCKERFILE, "Dockerfile"); ASSERT_NOT_NULL(r); - if (r->parse_incomplete) { + bool flagged = r->parse_incomplete; + cbm_free_result(r); + if (flagged) { FAIL("a terminated Dockerfile must not be parse_partial"); } PASS(); @@ -310,9 +311,13 @@ TEST(missing_final_newline_not_flagged_across_grammars_issue1610) { 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); - if (r->parse_incomplete) { + 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"); } } @@ -335,11 +340,11 @@ TEST(real_error_before_eof_still_flagged_without_final_newline_issue1610) { unterminated[n - 1] = '\0'; /* drop the final newline */ CBMFileResult *r = do_extract(unterminated, CBM_LANG_C, "split.c"); - bool flagged = r && r->parse_incomplete; - bool has_ranges = r && r->error_ranges != NULL; 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"); } @@ -356,7 +361,9 @@ 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); - if (!r->parse_incomplete) { + bool flagged = r->parse_incomplete; + cbm_free_result(r); + if (!flagged) { FAIL("a width-bearing parse failure at EOF must still be reported"); } PASS();