feat(extract): support PHP closures (#3409) - #3461
Conversation
The bug:
PHP closures (`anonymous_function_creation_expression` and `arrow_function`)
were invisible to the graph, disconnecting API routes and callbacks from
the backend code.
The fix:
- Added closure syntax node types to _PHP_CONFIG's `function_types`.
- Updated engine.py to synthesize line-bound names (e.g., `{closure@42}`)
for unnamed closures matching these AST node types, enabling them to be
extracted as linkable nodes in the graph without ID collisions.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Extracts PHP anonymous functions and arrow functions as first-class function nodes, adding anonymous_function_creation_expression and arrow_function to the PHP config's function and boundary type sets. Since these lack a name, _extract_generic now synthesizes a positional label like {closure@2} from the node's start line rather than dropping the node.
Worth a look
- PHP closures on the same line collapse to the same symbol —
graphify/extractors/engine.py:4418· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- PHP closure symbol names collide for multiple closures on the same line —
graphify/extractors/engine.py:4419· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2117 functions depend on the 451 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 563 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 37 more — each is listed as a finding
Verification — 2117 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1952 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
105 of 263 test file(s) selected (40%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_php_closures.py— impact, changed-testtests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_catch_binding_shadow.py— impacttests/test_indirect_call_external_import_shadow.py— impacttests/test_indirect_call_for_of_binding_shadow.py— impacttests/test_indirect_call_function_expression_shadow.py— impacttests/test_indirect_call_nested_closure_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_inferred_confidence_rubric.py— impacttests/test_inherited_field_receivers.py— impacttests/test_java_member_calls.py— impacttests/test_java_type_resolution.py— impact- … and 55 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 45 more finding(s) on lines outside this diff (see the check run).
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.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Extracts PHP anonymous functions and arrow functions as first-class nodes by adding them to the function, boundary, and dispatch type sets. Names a closure passed directly to a routing call ($app->get('/api/users', fn)) after its route as GET /api/users, and otherwise assigns a stable per-scope ordinal like {closure#1} keyed by enclosing class or file so names survive line-only edits instead of shifting with line numbers.
Worth a look
- PHP closures now remove enclosing-function call edges —
graphify/extract.py:1044· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- PHP route-name derived closure label may collide across identical verb/path pairs —
graphify/extractors/engine.py:4459· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2120 functions depend on the 454 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 563 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 37 more — each is listed as a finding
Verification — 2120 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1955 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
105 of 263 test file(s) selected (40%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_php_closures.py— impact, changed-testtests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_catch_binding_shadow.py— impacttests/test_indirect_call_external_import_shadow.py— impacttests/test_indirect_call_for_of_binding_shadow.py— impacttests/test_indirect_call_function_expression_shadow.py— impacttests/test_indirect_call_nested_closure_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_inferred_confidence_rubric.py— impacttests/test_inherited_field_receivers.py— impacttests/test_java_member_calls.py— impacttests/test_java_type_resolution.py— impact- … and 55 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 45 more finding(s) on lines outside this diff (see the check run).
…Labs#3409) Address edge cases identified in review: - Nested route closures (e.g., inside group() or prefix()) now correctly compose their full path by walking up the AST across closure boundaries. - Non-routing closures passed to methods that share verbs (like ->get('user:42')) are no longer misidentified as routes. A route path must begin with '/' to qualify, otherwise it correctly falls back to an ordinal name.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Extracts PHP anonymous closures and arrow functions as first-class function nodes, adding them to the language's function, boundary, and body-walk sets so they become graph nodes with their own scopes. Names each closure by walking up the AST: a closure passed to a routing verb (get/post/group/etc.) with a /-prefixed path string gets a semantic VERB /path label, composing grouped prefixes inside-out; anything else falls back to a per-scope ordinal {closure#N} keyed by enclosing class or file stem, which stays stable across line-only edits. Guards against false positives like $cache->get('user:42', ...) by requiring the path to start with /, so non-routing get() calls degrade to ordinals rather than fake route labels.
Worth a look
- Nested-route walk uses stale prefixes when an intermediate call is not a routing method —
graphify/extractors/engine.py:3196· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2125 functions depend on the 459 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 563 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 37 more — each is listed as a finding
Verification — 2125 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1960 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
105 of 263 test file(s) selected (40%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_php_closures.py— impact, changed-testtests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_catch_binding_shadow.py— impacttests/test_indirect_call_external_import_shadow.py— impacttests/test_indirect_call_for_of_binding_shadow.py— impacttests/test_indirect_call_function_expression_shadow.py— impacttests/test_indirect_call_nested_closure_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_inferred_confidence_rubric.py— impacttests/test_inherited_field_receivers.py— impacttests/test_java_member_calls.py— impacttests/test_java_type_resolution.py— impact- … and 55 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 45 more finding(s) on lines outside this diff (see the check run).
…abs#3409) Address maintainer nits by moving _php_get_route_name and the _PHP_ROUTING_VERBS frozenset out of the _extract_generic body to the module level, preventing them from being repeatedly re-initialized per file on large codebases.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Extracts PHP anonymous functions and arrow functions as first-class nodes by adding them to the language config's function, boundary, and dispatch type sets, so closures now become named nodes in the graph. Names route-registration closures semantically as VERB /path — _php_get_route_name walks up through argument_list/call chains, requiring the innermost call to be a routing verb (get, post, group, etc.) with a path beginning /, and composes grouped prefixes inside-out; this rejects lookalikes like $cache->get('user:42', ...). Everything else falls back to a {closure#N} ordinal scoped to the enclosing class or file stem, keeping labels stable across line-only edits rather than encoding line numbers.
Worth a look
- arrow_function outer prefix walk misclassifies non-route first-arg strings as route paths —
graphify/extractors/engine.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2126 functions depend on the 460 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 563 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 26 callees - new:
extract_js()— 85 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
extract_objc()— 27 callers, 9 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 37 more — each is listed as a finding
Verification — 2126 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1961 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
105 of 263 test file(s) selected (40%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_php_closures.py— impact, changed-testtests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_catch_binding_shadow.py— impacttests/test_indirect_call_external_import_shadow.py— impacttests/test_indirect_call_for_of_binding_shadow.py— impacttests/test_indirect_call_function_expression_shadow.py— impacttests/test_indirect_call_nested_closure_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_inferred_confidence_rubric.py— impacttests/test_inherited_field_receivers.py— impacttests/test_java_member_calls.py— impacttests/test_java_type_resolution.py— impact- … and 55 more
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 44 more finding(s) on lines outside this diff (see the check run).
|
|
||
| return None | ||
|
|
||
| def _extract_generic( |
There was a problem hiding this comment.
_extract_generic()
fans out to 26 callees (efferent coupling); 18 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
The bug:
PHP closures (
anonymous_function_creation_expressionandarrow_function) were invisible to the graph, disconnecting API routes and callbacks from the backend code.The fix:
function_types.{closure@42}) for unnamed closures matching these AST node types, enabling them to be extracted as linkable nodes in the graph without ID collisions.