diff --git a/CHANGELOG.md b/CHANGELOG.md index abf791d0f..d6083f49e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- PHP method calls made through a class property — `$this->dep->method()`, the dominant call shape in constructor-injection codebases (Symfony, Laravel) — now resolve to the method on the property's declared type, so `codegraph_callers` and impact analysis see production callers instead of reporting a DI-heavy method as uncalled or test-only. All three declaration shapes count: a promoted constructor parameter (`private readonly Foo $dep`), a typed property (`private Foo $dep;`), and a classic constructor parameter assigned in `__construct`. Interface-typed properties resolve to the interface method, and a method the property's type inherits from a supertype resolves through the existing conformance retry once `extends`/`implements` edges are built. Resolution is deliberately exclusive: a property whose type can't be recovered statically (docblock-only, setter/container injection) stays unlinked rather than guessed, so no name-similarity false edges are introduced. (#1220) ## [1.3.0] - 2026-07-07 diff --git a/__tests__/php-property-receiver-resolution.test.ts b/__tests__/php-property-receiver-resolution.test.ts new file mode 100644 index 000000000..4c145802a --- /dev/null +++ b/__tests__/php-property-receiver-resolution.test.ts @@ -0,0 +1,225 @@ +/** + * PHP property-receiver resolution (#1108 family). + * + * `$this->prop->method()` reaches the resolver as `this->prop.method` (the + * extractor records the receiver's raw text with the leading `$` stripped, and + * — unlike a `foo()->bar()` chain — there are no `()` on the receiver). The + * property's declaration lives OUTSIDE the calling method: a promoted + * constructor parameter (`private readonly Greeter $greeter`), a classic typed + * property assigned in `__construct`, or a property typed by an interface. The + * resolver recovers the property's declared type (widening the local-receiver + * scan to the whole file, the treatment component-scoped fields already get) + * and validates it through `resolveMethodOnType`, so a property whose type + * can't be recovered stays UNLINKED rather than guessed — a wrong inference + * produces no edge instead of a wrong one. + * + * Method lookup runs EXCLUSIVELY through declared-type inference: the + * name-similarity fallbacks never see this shape, which is what makes the + * same-name-collision and no-type cases below negative. Inherited methods + * resolve only once `extends`/`implements` edges exist, so these refs defer to + * the conformance pass; the full `indexAll()` path here exercises that. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { CodeGraph } from '../src'; +import { Node } from '../src/types'; +import { ResolutionContext } from '../src/resolution'; +import { matchMethodCall } from '../src/resolution/name-matcher'; +import type { UnresolvedRef } from '../src/resolution/types'; + +describe('PHP property-receiver resolution', () => { + let dir: string; + beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'php-prop-recv-')); }); + afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); }); + + const write = (rel: string, body: string) => { + const p = path.join(dir, rel); + fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, body); + }; + + const load = async () => { + const cg = await CodeGraph.init(dir, { silent: true }); + await cg.indexAll(); + const db = (cg as any).db.db; + const calls: { src: string; tgt: string; tgtQn: string }[] = db + .prepare( + `SELECT s.name src, t.name tgt, t.qualified_name tgtQn + FROM edges e JOIN nodes s ON s.id = e.source JOIN nodes t ON t.id = e.target + WHERE e.kind = 'calls' AND t.kind = 'method'`, + ) + .all(); + cg.close?.(); + return calls; + }; + const hasCall = (calls: any[], src: string, tgtQn: string) => + calls.some((e) => e.src === src && e.tgtQn === tgtQn); + // Any resolved method call `src` makes to a method of the given bare name — + // used by the negative cases to assert nothing was guessed. + const callsMethodNamed = (calls: any[], src: string, tgt: string) => + calls.some((e) => e.src === src && e.tgt === tgt); + + const greeter = ` { + write('Greeter.php', greeter); + write('App.php', `greeter->greet(); } +} +`); + const calls = await load(); + expect(hasCall(calls, 'run', 'Greeter::greet')).toBe(true); + }); + + it('resolves a classic typed property assigned in the constructor', async () => { + write('Greeter.php', greeter); + write('App.php', `greeter = $greeter; } + public function run() { return $this->greeter->greet(); } +} +`); + const calls = await load(); + expect(hasCall(calls, 'run', 'Greeter::greet')).toBe(true); + }); + + it('resolves a property typed by an interface to the interface method', async () => { + write('GreeterInterface.php', `g->hello(); } +} +`); + const calls = await load(); + expect(hasCall(calls, 'run', 'GreeterInterface::hello')).toBe(true); + }); + + it('resolves an inherited method through the conformance pass (property typed by the subclass)', async () => { + // `baseMethod` is declared only on Base; the property is typed `Sub`. + // The `Sub extends Base` edge is what lets the deferred conformance walk + // find the method on the supertype — the whole point of deferring this ref. + write('Base.php', `s->baseMethod(); } +} +`); + const calls = await load(); + expect(hasCall(calls, 'run', 'Base::baseMethod')).toBe(true); + }); + + it('disambiguates by declared type when two classes share a method name (negative)', async () => { + // Both classes declare `greet`; the property is typed `Greeter`. A + // name-similarity fallback would happily link either — this shape must + // route to the RIGHT class and ONLY it. + write('Greeter.php', greeter); + write('OtherGreeter.php', `greeter->greet(); } +} +`); + const calls = await load(); + expect(hasCall(calls, 'run', 'Greeter::greet')).toBe(true); + expect(hasCall(calls, 'run', 'OtherGreeter::greet')).toBe(false); + // Exactly one method edge from `run` — no double-linking. + expect(calls.filter((e) => e.src === 'run')).toHaveLength(1); + }); + + it('creates no edge for an untyped property with only a docblock type (negative)', async () => { + // `@var Greeter` is a comment, not a declared type. Guessing from a + // docblock is out of scope — the property stays unlinked. + write('Greeter.php', greeter); + write('App.php', `greeter->greet(); } +} +`); + const calls = await load(); + expect(callsMethodNamed(calls, 'run', 'greet')).toBe(false); + }); + + it('creates no edge for a deep property chain `$this->a->b->method()` (negative)', async () => { + // The single-property pattern deliberately does not match a two-hop chain; + // the intermediate type is unknown, so nothing is guessed. + write('Greeter.php', greeter); + write('App.php', `a->b->greet(); } +} +`); + const calls = await load(); + expect(callsMethodNamed(calls, 'run', 'greet')).toBe(false); + }); + + it('a local variable shadowing a property routes to the local\'s type, not the property (#1108 regression)', async () => { + // `$greeter->greet()` has receiver `greeter` (no `this->`), so it takes the + // existing #1108 local-variable path, not the new property path. The local + // `new OtherGreeter()` must win by nearest-declaration-backward even though + // a property `$greeter` typed `Greeter` exists — the property change must + // not hijack a plain-variable receiver. + write('Greeter.php', greeter); + write('OtherGreeter.php', `greet(); } +} +`); + const calls = await load(); + expect(hasCall(calls, 'run', 'OtherGreeter::greet')).toBe(true); + expect(hasCall(calls, 'run', 'Greeter::greet')).toBe(false); + }); + + // Unit-level check of the confidence the integration DB does not expose: + // the property-receiver shape resolves through resolveMethodOnType at 0.9. + it('matchMethodCall resolves `this->prop.method` at confidence 0.9', () => { + const node = (id: string, name: string, qn: string, kind: Node['kind'], file: string): Node => ({ + id, kind, name, qualifiedName: qn, filePath: file, language: 'php', + startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0, + }); + const byName: Record = { + Greeter: [node('c:greeter', 'Greeter', 'Greeter', 'class', 'Greeter.php')], + greet: [node('m:greet', 'greet', 'Greeter::greet', 'method', 'Greeter.php')], + }; + const lines = [ + 'greeter->greet(); }', + '}', + ]; + const ctx: ResolutionContext = { + getNodesInFile: () => [], + getNodesByName: (name) => byName[name] ?? [], + getNodesByQualifiedName: () => [], + getNodesByKind: () => [], + fileExists: () => false, + readFile: () => null, + getFileLines: () => lines, + getProjectRoot: () => '', + getAllFiles: () => [], + getImportMappings: () => [], + }; + const ref: UnresolvedRef = { + fromNodeId: 'caller', referenceName: 'this->greeter.greet', referenceKind: 'calls', + line: 4, column: 0, filePath: 'App.php', language: 'php', + }; + const res = matchMethodCall(ref, ctx); + expect(res?.targetNodeId).toBe('m:greet'); + expect(res?.confidence).toBe(0.9); + expect(res?.resolvedBy).toBe('instance-method'); + }); +}); diff --git a/src/resolution/index.ts b/src/resolution/index.ts index dbf1e00f9..222e861df 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -16,7 +16,7 @@ import { FrameworkResolver, ImportMapping, } from './types'; -import { matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, sameLanguageFamily, crossesKnownFamily } from './name-matcher'; +import { matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesKnownFamily } from './name-matcher'; import { resolveViaImport, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef } from './import-resolver'; import { detectFrameworks } from './frameworks'; import { synthesizeCallbackEdges } from './callback-synthesizer'; @@ -44,6 +44,9 @@ const SCOPED_CHAIN_LANGUAGES = new Set(['rust']); /** The extractor's chained-receiver encoding: `().`. */ const CHAIN_SHAPE = /^(.+)\(\)\.(\w+)$/; +/** PHP `$this->prop->method()` encoded as `this->prop.method` — no `()`, so CHAIN_SHAPE misses it. */ +const PHP_PROP_SHAPE = /^this->\w+\.\w+$/; + /** * Cache size limits. Each per-resolver cache is bounded so memory * stays flat on large codebases (20k+ files). Sizes were chosen to @@ -889,6 +892,15 @@ export class ReferenceResolver { CHAIN_SHAPE.test(ref.referenceName) ) { this.deferredChainRefs.push(ref); + } else if ( + // PHP `$this->prop->method()` (encoded `this->prop.method`): its method + // may live on the property's declared supertype, resolvable only once + // implements/extends edges exist — defer to the same conformance pass. + ref.referenceKind === 'calls' && + ref.language === 'php' && + PHP_PROP_SHAPE.test(ref.referenceName) + ) { + this.deferredChainRefs.push(ref); } return null; } @@ -1030,9 +1042,13 @@ export class ReferenceResolver { const maybeYield = createYielder(); const resolved: ResolvedRef[] = []; for (const ref of deferred) { - // `::`-receiver languages (Rust) split on `::` (matchScopedCallChain); + // PHP `this->prop.method` resolves via matchMethodCall (declared-type + // inference + resolveMethodOnType conformance walk); `::`-receiver + // languages (Rust) split on `::` (matchScopedCallChain); other // dotted-receiver languages on `.` (matchDottedCallChain). - const chainMatch = SCOPED_CHAIN_LANGUAGES.has(ref.language) + const chainMatch = (ref.language === 'php' && PHP_PROP_SHAPE.test(ref.referenceName)) + ? matchMethodCall(ref, this.context) + : SCOPED_CHAIN_LANGUAGES.has(ref.language) ? matchScopedCallChain(ref, this.context) : matchDottedCallChain(ref, this.context); const match = this.gateLanguage(chainMatch, ref); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 141d213d0..059f469b5 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -1280,6 +1280,21 @@ function inferLocalReceiverType( componentScoped = scope === 'variables' || scope === 'this'; } } + // PHP `$this->prop` receiver — the property's declaration lives outside the + // calling method (a promoted constructor parameter `private readonly Foo $prop`, + // a typed property `private Foo $prop;`, or a classic constructor parameter + // `Foo $prop` assigned in __construct). Strip the prefix and widen the scan to + // the whole file, the same treatment CFML's component-scoped fields get above: + // the existing PHP typed-parameter pattern (`Foo $prop`) matches all three + // declaration shapes, and nearest-declaration-backward still prefers a local + // that shadows the property. + if (ref.language === 'php') { + const scoped = receiverName.match(/^this->(.+)$/); + if (scoped) { + scanReceiver = scoped[1]!; + componentScoped = true; + } + } const patterns = localReceiverTypePatterns( ref.language, @@ -1364,6 +1379,32 @@ export function matchMethodCall( ? ref.referenceName.match(/^([\w.]+)\$(\w+)$/) : null; + // PHP property receiver: `$this->prop->method()` reaches the resolver as + // `this->prop.method` (the extractor records the receiver's raw text with the + // leading `$` stripped). Resolve it EXCLUSIVELY through declared-type + // inference + resolveMethodOnType validation — the name-similarity strategies + // below must never see this shape, so a property whose type can't be + // recovered stays unlinked rather than guessed (a wrong inference produces no + // edge rather than a wrong one). Deeper chains (`this->a->b.method`) don't + // match the single-property pattern and stay unlinked, same as before. + const phpThisPropMatch = ref.language === 'php' + ? ref.referenceName.match(/^(this->\w+)\.(\w+)$/) + : null; + if (phpThisPropMatch) { + const [, receiver, phpMethodName] = phpThisPropMatch; + const inferredType = inferLocalReceiverType(receiver!, ref, context); + if (!inferredType) return null; + return resolveMethodOnType( + inferredType, + phpMethodName!, + ref, + context, + 0.9, + 'instance-method', + importedFqnOf(inferredType, ref, context), + ); + } + const match = dotMatch || colonMatch || luaColonMatch || rDollarMatch; if (!match) { return null;