diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e79cd420..23105eb67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -82,6 +82,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`, 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. - 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. - `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__/ts-array-annotated-receivers.test.ts b/__tests__/ts-array-annotated-receivers.test.ts index 70982b968..cd3aabfdb 100644 --- a/__tests__/ts-array-annotated-receivers.test.ts +++ b/__tests__/ts-array-annotated-receivers.test.ts @@ -9,6 +9,13 @@ * `OffsetRange[]` reached `OffsetRange.map`. A method arrays don't have can * only be called on an element that shares the array's name, as in * `d.forEach(d => d.dispose())`, and that one keeps the element's type. + * + * A local annotated as one of a type's properties (`editor: + * IChatWidget['inputEditor']`) was read as that type too, so its calls + * reached the type's methods of the same name, or none. It has the type the + * property is declared with, an array of it the same way; one whose type + * isn't written in the project (`Parameters[0]`) links to nothing, + * and the call isn't matched by its name either. */ import { describe, it, expect, afterEach } from 'vitest'; import * as fs from 'fs'; @@ -27,7 +34,7 @@ afterEach(() => { }); /** Index `files` and return each call edge as ` -> `. */ -async function callsOf(files: Record): Promise { +async function callsOf(files: Record, withFiles = false): Promise { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ts-array-recv-')); roots.push(root); for (const [file, text] of Object.entries(files)) { @@ -39,7 +46,13 @@ async function callsOf(files: Record): Promise { const nodes = graph.getFiles().flatMap((f) => graph.getNodesInFile(f.path)); const byId = new Map(nodes.map((n) => [n.id, n])); return graph.getOutgoingEdgesFrom(nodes.map((n) => n.id), ['calls']) - .map((e) => `${e.metadata?.refName} -> ${byId.get(e.target)?.qualifiedName}`) + .map((e) => { + const source = byId.get(e.source); + const target = byId.get(e.target); + return withFiles + ? `${source?.filePath}: ${e.metadata?.refName} -> ${target?.filePath}::${target?.qualifiedName}` + : `${e.metadata?.refName} -> ${target?.qualifiedName}`; + }) .sort(); } @@ -133,3 +146,164 @@ describe('a call on a TypeScript local annotated as an array of a class', () => expect(calls).toEqual(['item.find -> Item::find', 'repo.find -> Repository::find']); }, 60_000); }); + +describe("a call on a TypeScript local annotated as one of a type's properties", () => { + it('links to the method of the type that property is declared with', async () => { + // vscode's `editor: IChatWidget['inputEditor']`, then `editor.executeEdits(…)`. + const calls = await callsOf({ + ...CLASSES, + 'src/widget.ts': + 'export class CodeEditor {\n executeEdits() { return true; }\n}\n' + + 'export interface Widget {\n readonly inputEditor: CodeEditor;\n}\n' + + // Another `executeEdits`, so the method's name alone picks neither. + 'export class DiffEditor {\n executeEdits() { return false; }\n}\n', + 'src/use.ts': + "import { Widget } from './widget';\n" + + "export function edit(ed: Widget['inputEditor']) {\n" + + ' return ed.executeEdits();\n' + + '}\n', + }); + expect(calls.filter((c) => c.startsWith('ed.'))).toEqual(['ed.executeEdits -> CodeEditor::executeEdits']); + }, 60_000); + + it('links to that type rather than to a method of the type it indexes', async () => { + // vscode's `visibility: ISessionChatPillVisibilityService['pullRequests']`, + // then `visibility.isVisible(…)`, which reached the service's own `isVisible`. + const calls = await callsOf({ + ...CLASSES, + 'src/service.ts': + 'export class Visibility {\n isVisible() { return true; }\n}\n' + + 'export class VisibilityService {\n readonly pullRequests: Visibility = new Visibility();\n isVisible() { return false; }\n}\n', + 'src/use.ts': + "import { VisibilityService } from './service';\n" + + "export function shown(visibility: VisibilityService['pullRequests']) {\n" + + ' return visibility.isVisible();\n' + + '}\n', + }); + expect(calls.filter((c) => c.startsWith('visibility.'))).toEqual(['visibility.isVisible -> Visibility::isVisible']); + }, 60_000); + + it("links to nothing when the property's type lacks the method, not to a guess", async () => { + // The property is declared as an intersection, whose head names no + // `isVisible`: neither the indexed type's own method nor one found by + // the receiver's name, like `VisibilityService.isVisible`, is the call's. + const calls = await callsOf({ + ...CLASSES, + 'src/service.ts': + 'export interface Filter {\n showAll(): boolean;\n}\n' + + 'export class VisibilityService {\n readonly pullRequests!: Filter & { isVisible(): boolean };\n isVisible() { return false; }\n}\n', + 'src/use.ts': + "import { VisibilityService } from './service';\n" + + "export function shown(visibility: VisibilityService['pullRequests']) {\n" + + ' return visibility.isVisible();\n' + + '}\n', + }); + expect(calls.filter((c) => c.startsWith('visibility.'))).toEqual([]); + }, 60_000); + + it("links an array property's own methods to nothing", async () => { + // excalidraw's `groupIds: ExcalidrawElement["groupIds"]`, then `groupIds.filter(…)`. + const calls = await callsOf({ + ...CLASSES, + 'src/element.ts': 'export class Element {\n groupIds: string[] = [];\n filter() { return this; }\n}\n', + 'src/use.ts': + "import { Element } from './element';\n" + + "export function without(groupIds: Element['groupIds'], id: string) {\n" + + ' return groupIds.filter((g) => g !== id);\n' + + '}\n', + }); + expect(calls.filter((c) => c.startsWith('groupIds.'))).toEqual([]); + }, 60_000); + + it("links to the class the property's file imports, when several share its name", async () => { + // vscode's `disposables: ComponentFixtureContext['disposableStore']` + // reached copilot's vendored `DisposableStore`, not the one the + // fixture context's file imports. + const editors = { + 'src/one/editor.ts': 'export class CodeEditor {\n executeEdits() { return 1; }\n}\n', + 'src/two/editor.ts': 'export class CodeEditor {\n executeEdits() { return 2; }\n}\n', + }; + const calls = await callsOf({ + ...CLASSES, + ...editors, + 'src/widget-one.ts': "import { CodeEditor } from './one/editor';\nexport interface WidgetOne {\n readonly inputEditor: CodeEditor;\n}\n", + 'src/widget-two.ts': "import { CodeEditor } from './two/editor';\nexport interface WidgetTwo {\n readonly inputEditor: CodeEditor;\n}\n", + 'src/use-one.ts': + "import { WidgetOne } from './widget-one';\n" + + "export function edit(ed: WidgetOne['inputEditor']) {\n return ed.executeEdits();\n}\n", + 'src/use-two.ts': + "import { WidgetTwo } from './widget-two';\n" + + "export function edit(ed: WidgetTwo['inputEditor']) {\n return ed.executeEdits();\n}\n", + }, true); + expect(calls.filter((c) => c.includes('ed.'))).toEqual([ + 'src/use-one.ts: ed.executeEdits -> src/one/editor.ts::CodeEditor::executeEdits', + 'src/use-two.ts: ed.executeEdits -> src/two/editor.ts::CodeEditor::executeEdits', + ]); + }, 60_000); + + it('reads the property off the indexed type the call\'s file imports, and a class its file declares', async () => { + // Two `Widget`s, each declaring its own `CodeEditor` beside it. + const widget = (n: number) => + `export class CodeEditor {\n executeEdits() { return ${n}; }\n}\n` + + 'export interface Widget {\n readonly inputEditor: CodeEditor;\n}\n'; + const calls = await callsOf({ + ...CLASSES, + 'src/a/widget.ts': widget(1), + 'src/b/widget.ts': widget(2), + 'src/use-a.ts': + "import { Widget } from './a/widget';\n" + + "export function edit(ed: Widget['inputEditor']) {\n return ed.executeEdits();\n}\n", + 'src/use-b.ts': + "import type { Widget } from './b/widget';\n" + + "export function edit(ed: Widget['inputEditor']) {\n return ed.executeEdits();\n}\n", + }, true); + expect(calls.filter((c) => c.includes('ed.'))).toEqual([ + 'src/use-a.ts: ed.executeEdits -> src/a/widget.ts::CodeEditor::executeEdits', + 'src/use-b.ts: ed.executeEdits -> src/b/widget.ts::CodeEditor::executeEdits', + ]); + }, 60_000); + + it("reads one whose property's type isn't written there, a getter's, as the type it indexes, as before", async () => { + // vscode's `let mockPty: { onCommandExecuted: Emitter['event'] }` + // sits nearer `onCommandExecuted.fire(…)` than the local's own `let + // onCommandExecuted: Emitter`; `event` is a getter of `Emitter`, and + // the `event: T` of its `fire(event: T)` names no type. + const calls = await callsOf({ + ...CLASSES, + 'src/emitter.ts': + 'export class Emitter {\n' + + ' get event() { return (listener: (e: T) => void) => listener; }\n' + + ' fire(event: T) { return event; }\n' + + '}\n', + 'src/use.test.ts': + "import { Emitter } from './emitter';\n" + + 'declare function suite(name: string, body: () => void): void;\n' + + "suite('pty', () => {\n" + + ' let onDone: Emitter;\n' + + ' let pty: {\n' + + " onDone: Emitter['event'];\n" + + ' };\n' + + " suite('fires', () => {\n" + + ' onDone.fire(1);\n' + + ' });\n' + + '});\n', + }); + expect(calls.filter((c) => c.startsWith('onDone.'))).toEqual(['onDone.fire -> Emitter::fire']); + }, 60_000); + + it("is not matched to a method by the call's name when the property's type isn't written in the project", async () => { + // vscode's `invocation: Parameters[0]`: + // left untyped, the receiver's name alone would pick `Logger.info`. + const calls = await callsOf({ + ...CLASSES, + 'src/logger.ts': 'export class Logger {\n info(message: string) { return message; }\n}\n', + 'src/use.ts': + "import { Logger } from './logger';\n" + + 'function make(logger: Logger) { return logger; }\n' + + 'export function log(logger: Parameters[0]) {\n' + + " return logger.info('x');\n" + + '}\n', + }); + expect(calls.filter((c) => c.startsWith('logger.'))).toEqual([]); + }, 60_000); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 5bb2175e8..fa463210e 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -13112,7 +13112,7 @@ function buildLocalReceiverTypePatterns(language: Language, r: string): RegExp[] // A type followed by brackets is captured whole, so matchMethodCall // can tell an array of it (`items: Item[]`, `items: Item[]`), whose // own methods aren't an element's (arrayElementType), from one of its - // properties (`groupIds: Element["groupIds"]`). + // properties (`groupIds: Element["groupIds"]`), which isn't it. new RegExp(`\\b${r}\\b\\s*:\\s*([A-Z][\\w.$]*(?:(?:\\s*<[^;=(){}<>]*(?:<[^;=(){}<>]*>[^;=(){}<>]*)*>)?\\s*\\[[^\\]]*\\])?)`), // lg: Logger (annotation or typed param) ]; case 'python': @@ -14259,12 +14259,35 @@ export function matchMethodCall( // declares, where `distinctElements.find(…)` on vscode's `ExplorerItem[]` // reached `ExplorerItem.find`; any other is called on one of its elements // under the same name, as in `d.forEach(d => d.dispose())`, so it takes - // the element's type. A type's property (`Emitter['event']`, - // `Parameters[0]`) is still read as the type. + // the element's type. One of a type's properties (`editor: + // IChatWidget['inputEditor']`) has the type that property is declared + // with, an array of it the same way, where the type it indexes reached + // vscode's `ISessionChatPillVisibilityService.isVisible` through + // `['pullRequests']`. That type decides alone, as a `this.` + // call's does (matchTsThisFieldCall): the method is its, or nothing the + // method's name would guess. Its class is the one the declaring file + // imports, as a typed local's is the one the call's file imports: + // vscode's `ComponentFixtureContext['disposableStore']` reached + // copilot's vendored `DisposableStore`. A property whose type isn't + // written there (a getter like `Emitter`'s `event`, `typeof`), or a key + // that isn't a name (`Parameters[0]`), says nothing new, and + // is read as the type it indexes, as before: the nearest line declaring + // the name may be another variable's type's member, as vscode's `let + // mockPty: { onCommandExecuted: Emitter['event'] }` is above calls on + // the local `onCommandExecuted`, an `Emitter`. if (inferredType && ESM_FAMILY.has(ref.language) && inferredType.endsWith(']')) { const element = arrayElementType(inferredType); - if (element !== undefined && JS_ARRAY_METHODS.has(methodName!)) return null; - inferredType = element ?? inferredType.replace(/\s*\[[^\]]*\]$/, ''); + const declared = element !== undefined + ? { type: element, array: true, file: undefined } + : tsIndexedPropertyType(inferredType, ref, context); + if (!declared) { + inferredType = inferredType.replace(/\s*\[[^\]]*\]$/, ''); + } else { + if (declared.array && JS_ARRAY_METHODS.has(methodName!)) return null; + inferredType = normalizeInferredTypeName(declared.type); + if (!inferredType) return null; + if (element === undefined) return resolveMethodOnType(inferredType, methodName!, ref, context, 0.9, 'instance-method', declared.file); + } } if (inferredType) { // A Go local bound from a type assertion calls a method of the type Go @@ -15558,7 +15581,15 @@ function matchRustSelfFieldCall( * patterns each time, a third of all method-call matching on vscode. Derived * from file source: drops with clearNameMatcherMemos. */ -const TS_FIELD_DECL_MEMO = new WeakMap>(); +const TS_FIELD_DECL_MEMO = new WeakMap>(); +/** + * A TS field's declared type as tsFieldDeclaration reads it: the type it + * names (`Mailer` of `mailer: Mailer`), whether that is a value's + * (`typeof X`), and whether the field holds an array of it (`items: Item[]`). + */ +type TsFieldDecl = { valueType: boolean; type: string; array?: boolean }; +/** What follows a field's type when the field holds an array of it: `[]`, after any type arguments. */ +const TS_ARRAY_SUFFIX = /^\s*(?:<[^;=(){}<>]*(?:<[^;=(){}<>]*>[^;=(){}<>]*)*>)?\s*\[\s*\]/; /** A class's comment-stripped lines, and which of them hold each `[\w$#]` token. */ interface TsClassDecl { lines: string[]; @@ -15612,7 +15643,7 @@ const TS_FIELD_PATTERNS_AT: readonly TsFieldPattern[] = [ ]; /** The first declaration of `field` on one line, as `line.match` of its patterns in order would find it. */ -function tsFieldOnLine(line: string, field: string): { valueType: boolean; type: string } | null { +function tsFieldOnLine(line: string, field: string): TsFieldDecl | null { let at: number[] | null = null; for (let i = line.indexOf(field); i !== -1; i = line.indexOf(field, i + 1)) { const end = i + field.length; @@ -15624,7 +15655,7 @@ function tsFieldOnLine(line: string, field: string): { valueType: boolean; type: for (const i of at) { re.lastIndex = i; const m = re.exec(line); - if (m && m[1]) return { valueType, type: m[1] }; + if (m && m[1]) return { valueType, type: m[1], array: TS_ARRAY_SUFFIX.test(line.slice(re.lastIndex)) }; } } return null; @@ -15687,7 +15718,7 @@ function tsFieldDeclaration( cls: Node, field: string, context: ResolutionContext -): { valueType: boolean; type: string } | null { +): TsFieldDecl | null { let memo = TS_FIELD_DECL_MEMO.get(context); if (!memo) { memo = new Map(); @@ -15696,7 +15727,7 @@ function tsFieldDeclaration( const key = cls.id + '\0' + field; const hit = memo.get(key); if (hit !== undefined) return hit; - let found: { valueType: boolean; type: string } | null = null; + let found: TsFieldDecl | null = null; const decl = tsClassDecl(cls, context); if (decl && TS_FIELD_TOKEN.test(field)) { // Every pattern needs the field as a whole `[\w$#]` run — the lookbehind @@ -15718,7 +15749,7 @@ function tsFieldDeclaration( for (const { re, valueType } of patterns) { const m = line.match(re); if (!m || !m[1]) continue; - found = { valueType, type: m[1] }; + found = { valueType, type: m[1], array: TS_ARRAY_SUFFIX.test(line.slice(m.index! + m[0].length)) }; break scan; } } @@ -15727,6 +15758,65 @@ function tsFieldDeclaration( return found; } +/** + * What a TS indexed type (`editor: IChatWidget['inputEditor']`) is: the type + * that property is declared with in the project class or interface it + * indexes, whether the property holds an array of it, and the file that + * says which class of that name it is. The indexed type is the one the + * call's file imports, when it imports one; the property's type is the one + * the declaring file imports, or else declares (tsImportedTypeFile). + * Undefined when the key isn't a string literal (`Parameters[0]`, + * `T[K]`), the indexed type isn't the project's, or the property's type + * isn't written there (a getter, `typeof`). + */ +function tsIndexedPropertyType( + indexed: string, + ref: UnresolvedRef, + context: ResolutionContext +): { type: string; array: boolean; file: string } | undefined { + const m = /^([A-Za-z_$][\w$]*)\s*\[\s*(['"])([\w$]+)\2\s*\]$/.exec(indexed); + if (!m) return undefined; + const imported = tsImportedTypeFile(m[1]!, ref, context); + const owners = preferCallSiteFile(context.getNodesByName(m[1]!), imported ?? ref.filePath).filter((n) => + (n.kind === 'class' || n.kind === 'interface' || n.kind === 'type_alias' || n.kind === 'component') && + sameLanguageFamily(n.language, ref.language)); + for (const owner of owners) { + const decl = tsFieldDeclaration(owner, m[3]!, context); + if (!decl) continue; + // A type parameter of the class names no type: `Emitter`'s + // `fire(event: T)`, which the reader takes for its `event`. + if (decl.valueType || tsClassTypeParameter(owner, decl.type, context)) return undefined; + const at = owner.filePath === ref.filePath ? ref : { ...ref, filePath: owner.filePath, language: owner.language }; + return { type: decl.type, array: decl.array === true, file: tsImportedTypeFile(decl.type, at, context) ?? owner.filePath }; + } + return undefined; +} + +/** Is `name` one of the type parameters the head of the TS class or interface `cls` declares (`class Emitter`)? */ +function tsClassTypeParameter(cls: Node, name: string, context: ResolutionContext): boolean { + const lines = context.getFileLines?.(cls.filePath) ?? context.readFile(cls.filePath)?.split(/\r?\n/) ?? []; + const head = lines.slice(cls.startLine - 1, cls.startLine + 2).join(' ').split('{')[0]!; + const open = new RegExp(`\\b${cls.name.replace(/\$/g, '\\$')}\\s*<`).exec(head); + if (!open) return false; + // The parameters between the `<` after the name and its own `>`, split + // at their top level: `T`, `K extends keyof T = never`. + let depth = 0; + let from = open.index + open[0].length; + for (let i = from; i < head.length; i++) { + const c = head[i]; + if (c === '<' || c === '(' || c === '[' || c === '{') depth++; + else if (c === ')' || c === ']' || c === '}') depth--; + else if (c === '>' && head[i - 1] !== '=') { + if (depth === 0) return head.slice(from, i).trim().replace(/^(?:in|out|const)\s+/, '').split(/[\s=]/)[0] === name; + depth--; + } else if (c === ',' && depth === 0) { + if (head.slice(from, i).trim().replace(/^(?:in|out|const)\s+/, '').split(/[\s=]/)[0] === name) return true; + from = i + 1; + } + } + return false; +} + /** * Resolve a TS/JS `this..()` call (#1496) through the field's * declared type, read off the ENCLOSING class's own declaration lines: