diff --git a/CHANGELOG.md b/CHANGELOG.md index f58218b4c..c45336701 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 000000000..52631f848 --- /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 4675e3b65..81849a07e 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 78b8bf214..3a2fdfe16 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 ae27c17aa..cc5dc2667 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 f6a82da32..757a19fd1 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 };