fix(php): resolve method calls through $this-> properties on their declared type (#1220)#1221
Open
w0lan wants to merge 1 commit into
Open
fix(php): resolve method calls through $this-> properties on their declared type (#1220)#1221w0lan wants to merge 1 commit into
w0lan wants to merge 1 commit into
Conversation
…clared type (colbymchenry#1220) A call whose receiver is a class property — $this->dep->method(), the dominant call shape in DI-style PHP (Symfony/Laravel constructor injection) — produced no call edge: the extractor records the receiver as raw text (this->dep), which no resolution strategy could type, so callers/impact silently missed every production consumer of a method while reporting its unit-test callers (local-new receivers). Three coordinated pieces, all riding existing machinery: - inferLocalReceiverType: strip the PHP this-> prefix and widen the scan to the whole file — the same treatment CFML component-scoped fields already get. The existing PHP typed-parameter pattern then recovers the type from a promoted constructor parameter, a typed property declaration, or a classic constructor parameter alike. - matchMethodCall: resolve the this->prop.method shape EXCLUSIVELY via declared-type inference + resolveMethodOnType validation; the name-similarity strategies never see it, so a property whose type can't be recovered stays unlinked rather than guessed. - defer + conformance retry: a ref whose method lives on the property type's supertype resolves only once implements/extends edges exist — push it to the existing deferred-chain retry (PHP_PROP_SHAPE) and dispatch it back through matchMethodCall in the conformance pass. Validation: 15-case corpus (promoted/classic/interface/inherited/ same-name-collision/untyped/deep-chain/local-shadowing) all resolve at 0.9 or stay deliberately unlinked; 9 new vitest tests; full suite passes. On six real PHP codebases (PHP 7.1-8.5, Symfony 3.2-8.0) a sampled repository method went 0/7 -> 7/7 production callers and 49/49 manually verified property-receiver edges were correct with no false positives. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Fixes #1220
Problem
A PHP method call whose receiver is a class property —
$this->dep->method(), the dominant call shape in constructor-injection codebases (Symfony, Laravel) — produces nocallsedge. The extractor records the receiver as raw text (this->dep), which no resolution strategy can type, so the reference is dropped.codegraph_callersthen reports a heavily-used method as uncalled (or test-only, since unit tests calling through localnewreceivers DO resolve), and impact analysis misses every production consumer. Full evidence and measurement protocol in #1220.Fix
Three coordinated pieces, all riding existing machinery (no schema or extraction changes):
inferLocalReceiverType(name-matcher.ts): strip the PHPthis->prefix and widen the scan to the whole file — the same treatment CFML component-scoped fields (variables.svc/this.svc) already get in this function. The existing PHP typed-parameter pattern (Foo $prop) then recovers the type from a promoted constructor parameter, a typed property declaration, or a classic constructor parameter alike; nearest-declaration-backward still lets a shadowing local win.matchMethodCall(name-matcher.ts): resolve thethis->prop.methodshape exclusively through declared-type inference +resolveMethodOnTypevalidation at confidence 0.9 (the same tier as the Local-variable receiver-type inference is C++-only — instance method calls through a local don't resolve in other languages #1108/TypeScript/JavaScript: local-variable receiver-type inference doesn't cover typed function parameters (unlike Java/C#/Kotlin/Swift/Scala) #1125 typed paths). The name-similarity strategies below never see this shape, so a property whose type can't be recovered statically (docblock-only, setter/container injection) stays unlinked rather than guessed — a wrong inference produces no edge rather than a wrong one.resolution/index.ts): a method the property's type inherits from a supertype is resolvable only onceimplements/extendsedges exist, so refs matchingPHP_PROP_SHAPEare pushed to the existing deferred-chain retry and dispatched back throughmatchMethodCallin the conformance pass — the Tracking: chained factory/singleton call resolution across statically-typed languages #750/TS/JS class fields with type annotations are extracted as method-kind nodes #808 lifecycle, no new wiring.Validation
$this->a->b->m()(deliberately no edge, parity with other languages).__tests__/php-property-receiver-resolution.test.ts), modeled on the CFML receiver-inference tests; full suite passes.Known limitations (deliberately out of scope, candidates for follow-ups)
$this->prop->inner()->outer()— the first hop now resolves, the second needs the<inner>().<method>re-encoding other languages got in C++: method calls through singletons, factories, and chained getters resolve to the wrong class (or not at all) #645/PHP:codegraph_callersmisses chained calls on static-factory return values (Cls::for($x)->method(...)) — common Laravel per-credential client pattern #608.$this->prop?->method()) —nullsafe_member_call_expressionisn't in the PHP extractor'scallTypesat all (affects every nullsafe call, not just property receivers).@vardocblocks, or injected via setters/containers, stay unlinked by design.