Give PHP namespace identity, composer.json indexing, and PHPUnit test detection - #387
Merged
Conversation
Every PHP symbol was named by its bare short name, and every type
reference was lowered to a bare name too, so App\Models\User and
App\Entities\User were one symbol to every name-based lookup. That is not
a corner case: on laravel/framework 31.8% of type-directed edges land on a
short name that more than one class defines — `Post` names 37 classes,
`User` 27, `Builder` 5 across Contracts / Eloquent / Query.
The resolver was already built for this and had no producer. scope.go
documents MetaScopeNamespace ("scope_ns") with a PHP example and
MetaScopeUseAliases as a PHP-only key, and preferPhpScopeCandidate reads
both; the branches were unreachable because nothing stamped them.
The pass is additive — node IDs do not change. Each declaration gains the
namespace it lives in; types, interfaces and free functions gain a
fully-qualified QualName. QualName stops there on purpose: the graph's
qual-name index holds one node per name, and minting an entry for every
method and property would grow it by an order of magnitude for a lookup
nobody performs.
Each type reference gains the fully-qualified name the source names,
resolved the way PHP resolves a class reference: a leading `\` is already
absolute, a qualified name's first segment may itself be imported, a bare
name is an import when the file imports it and otherwise names a class in
the current namespace. `use function` feeds scope_use_aliases instead, so
a function import cannot qualify a class reference. Canonicalisation to
the bare name would have thrown the written form away, so the reference
passes carry it forward and the namespace pass consumes it.
On the resolver side, resolveTypeRef and resolveTypeOrFunc narrow
same-named candidates to the ones declared under the named namespace.
Narrowing only: when nothing matches — a reference into a vendor namespace
the graph does not hold — the full set is ranked exactly as before, so a
binding can be sharpened but never lost.
Measured on laravel/framework (3007 files): 99.4% of PHP declarations
carry a namespace, 98.3% of ambiguous type-directed edges carry a
fully-qualified target, and the share of those edges binding to the class
in the namespace the source actually names goes from 31.8% to 93.5%
(5690 -> 16709 of 17866). In-principle call resolution on monolog,
symfony/console and guzzle is unchanged to within 0.2pp.
…guesses Three follow-ons to PHP namespace identity, each measured separately. A `use App\Log\Handler;` names a CLASS, but resolveImport's cascade is package-directory-oriented and its candidate filter accepts only file nodes, so every PHP import fell through to an external stub. That starved buildImportClosure — which seeds each caller's reachable set from its RESOLVED import targets — so the cross-package guard saw no PHP file as reaching anything outside its own directory and reverted the calls. Binding the import to the class it names makes that class's directory reachable. In-principle call resolution: monolog 86.7 -> 90.5%, symfony/console 81.2 -> 90.6%, guzzle 81.7 -> 89.0%. PHP joins loneMemberLang. The bar there is not static typing as such: loneMemberDefnKeep already excludes bare free-function calls and receivers typed as something the repo does not define, and fires only when the name has exactly ONE in-repo method definition — the case duck typing cannot confuse. The bar is whether class-based member dispatch is the language's model, and PHP's is; since PHP call sites now stamp receiver_type, the external-receiver gate screens them better than it does for most of that list. Another +1.0 to +2.1pp. The PHP typed-receiver gate moves ahead of the caller-receiver fallback, which was mis-binding before it could run: asked to resolve `$this->handler->setFormatter()` inside BufferHandler::setFormatter, that fallback answers with BufferHandler::setFormatter — the caller itself — because it only checks whether the CALLER's class has a method of that name and ignores the stated receiver, a different type entirely. Node.QualName is deliberately NOT used. It backs a UNIQUE SQLite index (nodes_by_qual) whose second non-empty duplicate fails the insert outright, and PHP produces duplicate fully-qualified names in practice — indexing laravel/framework mints six, where a class and the annotation stub for a same-named attribute land in one namespace. Taking an index down beats degrading, so the namespace rides Meta, which tolerates repetition, and the import binding matches on it instead — requiring a unique match, so an ambiguous FQN declines rather than guessing.
The manifest table wired nine formats and composer.json was not one, so a PHP repo produced no module nodes and no dependency edges at all — only the generic JSON extractor's top-level key variables, which reach neither `require` nor `autoload`. ParseComposerJSON and ParseComposerLock follow the package.json template: require-dev is Indirect with the block tagged on Replace, versions stay verbatim because composer.lock carries the resolved ones, and a malformed manifest yields nothing rather than failing the index. Platform requirements are skipped — `php`, `hhvm`, `ext-*`, `lib-*` and the `composer-*` runtime pseudo-packages name no installable package, so a module node for them could never be depended on by anything. The manifest node is tagged "json", not "php": it shares its ID with the JSON extractor's file node, and a php-tagged file would vouch for PHP presence in a repo holding no PHP source. Module nodes do carry php via ecosystemLanguage, which the per-repo language census ignores for KindModule. composer.json's autoload map is the only statement a PHP repo makes about which namespaces are its own, so the PSR-4 and PSR-0 prefixes — from both `autoload` and `autoload-dev`, and accepting composer's string-or-array value — ride the manifest node. Attributing a VENDOR import to the package that provides it is not attempted: composer.json says which packages are required, not which namespace each one owns, and the vendor/pkg to Vendor\Pkg correspondence is convention rather than rule (monolog/monolog serves the Monolog namespace). Guessing it would mint wrong edges. Verified on monolog: 19 composer modules (1 production, 18 dev) with the autoload root `Monolog => src/Monolog, tests/Monolog`.
PHP was absent from every arm of test detection. TestRole had no case, so `testAddsTwoNumbers` was not a test symbol; AnnotationTestRole had none, so `#[Test]` said nothing; and there was no runner default or import signal, so no PHP file ever carried one. Everything reading test_role — the test targets a change should run, coverage gaps, the test-as-edges pass — saw a PHP repo as having no tests at all, even though IsTestFile already recognised `*Test.php`. All three of PHPUnit's ways of declaring a test now classify: the `test*` method prefix, the PHP 8 `#[Test]` attribute (bare or fully qualified, and `#[TestWith]`), and the `@test` docblock tag that predates attributes. `#[DataProvider]`, `#[TestDox]` and `#[CoversClass]` describe a test rather than declaring one and stay unmarked. The docblock tag must stand alone on its line, so `@testWith` and prose mentioning it do not mark the method. The prefix follows the same rule Go's does — it must be followed by an uppercase letter — so `testing()` and `testable()` are not swept in. Runner detection reads the imported namespace: Pest, PHPUnit, Codeception and Behat each claim their own, with phpunit as the language default when nothing more specific was imported. It reads the namespace off the import edge's meta rather than its target, because a PHP import now resolves to the class it names and no longer carries the specifier in the target.
phpSoleTypeAtom refused any declaration containing `|` or `&`, on the reasoning that it "names more than one possible runtime class". That is true of a union and backwards for an intersection: `Foo&Bar` says the value is EVERY branch at once, so any branch is a true statement about it, while `Foo|Bar` says it is one of them and picking a branch would bind the call to the wrong class at the resolved tier. The cost showed up when classifying what monolog still leaves unresolved: `private PushoverHandler&MockObject $handler;` is the idiomatic type of a mocked collaborator in a modern PHP test suite, and refusing it gave up every call through that property. The first non-builtin branch is taken. When the method being called lives on a different branch, the hierarchy walk from this one finds nothing and the call stays unresolved — the failure mode is a missing edge, never a wrong one. Unions, including nullable unions, still yield nothing. In-principle call resolution on monolog: 91.8% -> 92.3%.
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.
Follows #386, which fixed PHP call and property extraction. That change left three things explicitly deferred; this is those three.
Namespace identity
Every PHP symbol was named by its bare short name, and every type reference was lowered to a bare name too, so
App\Models\UserandApp\Entities\Userwere one symbol to every name-based lookup. That is not a corner case. On laravel/framework (3007 files) 31.8% of type-directed edges land on a short name that more than one class defines —Postnames 37 classes,User27,Builder5 across Contracts / Eloquent / Query.The resolver was already built for this and had no producer.
scope.godocumentsMetaScopeNamespace("scope_ns") with a PHP example andMetaScopeUseAliasesas a PHP-only key, andpreferPhpScopeCandidatereads both — the branches were unreachable because nothing ever stamped them.The pass is additive; node IDs do not change. Each declaration gains the namespace it lives in, and each type reference gains the fully-qualified name the source names, resolved the way PHP resolves a class reference: a leading
\is already absolute, a qualified name's first segment may itself be imported, a bare name is an import when the file imports one and otherwise names a class in the current namespace.use functionfeedsscope_use_aliasesinstead, so a function import cannot qualify a class reference. Canonicalisation to the bare name would have thrown the written form away, so the reference passes carry it forward and the namespace pass consumes it.resolveTypeRefandresolveTypeOrFuncthen narrow same-named candidates to the ones declared under that namespace. Narrowing only — when nothing matches, which is what a reference into a vendor namespace looks like, the full set is ranked exactly as before. A binding can be sharpened, never lost.Of the 17866 ambiguous edges that carry a fully-qualified target, the share binding to the class in the namespace the source actually names goes from 31.8% to 93.5% (5690 → 16709).
Node.QualNameis deliberately not usedThe obvious implementation is to put the FQN in
Node.QualNameand look it up. That field backsnodes_by_qual, a UNIQUE partial index — a second node with the same non-emptyqual_namefails the insert outright, which takes the whole index down rather than degrading. I tried it: indexing laravel/framework mints six duplicates, where a class and the annotation stub for a same-named PHP 8 attribute land in one namespace. Six is enough, and duplicate FQNs are common enough in PHP fixtures that no measurement on a handful of corpora would prove safety.So the namespace rides
Meta, which tolerates repetition, and every lookup matches on(name, scope_ns)and returns a slice — an ambiguous FQN declines rather than guessing.Imports that name a class, and two guard fixes
A
use App\Log\Handler;names a class, butresolveImport's cascade is package-directory-oriented and its candidate filter accepts only file nodes, so every PHP import fell through to an external stub. That starvedbuildImportClosure— which seeds each caller's reachable set from its resolved import targets — so the cross-package guard saw no PHP file as reaching anything outside its own directory, and reverted the calls. Binding theimport to the class it names makes that class's directory reachable.
PHP also joins
loneMemberLang. The bar there is not static typing as such:loneMemberDefnKeepalready excludes bare free-function calls and receivers typed as something the repo does not define, and fires only when the name has exactly one in-repo method definition — the case duck typing cannot confuse.The bar is whether class-based member dispatch is the language's model, and PHP's is; since #386 made PHP call sites stamp
receiver_type, the external-receiver gate screens them better than it does for most of that list.The two stale doc comments naming "java, go" are corrected.
Finally, #386's typed-receiver gate moves ahead of the caller-receiver fallback, which was mis-binding before it could run: asked to resolve
$this->handler->setFormatter()insideBufferHandler::setFormatter, that fallback answersBufferHandler::setFormatter— the calling method itself — because it only checks whether the caller's class has a method of that name and ignores the stated receiver, a different type entirely.In-principle call resolution (call edges whose target is defined in-repo):
composer.json
The manifest table wired nine formats and composer was not one, so a PHP repo produced no module nodes and no dependency edges — only the generic JSON extractor's top-level key variables, which reach neither
requirenorautoload.require/require-devnow mint module nodes following the package.json template (dev isIndirectwith the block tagged onReplace, versions stay verbatim, a malformed manifest yields nothing rather than failing the index),and composer.lock supersedes the ranges. Platform requirements are skipped —
php,hhvm,ext-*,lib-*and thecomposer-*pseudo-packages name no installable package, so a module node for them could never be depended on.The PSR-4 / PSR-0 autoload map — from both
autoloadandautoload-dev, accepting composer's string-or-array value — rides the manifest node. It is the only statement a PHP repo makes about which namespaces are its own.Attributing a vendor import to the package that provides it is not attempted: composer.json says which packages are required, not which namespace each one owns, and the
vendor/pkg↔Vendor\Pkgcorrespondence is convention rather than rule (monolog/monologserves theMonolognamespace). Guessing it would mint wrong edges. Doing it properly means reading each dependency's own autoload map out ofvendor/.Verified on monolog: 19 composer modules (1 production, 18 dev), autoload root
Monolog => src/Monolog, tests/Monolog.PHPUnit and Pest
PHP was absent from every arm of test detection.
TestRolehad no case, sotestAddsTwoNumberswas not a test symbol;AnnotationTestRolehad none, so#[Test]said nothing; and there was no runner default or import signal, so no PHP file carried one. Everything readingtest_role— the tests a change should run, coverage gaps, the test-as-edges pass — saw a PHP repo as having no tests, even thoughIsTestFilealready recognised*Test.php.All three of PHPUnit's declaration forms now classify: the
test*method prefix, the PHP 8#[Test]attribute (bare or fully qualified, plus#[TestWith]), and the@testdocblock tag that predates attributes.#[DataProvider],#[TestDox]and#[CoversClass]describe a test rather than declaring one and stay unmarked. The docblock tag must stand alone on its line, so@testWithand prose mentioning it do not mark the method. The prefix follows the same rule Go's does — followed by an uppercase letter — sotesting()andtestable()are not swept in.Runner detection reads the imported namespace: Pest, PHPUnit, Codeception and Behat each claim their own, with phpunit as the default. It reads the namespace off the import edge's meta rather than its target, because a PHP import now resolves to the class it names and no longer carries the specifier there.
Verification
go test -race ./...,golangci-lint run ./internal/..., and the static-link parser gate. 25 new test functions. In-principle call resolution on monolog, symfony/console and guzzle is measured above; the namespace figures come fromindexing laravel/framework and comparing, for every type-directed edge landing on an ambiguous short name, the bound target's namespace against the one the source names.
Still deferred
#[Route]pattern requires a literalmethods:clause and a positional path, so most real routes are dropped; routes never bind to their controller method;routes.yamlis unread; only#[AsEventListener]produces a dispatch edge, leavingAsCommand/AsMessageHandler/Autowireinert.dispatch(new Job)never wires to the job'shandle(); Eloquent relations are unmodeled.analyze modelsis empty for PHP.@covers/#[CoversClass]: the test-to-subject link is not read from PHPUnit's coverage annotations, only from the generic passes.add_action/add_filter/do_actiondispatch uncovered.entrypoints.Detecthas no PHP case;dead_codetreats every PHP symbol as exported; there are no PHPEdgeThrows, soanalyze error_surfaceis empty; there is no PHP CFG spec, soget_cfgandtaint_pathsare unavailable; andanalyze review/unsafe_patternshave no PHP rules.insteadof/as) aliases, PHP 8.4 property hooks, attributes on properties / constants / enum cases, andphpDocMethodRedropping any@methodwhose return type contains a space.