fix(extract): address review on PHP inline-HTML masking (#2000)

Signed-off-by: harshitaajoshi <j.harshitaa06@gmail.com>

internal/cbm/php_inline_html.c

@@ -263,6 +263,47 @@ static int php_scan_code(const char *s, int len, int i) {
     return len;
 }
 
+/* The rewritten buffer. It is allocated and filled from the source on the
+ * first write, so a file whose only `?>` bytes sit in strings or comments
+ * costs no copy. */
+typedef struct {
+    CBMArena *arena;
+    const char *source;
+    int len;
+    char *out;
+    bool failed;
+} PHPMask;
+
+static char *php_mask_writable(PHPMask *m) {
+    if (!m->out && !m->failed) {
+        m->out = (char *)cbm_arena_alloc(m->arena, (size_t)m->len + SKIP_ONE);
+        if (m->out) {
+            memcpy(m->out, m->source, (size_t)m->len);
+            m->out[m->len] = '\0';
+        } else {
+            m->failed = true;
+        }
+    }
+    return m->out;
+}
+
+/* Blank [from, to) of the rewritten buffer, keeping line breaks. */
+static void php_mask_blank(PHPMask *m, int from, int to) {
+    char *out = php_mask_writable(m);
+    if (out) {
+        php_blank(out, from, to);
+    }
+}
+
+/* Turn the `?>` at i into `; `. */
+static void php_mask_close_tag(PHPMask *m, int i) {
+    char *out = php_mask_writable(m);
+    if (out) {
+        out[i] = ';';
+        out[i + SKIP_ONE] = ' ';
+    }
+}
+
 const char *cbm_php_mask_inline_html(CBMArena *arena, const char *source, int source_len) {
     if (!arena || !source || source_len <= 0) {
         return source;
@@ -273,17 +314,10 @@ const char *cbm_php_mask_inline_html(CBMArena *arena, const char *source, int so
         return source;
     }
 
-    char *out = (char *)cbm_arena_alloc(arena, (size_t)source_len + SKIP_ONE);
-    if (!out) {
-        return source;
-    }
-    memcpy(out, source, (size_t)source_len);
-    out[source_len] = '\0';
-
-    bool changed = false;
+    PHPMask m = {.arena = arena, .source = source, .len = source_len};
     bool seen_open = false;
     int i = 0;
-    while (i < source_len) {
+    while (i < source_len && !m.failed) {
         /* Inline-HTML mode: blank up to the next open tag. */
         int html_start = i;
         int tag = 0;
@@ -291,15 +325,13 @@ const char *cbm_php_mask_inline_html(CBMArena *arena, const char *source, int so
             i++;
         }
         if (i > html_start) {
-            php_blank(out, html_start, i);
-            changed = true;
+            php_mask_blank(&m, html_start, i);
         }
         if (i >= source_len) {
             break;
         }
         if (seen_open) {
-            php_blank(out, i, i + tag);
-            changed = true;
+            php_mask_blank(&m, i, i + tag);
         }
         seen_open = true;
 
@@ -308,10 +340,9 @@ const char *cbm_php_mask_inline_html(CBMArena *arena, const char *source, int so
         if (close < 0 || close >= source_len) {
             break;
         }
-        out[close] = ';';
-        out[close + SKIP_ONE] = ' ';
-        changed = true;
+        php_mask_close_tag(&m, close);
         i = close + PAIR_LEN;
     }
-    return changed ? out : source;
+    /* On allocation failure parse the file as it is: the old behaviour. */
+    return (m.out && !m.failed) ? m.out : source;
 }

tests/test_extraction.c

@@ -938,6 +938,56 @@ TEST(php_inline_html_close_tag_in_literals_issue2000) {
     PASS();
 }
 
+/* Markup is blanked, never parsed: code-shaped text outside the PHP tags must
+ * not produce definitions or calls, wherever it sits. */
+TEST(php_inline_html_markup_never_reaches_graph_issue2000) {
+    CBMFileResult *r = extract("<p>function before() { return leakBefore(); }</p>\n"
+                               "<?php\n"
+                               "function real() { return 1; }\n"
+                               "?>\n"
+                               "<p>function fake() { return leak(); }</p>\n"
+                               "<script>function jsFake() { leakScript(); }</script>\n"
+                               "<?xml version=\"1.0\"?><x>function xmlFake() { leakXml(); }</x>\n"
+                               "<?php\n"
+                               "function after() { return real(); }\n"
+                               "?>\n"
+                               "<p>class Tail { function tailFake() { leakTail(); } }</p>\n",
+                               CBM_LANG_PHP, "t", "markup.php");
+    ASSERT_NOT_NULL(r);
+    ASSERT_FALSE(r->has_error);
+    ASSERT(has_def(r, "Function", "real"));
+    ASSERT(has_def(r, "Function", "after"));
+    ASSERT(has_call(r, "real"));
+    ASSERT_FALSE(has_def_any(r, "before"));
+    ASSERT_FALSE(has_def_any(r, "fake"));
+    ASSERT_FALSE(has_def_any(r, "jsFake"));
+    ASSERT_FALSE(has_def_any(r, "xmlFake"));
+    ASSERT_FALSE(has_def_any(r, "Tail"));
+    ASSERT_FALSE(has_def_any(r, "tailFake"));
+    ASSERT_FALSE(has_call(r, "leak"));
+    ASSERT_FALSE(r->parse_incomplete);
+    cbm_free_result(r);
+    PASS();
+}
+
+/* The mask hands back the caller's buffer, uncopied, when nothing needs
+ * rewriting, and a same-length rewrite otherwise. */
+TEST(php_inline_html_mask_copies_only_on_write_issue2000) {
+    CBMArena arena;
+    cbm_arena_init(&arena);
+    const char *plain = "<?php\nfunction a() { return 1; }\n";
+    const char *quoted = "<?php\n$s = '?>';\n/* ?> */\n$h = <<<EOT\n?>\nEOT;\n";
+    const char *mixed = "<?php\nfunction a() {}\n?>\n<b>x</b>\n";
+    ASSERT(cbm_php_mask_inline_html(&arena, plain, (int)strlen(plain)) == plain);
+    ASSERT(cbm_php_mask_inline_html(&arena, quoted, (int)strlen(quoted)) == quoted);
+    const char *masked = cbm_php_mask_inline_html(&arena, mixed, (int)strlen(mixed));
+    ASSERT(masked != mixed);
+    ASSERT_EQ((int)strlen(masked), (int)strlen(mixed));
+    ASSERT_STR_EQ(masked, "<?php\nfunction a() {}\n; \n        \n");
+    cbm_arena_destroy(&arena);
+    PASS();
+}
+
 /* A trailing `?>` (common at the end of older class files) is not a partial
  * parse, and CRLF line endings keep their line numbers. */
 TEST(php_inline_html_trailing_close_tag_crlf_issue2000) {
@@ -13252,6 +13302,8 @@ SUITE(extraction) {
     RUN_TEST(php_inline_html_tail_reaches_graph_issue2000);
     RUN_TEST(php_inline_html_template_shapes_issue2000);
     RUN_TEST(php_inline_html_close_tag_in_literals_issue2000);
+    RUN_TEST(php_inline_html_markup_never_reaches_graph_issue2000);
+    RUN_TEST(php_inline_html_mask_copies_only_on_write_issue2000);
     RUN_TEST(php_inline_html_trailing_close_tag_crlf_issue2000);
     RUN_TEST(ruby_class);
     RUN_TEST(ruby_module);

tests/test_php_lsp.c

@@ -110,6 +110,65 @@ TEST(phplsp_local_method_via_typed_param) {
     PASS();
 }
 
+/* #2000: with no cached tree (a result spilled under the memory budget comes
+ * back without one) the cross-file pass re-parses the on-disk source itself.
+ * It must blank inline HTML exactly as the per-file extract does, or a file's
+ * tail would lose cross-file resolution only when that file spilled. */
+static int find_resolved_arr(const CBMResolvedCallArray *arr, const char *callerSub,
+                             const char *calleeSub) {
+    for (int i = 0; i < arr->count; i++) {
+        const CBMResolvedCall *rc = &arr->items[i];
+        if (rc->caller_qn && strstr(rc->caller_qn, callerSub) && rc->callee_qn &&
+            strstr(rc->callee_qn, calleeSub))
+            return i;
+    }
+    return -1;
+}
+
+TEST(phplsp_cross_reparse_reads_past_inline_html_issue2000) {
+    const char *src = "<?php\n"
+                      "function gamma() { return 2; }\n"
+                      "?>\n"
+                      "<div class=\"box\">\n"
+                      "  <p>plain markup after the closing tag</p>\n"
+                      "</div>\n"
+                      "<?php\n"
+                      "function delta(Mailer $m) { return $m->send(); }\n";
+    CBMLSPDef defs[] = {
+        {.qualified_name = "test.main.gamma",
+         .short_name = "gamma",
+         .label = "Function",
+         .def_module_qn = "test.main"},
+        {.qualified_name = "test.main.delta",
+         .short_name = "delta",
+         .label = "Function",
+         .def_module_qn = "test.main"},
+        {.qualified_name = "test.mailer.Mailer",
+         .short_name = "Mailer",
+         .label = "Class",
+         .def_module_qn = "test.mailer"},
+        {.qualified_name = "test.mailer.Mailer.send",
+         .short_name = "send",
+         .label = "Method",
+         .def_module_qn = "test.mailer",
+         .receiver_type = "test.mailer.Mailer"},
+    };
+    const char *imp_names[] = {"Mailer"};
+    const char *imp_qns[] = {"test.mailer.Mailer"};
+
+    CBMArena arena;
+    cbm_arena_init(&arena);
+    CBMResolvedCallArray out = {0};
+
+    cbm_run_php_lsp_cross(&arena, src, (int)strlen(src), "test.main", defs, 4, imp_names, imp_qns,
+                          1, NULL, &out);
+
+    ASSERT_GTE(find_resolved_arr(&out, "main.delta", "Mailer.send"), 0);
+
+    cbm_arena_destroy(&arena);
+    PASS();
+}
+
 /* ── 3. Arrow function with typed parameter ───────────────────── */
 
 TEST(phplsp_arrow_function_typed_param) {
@@ -5841,6 +5900,7 @@ SUITE(php_lsp) {
     /* Phase 1 baseline regressions */
     RUN_TEST(phplsp_local_method_via_new_assignment);
     RUN_TEST(phplsp_local_method_via_typed_param);
+    RUN_TEST(phplsp_cross_reparse_reads_past_inline_html_issue2000);
     RUN_TEST(phplsp_arrow_function_typed_param);
     RUN_TEST(phplsp_static_call_resolved);
     RUN_TEST(phplsp_self_and_parent);

tests/test_pipeline.c

@@ -1405,15 +1405,7 @@ TEST(pipeline_php_inline_html_calls_resolve_issue2000) {
                           "function renderCard($item) { return formatPrice($item['cents']); }\n"},
     };
     for (size_t i = 0; i < sizeof(files) / sizeof(files[0]); i++) {
-        char path[512];
-        snprintf(path, sizeof(path), "%s/%s", g_tmpdir, files[i].name);
-        FILE *f = fopen(path, "w");
-        if (!f) {
-            teardown_test_repo();
-            FAIL("failed to write php fixture");
-        }
-        fputs(files[i].body, f);
-        fclose(f);
+        write_temp_file(g_tmpdir, files[i].name, files[i].body);
     }
 
     char db[512];