mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-03 02:21:44 +00:00
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
This commit is contained in:
parent
55b8790f75
commit
29e45f1971
7 changed files with 140 additions and 5 deletions
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
10
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ChildModel.php
vendored
Normal file
10
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ChildModel.php
vendored
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
<?php
|
||||
|
||||
namespace App\Models;
|
||||
|
||||
class ChildModel extends ParentModel
|
||||
{
|
||||
public function method(int $a, int $b): bool { return true; }
|
||||
|
||||
public function compat(int $a): bool { return true; }
|
||||
}
|
||||
8
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/Orphan.php
vendored
Normal file
8
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/Orphan.php
vendored
Normal file
|
|
@ -0,0 +1,8 @@
|
|||
<?php
|
||||
|
||||
namespace App\Models;
|
||||
|
||||
class Orphan
|
||||
{
|
||||
public function method(int $a, int $b): bool { return true; }
|
||||
}
|
||||
10
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ParentModel.php
vendored
Normal file
10
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Models/ParentModel.php
vendored
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
<?php
|
||||
|
||||
namespace App\Models;
|
||||
|
||||
class ParentModel
|
||||
{
|
||||
public function method(int $a): bool { return true; }
|
||||
|
||||
public function compat(int $a): bool { return true; }
|
||||
}
|
||||
42
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Services/Caller.php
vendored
Normal file
42
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/app/Services/Caller.php
vendored
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
<?php
|
||||
|
||||
namespace App\Services;
|
||||
|
||||
use App\Models\ChildModel;
|
||||
use App\Models\Orphan;
|
||||
|
||||
class Caller
|
||||
{
|
||||
public function callIncompatible(): void
|
||||
{
|
||||
// ChildModel::method takes 2 args; ParentModel::method takes 1.
|
||||
// Class-name receiver -> 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);
|
||||
}
|
||||
}
|
||||
5
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/composer.json
vendored
Normal file
5
gitnexus/test/fixtures/lang-resolution/php-mro-arity-mismatch/composer.json
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"autoload": {
|
||||
"psr-4": { "App\\": "app/" }
|
||||
}
|
||||
}
|
||||
|
|
@ -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,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue