Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- `codegraph sync` and the file watcher now also link a call or read on a name a file imports, like `NODE_PATH_EXCLUDELIST.includes(path)` after `import { NODE_PATH_EXCLUDELIST } from './constants'`, `store.get(key)` through a default import or `api.get()` through a namespace import, when the module gets the export in a later edit, is added after the importing file was indexed, or comes back after being deleted, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, such a call stayed unlinked until the importing file changed or the project was re-indexed, because sync looked it up only by its method's name, which the module seldom declares, and skipped a name as common as `includes`, `get` or `map` altogether. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now also link a call on a variable or parameter declared with a class a file imports, like `cache.get(key)` after `const cache = new LRUCache(10)` or in a function that takes `cache: LRUCache<string, number>`, when the class's module gets the class in a later edit, is added after the file was indexed, or comes back after being deleted, in JavaScript and TypeScript. Before, such a call stayed unlinked until its own file changed or the project was re-indexed whenever its method's name, like `get`, was too common for sync to look up. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now also link a name a file imports from a barrel module, like `import { useInvocation } from './context'` where `context/index.ts` re-exports it from the file that declares it, when an edit gives the barrel that re-export after the importing file was indexed, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call or a JSX or template tag, stayed unlinked until the importing file changed or the project was re-indexed, because a barrel declares none of the names it passes on. Re-index after upgrading to pick up links an earlier version missed.
- `codegraph sync` and the file watcher now also update the files that import through a barrel module when an edit points one of its re-exports at another module, like `export { foo } from './a'` changed to `'./b'` or an `export *` moved the same way, takes one away, or drops a name from it, in JavaScript and TypeScript. Before, those files kept the names linked to the old module until they changed or the project was re-indexed. Thanks @inth3shadows for the report and @danusha2345. (#2978)
- In TypeScript and JavaScript, a call of an array method, like `items.find(…)` or `items.push(…)`, on a variable or parameter annotated as an array of a class (`items: Item[]`) no longer links to the class's method of the same name. Before, the annotation was read as the class itself: in vscode, `distinctElements.find(…)` on an `ExplorerItem[]` linked to `ExplorerItem.find`, and `ranges.map(…)` on an `OffsetRange[]` to `OffsetRange.map`, so those methods showed callers they don't have. A call on an element that shares the array's name, as in `d.forEach(d => d.dispose())`, still links to the class. Re-index TypeScript and JavaScript projects after upgrading.
- In TypeScript and JavaScript, a call on a variable or parameter annotated as one of a type's properties, like `editor.executeEdits(…)` with `editor: IChatWidget['inputEditor']`, now links to the method of the type that property is declared with, or to nothing when that type has no such method. That type is the class the file declaring the property imports or declares, and the indexed type itself the one the call's file imports, when several classes share a name. Before, the annotation was read as the type it indexes, so in vscode `visibility.isVisible(…)` with `visibility: ISessionChatPillVisibilityService['pullRequests']` linked to the service's own `isVisible`, and a call that type didn't have was matched by the method's name alone: `chatModel.onDidChange(…)` with `chatModel: IInlineChatSession['chatModel']` reached a folding model's `onDidChange`. A property declared as an array follows the same rules as an array annotation, and one whose type isn't written there, like a getter, is read as the type it indexes, as before. Re-index TypeScript and JavaScript projects after upgrading.
- `codegraph sync` and the file watcher now do the same for a Python import under a name of its own, like `from .models import User as UserModel`, `from . import helpers as h` or `from app.pkg import sub as subpkg`: when the module or package is added after the importing file was indexed, comes back after being deleted, or gets the imported name in a later edit, the import and every use of the alias, such as a call, a base class or the class passed as a value, now link to it. Before, they stayed unlinked until the importing file changed or the project was re-indexed. Re-index Python projects after upgrading to pick up links an earlier version missed.
Expand Down
206 changes: 206 additions & 0 deletions __tests__/sync-barrel-retarget.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
/**
* 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);

it('a name dropped from a re-export whose module stays', async () => {
// The barrel still names `./a`, so only the clause's names tell the edit.
const before: Files = {
...BEFORE,
'src/a.ts': 'export function foo() { return 1; }\nexport function qux() { return 3; }\n',
'src/index.ts': "export { foo, qux } from './a';\n",
'src/c.ts': "import { foo, qux } from './index';\nexport function bar() { return foo(); }\nexport function baz() { return qux(); }\n",
};
const { root, graph, full } = await syncEdit(before, { 'src/index.ts': "export { foo } from './a';\n" });
expect(stateOf(root, graph)).toEqual(full);
}, 120_000);

it('a plain import edit in a module others import reports nothing', async () => {
// Importers bind through `util.ts`, which passes nothing on: dropping its
// own import moves none of their bindings.
const root = tempRoot();
write(root, {
...BEFORE,
'src/util.ts': "import { foo } from './a';\nexport function helper() { return foo(); }\n",
'src/c.ts': "import { helper } from './util';\nexport function bar() { return helper(); }\n",
});
const graph = await index(root);
write(root, { 'src/util.ts': 'export function helper() { return 5; }\n' });
const result = await graph.sync();
expect(result.reExportsChangedFilePaths).toBeUndefined();
const fullRoot = tempRoot();
write(fullRoot, { ...BEFORE, 'src/util.ts': 'export function helper() { return 5; }\n', 'src/c.ts': "import { helper } from './util';\nexport function bar() { return helper(); }\n" });
expect(stateOf(root, graph)).toEqual(stateOf(fullRoot, await index(fullRoot)));
}, 120_000);
});
22 changes: 22 additions & 0 deletions src/db/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5549,6 +5549,28 @@ export class QueryBuilder {
return pairs;
}

/**
* The `file\0statement` pairs of the `export … from` statements in
* `filePaths` (#2978). Each is an import node named by the module it names,
* with the statement as its signature, so a re-export pointed at another
* module, taken away, added, or passing other names on changes a pair, and a
* plain `import` does not. Same shape as {@link getNodeNamePairsByFiles}.
*/
getReExportStatementsByFiles(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, signature FROM nodes
WHERE kind = 'import' AND file_path IN (${placeholders}) AND signature LIKE 'export%'`)
.all(...chunk) as Array<{ file_path: string; signature: string }>;
for (const row of rows) pairs.add(`${row.file_path}\0${row.signature}`);
}
return pairs;
}

// ===========================================================================
// Statistics
// ===========================================================================
Expand Down
36 changes: 36 additions & 0 deletions src/extraction/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ export interface IndexResult {
durationMs: number;
}

/** The languages whose modules pass bindings on with `export … from` (#2978). */
const RE_EXPORTING_LANGUAGES = new Set<string>(['typescript', 'tsx', 'javascript', 'jsx']);

/**
* Result of a sync operation
*/
Expand Down Expand Up @@ -164,6 +167,15 @@ export interface SyncResult {
* nothing downstream.
*/
definitionDelta?: string[];
/**
* Modified ES modules whose `export … from` statements changed: a re-export
* that points at another module now (`export { foo } from './a'` →
* `'./b'`), one taken away or added, or a clause that passes other names on.
* Their importers' bindings through them must be re-resolved: nothing those
* files declare changed, and the importers never did (#2978). An edit to a
* plain `import` leaves this empty.
*/
reExportsChangedFilePaths?: string[];
/** Tracked files this sync found gone and deleted from the index. */
removedFilePaths?: string[];
/**
Expand Down Expand Up @@ -3951,6 +3963,9 @@ export class ExtractionOrchestrator {
if (filesToIndex.length > 0) {
for (const pair of this.queries.getNodeNamePairsByFiles(filesToIndex)) pairsBefore.add(pair);
}
// The `export … from` statements of each file, before the store replaces
// its import nodes (#2978).
const reExportsBefore = filesToIndex.length > 0 ? this.queries.getReExportStatementsByFiles(filesToIndex) : new Set<string>();

// The frameworks every file below is extracted with, current (#2516).
if (addedFilePaths.length > 0 || removedFilePaths.length > 0) this.detectionFileListChanged = true;
Expand Down Expand Up @@ -4005,6 +4020,26 @@ export class ExtractionOrchestrator {
for (const pair of pairsAfter) if (!pairsBefore.has(pair)) deltaNames.add(nameOf(pair));
const definitionDelta = [...deltaNames];

// Modified ES modules whose `export … from` statements changed (#2978): a
// re-export retargeted, removed or passing other names on changes what
// their importers bind, which no definition delta records. A plain import
// edit passes nothing on. An added module's importers are read by the
// added-files pass.
const reExportsChanged: string[] = [];
if (filesToIndex.length > 0) {
const reExportsAfter = this.queries.getReExportStatementsByFiles(filesToIndex);
const fileOf = (pair: string) => pair.slice(0, pair.indexOf('\0'));
const changed = new Set<string>();
for (const pair of reExportsBefore) if (!reExportsAfter.has(pair)) changed.add(fileOf(pair));
for (const pair of reExportsAfter) if (!reExportsBefore.has(pair)) changed.add(fileOf(pair));
const added = new Set(addedFilePaths);
for (const filePath of changed) {
if (added.has(filePath)) continue;
const record = this.queries.getFileByPath(filePath);
if (record && RE_EXPORTING_LANGUAGES.has(record.language)) reExportsChanged.push(filePath);
}
}

return {
filesChecked,
filesAdded,
Expand All @@ -4016,6 +4051,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 } : {}),
};
Expand Down
Loading