diff --git a/CHANGELOG.md b/CHANGELOG.md index a36bbe48a..dcf87f2b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`, 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. diff --git a/__tests__/sync-import-binding-retry.test.ts b/__tests__/sync-import-binding-retry.test.ts index 37b1d64d7..0122295c0 100644 --- a/__tests__/sync-import-binding-retry.test.ts +++ b/__tests__/sync-import-binding-retry.test.ts @@ -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'; @@ -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)) { @@ -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

{invocation}{useProject()}

;\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': "\n\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': '\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(); @@ -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)); } diff --git a/src/db/reference-tail.ts b/src/db/reference-tail.ts index dcc031a22..29ecb85d0 100644 --- a/src/db/reference-tail.ts +++ b/src/db/reference-tail.ts @@ -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); diff --git a/src/index.ts b/src/index.ts index 73b30f2a1..524ca04e1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -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([ diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 78bfea843..faf19f25b 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -17,7 +17,7 @@ import { makeLineAt } from './synth-utils'; import { dartDirectiveFile } from './dart-libraries'; import { ambientModuleDeclares, findAmbientModuleExport, findAmbientModuleFile, inBlock, isInModuleBlock, namespaceBlocks, type NamespaceBlock } from './ambient-modules'; import { isTestFile } from '../search/query-utils'; -import { moduleTail } from '../db/reference-tail'; +import { moduleReferenceKeys, moduleTail } from '../db/reference-tail'; import { resolveMethodOnType, resolveObjectLiteralMember, @@ -4228,14 +4228,27 @@ export function isBoundToOutOfRepoImport( /** * The tail a reference that failed to resolve is parked under when it goes * through an import binding the module's key finds and its own tail does not: - * - the binding's whole name, when the module it imports does not declare it - * by that name: a default import (`import tagsController from + * - the binding's whole name — the import itself, a call, `new`, a JSX tag, a + * value. A default import (`import tagsController from * './tag/tag.controller'`, a CommonJS `require`), a namespace import, or an - * aliased one (`import { Component as Wrapper }`, `{ default as X }`) — the - * import itself, a call, `new`, a JSX tag, a value. Its own name, the + * aliased one (`import { Component as Wrapper }`, `{ default as X }`) names + * what the module declares under another name, so its own name, the * default tail, is one the module never declares. A sync still looks it up * by that name as well: a binding its module never resolves is linked by * the name alone. + * A plain named import (`import { useInvocation } from './context'`) names + * what its module declares or passes on by that name. A file that declares + * it is found by the name, and the importers of a module that appears are + * re-opened through it (see module-importers), but an edit that makes a + * barrel pass the name on (`context/index.ts` gaining `export { + * useInvocation } from './context'`) declares nothing. So it waits under + * the key only when its module is a project file that has the key, the + * file whose edit that is, and gives no such name yet: not while the module + * is missing, nor for a workspace package's own name (`@vben/icons`), whose + * key is the package's folder, which its entry file (`src/index.ts`) never + * has, nor when the module already gives the name and the reference failed + * for another reason (a constant passed where a function is expected), + * which no edit of its exports changes. * - a direct member of any binding, a named import's included * (`NODE_PATH_EXCLUDELIST.includes(p)`, `store.get(k)`, `NS.make()`). It * reaches the binding's declaration, or a member of it, through the module, @@ -4263,10 +4276,9 @@ export function isBoundToOutOfRepoImport( * Only for a module of the project: a package's binding keeps its tail. A * bare specifier is the project's when the importing app's own config * (`baseUrl`, a catch-all `"*"`) maps it to a file the index holds and no - * package the app declares goes by it. Undefined for every other reference, - * and for the whole name of an unaliased named import, which is the name its - * module declares. A Python reference is {@link pythonImportBindingTail}'s, - * by its whole name only. + * package the app declares goes by it. Undefined for every other reference. + * A Python reference is {@link pythonImportBindingTail}'s, by its whole name + * only. */ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext): string | undefined { if (ref.language === 'python') return pythonImportBindingTail(ref, context); @@ -4278,7 +4290,7 @@ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext const mappings = context.getImportMappings(ref.filePath, ref.language); const binding = mappings.find((m) => m.localName === name) ?? (receiver && ref.referenceKind === 'calls' ? localTypeBinding(receiver, ref, context, mappings) : undefined); - if (!binding || (!receiver && !binding.isDefault && !binding.isNamespace && binding.exportedName === binding.localName)) return undefined; + if (!binding) return undefined; if (isExternalImport(binding.source, ref.language, context) && // A monorepo app's own tsconfig alias (`#/views/…`): isExternalImport reads the root's only. !context.getNearestAliases?.(ref.filePath)?.patterns.some((p) => p.prefix !== '' && binding.source.startsWith(p.prefix)) && @@ -4290,7 +4302,15 @@ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext const modulePath = binding.source.startsWith('.') ? path.posix.join(path.posix.dirname(ref.filePath), binding.source) : binding.source; - return moduleTail(modulePath) || undefined; + const tail = moduleTail(modulePath); + if (!tail) return undefined; + if (!receiver && !binding.isDefault && !binding.isNamespace && binding.exportedName === binding.localName) { + const moduleFile = resolveImportPath(binding.source, ref.filePath, ref.language, context); + if (!moduleFile || !(context.isIndexedFile?.(moduleFile) ?? true) || !moduleReferenceKeys(moduleFile).includes(tail)) return undefined; + const want = { isDefault: false, isNamespace: false, exportedName: binding.exportedName, memberName: null }; + if (findExportedSymbol(moduleFile, want, ref.language, context, new Set())) return undefined; + } + return tail; } /**