From 94574eb2398ce2a50fb1b6735947de6855a4cb3b Mon Sep 17 00:00:00 2001 From: Cintu07 Date: Thu, 1 Oct 2026 22:37:05 +0530 Subject: [PATCH] fix(php): stop language constructs binding to same-named methods (#3830) empty(), isset(), eval() and die() parse as an ordinary function_call_expression, so the call pass resolved them by bare name. A reserved word has been a legal method name since PHP 7, so a class that declares empty() collected every empty($x) in the corpus: EXTRACTED from its sibling methods, INFERRED from every other file through the case-insensitive fold. Drop the callee for these constructs in the PHP branch, before the in-file lookup and before raw_calls, the same way _GO_PREDECLARED_FUNCS handles Go builtins. $bag->empty() is a member call and still resolves. --- graphify/extractors/engine.py | 22 ++ tests/test_php_language_construct_calls.py | 258 +++++++++++++++++++++ 2 files changed, 280 insertions(+) create mode 100644 tests/test_php_language_construct_calls.py diff --git a/graphify/extractors/engine.py b/graphify/extractors/engine.py index d5f48054ac..636dcb652b 100644 --- a/graphify/extractors/engine.py +++ b/graphify/extractors/engine.py @@ -3575,6 +3575,23 @@ def _lua_is_require_call(node, source: bytes) -> bool: _PHP_ROUTING_VERBS = frozenset({"get", "post", "put", "patch", "delete", "options", "any", "match", "map"}) +# PHP language constructs that are written like functions. `empty($x)`, +# `isset($a)`, `eval($s)` and `die($m)` parse as an ordinary +# function_call_expression, so the bare-name lookup bound them to any method +# that shares the name: since PHP 7 a reserved word is a legal method name +# (`public function empty()`), while no function can ever be declared under +# one. Every `empty($x)` in a corpus became an edge into that method, EXTRACTED +# in the same file and INFERRED across files through the case-insensitive fold +# (#3830). +# +# Same policy as _GO_PREDECLARED_FUNCS: language-local, bare calls only +# (`$bag->empty()` is a member call and still resolves), and the manual's whole +# list of call-shaped keywords rather than the subset the pinned grammar emits +# as calls today (`unset`, `exit`, `list` and `array` get their own node types). +_PHP_LANGUAGE_CONSTRUCTS = frozenset({ + "array", "die", "empty", "eval", "exit", "isset", "list", "unset", +}) + def _php_get_route_name(closure_node, src: bytes) -> str | None: """Walk up the AST to extract grouped routing prefixes (#3409).""" prefixes = [] @@ -6451,6 +6468,11 @@ def walk_calls( func_node = node.child_by_field_name("function") if func_node: callee_name = _read_text(func_node, source) + # PHP names are case-insensitive, so `EMPTY($x)` is the + # same construct. Dropping the name here skips the + # in-file bind and keeps it out of raw_calls. + if callee_name.lower() in _PHP_LANGUAGE_CONSTRUCTS: + callee_name = None elif node.type == "scoped_call_expression": # Static method call: Helper::format() → callee = "Helper" scope_node = node.child_by_field_name("scope") diff --git a/tests/test_php_language_construct_calls.py b/tests/test_php_language_construct_calls.py new file mode 100644 index 0000000000..79b17e2c52 --- /dev/null +++ b/tests/test_php_language_construct_calls.py @@ -0,0 +1,258 @@ +"""PHP language constructs must not bind to same-named user methods (#3830). + +`empty($x)`, `isset($a)`, `eval($s)` and `die($m)` are keywords, but +tree-sitter-php parses them as an ordinary `function_call_expression`, and the +call pass resolves that callee by bare name. A reserved word is a legal method +name since PHP 7, so a class declaring `public function empty()` absorbed every +`empty(...)` in the corpus: an EXTRACTED edge from a sibling method in the same +file, and an INFERRED one from every other file through the case-insensitive +fold. On the Symfony corpus in #3830 one `ParseCollectionPaginator::empty()` +collected ~200 of these from ~150 files. + +The filter (`_PHP_LANGUAGE_CONSTRUCTS`) is PHP-local and bare-call-only, the +same shape as `_GO_PREDECLARED_FUNCS`. The boundary tests at the bottom are the +reason: `$bag->empty()` is a genuine member call into that method and must still +resolve, and the construct's arguments must still be walked for calls. +""" +from graphify.extract import extract + + +def _nodes_by_file(result, suffix): + return [n for n in result["nodes"] if str(n.get("source_file", "")).endswith(suffix)] + + +def _label(node): + return (node.get("label") or "").strip(".()") + + +def _ids(result, suffix, name): + return {n["id"] for n in _nodes_by_file(result, suffix) if _label(n) == name} + + +def _edges_between(result, source_ids, target_ids): + return [ + e for e in result["edges"] + if e.get("source") in source_ids and e.get("target") in target_ids + ] + + +def _extract_php(tmp_path): + return extract(sorted(tmp_path.glob("*.php")), cache_root=tmp_path, parallel=False) + + +_BAG = ( + "items === [];\n" + " }\n" + "\n" + " public function isset(string $k): bool\n" + " {\n" + " return array_key_exists($k, $this->items);\n" + " }\n" + "}\n" +) + + +def test_construct_in_another_file_does_not_bind_to_user_method(tmp_path): + """The cross-file case: `empty($to)` in Mailer.php is not Bag::empty().""" + (tmp_path / "Bag.php").write_text(_BAG) + (tmp_path / "Mailer.php").write_text( + "items === [];\n" + " }\n" + "\n" + " public function isset(string $k): bool\n" + " {\n" + " return array_key_exists($k, $this->items);\n" + " }\n" + "\n" + " public function add(string $k, $v): void\n" + " {\n" + " if (empty($k) || isset($this->items[$k])) {\n" + " return;\n" + " }\n" + " $this->items[$k] = $v;\n" + " }\n" + "}\n" + ) + result = _extract_php(tmp_path) + method_ids = _ids(result, "Bag.php", "empty") | _ids(result, "Bag.php", "isset") + add_ids = _ids(result, "Bag.php", "add") + assert method_ids and add_ids, "both methods and the caller must still be extracted" + + phantom = _edges_between(result, add_ids, method_ids) + assert phantom == [], f"a construct in add() bound to a sibling method: {phantom}" + + +def test_construct_is_matched_case_insensitively(tmp_path): + """`EMPTY($a)` is the same construct; PHP names fold case. + + The cross-file pass folds PHP names on purpose, so an upper-case spelling + that skipped the filter would still land on `empty()` through that fold. + """ + (tmp_path / "Bag.php").write_text(_BAG) + (tmp_path / "helpers.php").write_text( + "empty()` really does call the method, so the filter must not reach it. + + It is a member_call_expression, not a function_call_expression. Filtering + by name alone would drop this genuine edge. Kept in one file because + cross-file PHP member calls do not resolve yet (the other half of #3830), + so a two-file version would pass or fail for reasons unrelated to this + filter. + """ + (tmp_path / "Bag.php").write_text( + "empty();\n" + " }\n" + "}\n" + ) + result = _extract_php(tmp_path) + method_ids = _ids(result, "Bag.php", "empty") + caller_ids = _ids(result, "Bag.php", "bothEmpty") + assert method_ids and caller_ids, "the method and the caller must still be extracted" + + resolved = _edges_between(result, caller_ids, method_ids) + assert resolved, "a genuine $other->empty() member call must still resolve" + + +def test_calls_inside_construct_arguments_still_resolve(tmp_path): + """Dropping the construct must not drop the calls written inside it.""" + (tmp_path / "Repo.php").write_text( + "load());\n" + " }\n" + "}\n" + ) + result = _extract_php(tmp_path) + load_ids = _ids(result, "Repo.php", "load") + missing_ids = _ids(result, "Repo.php", "missing") + assert load_ids and missing_ids, "both methods must still be extracted" + + resolved = _edges_between(result, missing_ids, load_ids) + assert resolved, "$this->load() inside empty(...) must still resolve" + + +def test_non_construct_function_call_still_resolves(tmp_path): + """The guard is a no-op for a genuine user function.""" + (tmp_path / "helpers.php").write_text( + "