mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-02 02:11:29 +00:00
fix(php): PSR-4-compliant fixture + pre-existing test debt cleanup
Four tests were failing on lang/php-migration-v2 in the tests/ubuntu/coverage
job before this commit. Root causes and fixes:
1. cross-file-binding.test.ts: Consumer-Before-Provider PHP
The fixture had BProvider.php at app/BProvider.php while declaring
namespace App\Models — a PSR-4 violation that caused PHP's import-
target resolver to return null for 'use function App\Models\getUser'.
Without a resolved import target, no IMPORTS edge was emitted, the
SCC graph had no edge between consumer and provider, and the shared
propagateImportedReturnTypes pass never mirrored getUser's return
type into AConsumer's scope. Result: $u (bound to 'getUser' by
query.ts:167-169 type-binding.alias) never collapsed to User, so
$u->save() went unresolved.
Move BProvider.php to its PSR-4-compliant location at
app/Models/User.php. The class+function were already in namespace
App\Models; the rename just aligns the file path with the
namespace prefix mapped by composer.json. After the move:
- PSR-4 directory scan finds the file for 'use function
App\Models\getUser' (lines 65-86 of php import-resolver).
- IMPORTS edge AConsumer.php → app/Models/User.php emits.
- SCC reverse-topological walk in propagateImportedReturnTypes
mirrors getUser→User from User.php's module-scope typeBindings
into AConsumer's, then chain-follows $u → getUser → User.
- $u->save() resolves to User#save.
2. registry-primary-flag.test.ts: 'flipped languages' size === 1
When PHP was added to MIGRATED_LANGUAGES at commit 69786b16 (the
PHP scope-based resolution migration), the hard-coded opt-out list
in this test did not include REGISTRY_PRIMARY_PHP. With PHP
default-on and not opted out, enabled.size returned 2 (Java + PHP)
instead of 1. Rewrite the opt-out loop to iterate
MIGRATED_LANGUAGES dynamically so future Ring 3 additions land
without test churn.
3. overload-narrowing.test.ts: 'falls back to the full overload list'
Commit af9af4a9 (PR #1497 production review fixes) deliberately
tightened narrowOverloadCandidates to stop silently rescuing
empty filter sets when every candidate had definite bounds — the
call is genuinely arity-incompatible (e.g. PHP variadic with
required-prefix called with too few args). The unit test still
asserted the old soft-rescue behavior. Rename and update the
assertion to match the intentional new semantics; document that
the 'anyUnknownBounds' branch in the source is structurally
unreachable from this caller shape (an unknown-bounds candidate
always passes the arity filter, so arityMatches is always
non-empty when anyUnknownBounds is true).
4. registries.test.ts: 'keeps incompatible candidates (soft penalty)'
Same root cause as #3 at the registries layer: lookup-core.ts now
drops every candidate when all are 'incompatible' and none are
'unknown'. The 'soft penalty' kept-with-evidence behavior survives
only when at least one candidate's arity verdict is 'unknown' —
that signal differentiates definite mismatch from missing metadata.
Update the existing test to assert the new hard-rejection behavior
and add a new test exercising the surviving soft-penalty path with
one 'unknown' + one 'incompatible' candidate.
Verification:
- Originally-failing tests: 4 of 4 now pass (no skips).
- PHP integration suite: 205/205 in primary mode, 197+8 skipped in
legacy mode (unchanged).
- Cross-language unit + integration: no regressions.
This commit is contained in:
parent
fb093f7f07
commit
fcb0e30dd1
4 changed files with 68 additions and 11 deletions
|
|
@ -148,17 +148,20 @@ describe('primaryLanguages', () => {
|
|||
|
||||
it('returns exactly the flipped languages (env opts in unmigrated, opts out migrated)', () => {
|
||||
// Migrated languages are default-on; each must be opted out here when
|
||||
// testing explicit env overrides. Java (unmigrated) opts in; Go stays off.
|
||||
process.env['REGISTRY_PRIMARY_PYTHON'] = 'false';
|
||||
process.env['REGISTRY_PRIMARY_CSHARP'] = 'false';
|
||||
process.env['REGISTRY_PRIMARY_TYPESCRIPT'] = 'false';
|
||||
process.env['REGISTRY_PRIMARY_GO'] = 'false';
|
||||
process.env['REGISTRY_PRIMARY_C'] = 'false';
|
||||
// testing explicit env overrides. Java (unmigrated) opts in.
|
||||
// Opt out every member of MIGRATED_LANGUAGES dynamically so this test
|
||||
// does not have to be updated each time a new language ships its
|
||||
// Ring 3 migration (PHP joined the set in commit 69786b16; future
|
||||
// Ring 3 additions land here without test churn).
|
||||
for (const lang of MIGRATED_LANGUAGES) {
|
||||
process.env[envVarNameFor(lang)] = 'false';
|
||||
}
|
||||
process.env['REGISTRY_PRIMARY_JAVA'] = '1';
|
||||
const enabled = primaryLanguages();
|
||||
expect(enabled.has(SupportedLanguages.Python)).toBe(false);
|
||||
expect(enabled.has(SupportedLanguages.CSharp)).toBe(false);
|
||||
expect(enabled.has(SupportedLanguages.Go)).toBe(false);
|
||||
expect(enabled.has(SupportedLanguages.PHP)).toBe(false);
|
||||
expect(enabled.has(SupportedLanguages.Java)).toBe(true);
|
||||
// Only Java is on: migrated defaults overridden off, Java explicitly on.
|
||||
expect(enabled.size).toBe(1);
|
||||
|
|
|
|||
|
|
@ -69,11 +69,26 @@ describe('narrowOverloadCandidates — arity filtering', () => {
|
|||
expect(result.map((d) => d.nodeId)).toEqual(['v:1']);
|
||||
});
|
||||
|
||||
it('falls back to the full overload list when arity filter empties it', () => {
|
||||
it('returns empty when arity filter empties the set AND every candidate had definite bounds', () => {
|
||||
// argCount=5 doesn't match any overload (none variadic, all have max < 5).
|
||||
// Post-commit af9af4a9 (PR #1497 / U1): the empty result is now authoritative
|
||||
// because every rejected candidate had defined `parameterCount` /
|
||||
// `requiredParameterCount`. The old "always fall back to full list" rescue
|
||||
// was deliberately removed so resolvers actually drop calls that are
|
||||
// definitively arity-incompatible (e.g., PHP `f(int $req, ...$rest)`
|
||||
// called with zero args).
|
||||
const result = narrowOverloadCandidates([add1, add2, add3], 5, undefined);
|
||||
expect(result.map((d) => d.nodeId)).toEqual(['add:1', 'add:2', 'add:3']);
|
||||
expect(result.map((d) => d.nodeId)).toEqual([]);
|
||||
});
|
||||
|
||||
// Note: the `anyUnknownBounds ? overloads : []` branch in
|
||||
// narrowOverloadCandidates is structurally unreachable in this caller's
|
||||
// shape — a candidate with both `parameterCount` and `requiredParameterCount`
|
||||
// undefined always passes the arity filter (neither `argCount > max` nor
|
||||
// `argCount < min` can fire), so `arityMatches.length` is always > 0
|
||||
// whenever `anyUnknownBounds` is true. The branch is preserved in the
|
||||
// source as a defensive guard for future refactors that might add
|
||||
// additional rejection criteria in the filter.
|
||||
});
|
||||
|
||||
describe('narrowOverloadCandidates — type narrowing', () => {
|
||||
|
|
|
|||
|
|
@ -250,7 +250,13 @@ describe('Step 5: arity filter', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('keeps incompatible candidates when no compatible candidate exists (soft penalty)', () => {
|
||||
it('drops every candidate when ALL are incompatible AND none unknown (hard rejection)', () => {
|
||||
// Post-commit af9af4a9 (PR #1497 / U1): the old soft-penalty fallback
|
||||
// that kept incompatible candidates with `arityMatchIncompatible`
|
||||
// weight was deliberately removed at this layer too. When every
|
||||
// candidate is definitively arity-incompatible, the registry returns
|
||||
// no resolution — matching the PHP variadic case `f(int $req, ...$rest)`
|
||||
// called with zero args.
|
||||
const save3 = mkDef({
|
||||
nodeId: 'def:save-three',
|
||||
type: 'Method',
|
||||
|
|
@ -268,8 +274,41 @@ describe('Step 5: arity filter', () => {
|
|||
const results = buildMethodRegistry(ctx).lookup('save', 'scope:m', {
|
||||
callsite: { arity: 1 },
|
||||
});
|
||||
expect(results).toHaveLength(1);
|
||||
expect(evidenceOfKind(results[0]!, 'arity-match')?.weight).toBe(
|
||||
expect(results).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('keeps incompatible candidates when at least one verdict is unknown (soft penalty)', () => {
|
||||
// The soft-rescue path is still active when at least one candidate's
|
||||
// arity verdict is 'unknown' — that signals missing metadata rather
|
||||
// than a definitive mismatch, so all candidates (including incompatible
|
||||
// ones) are preserved with their evidence weights for downstream
|
||||
// tie-breaking.
|
||||
const save3 = mkDef({
|
||||
nodeId: 'def:save-three',
|
||||
type: 'Method',
|
||||
qualifiedName: 'User.save',
|
||||
parameterCount: 3,
|
||||
});
|
||||
const saveUnknown = mkDef({
|
||||
nodeId: 'def:save-unknown',
|
||||
type: 'Method',
|
||||
qualifiedName: 'User.save',
|
||||
});
|
||||
const mod = mkScope({
|
||||
id: 'scope:m',
|
||||
parent: null,
|
||||
bindings: { save: [mkBinding(save3, 'local'), mkBinding(saveUnknown, 'local')] },
|
||||
});
|
||||
const ctx = makeCtx([mod], [save3, saveUnknown], {
|
||||
arity: (_callsite, def) => (def.nodeId === 'def:save-unknown' ? 'unknown' : 'incompatible'),
|
||||
});
|
||||
const results = buildMethodRegistry(ctx).lookup('save', 'scope:m', {
|
||||
callsite: { arity: 1 },
|
||||
});
|
||||
expect(results).toHaveLength(2);
|
||||
const incompat = results.find((r) => r.def.nodeId === 'def:save-three');
|
||||
expect(incompat).toBeDefined();
|
||||
expect(evidenceOfKind(incompat!, 'arity-match')?.weight).toBe(
|
||||
EvidenceWeights.arityMatchIncompatible,
|
||||
);
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue