mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 07:29:33 +00:00
fix(extract): stable closure naming for PHP (#3409)
Address reviewer feedback:
- Replace line-based {closure@N} names with stable ordinal {closure#N}
names scoped per enclosing class or file. Ordinals are unaffected by
line insertions/deletions above a closure; only reordering closures
or inserting one earlier in the same scope shifts later ordinals,
which is a much rarer edit.
- Route closures passed to routing verbs (get/post/put/patch/delete/
options/any/match/map) receive a semantic 'VERB /path' label that
is both stable and directly meaningful to graph queries (the primary
use-case from the issue).
Note: adding anonymous_function_creation_expression and arrow_function
to function_boundary_types (introduced in the first commit) means calls
inside a closure now attribute to the closure rather than the enclosing
named function. This is the correct behaviour; existing codebases with
closures inside named functions will see those edges move on next
incremental update.
(cherry picked from commit 5400b9cbd7)
This commit is contained in:
1 parent
cd3fd9436e
commit
2c172e4969
2 files changed
+87
-26
No files matched your search
@@ -3554,6 +3554,9 @@ def _extract_generic(
|
||||
# walk_calls as extra_locals, so each closure sees only its own
|
||||
# params/locals instead of a shared union that over-suppresses siblings.
|
||||
closure_locals_by_body: dict[int, set[str]] = {}
|
||||
# PHP only: ordinal counter for anonymous closures, keyed by scope id
|
||||
# (parent_class_nid or stem). Stable across line-only edits (#3409).
|
||||
php_closure_counts: dict[str, int] = {}
|
||||
pending_listen_edges: list[tuple[str, str, int]] = []
|
||||
# tree-sitter-swift parses both `class Foo` and `extension Foo` as
|
||||
# `class_declaration`. Same-file pairs collapse via seen_ids, but cross-file
|
||||
@@ -4929,7 +4932,48 @@ def _extract_generic(
|
||||
|
||||
if not func_name:
|
||||
if t in ("anonymous_function_creation_expression", "arrow_function"):
|
||||
func_name = f"{{closure@{node.start_point[0] + 1}}}"
|
||||
# Prefer a route-derived name when the closure is passed
|
||||
# directly to a routing method (`$app->get('/api', fn)`):
|
||||
# walk up to find a sibling string argument and the method name.
|
||||
route_name: str | None = None
|
||||
_PHP_ROUTING_VERBS = frozenset({
|
||||
"get", "post", "put", "patch", "delete",
|
||||
"options", "any", "match", "map",
|
||||
})
|
||||
parent = node.parent # argument node
|
||||
if parent is not None and parent.type == "argument":
|
||||
arg_list = parent.parent # argument_list
|
||||
if arg_list is not None and arg_list.type == "argument_list":
|
||||
call_node = arg_list.parent
|
||||
# extract method/function name from the call
|
||||
if call_node is not None and call_node.type in (
|
||||
"member_call_expression", "function_call_expression"
|
||||
):
|
||||
method_name_node = call_node.child_by_field_name("name")
|
||||
if method_name_node is None:
|
||||
method_name_node = call_node.child_by_field_name("function")
|
||||
raw_method = (
|
||||
_read_text(method_name_node, source)
|
||||
if method_name_node else ""
|
||||
).lower()
|
||||
if raw_method in _PHP_ROUTING_VERBS:
|
||||
# look for the first string argument (the path)
|
||||
for sibling in arg_list.children:
|
||||
if sibling is parent:
|
||||
break
|
||||
if sibling.type in (
|
||||
"string", "encapsed_string"
|
||||
):
|
||||
path_text = _read_text(sibling, source).strip("'\"")
|
||||
route_name = f"{raw_method.upper()} {path_text}"
|
||||
break
|
||||
if route_name:
|
||||
func_name = route_name
|
||||
else:
|
||||
# Stable ordinal scoped to the enclosing class/file (#3409)
|
||||
_scope_key = parent_class_nid or stem
|
||||
php_closure_counts[_scope_key] = php_closure_counts.get(_scope_key, 0) + 1
|
||||
func_name = "{closure#" + str(php_closure_counts[_scope_key]) + "}"
|
||||
else:
|
||||
return
|
||||
sanitized_name = (
|
||||
|
||||
@@ -2,32 +2,49 @@ from pathlib import Path
|
||||
from graphify.extract import extract_php
|
||||
import pytest
|
||||
|
||||
def test_php_closures(tmp_path):
|
||||
src = b'''<?php
|
||||
$app->get('/api', function() {
|
||||
return 1;
|
||||
|
||||
def test_php_route_closure_gets_semantic_name(tmp_path):
|
||||
"""A closure passed to a routing method gets a 'VERB /path' label."""
|
||||
src = b"""<?php
|
||||
$app->get('/api/users', function() {
|
||||
return [];
|
||||
});
|
||||
$fn = fn($x) => $x + 1;
|
||||
'''
|
||||
php_file = tmp_path / 'dummy.php'
|
||||
$app->post('/api/users', function() {
|
||||
return 'created';
|
||||
});
|
||||
"""
|
||||
php_file = tmp_path / "routes.php"
|
||||
php_file.write_bytes(src)
|
||||
|
||||
res = extract_php(php_file)
|
||||
if res.get("error"):
|
||||
pytest.skip(res["error"])
|
||||
|
||||
labels = {n["label"] for n in res["nodes"]}
|
||||
|
||||
assert "GET /api/users()" in labels, "Expected route closure to have semantic label 'GET /api/users()'"
|
||||
assert "POST /api/users()" in labels, "Expected route closure to have semantic label 'POST /api/users()'"
|
||||
|
||||
|
||||
def test_php_generic_closure_gets_ordinal_name(tmp_path):
|
||||
"""Non-routing closures get stable ordinal names {closure#N}."""
|
||||
src = b"""<?php
|
||||
$fn1 = fn($x) => $x + 1;
|
||||
$fn2 = function() { return 'hello'; };
|
||||
"""
|
||||
php_file = tmp_path / "closures.php"
|
||||
php_file.write_bytes(src)
|
||||
|
||||
|
||||
res = extract_php(php_file)
|
||||
if res.get('error'):
|
||||
pytest.skip(res['error'])
|
||||
|
||||
nodes = {n['id']: n for n in res['nodes']}
|
||||
labels = {n['label']: n for n in res['nodes']}
|
||||
|
||||
# Assert closures are correctly generated
|
||||
assert '{closure@2}()' in labels, "Missing anonymous_function_creation_expression node"
|
||||
assert '{closure@5}()' in labels, "Missing arrow_function node"
|
||||
|
||||
closure_1 = labels['{closure@2}()']
|
||||
closure_2 = labels['{closure@5}()']
|
||||
|
||||
# Check edges
|
||||
edges = res['edges']
|
||||
assert any(e['source'] == 'dummy.php' and e['target'] == closure_1['id'] and e['relation'] == 'contains' for e in edges), "Missing contains edge for closure_1"
|
||||
assert any(e['source'] == 'dummy.php' and e['target'] == closure_2['id'] and e['relation'] == 'contains' for e in edges), "Missing contains edge for closure_2"
|
||||
if res.get("error"):
|
||||
pytest.skip(res["error"])
|
||||
|
||||
labels = {n["label"] for n in res["nodes"]}
|
||||
|
||||
# Ordinals, not line numbers
|
||||
assert "{closure#1}()" in labels, "Expected first generic closure to be '{closure#1}()'"
|
||||
assert "{closure#2}()" in labels, "Expected second generic closure to be '{closure#2}()'"
|
||||
# Ensure no old line-based names leak through
|
||||
assert not any("closure@" in l for l in labels), "Line-based closure names should not appear"
|
||||
|
||||
|
||||
Reference in new issue
Block a user