Skip to content

Commit e5a4dc0

Browse files
committed
fix(php): keep PHP after comment-ended close tags, CR comments and bare short tags
Addresses CodeRabbit review on PR #34 (internal/cbm/php_inline_html.c and two test findings). Inline-HTML masker: - Line comments now end at a lone CR as well as LF, so CR-only files do not scan into the next line and treat `?>` inside a string as a close tag. - A `?>` that ends a `//` or `#` comment used to become `; ` with the comment still open, hiding PHP reopened on the same line. The comment bytes up to the close tag are now blanked to spaces. Byte offsets and rows are unchanged; only that comment's text is lost to the parser. - With short tags, `<?` followed directly by an identifier (`<?work();`) now opens PHP. `<?xml` prologs stay markup. Tests: - test_daemon_application: two worker resource-failure tests reused th_mktempdir's static buffer for cache and root, so cleanup removed the root twice and leaked the cache directory. Both paths are now copied. - test_pipeline: swift_init_observe leaves init_usages at -1 unless the USAGE query succeeds, so a failed query fails the test. - test_extraction: php_inline_html_comment_close_tag_and_short_tag_masks checks exact mask output for these cases and that a function reopened after a comment-ended close tag is extracted.
1 parent 96a1ac5 commit e5a4dc0

4 files changed

Lines changed: 80 additions & 13 deletions

File tree

‎internal/cbm/php_inline_html.c‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ enum {
3131
PHP_TAG_OPEN_LEN = 5, /* <?php */
3232
PHP_TAG_ECHO_LEN = 3, /* <?= */
3333
PHP_TAG_SHORT_LEN = 2, /* <? */
34+
PHP_XML_LEN = 3, /* xml */
3435
PHP_HEREDOC_LEN = 3, /* <<< */
3536
PHP_FIRST_NON_ASCII = 0x80,
3637
PHP_NOT_FOUND = -1,
@@ -80,9 +81,15 @@ static bool php_ieq(const char *s, int len, int i, const char *word) {
8081
return true;
8182
}
8283

84+
/* True when the identifier at i is `xml` (an XML prolog target). */
85+
static bool php_xml_prolog(const char *s, int len, int i) {
86+
return php_ieq(s, len, i, "xml") &&
87+
!(i + PHP_XML_LEN < len && php_is_ident_char((unsigned char)s[i + PHP_XML_LEN]));
88+
}
89+
8390
/* Length of the open tag at i, or 0. `<?php` needs whitespace or EOF after it
84-
* and `<?=` is always a tag. A bare `<?` counts only when whitespace follows,
85-
* so an `<?xml ...?>` prolog in markup stays markup. */
91+
* and `<?=` is always a tag. A bare `<?` counts when whitespace or an
92+
* identifier follows, except `<?xml`, so an XML prolog in markup stays markup. */
8693
static int php_open_tag_len(const char *s, int len, int i) {
8794
if (!php_at(s, len, i, "<?")) {
8895
return 0;
@@ -98,6 +105,11 @@ static int php_open_tag_len(const char *s, int len, int i) {
98105
if (after < len && php_is_space((unsigned char)s[after])) {
99106
return PHP_TAG_SHORT_LEN;
100107
}
108+
/* `<?work();` is a short tag too, but `<?xml` stays an XML prolog. */
109+
if (after < len && php_is_ident_start((unsigned char)s[after]) &&
110+
!php_xml_prolog(s, len, after)) {
111+
return PHP_TAG_SHORT_LEN;
112+
}
101113
return 0;
102114
}
103115

@@ -141,7 +153,7 @@ static int php_skip_block_comment(const char *s, int len, int i) {
141153

142154
/* Index of the line break or `?>` that ends the one-line comment at i. */
143155
static int php_skip_line_comment(const char *s, int len, int i) {
144-
while (i < len && s[i] != '\n' && !php_at(s, len, i, "?>")) {
156+
while (i < len && s[i] != '\n' && s[i] != '\r' && !php_at(s, len, i, "?>")) {
145157
i++;
146158
}
147159
return i;
@@ -236,7 +248,8 @@ static int php_skip_ident(const char *s, int len, int i, bool *halt) {
236248

237249
/* Scan PHP code from i to the next close tag. Returns the index of that `?>`,
238250
* len when the file ends in PHP mode, or PHP_NOT_FOUND at __halt_compiler. */
239-
static int php_scan_code(const char *s, int len, int i) {
251+
static int php_scan_code(const char *s, int len, int i, int *comment) {
252+
*comment = PHP_NOT_FOUND;
240253
while (i < len) {
241254
unsigned char c = (unsigned char)s[i];
242255
if (php_at(s, len, i, "?>")) {
@@ -247,7 +260,11 @@ static int php_scan_code(const char *s, int len, int i) {
247260
} else if (php_at(s, len, i, "/*")) {
248261
i = php_skip_block_comment(s, len, i);
249262
} else if (php_at(s, len, i, "//") || (c == '#' && !php_at(s, len, i, "#["))) {
263+
int start = i;
250264
i = php_skip_line_comment(s, len, i);
265+
if (php_at(s, len, i, "?>")) {
266+
*comment = start; /* the close tag ends this comment */
267+
}
251268
} else if (php_at(s, len, i, "<<<")) {
252269
i = php_skip_heredoc(s, len, i);
253270
} else if (php_is_ident_start(c)) {
@@ -295,10 +312,15 @@ static void php_mask_blank(PHPMask *m, int from, int to) {
295312
}
296313
}
297314

298-
/* Turn the `?>` at i into `; `. */
299-
static void php_mask_close_tag(PHPMask *m, int i) {
315+
/* Turn the `?>` at i into `; `. When the tag ends a line comment starting at
316+
* `comment`, blank that comment first: `; ` cannot end it and a line break
317+
* would shift rows, so the reopened PHP on the same line would be swallowed. */
318+
static void php_mask_close_tag(PHPMask *m, int i, int comment) {
300319
char *out = php_mask_writable(m);
301320
if (out) {
321+
if (comment >= 0) {
322+
php_blank(out, comment, i);
323+
}
302324
out[i] = ';';
303325
out[i + SKIP_ONE] = ' ';
304326
}
@@ -336,11 +358,12 @@ const char *cbm_php_mask_inline_html(CBMArena *arena, const char *source, int so
336358
seen_open = true;
337359

338360
/* PHP mode, up to the close tag. */
339-
int close = php_scan_code(source, source_len, i + tag);
361+
int comment = PHP_NOT_FOUND;
362+
int close = php_scan_code(source, source_len, i + tag, &comment);
340363
if (close < 0 || close >= source_len) {
341364
break;
342365
}
343-
php_mask_close_tag(&m, close);
366+
php_mask_close_tag(&m, close, comment);
344367
i = close + PAIR_LEN;
345368
}
346369
/* On allocation failure parse the file as it is: the old behaviour. */

‎tests/test_daemon_application.c‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2774,7 +2774,9 @@ TEST(daemon_application_programmatic_index_forwards_set_doc_candidates) {
27742774
}
27752775

27762776
TEST(daemon_application_worker_resource_failure_is_structured_and_not_retried) {
2777-
char *cache = th_mktempdir("cbm_app_worker_resource_cache");
2777+
/* th_mktempdir returns one static buffer: copy the cache path. */
2778+
char *cache_made = th_mktempdir("cbm_app_worker_resource_cache");
2779+
char *cache = cache_made ? strdup(cache_made) : NULL;
27782780
cbm_config_t *stored_config = cache ? cbm_config_open(cache) : NULL;
27792781
app_fake_worker_context_t fake;
27802782
app_fake_worker_context_init(&fake);
@@ -2799,7 +2801,8 @@ TEST(daemon_application_worker_resource_failure_is_structured_and_not_retried) {
27992801
cbm_daemon_runtime_application_callbacks_t callbacks =
28002802
cbm_daemon_application_runtime_callbacks(application);
28012803
cbm_daemon_runtime_application_session_t *session = app_test_open(&callbacks, 4188);
2802-
char *root = th_mktempdir("cbm_app_worker_resource");
2804+
char *root_made = th_mktempdir("cbm_app_worker_resource");
2805+
char *root = root_made ? strdup(root_made) : NULL;
28032806
uint8_t *context = NULL;
28042807
uint32_t context_length = 0;
28052808
uint8_t *tool = NULL;
@@ -2847,6 +2850,8 @@ TEST(daemon_application_worker_resource_failure_is_structured_and_not_retried) {
28472850
free(tool);
28482851
th_cleanup(root);
28492852
th_cleanup(cache);
2853+
free(root);
2854+
free(cache);
28502855

28512856
ASSERT_TRUE(setup);
28522857
ASSERT_EQ(status, CBM_DAEMON_RUNTIME_APPLICATION_OK);
@@ -2858,7 +2863,9 @@ TEST(daemon_application_worker_resource_failure_is_structured_and_not_retried) {
28582863
}
28592864

28602865
TEST(daemon_application_worker_resource_failure_after_crash_stops_recovery) {
2861-
char *cache = th_mktempdir("cbm_app_worker_resource_crash_cache");
2866+
/* th_mktempdir returns one static buffer: copy the cache path. */
2867+
char *cache_made = th_mktempdir("cbm_app_worker_resource_crash_cache");
2868+
char *cache = cache_made ? strdup(cache_made) : NULL;
28622869
cbm_config_t *stored_config = cache ? cbm_config_open(cache) : NULL;
28632870
app_fake_worker_context_t fake;
28642871
app_fake_worker_context_init(&fake);
@@ -2887,7 +2894,8 @@ TEST(daemon_application_worker_resource_failure_after_crash_stops_recovery) {
28872894
cbm_daemon_runtime_application_callbacks_t callbacks =
28882895
cbm_daemon_application_runtime_callbacks(application);
28892896
cbm_daemon_runtime_application_session_t *session = app_test_open(&callbacks, 4189);
2890-
char *root = th_mktempdir("cbm_app_worker_resource_crash");
2897+
char *root_made = th_mktempdir("cbm_app_worker_resource_crash");
2898+
char *root = root_made ? strdup(root_made) : NULL;
28912899
uint8_t *context = NULL;
28922900
uint32_t context_length = 0;
28932901
uint8_t *tool = NULL;
@@ -2935,6 +2943,8 @@ TEST(daemon_application_worker_resource_failure_after_crash_stops_recovery) {
29352943
free(tool);
29362944
th_cleanup(root);
29372945
th_cleanup(cache);
2946+
free(root);
2947+
free(cache);
29382948

29392949
ASSERT_TRUE(setup);
29402950
ASSERT_EQ(status, CBM_DAEMON_RUNTIME_APPLICATION_OK);

‎tests/test_extraction.c‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,37 @@ TEST(php_inline_html_trailing_close_tag_crlf_issue2000) {
11101110
PASS();
11111111
}
11121112

1113+
/* Standalone CR ends a line comment, so a `?>` inside the next CR-only line's
1114+
* string is not a close tag; a `?>` that ends a `//` or `#` comment blanks the
1115+
* comment so the PHP reopened on that line is not swallowed; `<?work();` is a
1116+
* short tag while `<?xml` stays markup. */
1117+
TEST(php_inline_html_comment_close_tag_and_short_tag_masks) {
1118+
CBMArena arena;
1119+
cbm_arena_init(&arena);
1120+
const char *cr = "<?php\r// note\r$s = '?>';\rfunction a() {}\r";
1121+
ASSERT(cbm_php_mask_inline_html(&arena, cr, (int)strlen(cr)) == cr);
1122+
1123+
const char *cmt = "<?php\n// note ?><?php function b() {}\n";
1124+
const char *masked = cbm_php_mask_inline_html(&arena, cmt, (int)strlen(cmt));
1125+
ASSERT_STR_EQ(masked, "<?php\n ; function b() {}\n");
1126+
1127+
const char *hash = "<?php\n# n ?><?php function c() {}\n";
1128+
masked = cbm_php_mask_inline_html(&arena, hash, (int)strlen(hash));
1129+
ASSERT_STR_EQ(masked, "<?php\n ; function c() {}\n");
1130+
1131+
const char *shorttag = "<?work();\n?>\n<?xml version=\"1.0\"?>\n";
1132+
masked = cbm_php_mask_inline_html(&arena, shorttag, (int)strlen(shorttag));
1133+
ASSERT_STR_EQ(masked, "<?work();\n; \n \n");
1134+
cbm_arena_destroy(&arena);
1135+
1136+
CBMFileResult *r = extract("<?php\n// note ?><?php function later() { helper(); }\n",
1137+
CBM_LANG_PHP, "t", "c.php");
1138+
ASSERT_NOT_NULL(r);
1139+
ASSERT(has_def(r, "Function", "later"));
1140+
cbm_free_result(r);
1141+
PASS();
1142+
}
1143+
11131144
/* --- Ruby --- */
11141145
TEST(ruby_class) {
11151146
CBMFileResult *r = extract("class Animal\n def initialize(name)\n @name = name\n end\n "
@@ -15448,6 +15479,7 @@ SUITE(extraction) {
1544815479
RUN_TEST(php_inline_html_markup_never_reaches_graph_issue2000);
1544915480
RUN_TEST(php_inline_html_mask_copies_only_on_write_issue2000);
1545015481
RUN_TEST(php_inline_html_trailing_close_tag_crlf_issue2000);
15482+
RUN_TEST(php_inline_html_comment_close_tag_and_short_tag_masks);
1545115483
RUN_TEST(ruby_class);
1545215484
RUN_TEST(ruby_module);
1545315485
RUN_TEST(ruby_constructor_callee_is_constant_path);

‎tests/test_pipeline.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23126,8 +23126,9 @@ static void swift_init_observe(cbm_store_t *s, const char *project, SwiftInitObs
2312623126

2312723127
edges = NULL;
2312823128
count = 0;
23129-
o->init_usages = 0;
23129+
o->init_usages = -1; /* error sentinel until the USAGE query succeeds */
2313023130
if (cbm_store_find_edges_by_type(s, project, "USAGE", &edges, &count) == CBM_STORE_OK) {
23131+
o->init_usages = 0;
2313123132
for (int i = 0; i < count; i++) {
2313223133
cbm_node_t tgt = {0};
2313323134
if (cbm_store_find_node_by_id(s, edges[i].target_id, &tgt) == CBM_STORE_OK &&
@@ -23148,6 +23149,7 @@ static void swift_init_case(bool parallel, int pad, SwiftInitObservation *o) {
2314823149
memset(o, 0, sizeof(*o));
2314923150
o->run_rc = -1;
2315023151
o->calls = -1;
23152+
o->init_usages = -1;
2315123153
char tmp[256];
2315223154
snprintf(tmp, sizeof(tmp), "/tmp/cbm_swift_init_XXXXXX");
2315323155
if (!cbm_mkdtemp(tmp)) {

0 commit comments

Comments
 (0)