Improve PHP parsing coverage and call resolution - #386
Merged
Conversation
Every other AST extractor that records call sites stamps `receiver_type` on a member-call edge, which lets the resolver's exact-type passes bind `x.foo()` to the `foo` that belongs to x's type. PHP stamped it only for Laravel facades: every `$this->foo()`, `$obj->foo()` and `Foo::foo()` became a bare `unresolved::*.foo`, leaving the resolver nothing but a repo-wide name match. Because a name-only pick lands at the text_matched tier, the cross-package guard then reverted most of them — the calls were not merely imprecise, they were dropped. php_receiver.go builds a per-body environment from declarations only: `$this`, typed properties (including PHP 8 promoted constructor properties), typed parameters, `$v = new Foo`, `catch (Foo $e)` and `@var` annotations. It never guesses — a variable assigned two different classes is dropped rather than picked, and a union/intersection declaration types nothing, because a wrong receiver_type binds a call to the wrong method at the resolved tier instead of leaving it for the name-match fallback. Closures take a child scope so a closure parameter shadows an outer variable, and a `static` closure loses `$this`. Two shapes were dropped entirely and now emit edges: nullsafe calls (`$x?->m()`, never a case in extractCallSites) and static calls to an ordinary class (`Utils::pick()` was only typed when the scope happened to be a registered facade). On the resolver side a typed receiver binds through the class hierarchy, which is the case a same-name match cannot see: the method a receiver inherits is declared on an ancestor, not on the receiver's own class. A typed receiver that resolves to nothing stays unresolved rather than falling through to the name-only fan-out. In-principle call resolution (calls whose target is defined in-repo, so vendor and builtin calls are excluded), measured over three idiomatic corpora: monolog 32.6% -> 88.2% symfony/console 50.5% -> 79.9% guzzle 61.4% -> 82.9%
The extractor recorded calls but never accesses. `$this->handlers`,
`$obj->name`, `Foo::$registry` and `Foo::VERSION` produced no edge of any
kind, so find_usages on a PHP property or class constant returned its
declaration and nothing else — the member looked unused no matter how
often the code read or wrote it. Before this change monolog's whole graph
held 76 read edges and zero writes against 322 field nodes.
Each access now emits EdgeReads, or EdgeWrites in assignment position, to
`unresolved::*.<member>` with the receiver typed where the source states
it. That is the shape resolveFieldRef already consumes: with a
receiver_type it binds to that type's field exactly, and without one it
falls through the same locality cascade every other language uses.
Write detection climbs subscripts, because `$this->handlers[] = $h`
mutates the property — but only through the container operand, so a
member access used as an index (`$store[$this->key] = $v`) stays a read.
`++`/`--` and `unset()` are writes too.
Two shapes deliberately emit nothing: a dynamic member (`$obj->$name`,
`$obj->{$expr}`) names no symbol, and `Foo::class` is the class-name
literal rather than a constant member — its type reference is already
emitted by the reference-form pass.
Measured over three corpora (resolved/total):
monolog reads 1065/2259, writes 458/528
symfony/console reads 1478/2769, writes 591/618
guzzle reads 930/2329, writes 332/441
extractCallSites was reached only from extractFunction and extractMethod,
so every statement outside a named function or method was invisible: a
script body, a bootstrap file, a Laravel routes file, a WordPress theme
template, a fixture that returns a closure. 35 of symfony/console's 361
files produced a file node, their imports, and nothing else.
A second walk now attributes those statements to the file node, stopping
at every declaration so a method body is not counted once for its method
and again for the file. Its environment is seeded from the top-level
`$v = new Foo` bindings, so `$app = new Application(); $app->run();` in a
bootstrap script types its receiver exactly as it would inside a function.
A closure written at file scope is walked as part of it — the closure is
not a symbol, so its body belongs to the file that defines it.
Anonymous classes were not extracted at all: `new class extends B { ... }`
minted no type node, so its methods were not symbols, calls into them
resolved nowhere, and calls out of them were attributed to whichever
function enclosed the expression. They now mint a type carrying the
extends / implements clauses, with members extracted the same way a named
class's are. The name is line-qualified (`class@anonymous:12`) because PHP
allows several per file and the members' receiver meta has to tell them
apart for receiver-directed resolution to stay precise.
`use Monolog\Handler\{StreamHandler, NullHandler};` emitted one import
of the shared prefix `Monolog/Handler` and dropped every member — the
grammar puts the braced members under a `body` field the walk never
looked at, so a group-importing file appeared to import nothing it
actually named. Each member now emits its own import, joined to the
prefix.
Import edges also carry what the source wrote: the fully-qualified name,
the short symbol, the `as` alias, and whether `use function` / `use const`
qualified it — including a group that qualifies members individually
(`use Monolog\{Logger, function Utils\jsonEncode}`). The edge target keeps
its historical slash-separated shape, so nothing downstream has to change
to benefit from the extra members.
Four declaration forms produced no symbol or no metadata:
Constructor property promotion declares a property in the constructor's
parameter list, so the class body holds no property_declaration and the
member walk minted nothing. On a modern PHP codebase, where promotion is
the idiomatic way to declare dependencies, a class's fields were simply
absent from the graph. They now mint fields carrying visibility, readonly,
declared type and a member_of edge, like any other property.
Only `visibility` was stamped on members, so `static`, `abstract`, `final`
and `readonly` were invisible — an analyzer could not tell a static method
from an instance one, and readonly, the marker that a property is never
written after construction, was lost.
A file-scope `const NAME = ...;` and `define('NAME', ...)` defined no
symbol at all: const_declaration was only reached inside a class body, and
define() is an ordinary call with no declaration node. A constants file
indexed as zero symbols. A computed `define($k, ...)` still mints nothing,
since it names nothing statically.
A PHP enum may implement interfaces, but the clause was not stamped into
scope_interfaces, so the dispatch hierarchy could not see the enum as an
implementor and a call through the interface never reached its methods.
Also: `nullable_type` is not a node in tree-sitter-php v0.24.2 — the three
switches naming it were dead, while PHP 8.2 disjunctive normal form
(`(Countable&Traversable)|null`) fell through untyped. Swapping the dead
case for the real one required teaching the atom split about parentheses,
which otherwise leak into a target name as `unresolved::(Countable`.
…call
PHP lets a callee be any expression — `$fn()`, `$obj->$method()`,
`Foo::$m()`, `($resolver)()`, `$handlers[0]()` — and the extractor emitted
the callee's raw source text as the edge target. That produced placeholders
like `unresolved::*.$fn` and `unresolved::*.($this->resolver)` which can
never bind to anything: 371 of them in guzzle alone, where `*.$handler` was
the third most common unresolved target in the whole repo. A computed
callee now emits nothing; the receiver-typed member calls inside such an
expression are still captured by the walk.
`new Foo()` emitted only an instantiation edge, which points at the class
rather than at the code that runs — so a PHP constructor showed no callers
at all, no matter how often the class was instantiated. It now also lowers
to a call of `__construct` with the receiver typed, the same shape Java
uses, so the resolver binds it to Foo's own constructor or to the nearest
ancestor's when Foo inherits one. `new $cls` and `new class {}` name no
constructible type and still emit nothing.
Catch clauses named no types, so a class used only as an exception type
looked unreferenced and the handler was invisible to error-surface
analysis. Each caught type in `catch (A | B $e)` is now a reference.
langComplexityTables had no PHP entry, so StampFunctionMetrics was a no-op for the language and no PHP function or method carried complexity, cognitive or loop_depth meta. Everything reading those keys — analyze bottlenecks, health_score, impact — saw a PHP repo as uniformly trivial. The table follows the grammar: `else_if_clause` is a decision point while a bare `else_clause` is not, a `match` arm counts like a switch case, and `conditional_expression` covers both ternary forms. The skip set stops the walk at every nested declaration, so a closure's branches are scored against the closure rather than folded into the function returning it. `.phtml` — the PHP template extension Zend, Laminas and Magento use — was not in Extensions(), so those files were not indexed at all. The binding is the HTML-framing grammar, so a template already parses as PHP; the extension list was the only thing keeping it out.
extractCallSites became a wrapper with no callers once the walk split into emitPHPCallSiteEdges plus its two drivers, and phpReceiverEnv.withClass was superseded by childScope handling the static-closure case directly. Both trip golangci-lint's unused check.
A static property or class constant reached through a relative scope (`self::$items`, `static::LIMIT`) fell through untyped, because the scope is a relative_scope node rather than a name. Late static binding may pick a subclass at runtime, but the enclosing class is where the member is declared or inherited from, which is what the hierarchy walk needs. `parent::` stays untyped on purpose so the existing scope_kind path walks up from the enclosing class instead of pinning to it.
Once receivers carry a type, `$this->getFormatter()` inside AmqpHandler still bound to BufferHandler.getFormatter — a sibling class that happens to sort first. The exact-type passes cannot fire, because the method is inherited rather than declared on the receiver, so resolveMethodCall fell through to its locality fallback: prefer a same-name method in the caller's directory. That heuristic is sound where a directory is a package boundary, and wrong for PHP, where one directory holds every sibling implementation of an interface. When the receiver names a type this repo defines, the locality pick is now withheld and the edge left for resolvePHPOverrideDispatch, which walks the hierarchy from the stated receiver and binds the declaration it actually inherits. Verified on monolog: the calls above now land on FormattableHandlerInterface / FormattableHandlerTrait. The gate stops at in-repo receivers on purpose. A vendor-typed receiver has no hierarchy in the graph either, so withholding the locality pick would lose the edge with nowhere better to put it — that case keeps its existing behaviour. Gating this way costs 0.3-1.9pp of in-principle call resolution versus letting the guesses stand, in exchange for the wrong bindings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PHP was the weakest language in the graph, and the cause was not parsing.
tree-sitter-php parses the corpora below with zero error or missing nodes,
and the extractor already captured 98–99.9% of the call sites in the AST. The
edges it produced simply did not bind to anything: 67–85% of PHP call edges
died unresolved (85% on monolog), and properties produced no usage edges at
all.
What was wrong
Every other AST extractor that records call sites — Go, Java, TypeScript, C#,
Kotlin, Python — stamps
receiver_typeon a member-call edge, which is whatlets the resolver's exact-type passes bind
x.foo()to thefoobelonging tox's type. PHP stamped it only for Laravel facades. Every
$this->foo(),$obj->foo()andFoo::foo()became a bareunresolved::*.foo, leaving theresolver a repo-wide name match. Because a name-only pick lands at the
text_matchedtier, the cross-package guard then reverted most of them — sothe calls were not merely imprecise, they were dropped.
Measuring three idiomatic corpora, 78% of monolog's and 79% of symfony/console's
member/scoped call sites had a receiver the source states outright and the
extractor discarded.
Commits
Type PHP call receivers. A per-body environment built from declarations
only:
$this, typed properties (including promoted constructor properties),typed parameters,
$v = new Foo,catch (Foo $e),@var. It never guesses— a variable assigned two different classes is dropped rather than picked,
and a union/intersection declaration types nothing, because a wrong
receiver_typebinds a call to the wrong method at the resolved tierinstead of leaving it for the name-match fallback. On the resolver side a
typed receiver binds through the class hierarchy, which is the case a
same-name match cannot see: the method a receiver inherits is declared on an
ancestor. Also fixes two shapes that emitted nothing: nullsafe calls
(
$x?->m()) and static calls to an ordinary class.Property and class-constant usage edges.
$this->handlers,$obj->name,Foo::$registryandFoo::VERSIONproduced no edge of anykind — monolog's whole graph held 76 read edges and zero writes against 322
field nodes, so every PHP property looked unused. Write detection climbs
subscripts (
$this->handlers[] = $hmutates the property) but only throughthe container operand, so a member access used as an index stays a read.
File-scope statements and anonymous classes.
extractCallSiteswasreached only from
extractFunction/extractMethod, so a script body, abootstrap file, a Laravel routes file or a WordPress template was invisible;
35 of symfony/console's 361 files produced their file node and nothing else.
Anonymous classes minted no type at all, so their methods were not symbols.
Grouped
usedeclarations and import metadata.use A\{B, C};emittedone import of the shared prefix and dropped every member. Imports now carry
the FQN, short symbol,
asalias, anduse function/use constqualifier.Declaration forms the member walk skipped. Constructor property
promotion minted no fields — on a modern PHP codebase a class's dependencies
were absent from the graph entirely. Plus
static/abstract/final/readonlymodifiers, file-scopeconstanddefine(), and the enumimplementsclause (without which the dispatch hierarchy cannot see an enumas an implementor).
nullable_typeis not a node in tree-sitter-php v0.24.2— the three switches naming it were dead, while PHP 8.2 DNF types fell
through untyped.
Call-target hygiene and constructor calls. A computed callee (
$fn(),$obj->$method(),($resolver)()) had its raw source text emitted as theedge target, minting placeholders like
unresolved::*.$fnthat can neverbind — 371 in guzzle, where
*.$handlerwas the third most commonunresolved target in the repo.
new Foo()now also lowers to a__constructcall, so a PHP constructor has callers. Catch clauses nametheir types.
A same-directory guess stealing an inherited call. Once receivers were
typed,
$this->getFormatter()insideAmqpHandlerstill bound toBufferHandler::getFormatter— a sibling that happens to sort first. Theexact-type passes cannot fire when the method is inherited, so
resolveMethodCallfell through to its locality fallback: prefer a same-namemethod in the caller's directory. That is sound where a directory is a
package boundary and wrong for PHP, where one directory holds every sibling
implementation of an interface. When the receiver names a type this repo
defines, the locality pick is now withheld and the hierarchy walk binds the
declaration actually inherited. Gated at in-repo receivers on purpose: a
vendor-typed receiver has no hierarchy in the graph either, so withholding
would lose the edge with nowhere better to put it.
Complexity metrics and
.phtml.langComplexityTableshad no PHP entry,so
analyze bottlenecks/health_score/impactsaw every PHP repo asuniformly trivial.
.phtmlwas not inExtensions(), so Zend/Laminas/Magento templates were not indexed at all.
Measured
In-principle call resolution — call edges whose target is defined in-repo, so
vendor and builtin calls are excluded and the denominator is what could
actually bind:
Resolved call edges: monolog 903 → 3310, symfony/console 4957 → 9634, guzzle
4175 → 8668. Property usage on monolog went from 76 reads / 0 writes to 1229
resolved reads / 484 resolved writes.
Those figures are after the precision gate in commit 7, which trades
0.3–1.9pp of this metric for correcting bindings that pointed at the wrong
sibling class. Precision was spot-checked by dumping every resolved
getFormattercall in monolog with its receiver and target and reading themagainst the source.
Cost: parsing plus extracting all 710 corpus files goes from ~4.6s to ~5.3s
(+15%) for ~25% more edges — a second file-scope walk, the property-access
edges, and a per-body type environment. The tree-sitter parse still dominates.
Verified with
go test -race ./...,golangci-lint run ./internal/..., and thestatic-link parser gate. 66 PHP test functions added or rewritten.
One existing test changed meaning:
TestPHPFacade_NonFacadeStaticCallUnstampedasserted that an ordinary
Helper::frobnicate()carries no receiver hint.That was the gap, not a contract — the receiver of a static call is its scope
class. It is now
TestPHPFacade_NonFacadeStaticCallUsesScopeClass, still guarding the thing itwas really protecting: that facade mapping does not leak to non-facades.
Deliberately not in this change
An audit of the whole PHP surface found more than these; each was left out
because it is a separate piece of work, not because it is unimportant:
canonicalizePHPTypeRefdiscards the FQN, soApp\Models\UserandApp\Entities\Userare indistinguishable to every name-based lookup.Relatedly,
resolveImporttreats PHP import targets as directory paths, whichleaves the import closure empty and collapses the cross-package guard to
same-directory; PHP is absent from
loneMemberLang; andpreferPhpScopeCandidate's namespace and use-alias branches are unreachable.This is one coherent change to node identity and deserves its own review.
dependency nodes.
#[Route]pattern requires a literalmethods:clause and apositional path, so most real routes are dropped; routes never bind to their
controller method;
routes.yamlis unread; only#[AsEventListener]producesa dispatch edge, leaving
AsCommand/AsMessageHandler/Autowireinert.dispatch(new Job)never wires to the job'shandle(); Eloquentrelations are unmodeled.
analyze modelsis empty for PHP.#[Test]/@test/
@covershandling.add_action/add_filter/do_actiondispatch uncovered.entrypoints.Detecthas no PHP case;dead_codetreats every PHPsymbol as exported; there are no PHP
EdgeThrows, soanalyze error_surfaceis empty; there is no PHP CFG spec, so
get_cfgandtaint_pathsareunavailable; and
analyze review/unsafe_patternshave no PHP rules.insteadof/as) aliases, PHP 8.4property hooks, attributes on properties / constants / enum cases, and
phpDocMethodRedropping any@methodwhose return type contains a space.