diff --git a/CHANGELOG.md b/CHANGELOG.md index 37bfa094..b69f14ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Go, variables declared in a `var ( … )` block, like gin's `binding.JSON` or prometheus's metric counters, are now indexed, with their own doc comments. Before, every variable in such a block was missing from the graph: the calls and types in its value were not recorded, so a struct used only there looked unused, and a call made through it, like `fPointPool.Get()` or a counter's `.Inc()`, was matched by its method's name alone and could link to an unrelated type's method. Re-index Go projects after upgrading. - Indexing a project that includes large bundled JavaScript files, such as a copy of pdf.js or d3, is fast again: since 1.6.2, resolving the calls in a JavaScript or TypeScript file re-read the file's text above each call, so a single bundled library could add many seconds to an index. The graph it builds is unchanged. Thanks @bompus for the report. (#2334) - Indexing a project that ships a large minified JavaScript file, such as the GraphiQL bundle inside go-ethereum, no longer stalls at "Resolving refs" when CodeGraph runs on Node 22, and linking such a file is faster on any Node version. The graph it builds is unchanged. +- Indexing JavaScript and TypeScript projects is a little faster when their code calls methods the project doesn't define, like `list.push(…)`, `fs.writeFileSync(…)` or a test library's matchers: CodeGraph now sees from the names it has indexed that nothing in the project carries such a method, and no longer reads the calling code to work out what the call is made on. The graph it builds is unchanged. Thanks @luojiyin1987 for the report and the analysis. (#2829) - The time `codegraph init` and `codegraph index` print beside the node and edge counts now covers the whole run, resolving references and linking included, and `codegraph sync` reports its whole run the same way. Before, it counted only reading and parsing the files, which can be a small part of an index, so a slow index looked fast. Thanks @bompus for the report. (#2334) - The call paths `codegraph context` prints now say what bridges each dynamic-dispatch hop, in the same words `codegraph_explore` uses: a call through an interface reads `interface → impl`, and a C++ override, a Redux thunk or a Gin middleware chain is named as one. Before, every hop of a kind it didn't know was labeled an "event", so a call through an interface looked like an event being emitted. - In Rust, code that uses an enum only through its variants, like `mode::Mode::A` in an expression, a `Mode::B =>` match arm or `Self::A` inside the enum's own `impl`, now shows up in that enum's callers and impact, linked to the enum that actually declares the variant rather than to a same-named type elsewhere. Standard-library variants like `Ordering::Less` or `Option::Some` and associated items like `Foo::new()` or `Foo::MAX` don't count as using a project enum. Re-index Rust projects after upgrading. Thanks @mg-mg-mg for the report and @danusha2345. (#2328) diff --git a/__tests__/absent-member-calls.test.ts b/__tests__/absent-member-calls.test.ts new file mode 100644 index 00000000..f3ff961a --- /dev/null +++ b/__tests__/absent-member-calls.test.ts @@ -0,0 +1,269 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { DatabaseConnection } from '../src/db'; +import { QueryBuilder } from '../src/db/queries'; +import { createResolver, type ReferenceResolver } from '../src/resolution'; +import { matchMethodCall } from '../src/resolution/name-matcher'; +import type { ResolutionContext, UnresolvedRef } from '../src/resolution/types'; +import type { Node } from '../src/types'; + +/** + * #2829: a JS/TS call of a member that no node in the index is named — a + * library's `list.push()`, `fs.writeFileSync()`, a test framework's matcher — + * can't be linked by any strategy in matchMethodCall that looks the member up + * by its own name, so the matcher no longer reads the calling file to work + * out the receiver's type for it. The one exception is the object-literal + * reader, which follows a member to the binding the literal gives it. These + * tests count the work the skip saves, pin every way a call reaches a symbol + * of another name, and check a whole index against one made without the skip. + */ + +/** A context holding no nodes, which counts every read of the calling file. */ +function countingContext(reads: { n: number }, isAbsentName?: (name: string) => boolean): ResolutionContext { + const source = ['const svc = createService();', 'svc.absent();']; + return { + getNodesInFile: () => [], + getNodesByName: () => [], + getNodesByQualifiedName: () => [], + getNodesByKind: () => [], + getNodesByLowerName: () => [], + getImportMappings: () => [], + fileExists: () => false, + getProjectRoot: () => '/', + getAllFiles: () => [], + readFile: () => { + reads.n++; + return source.join('\n'); + }, + getFileLines: () => { + reads.n++; + return source; + }, + ...(isAbsentName ? { isAbsentName } : {}), + }; +} + +const absentCall: UnresolvedRef = { + fromNodeId: 'function:caller', + referenceName: 'svc.absent', + referenceKind: 'calls', + line: 2, + column: 0, + filePath: 'caller.ts', + language: 'typescript', +}; + +describe('A call of a member no node is named (#2829)', () => { + const dirs: string[] = []; + const graphs: CodeGraph[] = []; + + afterEach(() => { + for (const cg of graphs.splice(0)) cg.close(); + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true, maxRetries: 5 }); + }); + + function tempDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-absent-member-')); + dirs.push(dir); + return dir; + } + + it('reads nothing of the calling file when no node has the member\'s name', () => { + const reads = { n: 0 }; + expect(matchMethodCall(absentCall, countingContext(reads, () => true))).toBeNull(); + expect(reads.n).toBe(0); + }); + + it('still reads it when a node has the name, or nothing says whether one does', () => { + // The same call, with the name known: the receiver is read as before, so + // the count above is the skip's doing. + const known = { n: 0 }; + expect(matchMethodCall(absentCall, countingContext(known, () => false))).toBeNull(); + expect(known.n).toBeGreaterThan(0); + // A context that can't answer never skips. + const unknown = { n: 0 }; + expect(matchMethodCall(absentCall, countingContext(unknown))).toBeNull(); + expect(unknown.n).toBeGreaterThan(0); + }); + + it('answers absence only from a loaded set of names', () => { + const dir = tempDir(); + const db = DatabaseConnection.initialize(path.join(dir, 'names.db')); + try { + const queries = new QueryBuilder(db.getDb()); + const node: Node = { + id: 'method:Service::known', kind: 'method', name: 'known', qualifiedName: 'Service::known', + filePath: 'service.ts', language: 'typescript', startLine: 1, endLine: 2, startColumn: 0, endColumn: 0, + updatedAt: Date.now(), + }; + queries.insertNodes([node]); + const resolver = createResolver(dir, queries); + const context = resolver.getResolutionContext(); + + // Not loaded yet: no name is absent, so no caller skips on a guess. + expect(context.isAbsentName?.('absent')).toBe(false); + resolver.warmCaches(); + expect(context.isAbsentName?.('known')).toBe(false); + expect(context.isAbsentName?.('absent')).toBe(true); + // The name set is exact: case and a qualified name are other names. + expect(context.isAbsentName?.('Known')).toBe(true); + expect(context.isAbsentName?.('Service::known')).toBe(true); + // Dropped with the other caches, as nodes may change after. + resolver.clearCaches(); + expect(context.isAbsentName?.('absent')).toBe(false); + } finally { + db.close(); + } + }); + + /** + * Calls whose member no node is named, beside calls whose member is known: + * a literal's member bound to another function (#1932), a shorthand member + * bound to a renamed import, an `as const` literal, a literal typed + * `Record<…>` (whose answer the type decides before the literal is read), + * a literal imported from another file, a renamed re-export read through a + * namespace import, and library calls through a parameter, a field and a + * global. + */ + function writeFixture(dir: string): void { + const files: Record = { + 'a.ts': [ + "import { fetchUser as getUser } from './users';", + "import * as ns from './barrel';", + "import { holder } from './holder';", + 'function renamed() { return 4; }', + 'function renamedConst() { return 3; }', + 'export const api = { alias: renamed, getUser };', + 'const constApi = { aliasC: renamedConst } as const;', + 'const typedApi: Record number> = { aliasT: renamedConst };', + 'export function sameFileCaller(list: number[]) {', + ' list.push(1);', + ' return [api.alias(), api.getUser(), constApi.aliasC(), typedApi.aliasT(), ns.run(), holder.aliasH()];', + '}', + '', + ].join('\n'), + 'users.ts': 'export function fetchUser() { return 1; }\n', + 'impl.ts': 'export function implementation() { return 2; }\n', + 'barrel.ts': "export { implementation as run } from './impl';\n", + 'holder.ts': 'function renamedHolder() { return 2; }\nexport const holder = { aliasH: renamedHolder };\n', + 'service.ts': [ + 'export class Store { save(): number { return 1; } }', + 'export class Service {', + ' private store = new Store();', + ' private items: string[] = [];', + ' work(): number {', + " this.items.push('x');", + ' return this.store.save();', + ' }', + '}', + 'export function useService(s: Service, store: Store) {', + ' store.save();', + ' s.missing();', + " console.log('x');", + ' return s.work();', + '}', + '', + ].join('\n'), + 'local.js': [ + 'function impl() { return 5; }', + 'const local = { start: impl };', + 'function useLocal() { return [local.start(), local.stop(), Math.max(1, 2)]; }', + 'module.exports = { useLocal };', + '', + ].join('\n'), + }; + for (const [file, content] of Object.entries(files)) fs.writeFileSync(path.join(dir, file), content); + } + + /** Index the fixture; `isAbsentName` replaces the resolver's own answer when given. */ + async function indexFixture(isAbsentName?: (original: ResolutionContext['isAbsentName']) => ResolutionContext['isAbsentName']) { + const dir = tempDir(); + writeFixture(dir); + const cg = CodeGraph.initSync(dir); + graphs.push(cg); + const context = (cg as unknown as { resolver: ReferenceResolver }).resolver.getResolutionContext(); + if (isAbsentName) context.isAbsentName = isAbsentName(context.isAbsentName); + await cg.indexAll(); + return { cg, dir }; + } + + const callers = (cg: CodeGraph, name: string): string[] => { + const target = cg.getNodesByName(name).find((n) => n.kind === 'function'); + expect(target, name).toBeDefined(); + return cg.getIncomingEdges(target!.id) + .filter((e) => e.kind === 'calls') + .map((e) => cg.getNode(e.source)!.name) + .sort(); + }; + + it('keeps every way a call reaches a symbol of another name', async () => { + const { cg } = await indexFixture(); + // Each call below names a member no node is named, so the skip sees it. + for (const member of ['alias', 'getUser', 'aliasC', 'aliasT', 'run', 'aliasH', 'start', 'stop', 'push', 'missing']) { + expect(cg.getNodesByName(member), member).toEqual([]); + } + // The literal reader: `{ alias: renamed }`, `{ getUser }` bound to a + // renamed import, an `as const` literal, a holder in its own file. + expect(callers(cg, 'renamed')).toEqual(['sameFileCaller']); + expect(callers(cg, 'fetchUser')).toEqual(['sameFileCaller']); + expect(callers(cg, 'renamedConst')).toEqual(['sameFileCaller']); + expect(callers(cg, 'impl')).toEqual(['useLocal']); + // The import resolver: an imported literal's member, and a re-export + // renamed in a barrel read through a namespace import. + expect(callers(cg, 'renamedHolder')).toEqual(['sameFileCaller']); + expect(callers(cg, 'implementation')).toEqual(['sameFileCaller']); + }, 60_000); + + it('sees a method that a file indexed after a resolution pass declares', async () => { + // indexAll leaves the resolver's name set loaded; indexFiles then adds a + // method a new call names. The call is resolved against the new names, + // as it is without the skip. + const dir = tempDir(); + fs.writeFileSync(path.join(dir, 'store.ts'), 'export class Store { save(): number { return 1; } }\n'); + fs.writeFileSync(path.join(dir, 'main.ts'), "import { Store } from './store';\nexport function useStore(store: Store) { return store.save(); }\n"); + const cg = CodeGraph.initSync(dir); + graphs.push(cg); + await cg.indexAll(); + fs.writeFileSync(path.join(dir, 'store.ts'), 'export class Store {\n save(): number { return 1; }\n restock(): number { return 2; }\n}\n'); + fs.writeFileSync(path.join(dir, 'caller.ts'), "import { Store } from './store';\nexport function useRestock(store: Store) { return store.restock(); }\n"); + await cg.indexFiles(['store.ts', 'caller.ts']); + cg.resolveReferences(); + const restock = cg.getNodesByName('restock').find((n) => n.kind === 'method'); + expect(restock).toBeDefined(); + expect(cg.getIncomingEdges(restock!.id).filter((e) => e.kind === 'calls').map((e) => cg.getNode(e.source)!.name)) + .toEqual(['useRestock']); + }, 60_000); + + it('indexes the fixture exactly as it does without the skip', async () => { + let absent = 0; + const withSkip = await indexFixture((original) => (name) => { + const answer = original?.(name) ?? false; + if (answer) absent++; + return answer; + }); + const withoutSkip = await indexFixture(() => undefined); + // The skip was asked about the calls above and answered for some of them. + expect(absent).toBeGreaterThan(0); + expect(dumpGraph(withSkip.dir)).toEqual(dumpGraph(withoutSkip.dir)); + }, 60_000); +}); + +/** Edges and parked references by natural keys, sorted, as scripts/dump-graph.mjs dumps them. */ +function dumpGraph(dir: string): string[] { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { DatabaseSync } = require('node:sqlite') as typeof import('node:sqlite'); + const db = new DatabaseSync(path.join(dir, '.codegraph', 'codegraph.db'), { readOnly: true }); + try { + const rows = [ + ...db.prepare(`SELECT s.qualified_name AS source, t.qualified_name AS target, e.kind, e.line, e.col, e.metadata + FROM edges e JOIN nodes s ON s.id = e.source JOIN nodes t ON t.id = e.target`).all(), + ...db.prepare(`SELECT from_node_id, reference_name, reference_kind, line, col, status FROM unresolved_refs`).all(), + ]; + return rows.map((r) => JSON.stringify(r)).sort(); + } finally { + db.close(); + } +} diff --git a/src/index.ts b/src/index.ts index 0a16f6f7..c135aa99 100644 --- a/src/index.ts +++ b/src/index.ts @@ -884,7 +884,11 @@ export class CodeGraph { return { success: false, filesIndexed: 0, filesSkipped: 0, filesErrored: 0, nodesCreated: 0, edgesCreated: 0, errors: [{ message: 'Could not acquire file lock - another process may be indexing', severity: 'error' as const }], durationMs: 0 }; } try { - return this.orchestrator.indexFiles(filePaths); + // The files' nodes are new to the resolver: a resolution after this + // reloads its caches, the set of node names included, as indexAll and + // sync do before they resolve, instead of reading the graph as it was + // before (#2829). + return this.orchestrator.indexFiles(filePaths).finally(() => this.resolver.clearCaches()); } finally { this.fileLock.release(); } diff --git a/src/resolution/index.ts b/src/resolution/index.ts index b95b2660..1df37f6f 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -774,6 +774,10 @@ export class ReferenceResolver { return result; }, + // The set warmCaches loads (`SELECT DISTINCT name FROM nodes`, the exact + // match getNodesByName makes); unloaded, no name is absent. + isAbsentName: (name: string) => this.knownNames !== null && !this.knownNames.has(name), + getMethodMatches: (typeName: string, methodName: string, language: Language) => { const key = `${language} ${typeName}::${methodName}`; const cached = this.methodMatchCache.get(key); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 1eb0a6a6..4e1f975d 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -19728,6 +19728,22 @@ export function matchMethodCall( } const [, objectOrClass, methodName] = match; + + // A JS/TS call of a member no node in the index is named — `list.push()`, + // `fs.writeFileSync()`, a test library's matcher — is one only the + // object-literal reader below can link (#2829). It follows a member to the + // binding the literal gives it: `const api = { alias: renamed }` makes + // `api.alias()` a call of `renamed` (#1932). Every other strategy looks the + // member up by its own name and finds nothing, so reading the calling file + // for the receiver's type is wasted work. When the reader answers, the + // whole path below runs and decides, as before. + if (ref.referenceKind === 'calls' && JS_FAMILY.has(ref.language) && context.isAbsentName?.(methodName!)) { + const literal = nmTimedT('mc-absent', ref, () => dotMatch && !objectOrClass!.includes('.') + ? resolveObjectPathMember(objectOrClass!, methodName!, ref, context, null) + : null); + if (!literal) return null; + } + // A simple `receiver.method` / `receiver:method` / `receiver$method` shape whose // receiver type we can try to infer from its local declaration. // A python attribute receiver (`self.conn`, `cfg.client`) is not one: the diff --git a/src/resolution/types.ts b/src/resolution/types.ts index d5d5dcb6..f580b03f 100644 --- a/src/resolution/types.ts +++ b/src/resolution/types.ts @@ -101,6 +101,15 @@ export interface ResolutionContext { getNodesInFileNamed?(filePath: string, name: string): Node[]; /** Get all nodes by name */ getNodesByName(name: string): Node[]; + /** + * Whether no node in the index has exactly this name, answered from the + * resolver's set of every node name rather than a query (#2829). False + * whenever that set isn't loaded (before `warmCaches`, after + * `clearCaches`), so a caller only ever acts on a certain absence. A view + * that hides nodes keeps its base's answer, which stays certain: hiding + * nodes adds no name. Optional: a context without it never answers. + */ + isAbsentName?(name: string): boolean; /** Get all nodes by qualified name */ getNodesByQualifiedName(qualifiedName: string): Node[]; /** Get all nodes of a kind */