From 53ad0d48b3dc2e50ab95e4ecb3041d614a950e1a Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sun, 18 Oct 2026 20:06:03 +0000 Subject: [PATCH] fix(sync): a member call on a local typed by an imported class links when the class's module changes (#2637) `cache.get(k)` after `const cache = new LRUCache(10)` (or an annotated local, a typed parameter, a default or namespace import of the class) is linked by a fresh index through the local's declaration. When the class's module was missing or empty, the failed call was parked under its member's name, `get`, which more failed refs share than the retry's ceiling, so a sync that added, restored or filled in the module never retried it. Such a call now waits under the key of the module the type's binding imports, as a member of the binding itself does since the previous change (2630), and keeps that change's member and receiver triggers. Only calls are parked this way: in TypeScript a typed local's member is only ever a call, and a method of `this` passed as a value is looked up in the enclosing class. A local declared as an array of the class (`results: URI[]`) keeps its member's tail. Resolution reads the element type there too, but `results.push` is the array's method, which no edit of the class's module links. On vscode those calls were 60% of what the parking added and nearly doubled what an edit of `uri.ts` retried. Inference now reports the line it read the declaration from. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../sync-typed-local-member-retry.test.ts | 227 ++++++++++++++++++ src/resolution/import-resolver.ts | 48 +++- src/resolution/name-matcher.ts | 28 ++- 4 files changed, 294 insertions(+), 10 deletions(-) create mode 100644 __tests__/sync-typed-local-member-retry.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d921ed2ba..b35d11dc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph_explore` now leaves out of its answers every file that search already treats as a test or an example: a C++ `FooTest.cpp` or `foo_test.cc`, a Google `foo_unittest.cc`, anything in an LLVM-style `unittests/` folder or an `e2e/` or `testing/` folder, and a project's `examples/`, `samples/` and `benchmarks/`. Before, explore went by a shorter list of its own, so a unit test or an example that called the code you asked about could take a place in the answer meant for that code: in protocolbuffers/protobuf, a question about `DescriptorPool` came back with three of its unit tests. The same files are no longer taken for a project's main routes file either, such as an end-to-end suite's mock server. No re-index is needed. - `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 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. - `codegraph sync` and the file watcher now also link a Python import of a name a package re-exports, like `from fastapi import FastAPI` through `fastapi/__init__.py`, when that `__init__.py` is added after the importing files were indexed or gains the re-export in a later edit. Before, calls through the name stayed unlinked until each importing file changed or the project was re-indexed. Large projects gain the most: sync also used to skip, in any language, a module or package that more than 500 references were waiting for, and now restoring Django's `django/urls/__init__.py` relinks every file that imports `path` from it. Re-index Python projects after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now update the files that import through a module that re-exports what it imports, like a package's `__init__.py` or an `index.ts` barrel, when that module is deleted or added, and in Python also the files that reach it through another package re-exporting it. Before, deleting `shop/__init__.py` left every `from shop import Order` linked to the class it re-exported, and deleting `components/index.ts` left every `import { Button } from './components'` linked, until each importing file changed or the project was re-indexed. When such a module came back, the imports stayed linked the way they were while it was missing, and in a large Python project an import of a module whose name is as common as `models` or `test`, like `from django.test import TestCase`, stayed unlinked. Python, JavaScript, TypeScript, Vue, Svelte and Astro. diff --git a/__tests__/sync-typed-local-member-retry.test.ts b/__tests__/sync-typed-local-member-retry.test.ts new file mode 100644 index 000000000..2323c0b97 --- /dev/null +++ b/__tests__/sync-typed-local-member-retry.test.ts @@ -0,0 +1,227 @@ +/** + * A sync links a member call on a local typed by an imported class once the + * class's module appears. + * + * `const cache = new LRUCache(2)` after `import { LRUCache } + * from './lru-cache'`, then `cache.get(key)`: a fresh index reads the local's + * declaration and links the call to `LRUCache.get` (folo's lru-cache tests, + * milo-ui's `astCache.get`). The same holds for an annotated local, a typed + * parameter and a class imported by default or through a namespace. While the + * module was missing or empty the call failed, and it was parked under its + * member's name, `get`, which more failed calls share than the retry's + * ceiling: a sync that brought the class back never retried it. + * + * Such a call is now parked under the key of the module the type's binding + * imports, as #2630 parks a member of the binding itself, and keeps its + * member's name as a key under that name's ceiling. + */ +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; + +/** 501 failed `get` calls on a receiver nothing declares: `get` is past the retry's ceiling. */ +const NOISE = `export function noise() {\n${Array.from({ length: 501 }, (_, i) => ` extApi.get(${i});`).join('\n')}\n}\n`; + +/** The files that call members of locals typed by a class they import. */ +const CALLERS: Files = { + 'package.json': JSON.stringify({ name: 'app' }), + 'src/use-new.ts': "import { LRUCache } from './lru-cache';\nexport function a() {\n const cache = new LRUCache(10);\n cache.set('k', 1);\n return cache.get('k');\n}\n", + 'src/use-annotated.ts': "import { LRUCache } from './lru-cache';\nexport function b(make: () => LRUCache) {\n const cache: LRUCache = make();\n return cache.get('k');\n}\n", + 'src/use-param.ts': "import { LRUCache } from './lru-cache';\nexport function c(cache: LRUCache) {\n return cache.get('k');\n}\n", + 'src/use-namespace.ts': "import * as lib from './lru-cache';\nexport function g() {\n const cache = new lib.LRUCache(3);\n return cache.get('k');\n}\n", + 'src/use-default.ts': "import Store from './store';\nexport function e() {\n const s = new Store();\n return s.get();\n}\n", + 'src/noise.ts': NOISE, +}; + +const MODULES: Files = { + 'src/lru-cache.ts': 'export class LRUCache {\n constructor(private max: number) {}\n get(key: K): V | undefined { return undefined; }\n set(key: K, value: V) { return this; }\n}\n', + 'src/store.ts': 'export default class Store {\n get() { return 1; }\n}\n', +}; + +/** What a full index links: `: -> ::`. */ +const LINKS = [ + 'cache.get: calls src/use-annotated.ts -> method src/lru-cache.ts::LRUCache::get', + 'cache.get: calls src/use-namespace.ts -> method src/lru-cache.ts::LRUCache::get', + 'cache.get: calls src/use-new.ts -> method src/lru-cache.ts::LRUCache::get', + 'cache.get: calls src/use-param.ts -> method src/lru-cache.ts::LRUCache::get', + 'cache.set: calls src/use-new.ts -> method src/lru-cache.ts::LRUCache::set', + 's.get: calls src/use-default.ts -> method src/store.ts::Store::get', +]; + +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-sync-typed-local-')); + 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, with where it is written and how it resolved. */ +function edgesOf(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) + ); + return 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 ?? {})}`) + .sort(); +} + +/** The calls on a lowercase local's member (`cache.get`): `: -> ::`. */ +function localMemberLinksOf(graph: CodeGraph): string[] { + const ids = graph.getFiles().flatMap((file) => graph.getNodesInFile(file.path)).map((n) => n.id); + const links: string[] = []; + for (const edge of graph.getOutgoingEdgesFrom(ids, ['calls'])) { + const ref = edge.metadata?.refName; + if (typeof ref !== 'string' || !/^[a-z]\w*\.\w+$/.test(ref) || ref.startsWith('extApi.')) continue; + const source = graph.getNode(edge.source)!; + const target = graph.getNode(edge.target)!; + links.push(`${ref}: ${edge.kind} ${source.filePath} -> ${target.kind} ${target.filePath}::${target.qualifiedName}`); + } + return [...new Set(links)].sort(); +} + +/** The final tree, indexed from scratch in a folder of its own: what the sync has to match. */ +async function fullIndexOf(files: Files): Promise { + const root = tempRoot(); + write(root, files); + return edgesOf(await index(root)); +} + +describe('sync links a member call on a local typed by an imported class', () => { + it('when the class\'s module is added after the callers', async () => { + const root = tempRoot(); + write(root, CALLERS); + const graph = await index(root); + expect(localMemberLinksOf(graph)).toEqual([]); + + write(root, MODULES); + const result = await graph.sync(); + // The callers did not change: only the retry of their parked refs links them. + expect(result.filesAdded).toBe(Object.keys(MODULES).length); + expect(result.filesModified).toBe(0); + expect(localMemberLinksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...CALLERS, ...MODULES })); + expect(graph.getPendingReferenceCount()).toBe(0); + }, 120_000); + + it('when the class\'s module comes back after it was deleted', async () => { + const root = tempRoot(); + write(root, { ...CALLERS, ...MODULES }); + const graph = await index(root); + expect(localMemberLinksOf(graph)).toEqual(LINKS); + + for (const file of Object.keys(MODULES)) fs.rmSync(path.join(root, file)); + expect((await graph.sync()).filesRemoved).toBe(Object.keys(MODULES).length); + expect(localMemberLinksOf(graph)).toEqual([]); + + write(root, MODULES); + expect((await graph.sync()).filesAdded).toBe(Object.keys(MODULES).length); + expect(localMemberLinksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...CALLERS, ...MODULES })); + }, 120_000); + + it('when an edit gives the module the class', async () => { + const root = tempRoot(); + write(root, CALLERS); + write(root, Object.fromEntries(Object.keys(MODULES).map((file) => [file, 'export const placeholder = 1;\n']))); + const graph = await index(root); + + write(root, MODULES); + const result = await graph.sync(); + expect(result.filesModified).toBe(Object.keys(MODULES).length); + expect(result.filesAdded).toBe(0); + expect(localMemberLinksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...CALLERS, ...MODULES })); + }, 120_000); +}); + +describe('a failed member call on a typed local', () => { + it('waits for the module of the class its declaration imports, and every other one keeps its own tail', async () => { + const root = tempRoot(); + write(root, { + ...CALLERS, + // A local of a built-in type, of a package's, of a type the file + // declares itself and one a factory returns: none waits for a module of + // the project. Nor does a local or a parameter declared as an array of + // an imported class, whose calls are the array's, or a method of `this` + // passed as a value, which resolution looks up in the enclosing class, + // whatever `this` is declared as. + 'src/other.ts': + "import { createCache, LRUCache } from './factory';\n" + + "import QuickLRU from 'quick-lru';\n" + + 'class Local { get() { return 1; } }\n' + + 'export function run() {\n' + + ' const seen = new Map();\n' + + " seen.get('k');\n" + + ' const pkg = new QuickLRU();\n' + + " pkg.get('k');\n" + + ' const mine: Local = new Local();\n' + + ' mine.peek();\n' + + ' const made = createCache();\n' + + " made.get('k');\n" + + ' const caches: LRUCache[] = [];\n' + + ' caches.push(made);\n' + + '}\n' + + 'export function each(all: LRUCache[]) {\n' + + ' return all.map((c) => c);\n' + + '}\n' + + 'export function bound(this: LRUCache) {\n' + + " return ['k'].map(this.peek);\n" + + '}\n', + }); + (await CodeGraph.init(root, { index: true })).close(); + + const db = DatabaseConnection.open(getDatabasePath(root)); + try { + const rows = db.getDb() + .prepare("SELECT reference_name AS name, file_path AS file, name_tail AS tail FROM unresolved_refs WHERE status = 'failed' AND reference_kind IN ('calls', 'function_ref')") + .all() as Array<{ name: string; file: string; tail: string }>; + const tails = (file: string, name: string) => [...new Set(rows.filter((r) => r.file === file && r.name === name).map((r) => r.tail))]; + + expect(tails('src/use-new.ts', 'cache.get')).toEqual(['module:lru-cache']); + expect(tails('src/use-new.ts', 'cache.set')).toEqual(['module:lru-cache']); + expect(tails('src/use-annotated.ts', 'cache.get')).toEqual(['module:lru-cache']); + expect(tails('src/use-param.ts', 'cache.get')).toEqual(['module:lru-cache']); + expect(tails('src/use-namespace.ts', 'cache.get')).toEqual(['module:lru-cache']); + expect(tails('src/use-default.ts', 's.get')).toEqual(['module:store']); + + expect(tails('src/other.ts', 'seen.get')).toEqual(['get']); + expect(tails('src/other.ts', 'pkg.get')).toEqual(['get']); + expect(tails('src/other.ts', 'mine.peek')).toEqual(['peek']); + expect(tails('src/other.ts', 'made.get')).toEqual(['get']); + expect(tails('src/other.ts', 'caches.push')).toEqual(['push']); + expect(tails('src/other.ts', 'all.map')).toEqual(['map']); + expect(tails('src/other.ts', 'this.peek')).toEqual(['peek']); + } finally { + db.close(); + } + }, 60_000); +}); diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 24a273290..a5e2a1074 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -22,6 +22,7 @@ import { localReceiverTypePatterns, normalizeInferredTypeName, goRefQualifier, + inferLocalReceiverType, } from './name-matcher'; /** @@ -3131,7 +3132,13 @@ export function isBoundToOutOfRepoImport( * member hangs off is what another member returns or holds, seldom a type * the module declares. Parked under the module as well, they tripled the * refs an edit of a hot module retries on vscode, and none of them resolved. - * Resolution reaches either through the module, so a sync's retry has to find + * - a direct member call on a local the file declares with a type it + * imports (`const cache = new LRUCache(10)`, `cache: LRUCache`, a + * typed parameter, then `cache.get(k)`), but not an array of it: + * resolution reads that declaration and calls the member of the class, + * which the type's binding imports, so the call waits for that module as a + * member of the binding itself does. + * Resolution reaches each through the module, so a sync's retry has to find * it by the module: under the module's {@link moduleTail} ('module:tag'), the * key a sync looks up for every file it adds or changes, so the file that * appears or gains the export finds it. @@ -3148,7 +3155,9 @@ export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext const receiver = ref.referenceKind === 'imports' ? undefined : MEMBER_RECEIVER.exec(ref.referenceName)?.[1]; const name = receiver ?? ref.referenceName; if (!receiver && !IDENTIFIER.test(name)) return undefined; - const binding = context.getImportMappings(ref.filePath, ref.language).find((m) => m.localName === name); + 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 (isExternalImport(binding.source, ref.language, context) && // A monorepo app's own tsconfig alias (`#/views/…`): isExternalImport reads the root's only. @@ -3256,6 +3265,41 @@ export function pythonModulePath(source: string, fromFile: string): string | nul return modulePath === '..' || modulePath.startsWith('../') ? null : modulePath; } +/** + * The import binding of the type a call's receiver is declared with, read the + * way resolution reads it for the call (inferLocalReceiverType, memoized per + * file, scope and receiver, so a call the matcher just tried costs a lookup): + * the binding the type is written through, `LRUCache` of `new LRUCache(…)` or `lib` of `new lib.LRUCache()`. Undefined for a type the file + * doesn't import, in a file that imports nothing, and for a local declared as + * an array of the type (`results: URI[]`). Resolution reads the element type + * there too, but the call is the array's (`results.push(…)`), which no edit + * of the type's module links: such calls were most of what this parked on + * vscode, and retrying them nearly doubled what an edit of `uri.ts` retried. + */ +function localTypeBinding(receiver: string, ref: UnresolvedRef, context: ResolutionContext, mappings: ImportMapping[]): ImportMapping | undefined { + if (mappings.length === 0) return undefined; + const decl: { raw?: string; line?: number } = {}; + if (!inferLocalReceiverType(receiver, ref, context, decl) || !decl.raw) return undefined; + const head = decl.raw.split('.')[0]; + const binding = mappings.find((m) => m.localName === head); + if (!binding || decl.line === undefined) return binding; + const lines = context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split(/\r?\n/); + const declaration = lines?.[decl.line]; + return declaration !== undefined && declaresArrayOf(declaration, receiver, decl.raw) ? undefined : binding; +} + +/** A `name: Type` annotation, the type's arguments, and the `[` of an array of it: `items: Item[]`. */ +const ANNOTATION = /([A-Za-z_$][\w$]*)\s*:\s*([A-Za-z_$][\w.$]*)\s*(?:<[^;=(){}<>]*(?:<[^;=(){}<>]*>[^;=(){}<>]*)*>)?\s*(\[)?/g; + +/** Whether `line` annotates `name` as an array of `type`: `items: Item[]`, `items: Item[] = []`. */ +function declaresArrayOf(line: string, name: string, type: string): boolean { + for (const m of line.matchAll(ANNOTATION)) { + if (m[1] === name && m[2] === type) return m[3] !== undefined; + } + return false; +} + const IDENTIFIER = /^[A-Za-z_$][\w$]*$/; /** The receiver of a direct member reference: `store` of `store.get`, none of `store.get().then` or `a.b.c`. */ const MEMBER_RECEIVER = /^([A-Za-z_$][\w$]*)\.[A-Za-z_$][\w$]*$/; diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index d3cf05b88..55162fdd9 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -13584,7 +13584,7 @@ function enclosingScopeStartLine(ref: UnresolvedRef, context: ResolutionContext) * without patterns or when no declaration is found. Bounded to the enclosing * scope so a same-named variable in another function can't leak in. */ -function inferLocalReceiverType( +export function inferLocalReceiverType( receiverName: string, ref: UnresolvedRef, context: ResolutionContext, @@ -13713,7 +13713,10 @@ function inferLocalReceiverType( const type = matchLine(i); if (type) { states.set(key, { hi: callIdx, ansIdx: i, ansType: type, ansRaw: lastRaw }); - if (decl) decl.raw = lastRaw; + if (decl) { + decl.raw = lastRaw; + decl.line = i; + } return type; } } @@ -13732,13 +13735,19 @@ function inferLocalReceiverType( } state.hi = callIdx; if (state.ansIdx < startIdx) return null; - if (decl) decl.raw = state.ansRaw; + if (decl) { + decl.raw = state.ansRaw; + decl.line = state.ansIdx; + } return state.ansType; } for (let i = callIdx; i >= startIdx; i--) { const type = matchLine(i); if (type) { - if (decl) decl.raw = lastRaw; + if (decl) { + decl.raw = lastRaw; + decl.line = i; + } return type; } } @@ -14956,13 +14965,16 @@ interface GoProjectType { /** * What inferLocalReceiverType tells its caller besides the type: the type as - * the declaration spells it (`raw`), and for a Go local bound from a type - * assertion, the type goWrittenType finds for it (`asserted`; null for one - * with no method the project declares). `asserted` is unset when the local - * is declared some other way, or goWrittenType can't tell its type. + * the declaration spells it (`raw`), the 0-based index of the line it was + * read from (`line`, set with `raw` by the line scan), and for a Go local + * bound from a type assertion, the type goWrittenType finds for it + * (`asserted`; null for one with no method the project declares). `asserted` + * is unset when the local is declared some other way, or goWrittenType can't + * tell its type. */ interface GoReceiverDecl { raw?: string; + line?: number; asserted?: GoProjectType | null; }