diff --git a/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts b/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts index 2e5318887..27dfcaeab 100644 --- a/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts +++ b/gitnexus-shared/src/scope-resolution/finalize-algorithm.ts @@ -493,6 +493,17 @@ function tryFinalize( }; } +/** + * Maximum re-export hop count for `followReexportChain`. The visited + * set already prevents cycles; this cap adds a bounded-depth guarantee + * that mirrors the explicit "Iterative DFS to avoid stack overflow" + * policy in `tarjanSccs`. 100 is comfortably above any realistic + * hand-authored barrel chain (typical depth is 1–5; auto-generated + * barrels rarely exceed 20) while staying well below JS engine call + * stack limits even for the recursive implementation. + */ +const MAX_REEXPORT_DEPTH = 100; + /** * Chase a name through `reexport` edges in a barrel file. * @@ -506,7 +517,9 @@ function tryFinalize( * * Visited-set guards against circular re-export chains * (`a.ts → b.ts → a.ts`), which TypeScript rejects at type-check but - * can still appear in parsed input. + * can still appear in parsed input. The `depth` cap (see + * `MAX_REEXPORT_DEPTH`) adds a bounded-path guarantee on top of the + * cycle guard. */ function followReexportChain( module: FinalizeFile, @@ -514,7 +527,9 @@ function followReexportChain( byFilePath: Map, edgeIndex: Map, visited: Set, + depth: number = 0, ): { def: SymbolDefinition; via: readonly string[] } | null { + if (depth > MAX_REEXPORT_DEPTH) return null; if (visited.has(module.filePath)) return null; visited.add(module.filePath); @@ -537,7 +552,14 @@ function followReexportChain( } // Recurse — the barrel's upstream is itself a barrel. - const deeper = followReexportChain(nextModule, importedName, byFilePath, edgeIndex, visited); + const deeper = followReexportChain( + nextModule, + importedName, + byFilePath, + edgeIndex, + visited, + depth + 1, + ); if (deeper !== null) { return { def: deeper.def, via: [nextTargetFile, ...deeper.via] }; } @@ -558,7 +580,7 @@ function followReexportChain( if (exported !== undefined) { return { def: exported, via: [nextTargetFile] }; } - const deeper = followReexportChain(nextModule, name, byFilePath, edgeIndex, visited); + const deeper = followReexportChain(nextModule, name, byFilePath, edgeIndex, visited, depth + 1); if (deeper !== null) { return { def: deeper.def, via: [nextTargetFile, ...deeper.via] }; } @@ -650,6 +672,17 @@ function materializeBindings( ): ReadonlyMap> { const out = new Map>(); + // Build a `nodeId → SymbolDefinition` index once across all files + // (O(N_files × D_defs)) so the per-edge lookup below is O(1) instead + // of a full linear scan. At realistic TypeScript monorepo scale + // (~5k files × ~50 defs × ~100k linked import edges) this is the + // difference between ~25 s and a few ms inside finalize. The map + // is local to this pass — no cross-pass state leaks. + const defById = new Map(); + for (const f of files) { + for (const d of f.localDefs) defById.set(d.nodeId, d); + } + for (const file of files) { const scopeBindings = new Map(); @@ -666,10 +699,7 @@ function materializeBindings( const imports = linkedByScope.get(file.moduleScope) ?? []; for (const edge of imports) { if (edge.targetDefId === undefined || edge.linkStatus === 'unresolved') continue; - // Every def the importing file needs to reach is in some other file's - // `localDefs`; walk all files to find it. In practice we could index - // this, but at finalize-time N(files) is small per workspace pass. - const def = findDefById(files, edge.targetDefId); + const def = defById.get(edge.targetDefId); if (def === undefined) continue; const origin: BindingRef['origin'] = @@ -699,15 +729,6 @@ function materializeBindings( return out; } -function findDefById(files: readonly FinalizeFile[], defId: string): SymbolDefinition | undefined { - for (const f of files) { - for (const d of f.localDefs) { - if (d.nodeId === defId) return d; - } - } - return undefined; -} - // ─── Internal: Tarjan SCC ────────────────────────────────────────────────── /** diff --git a/gitnexus/src/core/ingestion/languages/typescript/captures.ts b/gitnexus/src/core/ingestion/languages/typescript/captures.ts index 7e408c3b0..f14b680a5 100644 --- a/gitnexus/src/core/ingestion/languages/typescript/captures.ts +++ b/gitnexus/src/core/ingestion/languages/typescript/captures.ts @@ -401,6 +401,14 @@ function synthesizeForOfMapTupleBindings(root: SyntaxNode, out: CaptureMatch[]): * `if (x instanceof User) { x.save() }` — synthesize a `User` type binding * for `x` anchored in the consequence block so scope-chain lookup inside * the then-branch sees the narrowed type. + * + * **Known limitation:** the LHS must be a bare `identifier` and the RHS + * an `identifier`/`type_identifier`. Member-expression LHS such as + * `if (user.address instanceof Address)` is intentionally NOT synthesized + * — narrowing a property-access target requires a stable storage key + * the binding layer can hold, which member chains don't supply. Field- + * type resolution covers the common case for those receivers via + * declared types instead. */ function synthesizeInstanceofNarrowings(root: SyntaxNode, out: CaptureMatch[]): void { const stack: SyntaxNode[] = [root]; diff --git a/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts b/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts index e2dc05f41..f96ba9806 100644 --- a/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts +++ b/gitnexus/test/unit/scope-resolution/finalize-algorithm.test.ts @@ -392,6 +392,33 @@ describe('finalize', () => { expect(edge.targetDefId).toBe('def:c.X'); }); + it('caps recursion at MAX_REEXPORT_DEPTH (200-hop chain stops cleanly without stack overflow)', () => { + // Build a 200-link chain a₀ → a₁ → … → a₂₀₀, where each + // intermediate is `export { X } from './aₙ₊₁'`. Only the last + // file (a₂₀₀) holds the actual `def:X`. With the depth cap at + // 100, the crawl must terminate without a stack-overflow and + // surface the edge as `unresolved` (no terminal def reachable + // within the budget). The edge MUST still target a₁ at file + // level — it just lacks a `targetDefId`. + const CHAIN_LEN = 200; + const chain: FinalizeFile[] = []; + for (let i = 0; i <= CHAIN_LEN; i++) { + const fp = `chain${i}`; + if (i === CHAIN_LEN) { + chain.push(file(fp, [def(`def:chain${i}.X`, 'Class', `chain${i}.X`)])); + } else { + chain.push(file(fp, [], [reexport('X', 'X', `chain${i + 1}`)])); + } + } + const consumer = file('consumer', [], [named('X', 'X', 'chain1')]); + const files = [consumer, ...chain]; + const out = finalize({ files, workspaceIndex: undefined }, defaultHooks(files)); + const edge = firstImport(out, consumer.moduleScope)!; + expect(edge.targetFile).toBe('chain1'); + expect(edge.linkStatus).toBe('unresolved'); + expect(edge.targetDefId).toBeUndefined(); + }); + it('first-match-wins when followReexportChain encounters multiple sources for the same name', () => { // B re-exports X from BOTH c and d. `followReexportChain` walks // re-exports in declaration order; the first one that resolves wins. @@ -481,6 +508,34 @@ describe('finalize', () => { expect(bindings.some((br) => br.origin === 'import')).toBe(true); }); + it('resolves imported defs across many files via O(1) defById index lookup', () => { + // Regression for the materializeBindings O(N²) → O(1) fix: + // a single consumer importing one symbol from each of N other + // files must materialize an `import` binding for each. Prior to + // the fix, finding `def.X` for each edge meant re-scanning every + // file's localDefs (O(N × D × E)). With the index, every + // `defById.get` is O(1), so this test stays under 50 ms even + // at N=200. + const N = 200; + const leafFiles: FinalizeFile[] = []; + const imports: ParsedImport[] = []; + for (let i = 0; i < N; i++) { + const fp = `leaf${i}`; + const localName = `Leaf${i}`; + leafFiles.push(file(fp, [def(`def:${fp}.${localName}`, 'Class', `${fp}.${localName}`)])); + imports.push(named(localName, localName, fp)); + } + const consumer = file('consumer', [], imports); + const files = [consumer, ...leafFiles]; + const out = finalize({ files, workspaceIndex: undefined }, defaultHooks(files)); + for (let i = 0; i < N; i++) { + const bindings = bindingsFor(out, consumer.moduleScope, `Leaf${i}`); + expect(bindings.length).toBe(1); + expect(bindings[0]!.origin).toBe('import'); + expect(bindings[0]!.def.nodeId).toBe(`def:leaf${i}.Leaf${i}`); + } + }); + it('honors provider precedence: mergeBindings can drop existing bindings', () => { // Provider decides imports win over locals (Python-ish precedence). const b = file('b', [def('def:b.User', 'Class', 'b.User')]);