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 @@ -96,6 +96,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- `codegraph sync` and the file watcher now link a file to a module it imports under a name of its own when the module is added after the file was indexed, comes back after being deleted, or gets the export in a later edit: a default import like `import tagsController from './tag/tag.controller'`, a namespace import like `import * as api from './api'`, or an aliased one like `import { Component as GroupsPageWrapper } from './groups_page'`, in JavaScript, TypeScript, Vue, Svelte and Astro. Before, the import and every use of the name, such as a call, `new`, a base class, a JSX or template tag or an Express `app.use` mount, stayed unlinked until the importing file changed or the project was re-indexed, so the module looked unused and `codegraph affected` missed the files that use it. Re-index after upgrading to pick up links an earlier version missed.
- `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.
- 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
130 changes: 122 additions & 8 deletions __tests__/sync-import-binding-retry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@
* its own name: a binding its module never resolves is linked by that name
* alone, to whichever declaration carries it (vben's `{ VbenFormSchema as
* FormSchema }`), and that link must come back when the declaration does.
*
* A plain named import through a barrel (`import { useInvocation } from
* '@/test_investigation/context'`) waits the same way: `context/index.ts`
* declares none of the names it forwards. When the barrel or a file behind it
* is added, sync re-opens the barrel's importers (module-importers.ts); an
* edit that gives an existing barrel the name re-opened nothing, so the
* import and its uses stayed unlinked. Such a reference is parked under its
* module's key as well when its module is a file of the project that has the
* key, as the barrel does.
*/
import { describe, it, expect, afterEach, vi } from 'vitest';
import * as fs from 'fs';
Expand Down Expand Up @@ -125,9 +134,9 @@ function edgesOf(graph: CodeGraph): string[] {
.sort();
}

/** The {@link LINKS} the graph has. */
function linksOf(graph: CodeGraph): string[] {
const names = new Set(LINKS.map((link) => link.slice(0, link.indexOf(':'))));
/** The links the graph has through the bindings `expected` names. */
function linksOf(graph: CodeGraph, expected: string[] = LINKS): string[] {
const names = new Set(expected.map((link) => link.slice(0, link.indexOf(':'))));
const ids = graph.getFiles().flatMap((file) => graph.getNodesInFile(file.path)).map((n) => n.id);
const links: string[] = [];
for (const edge of graph.getOutgoingEdgesFrom(ids)) {
Expand Down Expand Up @@ -219,15 +228,113 @@ describe('sync links references through an import binding to their module', () =
}, 120_000);
});

/** Files that import a folder's barrel by the names it forwards. */
const BARREL_IMPORTERS: Files = {
'package.json': JSON.stringify({ name: 'app', dependencies: { react: '^18', vue: '^3' } }),
'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', jsx: 'react-jsx', paths: { '@/*': ['src/*'] } } }),
// Hooks imported through a tsconfig alias from a folder's barrel (luci-go's milo).
'src/pages/invocation_page.tsx': "import { useInvocation, useProject } from '@/test_investigation/context';\n\nexport function InvocationPage() {\n const invocation = useInvocation();\n return <p>{invocation}{useProject()}</p>;\n}\n",
// Namesakes elsewhere, as milo has: without the barrel the hooks link to nothing.
'src/common/store/invocation_state.ts': 'export function useInvocation() {\n return null;\n}\n',
'src/test_verdict/context/hooks.ts': 'export function useProject() {\n return null;\n}\n',
// A component its barrel forwards under another name, rendered by a Vue template (vben).
'src/layouts/toolbar.vue': "<template><ThemeToggle /></template>\n<script setup lang=\"ts\">\nimport { ThemeToggle } from '../widgets/theme-toggle';\n</script>\n",
};

const BARRELS: Files = {
'src/test_investigation/context/index.ts': "export { useInvocation, useProject } from './context';\n",
'src/widgets/theme-toggle/index.ts': "export { default as ThemeToggle } from './theme-toggle.vue';\n",
};

/** The files behind the barrels, in the barrels' folders. */
const BEHIND: Files = {
'src/test_investigation/context/context.ts': "export function useInvocation() {\n return 'invocation';\n}\n\nexport function useProject() {\n return 'project';\n}\n",
'src/widgets/theme-toggle/theme-toggle.vue': '<template><button>toggle</button></template>\n',
};

const BARREL_LINKS = [
'ThemeToggle: imports file src/layouts/toolbar.vue -> component src/widgets/theme-toggle/theme-toggle.vue::theme-toggle',
'ThemeToggle: references component src/layouts/toolbar.vue -> component src/widgets/theme-toggle/theme-toggle.vue::theme-toggle',
'useInvocation: calls function src/pages/invocation_page.tsx -> function src/test_investigation/context/context.ts::useInvocation',
'useInvocation: imports file src/pages/invocation_page.tsx -> function src/test_investigation/context/context.ts::useInvocation',
'useInvocation: imports file src/test_investigation/context/index.ts -> function src/test_investigation/context/context.ts::useInvocation',
'useProject: calls function src/pages/invocation_page.tsx -> function src/test_investigation/context/context.ts::useProject',
'useProject: imports file src/pages/invocation_page.tsx -> function src/test_investigation/context/context.ts::useProject',
'useProject: imports file src/test_investigation/context/index.ts -> function src/test_investigation/context/context.ts::useProject',
];

const ALL_BARREL_FILES: Files = { ...BARREL_IMPORTERS, ...BARRELS, ...BEHIND };

describe('sync links a plain named import through a barrel', () => {
it('when the barrel is added after its importers', async () => {
const root = tempRoot();
write(root, { ...BARREL_IMPORTERS, ...BEHIND });
const graph = await index(root);
expect(linksOf(graph, BARREL_LINKS)).toEqual([]);

write(root, BARRELS);
const result = await graph.sync();
// The importers did not change: only what sync re-opens or retries links them.
expect(result.filesAdded).toBe(Object.keys(BARRELS).length);
expect(result.filesModified).toBe(0);
expect(linksOf(graph, BARREL_LINKS)).toEqual(BARREL_LINKS);
expect(edgesOf(graph)).toEqual(await fullIndexOf(ALL_BARREL_FILES));
}, 120_000);

it('when a file behind the barrel is added after its importers', async () => {
// The hooks' file declares them by name, which a sync always looked up
// (until it appears, they link by name to their namesakes). The
// component's file declares only `theme-toggle`: the barrel that forwards
// it is what leads to it.
const root = tempRoot();
write(root, { ...BARREL_IMPORTERS, ...BARRELS });
const graph = await index(root);
expect(linksOf(graph, BARREL_LINKS).filter((link) => link.startsWith('ThemeToggle:'))).toEqual([]);

write(root, BEHIND);
expect((await graph.sync()).filesAdded).toBe(Object.keys(BEHIND).length);
expect(linksOf(graph, BARREL_LINKS)).toEqual(BARREL_LINKS);
expect(edgesOf(graph)).toEqual(await fullIndexOf(ALL_BARREL_FILES));
}, 120_000);

it('when an edit gives the barrel the names it forwards', async () => {
// The barrel was there all along: no file appears, and the edit declares
// none of the names it now passes on. Only the barrel's module key finds
// the importers' failed references.
const root = tempRoot();
write(root, { ...BARREL_IMPORTERS, ...BEHIND });
write(root, Object.fromEntries(Object.keys(BARRELS).map((file) => [file, 'export {};\n'])));
const graph = await index(root);
expect(linksOf(graph, BARREL_LINKS)).toEqual([]);

write(root, BARRELS);
const result = await graph.sync();
expect(result.filesModified).toBe(Object.keys(BARRELS).length);
expect(result.filesAdded).toBe(0);
expect(linksOf(graph, BARREL_LINKS)).toEqual(BARREL_LINKS);
expect(edgesOf(graph)).toEqual(await fullIndexOf(ALL_BARREL_FILES));
}, 120_000);
});

describe('a failed reference through an import binding', () => {
it('waits for its module, and every other reference keeps its own tail', async () => {
const root = tempRoot();
write(root, {
...IMPORTERS,
// A package's binding and an unaliased named import wait for the name
// they use; a member of a namespace import waits for its module too
// A package's binding waits for the name it uses, and so does a plain
// named import while its module is missing (`./util`), when the file its
// module resolves to has another key (`@main`, an alias of
// `src/lib/main.ts`), or when its module gives the name and the
// reference failed anyway (`LIMIT`, a constant passed as a function).
// One whose module is a file of the project with the key and lacks the
// name (`./lib`, a barrel yet to pass it on) waits for that module, as
// the other bindings do, and so does a member of a namespace import
// (sync-import-member-retry.test.ts).
'src/other.ts': "import express from 'express';\nimport { helper } from './util';\nimport * as NS from './ns';\nexport const app = express();\nexport const values = [helper, NS.make()];\n",
'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@/*': ['src/*'], '@main': ['src/lib/main.ts'] } } }),
'src/other.ts': "import express from 'express';\nimport { helper } from './util';\nimport { forwarded } from './lib';\nimport { mainThing } from '@main';\nimport { LIMIT } from './consts';\nimport * as NS from './ns';\nexport const app = express();\nexport const values = [helper, forwarded, mainThing, NS.make()];\nexport const mapped = [1, 2].map(LIMIT);\n",
'src/lib/index.ts': 'export {};\n',
'src/lib/main.ts': 'export const unrelated = 1;\n',
'src/consts.ts': 'export const LIMIT = 5;\n',
});
(await CodeGraph.init(root, { index: true })).close();

Expand All @@ -249,16 +356,23 @@ describe('a failed reference through an import binding', () => {
expect(tailOf('src/layouts/LayoutMain.vue', 'Error404', 'imports')).toBe('module:404');
expect(tailOf('src/App.svelte', 'Banner', 'references')).toBe('module:Hero');
expect(tailOf('src/pages/index.astro', 'Shell', 'imports')).toBe('module:BaseLayout');
expect(tailOf('src/other.ts', 'forwarded', 'imports')).toBe('module:lib');
expect(tailOf('src/other.ts', 'NS.make', 'calls')).toBe('module:ns');

expect(tailOf('src/other.ts', 'express', 'imports')).toBe('express');
expect(tailOf('src/other.ts', 'helper', 'imports')).toBe('helper');
expect(tailOf('src/other.ts', 'NS.make', 'calls')).toBe('module:ns');
expect(tailOf('src/other.ts', 'mainThing', 'imports')).toBe('mainThing');
expect(tailOf('src/other.ts', 'LIMIT', 'function_ref')).toBe('LIMIT');

// Each module's file is one of the keys its references wait under.
// Each module's file is one of the keys its references wait under, and
// so is a barrel's folder for every file in it.
for (const [file, module] of [
['src/routes/tag/tag.controller.ts', './tag/tag.controller'],
['src/views/error/404.vue', '@/views/error/404.vue'],
['src/pages/Team/index.tsx', './pages/Team'],
['src/test_investigation/context/index.ts', '@/test_investigation/context'],
['src/test_investigation/context/context.ts', '@/test_investigation/context'],
['src/widgets/theme-toggle/theme-toggle.vue', '../widgets/theme-toggle'],
]) {
expect(moduleReferenceKeys(file)).toContain(moduleTail(module));
}
Expand Down
6 changes: 3 additions & 3 deletions src/db/reference-tail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ export function importPathKeys(filePath: string): string[] {
* same path through its folder. A sync looks them up for the files it adds
* AND the ones it changes: a route renders the component its module exports,
* so an edit that gives the module one is what the route waited for. So does
* a reference through an import binding the module declares under another
* name, and a direct member reference through any binding
* (`importBindingTail`), which are parked under the same keys.
* a reference through an import binding of the module, and a direct member
* reference through any binding (`importBindingTail`), which are parked under
* the same keys.
*/
export function moduleReferenceKeys(filePath: string): string[] {
const keys = importPathKeys(filePath);
Expand Down
17 changes: 9 additions & 8 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1058,14 +1058,15 @@ export class CodeGraph {
// (`snippets/price.liquid`) waits under, and the keys a route's
// lazily loaded module waits under (`module:Team` for
// `lazy-import:./pages/Team`), as does a reference through an
// import binding the module declares under another name
// (`module:tag` for `import tagsController from
// './tag/tag.controller'`) and a direct member reference through
// any binding (`module:constants` for `LIST.includes(x)` after
// `import { LIST } from './constants'`): each resolves through the
// module, so an edit can satisfy it as well as an added file. On
// a sync where no failed ref matches, this is a few indexed
// lookups.
// import binding of the module (`module:tag` for `import
// tagsController from './tag/tag.controller'`, `module:context`
// for `import { useInvocation } from './context'`, which the
// barrel `context/index.ts` forwards) and a direct member
// reference through any binding (`module:constants` for
// `LIST.includes(x)` after `import { LIST } from './constants'`):
// each resolves through the module, so an edit can satisfy it as
// well as an added file. On a sync where no failed ref matches,
// this is a few indexed lookups.
const tRetry = Date.now();
edgesBeforeRetry = this.queries.getMaxEdgeId();
const retryable = this.queries.getRetryableFailedReferences([...new Set([
Expand Down
Loading