GitNexus/gitnexus/test/unit/scope-resolution/method-dispatch-index.test.ts
Gergő Magyar e944f90879
chore(shared): apply Ring 2 SHARED review follow-ups in one diff (#964)
* chore(shared): apply Ring 2 SHARED review follow-ups in one diff

Aggregates all actionable follow-ups from the 9 Ring 2 SHARED PRs
(#949–#963) before proceeding to Ring 2 PKG. No behavior changes;
docstring edits, test refinements, and one structural cleanup.

## #913 (DefIndex / ModuleScopeIndex / QualifiedNameIndex)
  - Rename `freezeIndex` → `wrapIndex` across all three index builders.
    The old name implied `Object.freeze` on the wrapper, which we never
    applied; `wrapIndex` more accurately describes the lightweight
    readonly-interface wrap. Safety surface (frozen bucket arrays,
    frozen miss-empty array, readonly Maps) is unchanged.
  - Document in `buildModuleScopeIndex` JSDoc that callers must
    pre-normalize `filePath` keys (no path-separator canonicalization
    happens here). Prevents silent cross-platform misses.
  - Add an explicit hit-path freeze assertion in
    `qualified-name-index.test.ts` (the existing test covered only the
    miss-path `EMPTY` array).

## #914 (MethodDispatchIndex)
  - Differentiate the C3 and BFS test cases: both tests now use
    distinct MRO orderings so they prove the materializer stores
    whatever order the `computeMro` callback produces (not that C3 and
    BFS yield identical output).
  - Add `implementsOfCalls` counter in the first-write-wins test, and
    document the call-count contract in `MethodDispatchInput.implementsOf`
    JSDoc: `implementsOf` fires **per occurrence** in `input.owners`
    (not per unique owner); `computeMro` fires at most once per unique
    owner. Callers with expensive `implementsOf` implementations should
    pre-dedupe `owners`.

## #916 (resolveTypeRef)
  - Document the deliberate exclusion of `'Type'` from `TYPE_KINDS`
    (verified no extractor in `gitnexus/src/core/ingestion/` emits
    `type: 'Type'` for annotation-relevant symbols).
  - Rename the namespace-origin test from `'resolves ...'` to
    `'returns null for a namespace-origin binding whose def is not a
    type kind'`, matching the failure-case intent.

## #918 (shadow diff + aggregate)
  - Remove the partial re-export `export type { ShadowAgreement, ShadowDiff };`
    from `aggregate.ts` — it omitted `ShadowCallsite` and diverged
    from the top-level barrel. Consumers import all three from the
    `gitnexus-shared` entry point.
  - Fix the invalid `'wildcard'` evidence kind in `diff.test.ts` fixture
    (that kind is not a valid `ResolutionEvidence.kind`). Replaced with
    `'global-name'`, a real kind the test treats identically.

## #912 (ScopeTree / PositionIndex / makeScopeId)
  - Document the touching-boundary semantics on `PositionIndex.atPosition`:
    when siblings share a boundary point, the right (later-start) sibling
    wins per the existing innermost-wins sort contract.
  - Resolve the layer-inversion flagged by review: move `ScopeLookup`
    from `resolve-type-ref.ts` to `types.ts` (its natural home in the
    data-model layer). `scope-tree.ts` now imports `ScopeLookup` from
    `types.js` directly; the old re-export from `resolve-type-ref.ts`
    is removed per repo convention (`feedback_no_reexport`). Barrel
    export moved alongside.

## #917 (ClassRegistry / MethodRegistry / FieldRegistry)
  - Replace the dangling "try a name-match among class-like defs"
    comment in `lookupReceiverType` with explicit prose that callers
    must pre-resolve via `resolveTypeRef` if they want richer semantics.
    No behavior change — the function already returned `undefined` on
    ambiguous/missing qnames.
  - Fix `tieBreakKey.origin` default for pure Step-2 candidates.
    Type-binding-only hits no longer falsely inherit `'local'` from
    `ensureCandidate`'s neutral default; they now demote to `'import'`
    on their first type-binding hit, and only a later Step-1 lexical
    hit can upgrade them back to `'local'`. Keeps the Appendix B
    cascade faithful to the true origin.
  - Document `'global-name'` in `evidence.ts`: currently reserved for
    Ring 3's byName global index; `lookupCore` never emits it today.
    The weight stays live so `composeEvidence` remains exhaustive over
    the origin union.
  - Rename the mislabeled Step-7 test from `'confidence DESC is the
    primary key'` (which actually tested hard-shadow baseline) to
    `'inner scope shadows outer, yielding single result'`, and add a
    separate test that actually exercises multi-candidate confidence
    ordering (local vs wildcard at the same scope).

## Verification
  - `tsc --noEmit` clean (both `gitnexus-shared` and `gitnexus`)
  - `gitnexus-shared` build clean
  - Combined scope-resolution / model / shadow suite: **260/260 pass**
    (+1 from the new multi-candidate ordering test in #917)

## Not addressed (non-actionable)
  - #949 CI "failure with zero failing tests": pre-existing Swift Node 22
    grammar flake unrelated to #910 scope.
  - #950: the two non-blocking findings were already addressed in
    follow-up commit `cbac32ba` (ParsedImport discriminated union +
    `ScopeId | null` on the two hooks).
  - #915: the five in-scope findings were already addressed in
    follow-up commit `54515a7e` (dead code, unused params, multi-hop
    docs, cap-hit test, stats granularity).
  - #915 LanguageProvider.resolveImportTarget signature divergence +
    `findDefById` O(F×D) perf: tracked separately as follow-up issues
    for the Ring 3 migration window.

* chore(shared): address ce:review findings on the follow-up diff

ce:review (interactive) on PR #964 surfaced two P2s and several P3s. This
commit applies all `safe_auto` fixes + both manual tests in-line so the
PR ships with a cleaner review trail.

## P2 fixes

- **Complete `freezeIndex` → `wrapIndex` rename.** The prior commit renamed
  3 of 5 sibling index files; `method-dispatch-index.ts` and
  `position-index.ts` still carried the old name. Now all 5 helpers use
  the consistent `wrapIndex` naming.
  (maintainability + project-standards reviewers both flagged this.)

- **Add regression tests for the `recordTypeBindingHit` origin demotion.**
  The prior commit introduced the `tieBreakKey.origin = 'import'`
  demotion for Step-2-only candidates without a direct test. Added:
    - `registries.test.ts`: two Step-2-only siblings under the same
      interface, asserting deterministic DefId.localeCompare tie-break
      AND the stronger invariant that composeEvidence never emits a
      where-found signal for Step-2-only candidates (no `signals.origin`).
    - `position-index.test.ts`: touching-boundary test proving the
      right-sibling-wins rule documented in the new JSDoc.
  (testing + kieran-typescript + api-contract reviewers all flagged these gaps.)

## P3 fixes

- Fix wrong comment in `recordTypeBindingHit` that claimed Step 1 could
  later upgrade a demoted origin. Step 1 runs BEFORE Step 2 — the actual
  upgrade path is Step 3 (`seedFromOwnerScopedContributor`). Comment now
  describes execution order correctly.

- Fix inaccurate "re-exported there" comment in `index.ts`. `types.ts`
  *defines* ScopeLookup natively; it's not a re-export. Phrasing now
  says "defined in types.ts and exported from the type-export block
  above — not from this module."

- Update stale `scope-tree.ts` file-header prose that still referenced
  `ScopeLookup` as living in #916/resolve-type-ref.ts. Now points to
  `./types.js` with a cross-ref to both #916 and #917 consumers.

- Expand `atPosition` touching-boundary JSDoc to name the mechanism
  (backward scan through start-sorted array) so readers can trace the
  binary-search code to the claim.

- Add breadcrumb to `aggregate.ts` module header pointing future readers
  to `./diff.ts` / the top-level barrel for `ShadowAgreement`,
  `ShadowCallsite`, and `ShadowDiff`.

- Remove unnecessary non-null assertion in `recordTypeBindingHit`. Local
  `const existingMroDepth = ...` lets TS narrow to `number` in the
  else-branch, eliminating the `!` without behavior change.

## Verification

- `tsc --noEmit` clean (both `gitnexus-shared` and `gitnexus`)
- `gitnexus-shared` build clean
- Combined scope-resolution / model / shadow suite: **262/262 pass** (+2
  from the new origin-demotion + touching-boundary regression tests)
2026-04-18 18:40:29 +01:00

235 lines
9.8 KiB
TypeScript

/**
* Unit tests for `buildMethodDispatchIndex` / `MethodDispatchIndex`
* (RFC #909 Ring 2 SHARED #914).
*
* Covers: empty input, single-inheritance chain, diamond inheritance (caller-
* determined MRO order), interface-only dispatch, multiple implementors,
* dedup, first-write-wins, C3 vs BFS strategy parity (both honored verbatim),
* readonly surface + frozen output.
*/
import { describe, it, expect } from 'vitest';
import { buildMethodDispatchIndex, type MethodDispatchInput, type DefId } from 'gitnexus-shared';
// ─── Test helpers ───────────────────────────────────────────────────────────
const input = (
owners: readonly DefId[],
mroByOwner: Record<DefId, readonly DefId[]>,
implementsByOwner: Record<DefId, readonly DefId[]> = {},
): MethodDispatchInput => ({
owners,
computeMro: (owner) => mroByOwner[owner] ?? [],
implementsOf: (owner) => implementsByOwner[owner] ?? [],
});
// ─── Tests ──────────────────────────────────────────────────────────────────
describe('buildMethodDispatchIndex', () => {
describe('empty / degenerate inputs', () => {
it('builds an empty index from no owners', () => {
const idx = buildMethodDispatchIndex(input([], {}));
expect(idx.mroByOwnerDefId.size).toBe(0);
expect(idx.implsByInterfaceDefId.size).toBe(0);
expect(idx.mroFor('anything')).toEqual([]);
expect(idx.implementorsOf('anything')).toEqual([]);
});
it('indexes an owner with no parents and no interfaces', () => {
const idx = buildMethodDispatchIndex(input(['def:A'], { 'def:A': [] }));
expect(idx.mroByOwnerDefId.size).toBe(1);
expect(idx.implsByInterfaceDefId.size).toBe(0);
expect(idx.mroFor('def:A')).toEqual([]);
});
});
describe('MRO materialization (single / multi inheritance)', () => {
it('records a single-inheritance chain verbatim from the callback', () => {
// A extends B extends C
const idx = buildMethodDispatchIndex(
input(['def:A', 'def:B', 'def:C'], {
'def:A': ['def:B', 'def:C'],
'def:B': ['def:C'],
'def:C': [],
}),
);
expect(idx.mroFor('def:A')).toEqual(['def:B', 'def:C']);
expect(idx.mroFor('def:B')).toEqual(['def:C']);
expect(idx.mroFor('def:C')).toEqual([]);
});
it('records a C3 linearization verbatim (Python diamond)', () => {
// D(B, C) where B(A), C(A). Classical C3 keeps A last because the
// merge step defers A until both B and C have been emitted.
// Our index stores MRO excluding self: [B, C, A].
const idx = buildMethodDispatchIndex(
input(['def:D'], { 'def:D': ['def:B', 'def:C', 'def:A'] }),
);
expect(idx.mroFor('def:D')).toEqual(['def:B', 'def:C', 'def:A']);
});
it('records a BFS linearization verbatim (Java-style first-wins)', () => {
// Same class hierarchy as the C3 case, but the BFS walker visits
// A before C via the B→A edge. Expected MRO differs from C3: [B, A, C].
// This test proves the materializer preserves whatever ordering the
// per-language `computeMro` callback produces — NOT that C3 and BFS
// produce identical output.
const idx = buildMethodDispatchIndex(
input(['def:D'], { 'def:D': ['def:B', 'def:A', 'def:C'] }),
);
expect(idx.mroFor('def:D')).toEqual(['def:B', 'def:A', 'def:C']);
});
it('records a Ruby-style kind-aware ancestry verbatim', () => {
// class C prepend P1 prepend P2; include M1 include M2
// ruby-mixin walk order (per callback): [P2, P1, M2, M1]
const idx = buildMethodDispatchIndex(
input(['def:C'], { 'def:C': ['def:P2', 'def:P1', 'def:M2', 'def:M1'] }),
);
expect(idx.mroFor('def:C')).toEqual(['def:P2', 'def:P1', 'def:M2', 'def:M1']);
});
it('records an empty chain for Rust qualified-syntax owners', () => {
// Rust: no auto-MRO; callback returns []
const idx = buildMethodDispatchIndex(input(['def:RustStruct'], { 'def:RustStruct': [] }));
expect(idx.mroFor('def:RustStruct')).toEqual([]);
});
});
describe('implements inversion', () => {
it('inverts a single class → interface mapping', () => {
const idx = buildMethodDispatchIndex(
input(['def:Impl'], { 'def:Impl': [] }, { 'def:Impl': ['def:IFace'] }),
);
expect(idx.implementorsOf('def:IFace')).toEqual(['def:Impl']);
});
it('aggregates multiple classes implementing the same interface', () => {
const idx = buildMethodDispatchIndex(
input(
['def:A', 'def:B', 'def:C'],
{ 'def:A': [], 'def:B': [], 'def:C': [] },
{ 'def:A': ['def:I'], 'def:B': ['def:I'], 'def:C': ['def:J'] },
),
);
expect(idx.implementorsOf('def:I')).toEqual(['def:A', 'def:B']);
expect(idx.implementorsOf('def:J')).toEqual(['def:C']);
});
it('preserves iteration order of owners in each implementors bucket', () => {
const idx = buildMethodDispatchIndex(
input(
['def:Z', 'def:Y', 'def:X'],
{ 'def:Z': [], 'def:Y': [], 'def:X': [] },
{ 'def:Z': ['def:I'], 'def:Y': ['def:I'], 'def:X': ['def:I'] },
),
);
expect(idx.implementorsOf('def:I')).toEqual(['def:Z', 'def:Y', 'def:X']);
});
it('deduplicates repeated (interface, owner) pairs within a single callback call', () => {
// Caller may legally return the same interface twice (e.g., a class that
// both `implements IFace` and inherits from a parent that also does).
const idx = buildMethodDispatchIndex(
input(['def:Impl'], { 'def:Impl': [] }, { 'def:Impl': ['def:I', 'def:I', 'def:I'] }),
);
expect(idx.implementorsOf('def:I')).toEqual(['def:Impl']);
});
it('deduplicates when the same owner is listed in `owners` twice (first-write-wins)', () => {
// First-write-wins parity with sibling indexes; subsequent owner entries
// should not re-invoke `computeMro` for existing MRO, and should not
// create duplicate implementor entries.
//
// NOTE on `implementsOf` call count: the builder calls `implementsOf`
// ONCE PER OCCURRENCE of an owner in `input.owners`, not once per
// unique owner. Duplicate owners therefore re-invoke `implementsOf`;
// the dedup lives at the bucket layer (via `implsSeen`), not the
// callback layer. Callers with expensive `implementsOf` callbacks
// should dedupe `input.owners` upfront. This counter assertion pins
// that contract so a future refactor can't silently collapse the
// second call without updating the docstring.
let mroCalls = 0;
let implementsOfCalls = 0;
const impls: Record<DefId, readonly DefId[]> = { 'def:A': ['def:I'] };
const idx = buildMethodDispatchIndex({
owners: ['def:A', 'def:A'],
computeMro: (_) => {
mroCalls++;
return ['def:B'];
},
implementsOf: (o) => {
implementsOfCalls++;
return impls[o] ?? [];
},
});
expect(mroCalls).toBe(1); // MRO dedup is at the callback layer (first-write-wins)
expect(implementsOfCalls).toBe(2); // implementsOf fires per occurrence; dedup at bucket
expect(idx.mroFor('def:A')).toEqual(['def:B']);
expect(idx.implementorsOf('def:I')).toEqual(['def:A']);
});
});
describe('lookup miss / safety surface', () => {
it('returns a frozen empty array on MRO miss', () => {
const idx = buildMethodDispatchIndex(input(['def:A'], { 'def:A': [] }));
const miss = idx.mroFor('def:Missing');
expect(miss).toEqual([]);
expect(() => (miss as unknown as DefId[]).push('x')).toThrow();
});
it('returns a frozen empty array on implementors miss', () => {
const idx = buildMethodDispatchIndex(input(['def:A'], { 'def:A': [] }));
const miss = idx.implementorsOf('def:Missing');
expect(miss).toEqual([]);
expect(() => (miss as unknown as DefId[]).push('x')).toThrow();
});
it('freezes stored MRO arrays (readonly surface)', () => {
const idx = buildMethodDispatchIndex(input(['def:A'], { 'def:A': ['def:B'] }));
const chain = idx.mroFor('def:A');
expect(() => (chain as unknown as DefId[]).push('x')).toThrow();
});
it('freezes stored implementors arrays (readonly surface)', () => {
const idx = buildMethodDispatchIndex(
input(['def:A'], { 'def:A': [] }, { 'def:A': ['def:I'] }),
);
const impls = idx.implementorsOf('def:I');
expect(() => (impls as unknown as DefId[]).push('x')).toThrow();
});
it('isolates stored MRO from later mutation of the callback-returned array', () => {
const mutable = ['def:B', 'def:C'];
const idx = buildMethodDispatchIndex({
owners: ['def:A'],
computeMro: () => mutable,
implementsOf: () => [],
});
mutable.push('def:D');
expect(idx.mroFor('def:A')).toEqual(['def:B', 'def:C']);
});
});
describe('readonly surface', () => {
it('exposes `mroByOwnerDefId` as a read-only Map for direct iteration', () => {
const idx = buildMethodDispatchIndex(
input(['def:A', 'def:B'], { 'def:A': [], 'def:B': ['def:A'] }),
);
const owners = Array.from(idx.mroByOwnerDefId.keys()).sort();
expect(owners).toEqual(['def:A', 'def:B']);
});
it('exposes `implsByInterfaceDefId` as a read-only Map for direct iteration', () => {
const idx = buildMethodDispatchIndex(
input(
['def:A', 'def:B'],
{ 'def:A': [], 'def:B': [] },
{ 'def:A': ['def:I'], 'def:B': ['def:J'] },
),
);
const keys = Array.from(idx.implsByInterfaceDefId.keys()).sort();
expect(keys).toEqual(['def:I', 'def:J']);
});
});
});