fix(sync): a barrel whose re-export points elsewhere re-links the files importing through it

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<User>("/me")` or `await api.get<T>(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(<App />)` or `render(<Card />)`, now counts as rendered there, so an app's entry reaches its root component and a component's tests show among its callers. (#2587)

__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<string, string>;
+
+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<CodeGraph> {
+  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<Record<string, unknown>>)
+      .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: `<file>::<qualified name>`, 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);
+});

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<string> {
+    const pairs = new Set<string>();
+    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<string> {
     const pairs = new Set<string>();
     if (filePaths.length === 0) return pairs;

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<string>(['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<string>();
 
     // 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<string>();
+      const lostImports = new Set<string>();
+      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 } : {}),
     };

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();

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<string>();
+  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 };