PHP: receiver-typed member calls, and park them for cross-repo merges - #3391
PHP: receiver-typed member calls, and park them for cross-repo merges#3391xiongjianxu wants to merge 4 commits into
Conversation
`$greeter->greet()` produced no edge when `Greeter` was declared in another file: the shared cross-file pass skips member calls, and the PHP extractor never read `member_call_expression`'s object, so the receiver was not even recorded. Every call through an injected dependency was invisible to `affected` and to every reverse-dependency query. The extractor now captures the receiver — the variable for `$greeter->greet()`, the property name for `$this->greeter->greet()` — and exports a per-file `php_type_table` built from the four places a receiver's type is written down: a typed property, a promoted constructor parameter, a typed parameter, and `$g = new Greeter()`. `_resolve_php_member_calls` types the receiver from that table and emits `calls` to the single class declaring that type. Only a single class name binds: a union, an intersection or a primitive names no one class. In-file resolution is untouched — PHP does not defer to this pass, so a call whose bare callee name already matched in its own file resolves exactly as before, and the receiver type is consulted only after that miss.
A PHP receiver typed to a class this build declares nowhere is a call into another repository, not a mistake. The resolver held the receiver type and dropped the call, so graph.json — the only artifact merge-graphs and global add read — recorded nothing and no merge-time pass could recover it. Those calls are now parked on the caller node by name, and the merge pass binds them when the type resolves to exactly one declaration in another repo. The suffix set covers every extension the PHP extractor claims, so a `.phtml` template that declares the class still answers.
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
Adds cross-file PHP member-call resolution so a call like $greeter->greet() on a typed receiver whose method lives in another file now gets a calls edge. _resolve_php_member_calls types the receiver from a new per-file php_type_table (built by _php_receiver_type_table from typed properties, promoted/typed parameters, and new bindings, first binding wins), emits an INFERRED edge at 0.85 only when exactly one class declares that type, bails on ambiguity, and parks receivers whose type this build declares nowhere for a later merge. Registers php suffixes for cross-repo binding, extracts receiver names for $var->m() and $this->prop->m() chains, and keeps PHP member calls in-file rather than deferring so an unresolved bare callee routes through raw_calls.
Worth a look
- PHP member resolver can bind calls to non-PHP declarations —
graphify/extract.py:4459· Escalate · high- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Existing non-call edge suppresses required PHP calls edge —
graphify/extract.py:4471· 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 — 2162 functions depend on the 519 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 546 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 27 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 43 more — each is listed as a finding
Verification — 2162 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: 1997 function(s) in the blast radius were not formally verified this run
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
· 51 more finding(s) on lines outside this diff (see the check run).
A corpus-wide type index let a same-named class in another language answer a PHP receiver, and it hid from the parking branch that no PHP file declares the type.
…hared resolver does
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 4 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds cross-file PHP member-call resolution: _resolve_php_member_calls looks each $greeter->greet() receiver up in a per-file php_type_table, resolves the single class declaring that type, and emits an INFERRED calls edge (score 0.85) to its method, bailing when the type is ambiguous or a builtin. Builds that table via _php_receiver_type_table, which records the written-down type of typed properties, promoted/typed parameters, and new bindings (file-scoped, first binding wins), and types member-call receivers during generic extraction from $var or $this->prop. Parks receivers whose type is declared nowhere in the corpus for a later merge (#3152), and extends the PHP suffix set (including .phtml, .php3–.php7, .phps) to the cross-repo indexer and resolver activation.
Worth a look
- PHP builtin globals are checked against the language map keys —
graphify/extract.py:4484· 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 member-call resolver suppresses calls edges when any other edge already connects the nodes —
graphify/extract.py:4495· 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 receiver type table conflates same-named parameters across methods —
graphify/extractors/engine.py:693· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- member_receiver may be referenced before assignment for PHP member_access chains —
graphify/extractors/engine.py:5551· 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 — 2163 functions depend on the 520 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 546 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 27 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 43 more — each is listed as a finding
Verification — 2163 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: 1998 function(s) in the blast radius were not formally verified this run
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
· 51 more finding(s) on lines outside this diff (see the check run).
|
Self-audit pass over the bot review. One fix pushed, two findings I am deliberately not acting on. Fixed — the declaration index was corpus-wide. Not acting on — Not acting on — the type table is per-file and flat. Swift (#1356), TS/JS and C++ all scope theirs this way, and Verification: |
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
Adds cross-file PHP receiver-typed member-call resolution so calls like $greeter->greet() reach a method declared in another file. The engine now builds a per-file php_type_table (via _php_receiver_type_table) mapping receivers to their written-down types from typed properties, promoted constructor params, typed params, and new bindings, and captures member receivers for both $var->m() and $this->prop->m() chains; the new _resolve_php_member_calls pass looks the receiver up, and when exactly one PHP-declared class of that type exists emits an INFERRED calls edge at 0.85, bailing on ambiguity and skipping builtins. A receiver typed to a class this build contains nowhere is parked on the caller (#3152) for a later merge rather than dropped, and PHP suffixes are registered so cross-repo scanning picks up .php/.phtml/etc.
Worth a look
- Nullable PHP builtin types are parked as unresolved —
graphify/extract.py:4476· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- member_receiver may be used before assignment for PHP chained access —
graphify/extractors/engine.py:5551· 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 — 2163 functions depend on the 520 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 546 callers, 43 callees - new:
_rebuild_code()— 115 callers, 51 callees - new:
_extract_generic()— 18 callers, 27 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 43 more — each is listed as a finding
Verification — 2163 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: 1998 function(s) in the blast radius were not formally verified this run
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
· 51 more finding(s) on lines outside this diff (see the check run).
|
Both advisory findings were checked against the branch; neither holds.
Nullable builtin types parked as unresolved (extract.py:4476) — the premise does not hold. What is true, and deliberate, is that a stdlib class such as |
Fixes #3390. Two cherry-pickable commits: the single-repo resolver, then the park that
lets a merged graph finish what one build cannot.
Commit 1 — resolve PHP member calls through the receiver's declared type
PHP recorded no receiver on a member call and exported no type table, so
$greeter->greet()across files produced no edge. Both halves are added:$greeter->greet(), theproperty name for
$this->greeter->greet();php_type_tableis exported, and_resolve_php_member_callstypes thereceiver from it and emits
callsto the single class declaring that type.Table sources, all four needed:
private Greeter $greeter;__construct(private Greeter $g)declares no property, so nothing else names the typefunction run(Greeter $g)newbinding$g = new Greeter();Design calls:
type-qualified receiver (
Type.staticMethod()) 1.0/EXTRACTED, but in PHP thatspelling is
Helper::format()— ascoped_call_expression, which already has its ownpath and is left alone. Every receiver this pass types is a variable, so there is no
spelling of a member call that names the type exactly.
resolves there, exactly as before; the receiver type is consulted only after that miss,
which is the only situation where it can add an edge. This keeps in-file behaviour
byte-identical rather than trading known edges for better-typed ones.
binding the first arm of
Greeter|Otherwould be a guess.?Greeterdoes name one.$this->prop->call()and$var->call()are typed;$this->a->b->call()types neitheranorband is left unresolved.method cannot retype the property's own calls. The table walk pushes children reversed
to yield document order — without that, "first" is DFS order and the wrong binding wins
(this is what the shadowing test caught).
confidence_scoreis 0.85, the high-confidence rung of the rubric's discreteINFERRED scale in
references/extraction-spec.md. The sibling member-call resolversstill emit the off-rubric 0.8;
test_no_module_hardcodes_an_off_rubric_inferred_scoreexists to stop new ones, and snapping the pre-existing sites is out of scope here.
Commit 2 — park what this build cannot answer (#3152)
A receiver typed to a class with zero declarations in this corpus is parked on the
caller as a
metadata.unresolved_callsentry (names only, never node ids — those arerewritten by the #1529 remap and again by repo prefixing).
> 1declarations staysdropped: local ambiguity is not something merging can narrow. The merge pass then binds
the entry when exactly one declaration in another repo answers it.
_LANG_SUFFIXES["php"]covers every extension the PHP extractors claim (.php,.phtml,.php3–.php7,.phps), so a class declared in a template file still answers.Known cost
Class name matching is case-sensitive on both sides. PHP class names are case-insensitive,
so
private greeter $gbinds nothing. Folding case in the merge pass would let thelanguages that park alongside PHP bind
greetertoGreeter, which is why the pass keepscase; a PHP-only fold is a separate change.
Verification
tests/test_php_receiver_member_calls.py— 11 cases: one per type source (including?Greeter), the negatives that must stay unresolved (untyped parameter, union type,longer chain, two same-named classes), the shadowing case, and one pinning that
Helper::format()keeps its existing path. 6 of the 11 fail onv8.tests/test_cross_repo_member_calls.py— aphp-typed-propertyarm in the per-languagepark→merge test.
ruff check graphify testsclean.