From 4fcb4e3553be3659ba3188c0b899323b66db0adc Mon Sep 17 00:00:00 2001 From: Gadi Evron Date: Thu, 28 May 2026 11:59:32 -0700 Subject: [PATCH] fix(parsers/php): repair the PHP call-graph builder (tag grammar, callbacks, new, scopes, imports) Several defects in parsers/php/call_graph_builder.py + function_extractor.py; before this the PHP call graph was effectively empty (see #1). 1. Tag grammar: the call extractor re-parsed tag-stripped function bodies with ts_php.language_php() (the tag-REQUIRING grammar), which treats tagless input as inert `text` -- so ZERO call nodes were found and the entire PHP call graph was empty. Use language_php_only(). Foundational: every fix below only produces edges because of it. 2. Higher-order callbacks: call_user_func / call_user_func_array / array_map / array_filter / array_walk / array_reduce / usort / uasort / uksort / preg_replace_callback are in PHP_BUILTINS, so _resolve_function_call returned None before inspecting their callback argument. A CALLBACK_BUILTINS map now resolves the string-literal callback ('fn', 'Class::method', or ['Class','method']) at the builtin's callback position. The outer builtin call stays filtered; variable/closure callbacks have no static target. 3. use-import node type: _extract_imports matched `use_declaration`, but tree-sitter-php emits `namespace_use_declaration`, so `use App\Service\Foo;` imports were never recorded. Match the real node type; handle grouped `use A, B;` and strip `as Alias`. 4. Import-file matching: _resolve_simple_call matched `import_name in file_path` -- an unanchored substring -- so an import 'Bar' matched 'app/BarBaz/x.php'. Match the import file name (anchored: endswith the basename `.php`, or exact). 5. new/__construct: `new Foo(...)` parses as object_creation_expression, which the traversal never visited, so constructors were untracked. Add the node type + resolve the class __construct. 6. self/static/parent scopes: in tree-sitter-php `self`/`static`/`parent` are `relative_scope` nodes, not `name`, so _resolve_scoped_call never captured the scope and ALL self::/static::/parent:: calls were silently dropped. Handle relative_scope; resolve parent:: in the superclass (from the class's `extends`, namespace-stripped), with no fall-back to the caller's own class (which would mis-link an override's parent:: call to itself). 7. Case-insensitive builtins: _is_builtin compared case-sensitively, but PHP function names are case-insensitive, so e.g. CALL_USER_FUNC was not recognized. Compare name.lower(). Scope: PHP-specific (the c/python/ruby/zig/go call-graph builders are separate implementations). Tests: tests/test_php_call_graph.py (new; the package had no php call-graph tests) -- loaded under a unique importlib name (call_graph_builder/function_extractor are basenames shared by every parser). Eight behavioral tests: tagless body produces edges, callback builtins resolve, new->__construct, parent::->superclass, parent:: with a namespaced superclass, case-insensitive builtins, anchored import match, namespace_use_declaration import. RED 8 failed (pre-fix) -> GREEN 8 passed; ruff clean; full suite 184 passed / 63 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../parsers/php/call_graph_builder.py | 110 +++++++++++++- .../parsers/php/function_extractor.py | 16 ++- .../openant-core/tests/test_php_call_graph.py | 134 ++++++++++++++++++ 3 files changed, 250 insertions(+), 10 deletions(-) create mode 100644 libs/openant-core/tests/test_php_call_graph.py diff --git a/libs/openant-core/parsers/php/call_graph_builder.py b/libs/openant-core/parsers/php/call_graph_builder.py index 42e37bbf..11240a66 100644 --- a/libs/openant-core/parsers/php/call_graph_builder.py +++ b/libs/openant-core/parsers/php/call_graph_builder.py @@ -42,7 +42,9 @@ from utilities.file_io import read_json, write_json, open_utf8 -PHP_LANGUAGE = Language(ts_php.language_php()) +# Use the tagless grammar variant: function bodies are re-parsed WITHOUT their the 0-based +# position of the callback argument. The outer builtin call is still filtered (it is in PHP_BUILTINS); +# we additionally resolve the callback it dispatches to. +CALLBACK_BUILTINS = { + 'call_user_func': 0, 'call_user_func_array': 0, + 'array_map': 0, 'array_filter': 1, 'array_walk': 1, 'array_reduce': 1, + 'usort': 1, 'uasort': 1, 'uksort': 1, 'preg_replace_callback': 1, +} + # PHP keywords to skip in regex fallback PHP_KEYWORDS = { 'if', 'else', 'elseif', 'while', 'for', 'foreach', @@ -153,7 +164,7 @@ def _build_indexes(self) -> None: def _is_builtin(self, name: str) -> bool: """Check if name is a PHP builtin or common function.""" - return name in PHP_BUILTINS + return name.lower() in PHP_BUILTINS # PHP function names are case-insensitive def _extract_calls_from_code(self, code: str, caller_id: str) -> Set[str]: """Extract function call references from code using tree-sitter.""" @@ -172,7 +183,7 @@ def _extract_calls_from_code(self, code: str, caller_id: str) -> Set[str]: while stack: node = stack.pop() if node.type in ('function_call_expression', 'member_call_expression', - 'scoped_call_expression'): + 'scoped_call_expression', 'object_creation_expression'): resolved = self._resolve_call_node(node, code_bytes, caller_file, caller_class) if resolved: calls.add(resolved) @@ -189,6 +200,8 @@ def _resolve_call_node(self, node, source: bytes, caller_file: str, return self._resolve_member_call(node, source, caller_file, caller_class) elif node.type == 'scoped_call_expression': return self._resolve_scoped_call(node, source, caller_file, caller_class) + elif node.type == 'object_creation_expression': + return self._resolve_new(node, source, caller_file, caller_class) return None def _resolve_function_call(self, node, source: bytes, caller_file: str, @@ -211,10 +224,69 @@ def _resolve_function_call(self, node, source: bytes, caller_file: str, return None if self._is_builtin(func_name): + # Higher-order builtins (call_user_func, array_map, ...) drop the OUTER call, but the + # callback they dispatch to is a real edge -- resolve it instead of returning None. + cb_idx = CALLBACK_BUILTINS.get(func_name.lower()) + if cb_idx is not None: + return self._resolve_callback_arg(node, source, caller_file, caller_class, cb_idx) return None return self._resolve_simple_call(func_name, caller_file, caller_class) + def _resolve_callback_arg(self, node, source: bytes, caller_file: str, + caller_class: Optional[str], idx: int) -> Optional[str]: + """Resolve the callback argument at position `idx` of a higher-order builtin call. + + Only string-literal callbacks have a static target: 'fn', 'Class::method', or the array form + ['Class', 'method']. Variable/closure callbacks ($cb, fn() => ...) have no resolvable target. + """ + args_node = next((c for c in node.children if c.type == 'arguments'), None) + if args_node is None: + return None + arg_nodes = [c for c in args_node.children if c.type == 'argument'] + if idx >= len(arg_nodes) or not arg_nodes[idx].children: + return None + value = arg_nodes[idx].children[0] + if value.type == 'string': + name = source[value.start_byte:value.end_byte].decode('utf-8', errors='replace').strip('\'"') + return self._resolve_callback_name(name, caller_file, caller_class) + if value.type == 'array_creation_expression': + # ['ClassName', 'method'] static callback (instance [$obj,'m'] needs a type we don't track). + strings = [] + stack = [value] + while stack: + n = stack.pop() + if n.type == 'string': + strings.append(source[n.start_byte:n.end_byte].decode('utf-8', errors='replace').strip('\'"')) + stack.extend(reversed(n.children)) + if len(strings) >= 2: + return self._resolve_class_call(strings[0], strings[1], caller_file) + return None + + def _resolve_callback_name(self, name: str, caller_file: str, + caller_class: Optional[str]) -> Optional[str]: + """Resolve a string callback: 'Class::method' (static) or a plain function name.""" + if '::' in name: + cls, _, method = name.partition('::') + return self._resolve_class_call(cls, method, caller_file) + if self._is_builtin(name): + return None + return self._resolve_simple_call(name, caller_file, caller_class) + + def _resolve_new(self, node, source: bytes, caller_file: str, + caller_class: Optional[str]) -> Optional[str]: + """Resolve `new ClassName(...)` to the class's __construct method, if one is defined.""" + class_name = None + for child in node.children: + if child.type in ('name', 'qualified_name'): + class_name = source[child.start_byte:child.end_byte].decode('utf-8', errors='replace') + if '\\' in class_name: + class_name = class_name.rsplit('\\', 1)[-1] + break + if not class_name: + return None + return self._resolve_class_call(class_name, '__construct', caller_file) + def _resolve_member_call(self, node, source: bytes, caller_file: str, caller_class: Optional[str]) -> Optional[str]: """Resolve a member call like $obj->method().""" @@ -250,6 +322,10 @@ def _resolve_scoped_call(self, node, source: bytes, caller_file: str, for child in node.children: if child.type == 'name' and scope is not None: method_name = source[child.start_byte:child.end_byte].decode('utf-8', errors='replace') + elif child.type == 'relative_scope' and scope is None: + # self / static / parent are `relative_scope` nodes, not `name` -- without this branch + # the scope was never captured and self::/static::/parent:: calls were silently dropped. + scope = source[child.start_byte:child.end_byte].decode('utf-8', errors='replace') elif child.type in ('name', 'qualified_name') and scope is None: scope = source[child.start_byte:child.end_byte].decode('utf-8', errors='replace') if '\\' in scope: @@ -264,12 +340,30 @@ def _resolve_scoped_call(self, node, source: bytes, caller_file: str, return None # self::method() or static::method() - same class - if scope in ('self', 'static', 'parent') and caller_class: + if scope in ('self', 'static') and caller_class: return self._resolve_self_call(method_name, caller_file, caller_class) + # parent::method() - resolve in the superclass (extends), not the caller's own class. + if scope == 'parent' and caller_class: + superclass = self._superclass_of(caller_file, caller_class) + if not superclass: + return None + if '\\' in superclass: + superclass = superclass.rsplit('\\', 1)[-1] # `extends App\Base` -> match `Base` + # Resolve in the superclass only; do NOT fall back to the caller's own class, which would + # mis-link an overriding child's parent:: call to the child's own method. + return self._resolve_class_call(superclass, method_name, caller_file) + # ClassName::method() return self._resolve_class_call(scope, method_name, caller_file) + def _superclass_of(self, caller_file: str, caller_class: str) -> Optional[str]: + """Return the superclass (extends) name of caller_class defined in caller_file, or None.""" + for class_data in self.classes.values(): + if class_data.get('name') == caller_class and class_data.get('file_path') == caller_file: + return class_data.get('superclass') + return None + def _resolve_simple_call(self, func_name: str, caller_file: str, caller_class: Optional[str]) -> Optional[str]: """Resolve a simple function call to a function ID.""" @@ -290,9 +384,13 @@ def _resolve_simple_call(self, func_name: str, caller_file: str, file_imports = self.imports.get(caller_file, {}) for import_name, import_type in file_imports.items(): if import_type in ('require', 'require_once', 'include', 'include_once', 'use'): - # Try matching import path to file + # Match the import by file name (anchored), not an unanchored `in` substring which + # over-matched any path merely containing the import string. + imp_base = import_name.replace('\\', '/').rsplit('/', 1)[-1] for file_path in self.functions_by_file: - if file_path.endswith(f"{import_name}.php") or import_name in file_path: + if (file_path.endswith(f"{import_name}.php") + or file_path.endswith(f"/{imp_base}.php") + or file_path == f"{imp_base}.php"): file_funcs = self.functions_by_file[file_path] for func_id in file_funcs: func_data = self.functions.get(func_id, {}) diff --git a/libs/openant-core/parsers/php/function_extractor.py b/libs/openant-core/parsers/php/function_extractor.py index 2c9039ad..162967b4 100644 --- a/libs/openant-core/parsers/php/function_extractor.py +++ b/libs/openant-core/parsers/php/function_extractor.py @@ -195,15 +195,23 @@ def _extract_imports(self, tree, source: bytes) -> Dict[str, str]: while stack: node = stack.pop() - if node.type == 'use_declaration': - # Extract the full use statement value + if node.type in ('namespace_use_declaration', 'use_declaration'): + # tree-sitter-php emits `namespace_use_declaration` (not `use_declaration`) for + # `use App\Service\Foo;`, so the old node-type check matched nothing and use-imports + # were never recorded. import_text = self._node_text(node, source) - # Clean up: remove 'use ' prefix and trailing ';' cleaned = import_text.strip() if cleaned.startswith('use '): cleaned = cleaned[4:] cleaned = cleaned.rstrip(';').strip() - imports[cleaned] = 'use' + # Handle grouped `use A\B, C\D;` and drop trailing `as Alias` so the stored import is + # the namespace path (matched by file name downstream). + for part in cleaned.split(','): + entry = part.strip() + if not entry: + continue + path = entry.split(' as ')[0].strip() if ' as ' in entry else entry + imports[path] = 'use' elif node.type == 'namespace_definition': # Extract namespace name diff --git a/libs/openant-core/tests/test_php_call_graph.py b/libs/openant-core/tests/test_php_call_graph.py new file mode 100644 index 00000000..980175e7 --- /dev/null +++ b/libs/openant-core/tests/test_php_call_graph.py @@ -0,0 +1,134 @@ +"""Regression tests for defects in parsers/php/call_graph_builder.py + function_extractor.py. + +#1 (tag): the re-parser used the tag-requiring grammar on tag-stripped function bodies, so + the whole PHP call graph was empty (0 edges). Fixed by language_php_only(). +#2 (callbacks): higher-order builtins (call_user_func, array_map, + ...) dropped their callback argument. Now resolved via CALLBACK_BUILTINS. +#5 (new): `new Foo()` (object_creation_expression) was not traversed -> __construct untracked. +#6 (parent::): parent:: was resolved in the caller's own class, not the superclass. +#4 (import): use/require resolution used an unanchored `import_name in file_path` substring. +#7 (case): _is_builtin compared case-sensitively though PHP names are case-insensitive. +#3 (use node type): _extract_imports checked `use_declaration` but tree-sitter-php emits + `namespace_use_declaration`, so use-imports were never recorded. +(#8: `indirect_calls` is a phantom field -- documented, no code.) + +Loads both modules under UNIQUE importlib names (call_graph_builder / function_extractor are basenames +shared by every parser). +""" +import importlib.util +import sys +from pathlib import Path + +import tree_sitter_php as _tsphp +from tree_sitter import Language, Parser + +CORE = Path(__file__).resolve().parents[1] +if str(CORE) not in sys.path: + sys.path.insert(0, str(CORE)) + + +def _load(unique, relpath): + spec = importlib.util.spec_from_file_location(unique, str(CORE / relpath)) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +_cgb = _load("php_call_graph_builder_isolated", "parsers/php/call_graph_builder.py") +_fe = _load("php_function_extractor_isolated", "parsers/php/function_extractor.py") +CallGraphBuilder = _cgb.CallGraphBuilder +FunctionExtractor = _fe.FunctionExtractor + + +def _build(funcs, classes=None, imports=None): + b = CallGraphBuilder({"functions": funcs, "classes": classes or {}, "imports": imports or {}, + "repository": "/r"}) + b.build_call_graph() + return b + + +# #1 tag-grammar: tagless function bodies now parse -> edges exist at all +def test_tagless_body_produces_edges(): + b = _build({ + "a.php:a": {"name": "a", "file_path": "a.php", "class_name": None, "code": "function a() { b(); }"}, + "a.php:b": {"name": "b", "file_path": "a.php", "class_name": None, "code": "function b() {}"}, + }) + assert b.call_graph.get("a.php:a") == ["a.php:b"], b.call_graph + + +# #2 callbacks: call_user_func('cb') and array_map('cb2', ...) resolve their callback arg +def test_callback_builtins_resolve_callback_arg(): + b = _build({ + "f.php:f": {"name": "f", "file_path": "f.php", "class_name": None, + "code": "function f() { call_user_func('cb'); array_map('cb2', $x); }"}, + "f.php:cb": {"name": "cb", "file_path": "f.php", "class_name": None, "code": "function cb() {}"}, + "f.php:cb2": {"name": "cb2", "file_path": "f.php", "class_name": None, "code": "function cb2($v) {}"}, + }) + edges = set(b.call_graph.get("f.php:f", [])) + assert edges == {"f.php:cb", "f.php:cb2"}, edges + + +# #5 new Foo() -> Foo::__construct +def test_new_resolves_to_construct(): + b = _build({ + "a.php:f": {"name": "f", "file_path": "a.php", "class_name": None, "code": "function f() { new Foo(); }"}, + "a.php:Foo.__construct": {"name": "__construct", "file_path": "a.php", "class_name": "Foo", + "code": "function __construct() {}"}, + }, classes={"a.php:Foo": {"name": "Foo", "file_path": "a.php", "superclass": None}}) + assert b.call_graph.get("a.php:f") == ["a.php:Foo.__construct"], b.call_graph + + +# #6 parent::m() resolves in the superclass +def test_parent_resolves_in_superclass(): + b = _build({ + "a.php:Child.doit": {"name": "doit", "file_path": "a.php", "class_name": "Child", + "code": "function doit() { parent::base_m(); }"}, + "a.php:Base.base_m": {"name": "base_m", "file_path": "a.php", "class_name": "Base", + "code": "function base_m() {}"}, + }, classes={ + "a.php:Child": {"name": "Child", "file_path": "a.php", "superclass": "Base"}, + "a.php:Base": {"name": "Base", "file_path": "a.php", "superclass": None}, + }) + assert b.call_graph.get("a.php:Child.doit") == ["a.php:Base.base_m"], b.call_graph + + +# #6 parent:: with a NAMESPACED superclass (extends App\Base) resolves by the unqualified class name +def test_parent_resolves_namespaced_superclass(): + b = _build({ + "a.php:Child.doit": {"name": "doit", "file_path": "a.php", "class_name": "Child", + "code": "function doit() { parent::base_m(); }"}, + "b.php:Base.base_m": {"name": "base_m", "file_path": "b.php", "class_name": "Base", + "code": "function base_m() {}"}, + }, classes={ + "a.php:Child": {"name": "Child", "file_path": "a.php", "superclass": "App\\Base"}, + "b.php:Base": {"name": "Base", "file_path": "b.php", "superclass": None}, + }) + assert b.call_graph.get("a.php:Child.doit") == ["b.php:Base.base_m"], b.call_graph + + +# #7 _is_builtin is case-insensitive (PHP function names are case-insensitive) +def test_is_builtin_case_insensitive(): + b = CallGraphBuilder({"functions": {}, "classes": {}, "imports": {}, "repository": "/r"}) + assert b._is_builtin("CALL_USER_FUNC") is True + assert b._is_builtin("StrLen") is True + assert b._is_builtin("myCustomFunc") is False + + +# #4 import resolution matches the import FILE name, not an unanchored substring +def test_import_match_anchored_not_substring(): + # 'app/BarBaz/x.php' contains the substring 'Bar' but is NOT Bar.php; the real target is sub/Bar.php. + b = CallGraphBuilder({"functions": { + "app/BarBaz/x.php:helper": {"name": "helper", "file_path": "app/BarBaz/x.php", "class_name": None, "code": ""}, + "sub/Bar.php:helper": {"name": "helper", "file_path": "sub/Bar.php", "class_name": None, "code": ""}, + }, "classes": {}, "imports": {"caller.php": {"Bar": "use"}}, "repository": "/r"}) + assert b._resolve_simple_call("helper", "caller.php", None) == "sub/Bar.php:helper" + + +# #3 _extract_imports records `namespace_use_declaration` (the real tree-sitter-php node type) +def test_extract_imports_namespace_use_declaration(): + src = b"