From 60f2cc821bbdbba6e249961bf0194eea89b040bc Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Wed, 28 Oct 2026 12:09:10 +0300 Subject: [PATCH] fix(sync): a barrel whose re-export points elsewhere re-links the files importing through it `c.ts` imports `foo` through `index.ts`, which re-exports it from `a.ts`. When the barrel's clause is changed to `b.ts`, the barrel declares nothing of `foo` and `c.ts` did not change, so no definition delta, added file or failed reference named `c.ts`'s binding: it stayed on `a.ts` until a full re-index, and an `export *` moved the same way, in a full or a scoped sync (#2978). An import node is named by the module it names, so the orchestrator samples the `file\0module` pairs of a changed file's import nodes before the store replaces them, as it samples the definition pairs, and reports the ES modules whose set of imported modules changed when they pass bindings on now or lost an import: `reExportsChangedFilePaths`. For those, the sync re-opens what their importers bind through them, up through the modules that pass them on in turn, with the failures parked under them, and the edges into the members of what the bindings pointed at, which a value typed by a binding reached by their own names (reopenReferencesThroughRetargetedModules, built on the importer walk the removed-files pass uses). It runs after the name-matched-import pass, which reads the bindings a name match took before they go back. The sweep binds them against the barrel as it is now. A module given re-exports is read the same way; one whose body alone changed reports nothing. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 1 + __tests__/sync-barrel-retarget.test.ts | 176 +++++++++++++++++++++++++ src/db/queries.ts | 20 +++ src/extraction/index.ts | 37 ++++++ src/index.ts | 19 ++- src/resolution/module-importers.ts | 37 ++++++ 6 files changed, 289 insertions(+), 1 deletion(-) create mode 100644 __tests__/sync-barrel-retarget.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f58218b4c7..c453367019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- In TypeScript and JavaScript, a sync now follows a barrel whose re-export points at another module (`export { foo } from './a'` changed to `'./b'`, an `export *` moved the same way) or drops one: the files importing through the barrel are linked again, where they kept the old module until a full re-index. (#2978) - In TypeScript, an awaited call with type arguments like `await fetchApi("/me")` or `await api.get(url)` now links to the function it calls. Before, the call was dropped, so typed fetch wrappers looked like they had no callers. (#2586) - In JavaScript and TypeScript, a call or `new` in a parameter's default value, like `function renderWithNote(note = buildNote())`, a destructured `{ size = defaultSize() }` or `constructor(private svc = new Service())`, now counts as the function's own call. Before, it was dropped, so `buildNote` was missing that caller and impact on it came back incomplete. Re-index JavaScript and TypeScript projects after upgrading. Thanks @inth3shadows for the report and the pull request. (#2588) - In React, a component rendered at module scope, in a top-level constant or inside a test's `it(…)` callback, like `createRoot(el).render()` or `render()`, now counts as rendered there, so an app's entry reaches its root component and a component's tests show among its callers. (#2587) diff --git a/__tests__/sync-barrel-retarget.test.ts b/__tests__/sync-barrel-retarget.test.ts new file mode 100644 index 0000000000..52631f848e --- /dev/null +++ b/__tests__/sync-barrel-retarget.test.ts @@ -0,0 +1,176 @@ +/** + * A sync follows a barrel whose re-export points elsewhere (#2978). + * + * `c.ts` imports `foo` through `index.ts`, which re-exports it from `a.ts`. + * When the barrel's clause is changed to `b.ts`, the barrel declares nothing + * of `foo` and `c.ts` did not change, so no definition delta, added file or + * failed reference named `c.ts`'s binding, and it stayed on `a.ts` until a + * full re-index. The importers of a module whose re-exports changed now have + * their bindings through it re-resolved. + * + * Each case indexes the first tree, edits it, syncs, and compares edges and + * unresolved references with a full index of the final tree in a folder of + * its own. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { DatabaseConnection, getDatabasePath } from '../src/db'; + +type Files = Record; + +let roots: string[] = []; +let graphs: CodeGraph[] = []; + +afterEach(() => { + for (const graph of graphs) graph.close(); + graphs = []; + for (const root of roots) fs.rmSync(root, { recursive: true, force: true }); + roots = []; +}); + +function tempRoot(): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-barrel-retarget-')); + roots.push(root); + return root; +} + +function write(root: string, files: Files): void { + for (const [file, text] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true }); + fs.writeFileSync(path.join(root, file), text); + } +} + +async function index(root: string): Promise { + const graph = await CodeGraph.init(root, { index: true }); + graphs.push(graph); + return graph; +} + +/** Every edge by its ends' natural keys, and every reference left unresolved. */ +function stateOf(root: string, graph: CodeGraph): string[] { + const keys = new Map( + graph.getFiles() + .flatMap((file) => graph.getNodesInFile(file.path)) + .map((n) => [n.id, `${n.kind} ${n.filePath}:${n.qualifiedName}:${n.startLine}`] as const) + ); + const edges = graph.getOutgoingEdgesFrom([...keys.keys()]) + .map((e) => `${e.kind} ${keys.get(e.source)} -> ${keys.get(e.target) ?? e.target} @${e.line}:${e.column} ${JSON.stringify(e.metadata ?? {})}`); + const db = DatabaseConnection.open(getDatabasePath(root)); + try { + const refs = (db.getDb() + .prepare('SELECT reference_name, reference_kind, file_path, line, col, status, name_tail FROM unresolved_refs') + .all() as Array>) + .map((r) => `ref ${r.file_path}:${r.line}:${r.col} ${r.reference_kind} ${r.reference_name} ${r.status} ${r.name_tail}`); + return [...edges, ...refs].sort(); + } finally { + db.close(); + } +} + +/** Where `bar`'s call to `foo` lands: `::`, or nothing. */ +function fooCalledBy(graph: CodeGraph): string[] { + const bar = graph.getNodesByQualifiedName('bar')[0]!; + return graph.getOutgoingEdges(bar.id) + .filter((e) => e.kind === 'calls') + .map((e) => `${graph.getNode(e.target)!.filePath}::${graph.getNode(e.target)!.qualifiedName}`) + .sort(); +} + +/** Index `before`, write `after` over it, sync (scoped to `paths` when given); return the graph and a full index of the final tree. */ +async function syncEdit(before: Files, after: Files, paths?: string[]): Promise<{ root: string; graph: CodeGraph; full: string[] }> { + const root = tempRoot(); + write(root, before); + const graph = await index(root); + expect(fooCalledBy(graph)).toEqual(['src/a.ts::foo']); + write(root, after); + await graph.sync(paths ? { paths } : {}); + const fullRoot = tempRoot(); + write(fullRoot, { ...before, ...after }); + return { root, graph, full: stateOf(fullRoot, await index(fullRoot)) }; +} + +const BEFORE: Files = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/a.ts': 'export function foo() { return 1; }\n', + 'src/b.ts': 'export function foo() { return 2; }\n', + 'src/index.ts': "export { foo } from './a';\n", + 'src/c.ts': "import { foo } from './index';\nexport function bar() { return foo(); }\n", +}; + +describe('a sync follows a barrel whose re-export points elsewhere (#2978)', () => { + it('a named re-export retargeted to another module', async () => { + const { root, graph, full } = await syncEdit(BEFORE, { 'src/index.ts': "export { foo } from './b';\n" }); + expect(fooCalledBy(graph)).toEqual(['src/b.ts::foo']); + expect(stateOf(root, graph)).toEqual(full); + expect(graph.getPendingReferenceCount()).toBe(0); + }, 120_000); + + it('an `export *` retargeted to another module', async () => { + const before = { ...BEFORE, 'src/index.ts': "export * from './a';\n" }; + const { root, graph, full } = await syncEdit(before, { 'src/index.ts': "export * from './b';\n" }); + expect(fooCalledBy(graph)).toEqual(['src/b.ts::foo']); + expect(stateOf(root, graph)).toEqual(full); + }, 120_000); + + it('a scoped sync of the barrel alone', async () => { + const { root, graph, full } = await syncEdit(BEFORE, { 'src/index.ts': "export { foo } from './b';\n" }, ['src/index.ts']); + expect(fooCalledBy(graph)).toEqual(['src/b.ts::foo']); + expect(stateOf(root, graph)).toEqual(full); + }, 120_000); + + it('a re-export taken away leaves the importer where a full index leaves it', async () => { + // The import fails through the barrel now; whatever a full index then + // links the call to by name, the sync links it to the same way. + const { root, graph, full } = await syncEdit(BEFORE, { 'src/index.ts': 'export const version = 1;\n' }); + expect(stateOf(root, graph)).toEqual(full); + }, 120_000); + + it('through a second barrel that passes the first one on', async () => { + const before = { + ...BEFORE, + 'src/lib.ts': "export * from './index';\n", + 'src/c.ts': "import { foo } from './lib';\nexport function bar() { return foo(); }\n", + }; + const { root, graph, full } = await syncEdit(before, { 'src/index.ts': "export { foo } from './b';\n" }); + expect(fooCalledBy(graph)).toEqual(['src/b.ts::foo']); + expect(stateOf(root, graph)).toEqual(full); + }, 120_000); + + it('a value typed by the binding reaches the members of the new target', async () => { + const before: Files = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/a.ts': 'export class Widget { run(): number { return 1; } }\n', + 'src/b.ts': 'export class Widget { run(): number { return 2; } }\n', + 'src/index.ts': "export { Widget } from './a';\n", + 'src/c.ts': "import { Widget } from './index';\nexport function make(): number { const w = new Widget(); return w.run(); }\n", + }; + const root = tempRoot(); + write(root, before); + const graph = await index(root); + const runTargets = (g: CodeGraph) => g.getOutgoingEdges(g.getNodesByQualifiedName('make')[0]!.id) + .filter((e) => e.kind === 'calls' && e.metadata?.refName === 'w.run') + .map((e) => `${g.getNode(e.target)!.filePath}::${g.getNode(e.target)!.qualifiedName}`); + expect(runTargets(graph)).toEqual(['src/a.ts::Widget::run']); + write(root, { 'src/index.ts': "export { Widget } from './b';\n" }); + await graph.sync(); + expect(runTargets(graph)).toEqual(['src/b.ts::Widget::run']); + const fullRoot = tempRoot(); + write(fullRoot, { ...before, 'src/index.ts': "export { Widget } from './b';\n" }); + expect(stateOf(root, graph)).toEqual(stateOf(fullRoot, await index(fullRoot))); + }, 120_000); + + it('a body-only edit of the barrel re-resolves nothing', async () => { + const root = tempRoot(); + write(root, { ...BEFORE, 'src/index.ts': "export { foo } from './a';\nexport const version = 1;\n" }); + const graph = await index(root); + const before = stateOf(root, graph); + write(root, { 'src/index.ts': "export { foo } from './a';\nexport const version = 2;\n" }); + const result = await graph.sync(); + expect(result.reExportsChangedFilePaths).toBeUndefined(); + expect(stateOf(root, graph)).toEqual(before); + }, 120_000); +}); diff --git a/src/db/queries.ts b/src/db/queries.ts index 4675e3b65e..81849a07e4 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -5373,6 +5373,26 @@ export class QueryBuilder { * repo-wide may now bind elsewhere. Keying by file makes each definition its * own fact, so the move is seen as one removal plus one addition. */ + /** + * The `file\0name` pairs of the import nodes in `filePaths`: an import node + * is named by the module specifier it names, so these are the modules each + * file imports or re-exports from (#2978). Same shape as + * {@link getNodeNamePairsByFiles}. + */ + getImportNamePairsByFiles(filePaths: string[]): Set { + const pairs = new Set(); + if (filePaths.length === 0) return pairs; + for (let i = 0; i < filePaths.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = filePaths.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const rows = this.db + .prepare(`SELECT DISTINCT file_path, name FROM nodes WHERE kind = 'import' AND file_path IN (${placeholders})`) + .all(...chunk) as Array<{ file_path: string; name: string }>; + for (const row of rows) pairs.add(`${row.file_path}\0${row.name}`); + } + return pairs; + } + getNodeNamePairsByFiles(filePaths: string[]): Set { const pairs = new Set(); if (filePaths.length === 0) return pairs; diff --git a/src/extraction/index.ts b/src/extraction/index.ts index 78b8bf2142..3a2fdfe169 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -127,6 +127,9 @@ export interface IndexResult { /** * Result of a sync operation */ +/** The languages whose modules pass bindings on with `export … from` (#2978). */ +const RE_EXPORTING_LANGUAGES = new Set(['typescript', 'tsx', 'javascript', 'jsx']); + export interface SyncResult { /** References attempted by the pending-reference recovery sweep, if run. */ pendingRefsProcessed?: number; @@ -164,6 +167,15 @@ export interface SyncResult { * nothing downstream. */ definitionDelta?: string[]; + /** + * Modified ES modules whose set of imported modules changed and that pass + * bindings on, or stopped passing some on: a re-export that points at + * another module now (`export { foo } from './a'` → `'./b'`), one taken + * away, or a module given re-exports. Their importers' bindings through them + * must be re-resolved: nothing those files declare changed, and the + * importers never did (#2978). + */ + reExportsChangedFilePaths?: string[]; /** Tracked files this sync found gone and deleted from the index. */ removedFilePaths?: string[]; /** @@ -3951,6 +3963,9 @@ export class ExtractionOrchestrator { if (filesToIndex.length > 0) { for (const pair of this.queries.getNodeNamePairsByFiles(filesToIndex)) pairsBefore.add(pair); } + // The modules each file imported or re-exported from, before the store + // replaces its import nodes (#2978). + const importPairsBefore = filesToIndex.length > 0 ? this.queries.getImportNamePairsByFiles(filesToIndex) : new Set(); // The frameworks every file below is extracted with, current (#2516). if (addedFilePaths.length > 0 || removedFilePaths.length > 0) this.detectionFileListChanged = true; @@ -4005,6 +4020,27 @@ export class ExtractionOrchestrator { for (const pair of pairsAfter) if (!pairsBefore.has(pair)) deltaNames.add(nameOf(pair)); const definitionDelta = [...deltaNames]; + // Modified ES modules whose imported modules changed, when they pass + // bindings on or stopped passing some on (#2978): a re-export retargeted or + // removed changes what their importers bind, which no definition delta + // records. An added module's importers are read by the added-files pass. + const reExportsChanged: string[] = []; + if (filesToIndex.length > 0) { + const importPairsAfter = this.queries.getImportNamePairsByFiles(filesToIndex); + const fileOf = (pair: string) => pair.slice(0, pair.indexOf('\0')); + const changedImports = new Set(); + const lostImports = new Set(); + for (const pair of importPairsBefore) if (!importPairsAfter.has(pair)) { changedImports.add(fileOf(pair)); lostImports.add(fileOf(pair)); } + for (const pair of importPairsAfter) if (!importPairsBefore.has(pair)) changedImports.add(fileOf(pair)); + const added = new Set(addedFilePaths); + for (const filePath of changedImports) { + if (added.has(filePath)) continue; + const record = this.queries.getFileByPath(filePath); + if (!record || !RE_EXPORTING_LANGUAGES.has(record.language)) continue; + if (lostImports.has(filePath) || importReader.reExports(filePath, record.language).length > 0) reExportsChanged.push(filePath); + } + } + return { filesChecked, filesAdded, @@ -4016,6 +4052,7 @@ export class ExtractionOrchestrator { ...(addedFilePaths.length > 0 ? { addedFilePaths } : {}), ...(failedFilePaths.length > 0 ? { failedFilePaths } : {}), definitionDelta: definitionDelta.length > 0 ? definitionDelta : undefined, + ...(reExportsChanged.length > 0 ? { reExportsChangedFilePaths: reExportsChanged } : {}), ...(removedFilePaths.length > 0 ? { removedFilePaths } : {}), ...(detectedFrameworks ? { detectedFrameworks } : {}), }; diff --git a/src/index.ts b/src/index.ts index ae27c17aaf..cc5dc26673 100644 --- a/src/index.ts +++ b/src/index.ts @@ -54,7 +54,7 @@ import { ResolutionResult, } from './resolution'; import { hasSynthesisPattern, isJsxChildCandidate } from './resolution/callback-synthesizer'; -import { createImportReader, reopenNameMatchedImports, reopenReferencesThroughAddedFiles, reopenReferencesThroughBarrels, reopenReferencesThroughChangedDefaults, reopenReferencesThroughDeclaredPackages } from './resolution/module-importers'; +import { createImportReader, reopenNameMatchedImports, reopenReferencesThroughAddedFiles, reopenReferencesThroughBarrels, reopenReferencesThroughChangedDefaults, reopenReferencesThroughDeclaredPackages, reopenReferencesThroughRetargetedModules } from './resolution/module-importers'; import { GraphTraverser, GraphQueryManager } from './graph'; import { findNamedCopybooks, type NamedCopybook } from './graph/cobol-copybooks'; import { ContextBuilder, createContextBuilder } from './context'; @@ -1401,6 +1401,23 @@ export class CodeGraph { (ref) => this.resolver.resolveNow(ref) ); if (process.env.CODEGRAPH_SYNTH_TIMINGS) console.error(`[phase-timing] sync-name-matched-imports: ${Date.now() - tNamed}ms (${nameMatched} refs re-opened)`); + + // A module whose `export … from` points elsewhere now, or passes + // a name on no longer: `export { foo } from './a'` made `'./b'` + // keeps `import { foo } from './index'` on `a.ts` (#2978). The + // barrel declares nothing of `foo`, so no delta names it, and the + // walks above read files below a barrel, not the barrel's own + // importers, and the name-matched pass only bindings a name match + // took. What they bind through it goes back for the sweep, after + // that pass has read the bindings it is about. + const tRetargeted = Date.now(); + const retargeted = (result.reExportsChangedFilePaths ?? []).flatMap((filePath) => { + const file = this.queries.getFileByPath(filePath); + return file ? [{ path: filePath, language: file.language }] : []; + }); + const throughRetargeted = retargeted.length === 0 ? 0 + : reopenReferencesThroughRetargetedModules(this.queries, importReader, retargeted); + if (process.env.CODEGRAPH_SYNTH_TIMINGS) console.error(`[phase-timing] sync-retargeted-modules: ${Date.now() - tRetargeted}ms (${throughRetargeted} refs re-opened)`); } else { // No git info — use batched resolution to avoid OOM const unresolvedCount = this.queries.getUnresolvedReferencesCount(); diff --git a/src/resolution/module-importers.ts b/src/resolution/module-importers.ts index f6a82da321..757a19fd12 100644 --- a/src/resolution/module-importers.ts +++ b/src/resolution/module-importers.ts @@ -424,6 +424,43 @@ export function reopenReferencesThroughBarrels( queries.reopenFailedReferences([...failed]); } +/** + * Re-open, for the sync's sweep, what the files importing any of `modules` + * bind through them, after a module's `export … from` clauses changed what + * they point at (#2978): `export { foo } from './a'` made `'./b'`, an `export + * *` moved the same way, a re-export taken away. The importers did not change + * and the module declares nothing of what it passes on, so no definition + * delta, added file or failed reference names their bindings, and they stay + * on the module the clause used to name. Each importer's edges through the + * names it binds from the module go back, up through the modules that pass + * them on in turn, with the failures parked under them. Returns the number + * re-opened. + */ +export function reopenReferencesThroughRetargetedModules( + queries: QueryBuilder, + reader: ImportReader, + modules: ReadonlyArray<{ path: string; language: Language }>, +): number { + const barrels = modules.filter((module) => BARREL_LANGUAGES.has(module.language)); + if (barrels.length === 0) return 0; + const bindings = bindingsOf(importersOf(queries, barrels, reader), reader); + const failed = [...bindings].flatMap(([file, { modules: named, names }]) => queries.getFailedReferencesThrough(file, named, names)); + // What the bindings pointed at: a value typed by a binding reached the + // members of that (`w.run()` with `w: Widget`), by their own names, and + // those edges follow the binding to its new target. + const bound = new Set(); + for (const [file, { names }] of bindings) { + if (names.length === 0) continue; + for (const e of queries.getResolutionEdgesThrough(file, names)) if (!e.target.startsWith('file:')) bound.add(e.target); + } + let reopened = reopenEdges(queries, bindings, () => true) + queries.reopenFailedReferences(failed); + if (bound.size === 0) return reopened; + const members = queries.getOutgoingEdgesFrom([...bound], ['contains']).map((edge) => edge.target); + if (members.length === 0) return reopened; + reopened += reopenEdgeList(queries, queries.getResolutionEdgesInto(members).filter((e) => bindings.has(e.sourceFilePath))); + return reopened; +} + /** An import's module edge into a file that an added file now outranks. */ interface OutrankedImport { edge: Edge & { edgeId: number; sourceFilePath: string; sourceLanguage: Language };