fix(ts): an array method on a local annotated as an array of a class links to nothing, not the class's method

CHANGELOG.md

@@ -81,6 +81,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.
+- 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.
 - `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.

__tests__/sync-typed-local-member-retry.test.ts

@@ -35,6 +35,10 @@ const CALLERS: Files = {
   'src/use-param.ts': "import { LRUCache } from './lru-cache';\nexport function c(cache: LRUCache<string, number>) {\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<string, number>(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",
+  // A call on an element that shares the array's name: the element's class.
+  'src/use-each.ts': "import { LRUCache } from './lru-cache';\nexport function h(cs: LRUCache<string, number>[]) {\n  cs.forEach((cs) => cs.get('k'));\n}\n",
+  // One of the class's properties, which resolution reads as the class.
+  'src/use-prop.ts': "import { LRUCache } from './lru-cache';\nexport function i(max: LRUCache<string, number>['max']) {\n  return max.toFixed(0);\n}\n",
   'src/noise.ts': NOISE,
 };
 
@@ -50,6 +54,7 @@ const LINKS = [
   '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',
+  'cs.get: calls src/use-each.ts -> method src/lru-cache.ts::LRUCache::get',
   's.get: calls src/use-default.ts -> method src/store.ts::Store::get',
 ];
 
@@ -171,9 +176,9 @@ describe('a failed member call on a typed local', () => {
       // 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.
+      // an imported class, whose array methods 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" +
@@ -212,6 +217,9 @@ describe('a failed member call on a typed local', () => {
       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/use-each.ts', 'cs.get')).toEqual(['module:lru-cache']);
+      expect(tails('src/use-each.ts', 'cs.forEach')).toEqual(['forEach']);
+      expect(tails('src/use-prop.ts', 'max.toFixed')).toEqual(['module:lru-cache']);
 
       expect(tails('src/other.ts', 'seen.get')).toEqual(['get']);
       expect(tails('src/other.ts', 'pkg.get')).toEqual(['get']);

__tests__/ts-array-annotated-receivers.test.ts

@@ -0,0 +1,135 @@
+/**
+ * A call on a TypeScript local or parameter annotated as an array of a class
+ * is the array's method, not the class's.
+ *
+ * `items: Item[]` was read as an `Item`, because the annotation's capture
+ * stopped at `[`, so `items.find(…)` linked to `Item.find` whenever the class
+ * declares a method of that name: on vscode, `distinctElements.find(…)` on
+ * an `ExplorerItem[]` reached `ExplorerItem.find`, and `ranges.map(…)` on an
+ * `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.
+ */
+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';
+
+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 = [];
+});
+
+/** Index `files` and return each call edge as `<refName> -> <target qualified name>`. */
+async function callsOf(files: Record<string, string>): Promise<string[]> {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ts-array-recv-'));
+  roots.push(root);
+  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);
+  }
+  const graph = await CodeGraph.init(root, { index: true });
+  graphs.push(graph);
+  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}`)
+    .sort();
+}
+
+const CLASSES = {
+  'package.json': JSON.stringify({ name: 'app' }),
+  'src/item.ts':
+    'export class Item {\n' +
+    '  find(key: string) { return key; }\n' +
+    '  push(value: number) { return value; }\n' +
+    '  flush() { return 0; }\n' +
+    '}\n' +
+    'export class Repository<T> {\n' +
+    '  save(value: T) { return value; }\n' +
+    '  find(id: string) { return id; }\n' +
+    '}\n',
+  // The one project method named `findLast`, on an unrelated class.
+  'src/stack.ts': 'export class Stack {\n  findLast() { return 1; }\n}\n',
+};
+
+describe('a call on a TypeScript local annotated as an array of a class', () => {
+  it('links to no method of the element class', async () => {
+    const calls = await callsOf({
+      ...CLASSES,
+      'src/use.ts':
+        "import { Item } from './item';\n" +
+        'export function first(items: Item[]) {\n' +
+        "  return items.find((i) => i !== undefined);\n" +
+        '}\n' +
+        'export function fill() {\n' +
+        '  const list: Item[] = [];\n' +
+        '  list.push(new Item());\n' +
+        '  return list;\n' +
+        '}\n' +
+        'export function spaced(all: Item [], grid: Item[][]) {\n' +
+        "  all.find((i) => i !== undefined);\n" +
+        '  return grid.push([]);\n' +
+        '}\n',
+    });
+    expect(calls.filter((c) => /^(items|list|all|grid)\./.test(c))).toEqual([]);
+  }, 60_000);
+
+  it("is not matched to another class's method by the call's name instead", async () => {
+    const calls = await callsOf({
+      ...CLASSES,
+      'src/use.ts':
+        "import { Item } from './item';\n" +
+        'export function last(all: Item[]) {\n' +
+        '  return all.findLast((i) => i !== undefined);\n' +
+        '}\n',
+    });
+    expect(calls.filter((c) => c.startsWith('all.'))).toEqual([]);
+  }, 60_000);
+
+  it('does the same for an array of a generic class', async () => {
+    const calls = await callsOf({
+      ...CLASSES,
+      'src/use.ts':
+        "import { Repository } from './item';\n" +
+        'export function saveAll(repos: Repository<string>[], value: string) {\n' +
+        '  repos.forEach((r) => r.save(value));\n' +
+        "  return repos.find((r) => r !== undefined);\n" +
+        '}\n',
+    });
+    expect(calls.filter((c) => c.startsWith('repos.'))).toEqual([]);
+  }, 60_000);
+
+  it("still links a method arrays don't have to the element's class: the call is on an element of the same name", async () => {
+    // vscode's `d.forEach(d => d.dispose())` on an `IDisposable[]`, and
+    // `extensions.map(extensions => extensions.syncInstalledExtensionsWithGallery(…))`.
+    const calls = await callsOf({
+      ...CLASSES,
+      'src/use.ts':
+        "import { Item } from './item';\n" +
+        'export function flushAll(d: Item[]) {\n' +
+        '  d.forEach((d) => d.flush());\n' +
+        '}\n',
+    });
+    expect(calls.filter((c) => c.startsWith('d.'))).toEqual(['d.flush -> Item::flush']);
+  }, 60_000);
+
+  it('still links a call on a local annotated as the class, generic or not', async () => {
+    const calls = await callsOf({
+      ...CLASSES,
+      'src/use.ts':
+        "import { Item, Repository } from './item';\n" +
+        'export function one(item: Item, repo: Repository<string>) {\n' +
+        "  repo.find('x');\n" +
+        "  return item.find('k');\n" +
+        '}\n',
+    });
+    expect(calls).toEqual(['item.find -> Item::find', 'repo.find -> Repository::find']);
+  }, 60_000);
+});

src/resolution/import-resolver.ts

@@ -23,7 +23,9 @@ import {
   normalizeInferredTypeName,
   goRefQualifier,
   inferLocalReceiverType,
+  arrayElementType,
 } from './name-matcher';
+import { JS_ARRAY_METHODS } from './js-builtins';
 
 /**
  * Extension resolution order by language
@@ -3270,34 +3272,22 @@ export function pythonModulePath(source: string, fromFile: string): string | nul
  * 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<K,
- * V>(…)` 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.
+ * V>(…)` or `lib` of `new lib.LRUCache()`, of the element type of an array
+ * (`results: URI[]`), or of the type whose property it is (`Emitter<E>['event']`).
+ * Undefined for a type the file doesn't import, in a file that imports
+ * nothing, and for a method arrays have called on an array (`results.push(…)`):
+ * resolution links that to no project class, so no edit of the type's module
+ * does. 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<K, V>[]`. */
-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<K>[] = []`. */
-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 decl: { raw?: string } = {};
+  const type = inferLocalReceiverType(receiver, ref, context, decl);
+  if (!type || !decl.raw) return undefined;
+  if (arrayElementType(type) !== undefined && JS_ARRAY_METHODS.has(ref.referenceName.slice(receiver.length + 1))) return undefined;
+  const head = decl.raw.split(/[.<[\s]/)[0];
+  return mappings.find((m) => m.localName === head);
 }
 
 const IDENTIFIER = /^[A-Za-z_$][\w$]*$/;

src/resolution/js-builtins.ts

@@ -33,6 +33,21 @@ export const JS_BUILTIN_METHODS = new Set([
   'text', 'json', 'arrayBuffer', 'blob', 'formData',
 ]);
 
+/**
+ * The methods an array has: `Array.prototype`'s and those every object
+ * inherits. On a local annotated as an array (`items: Item[]`) a call of one
+ * is the array's, never a method of the element's class.
+ */
+export const JS_ARRAY_METHODS = new Set([
+  'at', 'concat', 'copyWithin', 'entries', 'every', 'fill', 'filter', 'find',
+  'findIndex', 'findLast', 'findLastIndex', 'flat', 'flatMap', 'forEach',
+  'includes', 'indexOf', 'join', 'keys', 'lastIndexOf', 'map', 'pop', 'push',
+  'reduce', 'reduceRight', 'reverse', 'shift', 'slice', 'some', 'sort', 'splice',
+  'toReversed', 'toSorted', 'toSpliced', 'unshift', 'values', 'with',
+  'toString', 'toLocaleString', 'valueOf', 'hasOwnProperty', 'isPrototypeOf',
+  'propertyIsEnumerable',
+]);
+
 /**
  * TypeScript primitive type names. Distinct from JS_BUILT_INS on purpose: those
  * are runtime globals a receiver can be constructed from, these only ever come