From 29e45f197176b2811135de1d4fc23c4ae1a392d6 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Tue, 12 May 2026 10:48:49 +0100 Subject: [PATCH] fix(php): stop MRO walk on arity-incompatible most-derived (U1) When the class-name receiver pass (Case 2 in receiver-bound-calls.ts) found a most-derived definition that was arity-incompatible with the call site, the previous code used `continue` to fall through to the next ancestor in the MRO chain. If an ancestor happened to be arity- compatible, the resolver emitted a false CALLS edge to it. This is incorrect: PHP dispatches to the most-derived override at runtime and throws `ArgumentCountError` when arities don't match - it never silently redirects to an ancestor. Replace the inner `continue` with `break` so the chain walk terminates and no edge is emitted for the site. Adds php-mro-arity-mismatch fixture and 5 regression tests covering: - the bug scenario (Child::method/2 + Parent::method/1, call with 1 arg) - arity-compatible happy path (Child::compat/1) - no-parent case (Orphan with arity mismatch) - happy path most-derived call - class detection sanity check --- .../passes/receiver-bound-calls.ts | 12 ++-- .../app/Models/ChildModel.php | 10 ++++ .../app/Models/Orphan.php | 8 +++ .../app/Models/ParentModel.php | 10 ++++ .../app/Services/Caller.php | 42 ++++++++++++++ .../php-mro-arity-mismatch/composer.json | 5 ++ .../test/integration/resolvers/php.test.ts | 58 +++++++++++++++++++ 7 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ChildModel.php create mode 100644 gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/Orphan.php create mode 100644 gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ParentModel.php create mode 100644 gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Services/Caller.php create mode 100644 gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/composer.json diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index 283f93b51..0cb544db9 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -293,15 +293,17 @@ export function emitReceiverBoundCalls( for (const ownerId of chain) { memberDef = findOwnedMember(ownerId, memberName, model); if (memberDef !== undefined) { - // Reject when arity is definitively incompatible (e.g., PHP - // f(int $req, ...$rest) called with zero args). Falls through - // to the next owner in the chain — a subclass may shadow with - // a different arity. + // The MRO chain is most-derived-first ([classDef, ...ancestors]). + // If the most-derived definition is arity-incompatible with the + // call site, PHP throws ArgumentCountError at runtime — it does + // NOT silently dispatch to an ancestor. Terminate the chain walk + // so no edge is emitted, rather than falling through to an + // arity-compatible ancestor (which would be a false positive). if ( narrowOverloadCandidates([memberDef], site.arity, site.argumentTypes).length === 0 ) { memberDef = undefined; - continue; + break; } break; } diff --git a/gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ChildModel.php b/gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ChildModel.php new file mode 100644 index 000000000..0dff935c9 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ChildModel.php @@ -0,0 +1,10 @@ + hits Case 2 (findClassBindingInScope) in + // receiver-bound-calls.ts. + // Pre-fix bug: MRO walk emits a false CALLS edge to ParentModel::method + // because Case 2 used `continue` on arity mismatch and fell through. + // Post-fix: zero edges (PHP throws ArgumentCountError at runtime). + ChildModel::method(1); + } + + public function callCompatible(): void + { + // Happy path: ChildModel::compat takes 1 arg; matches call site. + ChildModel::compat(1); + } + + public function callNoParent(): void + { + // Orphan::method takes 2 args; called with 1; no parent class exists. + // Pre-fix: same Case 2 bug — the loop exhausts with memberDef cleared, + // BUT with `continue` the loop simply ends after one iteration since + // the chain has only one entry; no edge would have been emitted here + // even pre-fix. Post-fix: same — zero edges. Documents the boundary. + Orphan::method(1); + } + + public function callMostDerivedHappy(): void + { + // Happy path: ChildModel::method takes 2 args; matches call site. + ChildModel::method(1, 2); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/composer.json b/gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/composer.json new file mode 100644 index 000000000..60ede80e5 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/composer.json @@ -0,0 +1,5 @@ +{ + "autoload": { + "psr-4": { "App\\": "app/" } + } +} diff --git a/gitnexus/test/integration/resolvers/php.test.ts b/gitnexus/test/integration/resolvers/php.test.ts index 6710c1dc1..14ef9bbd6 100644 --- a/gitnexus/test/integration/resolvers/php.test.ts +++ b/gitnexus/test/integration/resolvers/php.test.ts @@ -2035,3 +2035,61 @@ describe('PHP fully-qualified type-hint resolution (Codex #1497)', () => { expect(callsFromTo('saveLocal', 'record', 'app/Other/User.php').length).toBe(0); }); }); + +// --------------------------------------------------------------------------- +// MRO arity-mismatch: most-derived override with incompatible arity must NOT +// fall through to an arity-compatible ancestor (PHP throws ArgumentCountError +// at runtime). See receiver-bound-calls.ts Case 2. +// --------------------------------------------------------------------------- + +describe('PHP MRO arity-mismatch fallthrough', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'php-mro-arity-mismatch'), () => {}); + }, 60000); + + const callsFromTo = (source: string, target: string, targetFilePath: string) => + getRelationships(result, 'CALLS').filter( + (c) => c.source === source && c.target === target && c.targetFilePath === targetFilePath, + ); + + it('detects ParentModel, ChildModel, Orphan, and Caller classes', () => { + expect(getNodesByLabel(result, 'Class')).toEqual([ + 'Caller', + 'ChildModel', + 'Orphan', + 'ParentModel', + ]); + }); + + it('arity-incompatible most-derived override does NOT fall through to ParentModel::method', () => { + // Pre-fix bug: `$child->method(1)` with ChildModel::method(int,int) and + // ParentModel::method(int) would emit a false CALLS edge to ParentModel::method. + // Post-fix: zero CALLS edges from callIncompatible for this site. + expect(callsFromTo('callIncompatible', 'method', 'app/Models/ParentModel.php').length).toBe(0); + expect(callsFromTo('callIncompatible', 'method', 'app/Models/ChildModel.php').length).toBe(0); + }); + + it('arity-compatible most-derived override emits exactly one CALLS edge to ChildModel::compat', () => { + // Happy path: ChildModel::compat(int) matches the call site $child->compat(1). + expect(callsFromTo('callCompatible', 'compat', 'app/Models/ChildModel.php').length).toBe(1); + expect(callsFromTo('callCompatible', 'compat', 'app/Models/ParentModel.php').length).toBe(0); + }); + + it('arity-incompatible class with no parent emits zero CALLS edges (regression check)', () => { + // Orphan::method(int,int) called with one arg, no parent class — must remain + // unresolved both before and after the fix. + expect(callsFromTo('callNoParent', 'method', 'app/Models/Orphan.php').length).toBe(0); + }); + + it('arity-compatible most-derived call still resolves to ChildModel::method (happy path)', () => { + // Ensure the fix did not break compatible-arity resolution. + expect(callsFromTo('callMostDerivedHappy', 'method', 'app/Models/ChildModel.php').length).toBe( + 1, + ); + expect(callsFromTo('callMostDerivedHappy', 'method', 'app/Models/ParentModel.php').length).toBe( + 0, + ); + }); +});