diff --git a/CHANGELOG.md b/CHANGELOG.md index 30f4a0618..7571cc64c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - `codegraph sync` and the file watcher now link a Shopify section or snippet to the files that name it when it is added after them, or comes back after being deleted, whether they name it in a `{% render %}`, `{% include %}` or `{% section %}` tag or as a section `type` in a JSON template. Before, the link appeared only once a file naming it changed or the project was re-indexed, even in a repository with a single theme, so the new section or snippet looked unused and `codegraph affected` missed the files that use it. Re-index Shopify themes after upgrading to pick up links an earlier version missed. - `codegraph sync` and the file watcher now link a route to the page or layout it loads lazily when that file is added after the router was indexed, comes back after being deleted, or gets the component the route renders in a later edit, whether or not the import names the file's extension: React Router's `lazy: () => import('./pages/Team')`, Vue Router's `component: () => import('@/views/Login')` and Angular's `loadComponent: () => import('./home/home.component')`. Before, the route stayed unlinked until the router file changed or the project was re-indexed, so the page looked unused and `codegraph affected` missed the tests that reach it through the router. Re-index after upgrading to pick up links an earlier version missed. - A file named the way Google names its tests, like `wire_format_unittest.cc` in protocolbuffers/protobuf, `logging_unittest.cc` in google/glog or a Python `foo_unittest.py`, now counts as a test, as `wire_format_test.cc` already did. Before, it was taken for production code: search and `codegraph_explore` ranked it alongside the code it tests, `codegraph affected` never listed it, and calls in production code could link to a class or type a unittest declares for itself, as google/breakpad's `stack_frame_entries_.size()` did to a unittest's `StackHelper`. Re-index projects that have such files after upgrading. +- `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. - In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading. - In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. Re-index C++ projects after upgrading. - In C++, a call on a variable, parameter or member declared with template arguments nested inside its template arguments, like `autovector>` or `Striped>`, now links to the method of the class it is declared as. Before, the nested arguments hid that class: in RocksDB, `files_marked_for_compaction_.clear()` on such an `autovector` linked to an unrelated `CompactionInputFiles::clear` that only shares words with the variable's name, and `mutex_.Get(key)` on a `Striped` linked to nothing. Re-index C++ projects after upgrading. diff --git a/__tests__/sync-import-binding-retry.test.ts b/__tests__/sync-import-binding-retry.test.ts new file mode 100644 index 000000000..c34f4f1cb --- /dev/null +++ b/__tests__/sync-import-binding-retry.test.ts @@ -0,0 +1,338 @@ +/** + * A sync links a reference through an import binding once the module appears. + * + * `import tagsController from './tag/tag.controller'` binds a name the module + * never declares: its default export is a `router`. So does a namespace import + * and an aliased one (`import { Component as GroupsPageWrapper }`). Every + * reference through such a name — the import itself, a call, `new`, a base + * class, a JSX or Vue template tag, an Express mount — resolves only through + * the module. Sync retries a parked failed ref by its tail (#1240), and these + * were parked under the binding's own name, which neither the module's file + * nor its symbols carry. So a module added after its importers were indexed, + * one restored after a delete, or one that gained its export in a later edit + * stayed unlinked from them until each importer changed or the project was + * indexed again. #2392 fixed the same gap for the import of the file itself. + * + * Such a reference is now parked under its module's key, and still found by + * 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. + */ +import { describe, it, expect, afterEach, vi } 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'; +import { CURRENT_SCHEMA_VERSION, getCurrentVersion, runMigrations } from '../src/db/migrations'; +import { QueryBuilder } from '../src/db/queries'; +import { moduleReferenceKeys, moduleTail } from '../src/db/reference-tail'; + +type Files = Record; + +/** The files that reach a module through a binding the module doesn't declare by that name. */ +const IMPORTERS: Files = { + 'package.json': JSON.stringify({ name: 'app', dependencies: { express: '^4', vue: '^3', svelte: '^4', astro: '^4' } }), + 'tsconfig.json': JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '@/*': ['src/*'] } } }), + // A default import of a module whose default export has another name (ts-express). + 'src/routes/routes.ts': "import tagsController from './tag/tag.controller';\nexport const routes = [tagsController];\n", + // An aliased import through a tsconfig alias (luci-go's milo). + 'src/pages/groups_page.test.tsx': "import { Component as GroupsPageWrapper } from '@/pages/groups_page';\nexport const page = ;\n", + 'src/use-ns.ts': "import * as NS from './ns';\nexport const all = NS;\n", + 'src/call.ts': "import runIt from './handler';\nexport function go() { return runIt(); }\n", + 'src/draw.ts': "import { Widget as W } from './widgets/Widget';\nexport function paint() { return new W(); }\n", + 'src/admin.ts': "import Base from './base';\nexport class Admin extends Base {}\n", + // An Express router mounted through a default import (proshop). + 'backend/server.js': "import express from 'express';\nimport uploadRoutes from './routes/uploadRoutes.js';\nconst app = express();\napp.use('/api/upload', uploadRoutes);\nexport default app;\n", + // A Vue component imported through an alias and rendered by the template (vue3-element-admin). + 'src/layouts/LayoutMain.vue': "\n\n", + 'src/App.svelte': "\n\n", + 'src/pages/index.astro': "---\nimport Shell from '../layouts/BaseLayout.astro';\n---\n

Home

\n", +}; + +const MODULES: Files = { + 'src/routes/tag/tag.controller.ts': 'const router = { list() { return []; } };\nexport default router;\n', + 'src/pages/groups_page.tsx': 'export function Component() { return null; }\n', + 'src/ns.ts': 'export const member = 1;\n', + 'src/handler.ts': 'export default function handle() { return 1; }\n', + 'src/widgets/Widget.ts': 'export class Widget {}\n', + 'src/base.ts': 'export default class BaseUser {}\n', + 'backend/routes/uploadRoutes.js': "import express from 'express';\nconst router = express.Router();\nrouter.post('/', (req, res) => res.send('ok'));\nexport default router;\n", + 'src/views/error/404.vue': '\n', + 'src/lib/Hero.svelte': '

Hero

\n', + 'src/layouts/BaseLayout.astro': '\n', +}; + +/** What a full index links through the bindings: `: -> ::`. */ +const LINKS = [ + 'Banner: imports file src/App.svelte -> component src/lib/Hero.svelte::Hero', + 'Banner: references component src/App.svelte -> component src/lib/Hero.svelte::Hero', + 'Base: extends class src/admin.ts -> class src/base.ts::BaseUser', + 'Base: imports file src/admin.ts -> file src/base.ts::base.ts', + 'Error404: imports file src/layouts/LayoutMain.vue -> component src/views/error/404.vue::404', + 'Error404: references component src/layouts/LayoutMain.vue -> component src/views/error/404.vue::404', + 'GroupsPageWrapper: imports file src/pages/groups_page.test.tsx -> function src/pages/groups_page.tsx::Component', + 'NS: imports file src/use-ns.ts -> file src/ns.ts::ns.ts', + 'Shell: imports file src/pages/index.astro -> component src/layouts/BaseLayout.astro::BaseLayout', + 'Shell: references component src/pages/index.astro -> component src/layouts/BaseLayout.astro::BaseLayout', + 'W: imports file src/draw.ts -> class src/widgets/Widget.ts::Widget', + 'W: instantiates function src/draw.ts -> class src/widgets/Widget.ts::Widget', + 'runIt: calls function src/call.ts -> function src/handler.ts::handle', + 'runIt: imports file src/call.ts -> file src/handler.ts::handler.ts', + 'tagsController: imports file src/routes/routes.ts -> file src/routes/tag/tag.controller.ts::tag.controller.ts', + 'uploadRoutes: imports file backend/server.js -> file backend/routes/uploadRoutes.js::uploadRoutes.js', + 'uploadRoutes: references route backend/server.js -> constant backend/routes/uploadRoutes.js::router', +]; + +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-binding-')); + 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 {@link LINKS} the graph has. */ +function linksOf(graph: CodeGraph): string[] { + const names = new Set(LINKS.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)) { + const ref = edge.metadata?.refName; + if (typeof ref !== 'string' || !names.has(ref)) continue; + const source = graph.getNode(edge.source)!; + const target = graph.getNode(edge.target)!; + links.push(`${ref}: ${edge.kind} ${source.kind} ${source.filePath} -> ${target.kind} ${target.filePath}::${target.name}`); + } + 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)); +} + +/** A module before the edit that gives it what its importers import. */ +const stub = (file: string): string => + file.endsWith('.vue') ? '\n' : /\.(svelte|astro)$/.test(file) ? '

\n' : 'export const placeholder = 1;\n'; + +describe('sync links references through an import binding to their module', () => { + it('when the module is added after its importers', async () => { + const root = tempRoot(); + write(root, IMPORTERS); + const graph = await index(root); + expect(linksOf(graph)).toEqual([]); + + write(root, MODULES); + const result = await graph.sync(); + // The importers 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(linksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + expect(graph.getPendingReferenceCount()).toBe(0); + }, 120_000); + + it('when the module comes back after it was deleted', async () => { + const root = tempRoot(); + write(root, { ...IMPORTERS, ...MODULES }); + const graph = await index(root); + expect(linksOf(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(linksOf(graph)).toEqual([]); + + write(root, MODULES); + expect((await graph.sync()).filesAdded).toBe(Object.keys(MODULES).length); + expect(linksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + }, 120_000); + + it('when an edit gives the module what its importers import', async () => { + const root = tempRoot(); + write(root, IMPORTERS); + write(root, Object.fromEntries(Object.keys(MODULES).map((file) => [file, stub(file)]))); + 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(linksOf(graph)).toEqual(LINKS); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...IMPORTERS, ...MODULES })); + }, 120_000); + + it('and by its own name when its module never resolves it', async () => { + // vben: the module doesn't declare the imported name, so a full index + // links the local name to the declaration that carries it. + const initial: Files = { + 'src/lib.ts': 'export const unrelated = 1;\n', + 'src/form.ts': "import type { VbenFormSchema as FormSchema } from './lib';\nexport type Schema = FormSchema;\n", + }; + const added: Files = { 'src/types.ts': 'export type FormSchema = { field: string };\n' }; + const root = tempRoot(); + write(root, initial); + const graph = await index(root); + + write(root, added); + expect((await graph.sync()).filesAdded).toBe(1); + const formSchema = edgesOf(graph).filter((edge) => edge.includes('"refName":"FormSchema"')); + expect(formSchema).toHaveLength(2); + expect(formSchema.every((edge) => edge.includes('-> type_alias src/types.ts:'))).toBe(true); + expect(edgesOf(graph)).toEqual(await fullIndexOf({ ...initial, ...added })); + }, 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, an unaliased named import and a member of a + // namespace import wait for the name they use. + '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", + }); + (await CodeGraph.init(root, { index: true })).close(); + + const db = DatabaseConnection.open(getDatabasePath(root)); + try { + const rows = db.getDb() + .prepare("SELECT reference_name AS name, reference_kind AS kind, file_path AS file, name_tail AS tail FROM unresolved_refs WHERE status = 'failed'") + .all() as Array<{ name: string; kind: string; file: string; tail: string }>; + const tailOf = (file: string, name: string, kind: string): string | undefined => + rows.find((r) => r.file === file && r.name === name && r.kind === kind)?.tail; + + expect(tailOf('src/routes/routes.ts', 'tagsController', 'imports')).toBe('module:tag'); + expect(tailOf('src/pages/groups_page.test.tsx', 'GroupsPageWrapper', 'imports')).toBe('module:groups_page'); + expect(tailOf('src/use-ns.ts', 'NS', 'imports')).toBe('module:ns'); + expect(tailOf('src/call.ts', 'runIt', 'calls')).toBe('module:handler'); + expect(tailOf('src/draw.ts', 'W', 'instantiates')).toBe('module:Widget'); + expect(tailOf('src/admin.ts', 'Base', 'extends')).toBe('module:base'); + expect(tailOf('backend/server.js', 'uploadRoutes', 'references')).toBe('module:uploadRoutes'); + 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', 'express', 'imports')).toBe('express'); + expect(tailOf('src/other.ts', 'helper', 'imports')).toBe('helper'); + expect(tailOf('src/other.ts', 'NS.make', 'calls')).toBe('make'); + + // Each module's file is one of the keys its references wait under. + 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'], + ]) { + expect(moduleReferenceKeys(file)).toContain(moduleTail(module)); + } + } finally { + db.close(); + } + }, 60_000); +}); + +describe('schema v15', () => { + let db: DatabaseConnection | undefined; + + afterEach(() => { + db?.close(); + db = undefined; + }); + + function fixture(): QueryBuilder { + const root = tempRoot(); + db = DatabaseConnection.initialize(path.join(root, 'test.db')); + const queries = new QueryBuilder(db.getDb()); + queries.insertNode({ id: 'f', kind: 'file', name: 'form.ts', qualifiedName: 'src/form.ts', filePath: 'src/form.ts', + language: 'typescript', startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0 }); + for (const [referenceName, referenceKind, tail, line] of [ + ['FormSchema', 'imports', 'module:lib', 1], + ['FormSchema', 'references', 'module:lib', 2], + ['tagsController', 'imports', 'module:tag', 3], + // A ref its name parks: the name lookup finds it by its tail already. + ['helper', 'calls', 'helper', 4], + ] as const) { + queries.insertUnresolvedRef({ fromNodeId: 'f', referenceName, referenceKind, line, column: 0, filePath: 'src/form.ts', language: 'typescript' }); + db.getDb().prepare("UPDATE unresolved_refs SET status = 'failed', name_tail = ? WHERE line = ?").run(tail, line); + } + return queries; + } + + const retried = (queries: QueryBuilder, names: string[], ceiling?: number) => + queries.getRetryableFailedReferences(names, ceiling).map((ref) => `${ref.referenceKind} ${ref.referenceName}`).sort(); + + it('finds a reference parked under its module by its module and by its own name', () => { + const queries = fixture(); + expect(retried(queries, moduleReferenceKeys('src/lib.ts'))).toEqual(['imports FormSchema', 'references FormSchema']); + expect(retried(queries, ['FormSchema'])).toEqual(['imports FormSchema', 'references FormSchema']); + expect(retried(queries, ['FormSchema', 'module:lib', 'helper'])).toEqual(['calls helper', 'imports FormSchema', 'references FormSchema']); + expect(retried(queries, ['tag', 'module:tag.controller.ts'])).toEqual([]); + // The per-name ceiling holds for the name lookup too. + expect(retried(queries, ['FormSchema'], 1)).toEqual([]); + }); + + it('adds the name index to an older index, and replays cleanly', () => { + fixture(); + const indexes = () => db!.getDb().prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND name = 'idx_unresolved_failed_module_name'").all(); + db!.getDb().exec(`DROP INDEX idx_unresolved_failed_module_name; + DELETE FROM schema_versions WHERE version >= 15; + INSERT OR IGNORE INTO schema_versions(version, applied_at, description) VALUES (14, 0, 'legacy fixture');`); + expect(indexes()).toHaveLength(0); + runMigrations(db!.getDb(), 14); + expect(getCurrentVersion(db!.getDb())).toBe(CURRENT_SCHEMA_VERSION); + expect(indexes()).toHaveLength(1); + db!.getDb().exec('DELETE FROM schema_versions WHERE version >= 15'); + runMigrations(db!.getDb(), 14); + expect(indexes()).toHaveLength(1); + }); + + it('looks a reference up by its name through the name index, not every failed row', () => { + const queries = fixture(); + const prepare = vi.spyOn(db!.getDb(), 'prepare'); + queries.getRetryableFailedReferences(['FormSchema']); + const selects = prepare.mock.calls.map(([sql]) => sql as string).filter((sql) => sql.includes('reference_name IN')); + prepare.mockRestore(); + expect(selects).toHaveLength(2); + for (const select of selects) { + const plan = db!.getDb().prepare(`EXPLAIN QUERY PLAN ${select}`).all('FormSchema') + .map((row) => (row as { detail: string }).detail).join('; '); + expect(plan).toMatch(/USING (COVERING )?INDEX idx_unresolved_failed_module_name/); + } + }); +}); diff --git a/src/db/migrations.ts b/src/db/migrations.ts index 7c07a99e3..46ab416b9 100644 --- a/src/db/migrations.ts +++ b/src/db/migrations.ts @@ -10,7 +10,7 @@ import { referenceNameTail } from './reference-tail'; /** * Current schema version */ -export const CURRENT_SCHEMA_VERSION = 14; +export const CURRENT_SCHEMA_VERSION = 15; /** * Migration definition @@ -284,6 +284,21 @@ const migrations: Migration[] = [ } }, }, + { + version: 15, + description: 'Retry a failed reference through an import binding by its whole name: module-tail name index', + up: (db) => { + // A failed reference through an import binding the module declares + // under another name is parked under the module's key from this + // version on, and still looked up by its whole name. Rows parked before + // keep the tail their name gives, which the name lookup already finds; + // a re-index parks them under their modules. Keep the definition in + // lockstep with schema.sql. + db.exec(` + CREATE INDEX IF NOT EXISTS idx_unresolved_failed_module_name ON unresolved_refs(status, reference_name) WHERE status = 'failed' AND name_tail GLOB 'module:*'; + `); + }, + }, ]; /** diff --git a/src/db/queries.ts b/src/db/queries.ts index a063beff2..783f0740a 100644 --- a/src/db/queries.ts +++ b/src/db/queries.ts @@ -3785,9 +3785,10 @@ export class QueryBuilder { * terminate) but stay queryable by name_tail so a later sync can retry them * when a changed file introduces a symbol that could satisfy them. name_tail * is (re)written here so rows inserted before the v8 migration get their - * tail the first time they're attempted. + * tail the first time they're attempted. A ref's own `nameTail`, when the + * resolver gave it one, replaces the tail its name gives. */ - markReferencesFailed(refs: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }>): number { + markReferencesFailed(refs: Array<{ fromNodeId: string; referenceName: string; referenceKind: string; nameTail?: string }>): number { if (refs.length === 0) return 0; const stmt = this.db.prepare( "UPDATE unresolved_refs SET status = 'failed', name_tail = ? WHERE from_node_id = ? AND reference_name = ? AND reference_kind = ?" @@ -3795,7 +3796,7 @@ export class QueryBuilder { let changed = 0; const markMany = this.db.transaction((items: typeof refs) => { for (const ref of items) { - changed += stmt.run(referenceNameTail(ref.referenceName, ref.referenceKind), ref.fromNodeId, ref.referenceName, ref.referenceKind).changes; + changed += stmt.run(ref.nameTail ?? referenceNameTail(ref.referenceName, ref.referenceKind), ref.fromNodeId, ref.referenceName, ref.referenceKind).changes; } }); markMany(refs); @@ -3810,7 +3811,7 @@ export class QueryBuilder { * can differ per call site (receiver-type inference reads the ref's line), * so a sibling must not inherit this row's failure. */ - markReferencesFailedByRowIds(refs: Array<{ rowId: number; referenceName: string; referenceKind: string }>): number { + markReferencesFailedByRowIds(refs: Array<{ rowId: number; referenceName: string; referenceKind: string; nameTail?: string }>): number { if (refs.length === 0) return 0; const stmt = this.db.prepare( "UPDATE unresolved_refs SET status = 'failed', name_tail = ? WHERE id = ?" @@ -3818,7 +3819,7 @@ export class QueryBuilder { let changed = 0; const markMany = this.db.transaction((items: typeof refs) => { for (const ref of items) { - changed += stmt.run(referenceNameTail(ref.referenceName, ref.referenceKind), ref.rowId).changes; + changed += stmt.run(ref.nameTail ?? referenceNameTail(ref.referenceName, ref.referenceKind), ref.rowId).changes; } }); markMany(refs); @@ -3835,42 +3836,52 @@ export class QueryBuilder { * arbitrary subset would be both wasted work and incoherent coverage. * * A name can also be one of a changed file's `moduleReferenceKeys`, which a - * route's reference to the module it lazily loads is parked under. + * route's reference to the module it lazily loads is parked under, and so + * is a reference through an import binding of the module. Such a binding's + * reference still waits for its own name too, looked up by its whole name: + * a fresh index links a binding whose module never resolves it by that + * name alone, to whichever declaration carries it. */ getRetryableFailedReferences(names: string[], perNameCeiling: number = 500): UnresolvedReference[] { if (names.length === 0) return []; - // Pass 1: per-tail counts, chunked under the SQLite parameter limit. - const retryNames: string[] = []; - for (let i = 0; i < names.length; i += SQLITE_PARAM_CHUNK_SIZE) { - const chunk = names.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); - const placeholders = chunk.map(() => '?').join(','); - const counts = this.db - .prepare( - `SELECT name_tail, COUNT(*) as count FROM unresolved_refs WHERE status = 'failed' AND name_tail IN (${placeholders}) GROUP BY name_tail` - ) - .all(...chunk) as Array<{ name_tail: string; count: number }>; - for (const row of counts) { - if (row.count <= perNameCeiling) retryNames.push(row.name_tail); + const lookups = [ + { column: 'name_tail', where: "status = 'failed'" }, + // idx_unresolved_failed_module_name; its leading `status` is the + // equality term that keeps the planner off idx_unresolved_status and + // idx_unresolved_name, which read hundreds of thousands of rows here. + { column: 'reference_name', where: "status = 'failed' AND name_tail GLOB 'module:*'" }, + ]; + const rows = new Map(); + for (const { column, where } of lookups) { + // Pass 1: per-key counts, chunked under the SQLite parameter limit. + const retryKeys: string[] = []; + for (let i = 0; i < names.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = names.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const counts = this.db + .prepare(`SELECT ${column} AS key, COUNT(*) AS count FROM unresolved_refs WHERE ${where} AND ${column} IN (${placeholders}) GROUP BY ${column}`) + .all(...chunk) as Array<{ key: string; count: number }>; + for (const row of counts) { + if (row.count <= perNameCeiling) retryKeys.push(row.key); + } } - } - if (retryNames.length === 0) return []; - // Pass 2: load the surviving rows. - const rows: UnresolvedRefRow[] = []; - for (let i = 0; i < retryNames.length; i += SQLITE_PARAM_CHUNK_SIZE) { - const chunk = retryNames.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); - const placeholders = chunk.map(() => '?').join(','); - const chunkRows = this.db - .prepare(`SELECT * FROM unresolved_refs WHERE status = 'failed' AND name_tail IN (${placeholders})`) - .all(...chunk) as UnresolvedRefRow[]; - // Loop, not spread — same V8 argument-limit hazard as - // getUnresolvedReferencesByFiles (#1558): a large definition delta can - // select an unbounded number of failed rows per chunk. - for (const row of chunkRows) rows.push(row); + // Pass 2: load the surviving rows; a row both lookups find is kept once. + for (let i = 0; i < retryKeys.length; i += SQLITE_PARAM_CHUNK_SIZE) { + const chunk = retryKeys.slice(i, i + SQLITE_PARAM_CHUNK_SIZE); + const placeholders = chunk.map(() => '?').join(','); + const chunkRows = this.db + .prepare(`SELECT * FROM unresolved_refs WHERE ${where} AND ${column} IN (${placeholders})`) + .all(...chunk) as UnresolvedRefRow[]; + // Loop, not spread — same V8 argument-limit hazard as + // getUnresolvedReferencesByFiles (#1558): a large definition delta can + // select an unbounded number of failed rows per chunk. + for (const row of chunkRows) rows.set(row.id, row); + } } - return rows.map((row) => ({ + return [...rows.values()].map((row) => ({ fromNodeId: row.from_node_id, referenceName: row.reference_name, referenceKind: row.reference_kind as EdgeKind, diff --git a/src/db/reference-tail.ts b/src/db/reference-tail.ts index 61c16e4a1..a0b954c59 100644 --- a/src/db/reference-tail.ts +++ b/src/db/reference-tail.ts @@ -45,8 +45,8 @@ export function referenceNameTail(referenceName: string, referenceKind?: string) // Vue Router's route names the component it renders as a call. if (referenceKind === 'references' || referenceKind === 'calls') { const route = MODULE_REFERENCE.exec(referenceName); - const stem = route ? pathStem(route[1] ?? route[2]!) : ''; - if (stem) return MODULE_KEY + stem; + const tail = route ? moduleTail(route[1] ?? route[2]!) : ''; + if (tail) return tail; } if (referenceKind === 'references') { const fileName = referenceName.slice(referenceName.lastIndexOf('/') + 1); @@ -73,6 +73,18 @@ const MODULE_REFERENCE = /^(?:layout:)?(?:lazy-import:(.+)|import:(.+)#[^#]+)$/; /** What a module reference's tail starts with. No symbol's name does, so a lookup by symbol names never finds one. */ const MODULE_KEY = 'module:'; +/** + * The tail of a reference that waits for a module rather than for a name: the + * module path's stem behind 'module:' — `./pages/Team` → 'module:Team', + * `@/views/error/404.vue` → 'module:404' — the key + * {@link moduleReferenceKeys} gives each file that could be the module, or '' + * when the path's last segment is only dots. + */ +export function moduleTail(modulePath: string): string { + const stem = pathStem(modulePath); + return stem ? MODULE_KEY + stem : ''; +} + /** A path's last segment up to its first dot — `./pages/Team` → 'Team', `a/b.dart` → 'b' — or '' when that is only dots. */ function pathStem(path: string): string { const trimmed = path.replace(/\/+$/, ''); @@ -110,7 +122,9 @@ export function importPathKeys(filePath: string): string[] { * is the module of `lazy-import:./pages/Team`, `pages/Team/index.tsx` of the * 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 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 (`importBindingTail`), which is parked under the same keys. */ export function moduleReferenceKeys(filePath: string): string[] { return importPathKeys(filePath).map((key) => MODULE_KEY + key); diff --git a/src/db/schema.sql b/src/db/schema.sql index 6aff9163c..f9acf1f83 100644 --- a/src/db/schema.sql +++ b/src/db/schema.sql @@ -203,6 +203,10 @@ CREATE INDEX IF NOT EXISTS idx_unresolved_failed_tail ON unresolved_refs(name_ta -- hide how many failed calls a common tail (`index`, `types`) has. CREATE INDEX IF NOT EXISTS idx_unresolved_failed_import_tail ON unresolved_refs(reference_kind, name_tail) WHERE status = 'failed' AND reference_kind = 'imports'; CREATE INDEX IF NOT EXISTS idx_unresolved_failed_import_name ON unresolved_refs(reference_kind, reference_name) WHERE status = 'failed' AND reference_kind = 'imports'; +-- A failed ref through an import binding is parked under its module +-- (`module:tag`) and looked up by its whole name as well. The leading status +-- is the equality term that keeps the planner on this small index. +CREATE INDEX IF NOT EXISTS idx_unresolved_failed_module_name ON unresolved_refs(status, reference_name) WHERE status = 'failed' AND name_tail GLOB 'module:*'; CREATE INDEX IF NOT EXISTS idx_edges_provenance ON edges(provenance); -- Sync's third-file wiring lookup must not scan every synthesized edge. -- CASE short-circuits malformed metadata; keep these expressions identical diff --git a/src/index.ts b/src/index.ts index 127d9cf8d..5bf49cca2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1017,10 +1017,12 @@ export class CodeGraph { // own, which a reference written as a path // (`snippets/price.liquid`) waits under, and the keys a route's // lazily loaded module waits under (`module:Team` for - // `lazy-import:./pages/Team`): a route renders the component its - // module exports, so an edit can satisfy it as well as an added - // file. On a sync where no failed ref matches, this is one - // indexed lookup. + // `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'`): 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 one indexed lookup. const tRetry = Date.now(); const retryable = this.queries.getRetryableFailedReferences([...new Set([ ...this.queries.getNodeNamesByFiles(result.changedFilePaths), diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index 8de00e7ba..651825f41 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -13,6 +13,7 @@ import { extractLocalExportAliases } from './alias-binding'; import { resolveWorkspaceImport } from './workspace-packages'; import { stripCommentsForRegex } from './strip-comments'; import { dartDirectiveFile } from './dart-libraries'; +import { moduleTail } from '../db/reference-tail'; import { resolveMethodOnType, resolveObjectLiteralMember, @@ -3015,3 +3016,39 @@ export function isBoundToOutOfRepoImport( } return false; } + +/** + * The tail a reference that failed to resolve is parked under when its name + * is an import binding the module it imports does not declare by that name: + * 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 }`). Resolution reaches it 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. Its own name, the default tail, is one the + * module never declares, so the file that appears or gains the export never + * found it. A sync still looks it up by that name as well: a binding its + * module never resolves is linked by the name alone. + * + * Only a whole-name reference to the binding — the import itself, a call, + * `new`, a JSX tag, a value — and only for a module of the project: a member + * read (`NS.member`) waits for the member's name, and a package's binding + * keeps its tail. Undefined for every other reference. + */ +export function importBindingTail(ref: UnresolvedRef, context: ResolutionContext): string | undefined { + if (!ESM_IMPORT_LANGUAGES.has(ref.language) || !IDENTIFIER.test(ref.referenceName)) return undefined; + const binding = context.getImportMappings(ref.filePath, ref.language).find((m) => m.localName === ref.referenceName); + if (!binding || (!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. + !context.getNearestAliases?.(ref.filePath)?.patterns.some((p) => p.prefix !== '' && binding.source.startsWith(p.prefix))) { + return undefined; + } + // `..` names a folder through the importing file's own location. + const modulePath = binding.source.startsWith('.') + ? path.posix.join(path.posix.dirname(ref.filePath), binding.source) + : binding.source; + return moduleTail(modulePath) || undefined; +} + +const IDENTIFIER = /^[A-Za-z_$][\w$]*$/; diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 1305a2fcf..e04f9a274 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -33,7 +33,7 @@ import { gateDartLocal, clearDartLocalScopeMemos } from './dart-local-scope'; import { clearCppTypeAliasMemos } from './cpp-type-aliases'; import { clearCppIncluderMemos } from './cpp-includers'; import { matchShopifyThemeFile } from './shopify-themes'; -import { resolveViaImport, resolvePhpImportedStaticCall, resolvePhpQualifiedClassRef, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isDartImportRef, isLuaRequireRef, isJsPathImportRef, isBoundToOutOfRepoImport, clearImportResolverMemos, resolveImportPath, isExternalImport } from './import-resolver'; +import { resolveViaImport, resolvePhpImportedStaticCall, resolvePhpQualifiedClassRef, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isDartImportRef, isLuaRequireRef, isJsPathImportRef, isBoundToOutOfRepoImport, importBindingTail, clearImportResolverMemos, resolveImportPath, isExternalImport } from './import-resolver'; import { ResolverPool, minRefsForPool, shouldEngageAdaptively } from './resolver-pool'; import { resolveAliasBinding } from './alias-binding'; import { detectFrameworks, getResolvingFrameworks } from './frameworks'; @@ -1031,7 +1031,7 @@ export class ReferenceResolver { resolved.push(result); byMethod[result.resolvedBy] = (byMethod[result.resolvedBy] || 0) + 1; } else { - unresolved.push(ref); + unresolved.push(this.parkable(ref)); } // Report progress every 1% to avoid too many updates @@ -1654,22 +1654,36 @@ export class ReferenceResolver { * ref's line), so a sibling must not inherit this row's failure (#1269). */ private static partitionFailedCleanup(unresolved: UnresolvedRef[]): { - byRowId: Array<{ rowId: number; referenceName: string; referenceKind: string }>; - legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }>; + byRowId: Array<{ rowId: number; referenceName: string; referenceKind: string; nameTail?: string }>; + legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string; nameTail?: string }>; } { - const byRowId: Array<{ rowId: number; referenceName: string; referenceKind: string }> = []; - const legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string }> = []; + const byRowId: Array<{ rowId: number; referenceName: string; referenceKind: string; nameTail?: string }> = []; + const legacyKeys: Array<{ fromNodeId: string; referenceName: string; referenceKind: string; nameTail?: string }> = []; for (const r of unresolved) { - if (r.rowId != null) byRowId.push({ rowId: r.rowId, referenceName: r.referenceName, referenceKind: r.referenceKind }); + if (r.rowId != null) byRowId.push({ rowId: r.rowId, referenceName: r.referenceName, referenceKind: r.referenceKind, nameTail: r.nameTail }); else legacyKeys.push({ fromNodeId: r.fromNodeId, referenceName: r.referenceName, referenceKind: r.referenceKind, + nameTail: r.nameTail, }); } return { byRowId, legacyKeys }; } + /** + * `ref`, which no strategy resolved, with the tail it is parked under when + * that is not the one its name gives: a reference through an import + * binding the module doesn't declare by that name waits for the module + * (see importBindingTail). Decided here, where the file's import mappings + * are still cached from the attempt — in a resolver-pool worker too. + */ + private parkable(ref: UnresolvedRef): UnresolvedRef { + const tail = importBindingTail(ref, this.context); + if (tail) ref.nameTail = tail; + return ref; + } + /** A deferred attempt is unfinished work, not a final failure (#1577). */ private nonDeferredFailures(unresolved: UnresolvedRef[]): UnresolvedRef[] { return unresolved.filter((ref) => ref.rowId == null || !this.deferredRowIds.has(ref.rowId)); @@ -1904,7 +1918,7 @@ export class ReferenceResolver { resolved.push(result); byMethod[result.resolvedBy] = (byMethod[result.resolvedBy] || 0) + 1; } else { - unresolved.push(ref); + unresolved.push(this.parkable(ref)); } // Fast-path the per-ref yield check: awaiting the async no-op costs a // microtask hop per ref, which dominates at ~10⁵ refs (see MaybeYield). @@ -2024,7 +2038,7 @@ export class ReferenceResolver { resolved.push(result); byMethod[result.resolvedBy] = (byMethod[result.resolvedBy] || 0) + 1; } else { - unresolved.push(ref); + unresolved.push(this.parkable(ref)); } } this.deferredRowIds.clear(); // the admission side now owns both queues diff --git a/src/resolution/types.ts b/src/resolution/types.ts index 089afd4c7..52d62be2a 100644 --- a/src/resolution/types.ts +++ b/src/resolution/types.ts @@ -29,6 +29,9 @@ export interface UnresolvedRef { /** `unresolved_refs.id` when loaded from the database — post-pass cleanup * targets exactly this row instead of every same-key sibling (#1269). */ rowId?: number; + /** The tail a ref that failed to resolve is parked under, when it is not + * the one its name gives (see `importBindingTail`). */ + nameTail?: string; } /**