diff --git a/CHANGELOG.md b/CHANGELOG.md index 5498f3ee..cd658d3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -214,6 +214,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In React, a tag a component only mentions in a comment or a string no longer links it to that component: a JSDoc example, markup commented out like `{/* */}`, or an error message like react-router's "You cannot render a inside another " or shadcn's "useCarousel must be used within a ". Nothing in a TypeScript `.ts` file, which can't hold JSX, is read as a tag either, like the XML in outline's OpenSearch template. Before, these counted as renders, so react-router's hooks showed up among the callers of `Router`, `Routes` and `Route`, and `codegraph_explore`, callers and impact followed renders that never happen. A tag after an apostrophe, a URL or a backtick in a component's text, like `

Don't have an account? `, still links as before. Re-index React projects after upgrading. - In JavaScript and TypeScript, a name imported from a package your `package.json` lists no longer links to a project symbol that only shares its name when `tsconfig.json` or `jsconfig.json` has a catch-all path alias, like `"*": ["./typings/*"]` or `"*": ["src/*", "node_modules/*"]`, or an alias that points the package at a file in `node_modules`, like `"lit/decorators": ["./node_modules/lit/decorators.js"]`. Such an alias made every package look like part of the project, so `import { Typography } from '@mui/material'` was linked to the project's own `Typography` and every lit `@property()` decorator to an unrelated class's `property` field, and `codegraph callers`, impact and `codegraph affected` listed code that never used them. An import the alias does map to a file of your project, like a `.d.ts` you keep for an untyped package or `components/Button` through `"*": ["src/*"]`, links as before. Re-index affected projects after upgrading. - In JavaScript and TypeScript, a function the file imports from a package and passes along as a value, like react-router's `Link` in `styled(Link)`, lodash's `noop` in `createContext({ onClose: noop })` or chart.js's `Legend` in `Chart.register(Legend)`, no longer links to a project function that only shares its name. Before, whenever the project had exactly one function of that name, every such value linked to it: outline's breadcrumbs, menus and document lists showed up among the callers of an email template's `Link`, and a test's own `noop` stub in cord-field among the code the app's contexts use, so `codegraph callers`, impact and `codegraph_explore` followed references that never happen. A function the file declares itself, including one that hides the import inside a function, and one imported from a file of your project link as before. Re-index JavaScript and TypeScript projects after upgrading. +- In JavaScript and TypeScript, a function a file loads with a dynamic import from Node or a package, like `const { createServer } = await import('node:http')`, `const { default: chalk } = await import('chalk')` or `import('node:fs/promises').then(({ readFile }) => …)`, no longer links to a project function that only shares its name. Before, vite's call to Node's `createServer` showed up among the callers of vite's own `createServer`. The import counts only where it is in scope, so the rest of the file links as before, and a dynamic import of your own project's module still leads to that module. Thanks @Ancienttwo for the report. (#2491) - In JavaScript and TypeScript, a value the file imports from your own project and passes along now links only to what it imports: a constant like `isCloudHosted`, a class like a `Star` model, a whole module (`import * as Slack from '../slack'`), or a stylesheet, image or GraphQL document (`import TCO21 from './themes/TCO21.scss'`) no longer links to a function elsewhere that only shares its name. Before, when the project had exactly one function of that name, such a value linked to it: outline's settings screens showed up among the callers of the server's `isCloudHosted()` check, and its stores among those of the `` icon, so `codegraph callers`, impact and `codegraph_explore` followed references that never happen. A component that wraps the function declared beside it, like `export default styled(Disclosure)`, and a page loaded lazily, like `lazy(() => import('../pages/Home'))`, still lead to that function. Re-index JavaScript and TypeScript projects after upgrading. - In JavaScript, an import of a TypeScript module written without an extension, like `import AudioModal from './audio_modal'` in a `.jsx` file next to `audio_modal.tsx`, now links to that module, as it already did from a TypeScript file. Before, a `.js` or `.jsx` file's imports of TypeScript modules were never followed, so in a codebase moving to TypeScript they were matched by name instead: an import under another name linked nothing, and a name that repeats could link to the wrong file, so mastodon's `` and `` tags rendered other components of those names than the ones each file imports. A JavaScript module beside a TypeScript one of the same name still comes first. Re-index JavaScript projects after upgrading. - `codegraph_explore`'s Flow now goes through the implementation your query names when an interface method has several that lead to the same place. Before, it took whichever one came first in the index: asking about prometheus's `Engine.execEvalStmt Queryable.Querier fanout.Querier NewMergeQuerier` traced the call through the TSDB's `DB.Querier` and left the `fanout.Querier` you named off the Flow. Any two routes of the same length are now settled this way, in favor of the one that passes through more of the symbols you named. diff --git a/__tests__/dynamic-import-binding.test.ts b/__tests__/dynamic-import-binding.test.ts new file mode 100644 index 00000000..bed9ed84 --- /dev/null +++ b/__tests__/dynamic-import-binding.test.ts @@ -0,0 +1,168 @@ +/** + * Issue #2491: a function imported from a Node built-in names nothing in the + * project. The static forms (`import { resolve } from 'node:path'`, bare + * `'fs'`, `require('node:fs')`) stopped linking to same-named project + * functions in #2120; a DYNAMIC import still did. vite's + * `const { createServer } = await import('node:http'); return createServer(app)` + * linked to vite's own `createServer`, and `readFile` from + * `await import('node:fs/promises')` to a playground helper. + * + * The binding counts only where it is in scope: nitro's + * `async function createWebSocketProxy()` imports crossws's + * `createWebSocketProxy` inside itself, so the call there is crossws's, while + * the file's other calls still mean the local function. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +const FILES: Record = { + 'package.json': JSON.stringify({ name: 'app', type: 'module', dependencies: { crossws: '^0.3.0', chalk: '^5.0.0' } }), + 'src/server/index.ts': `export function createServer(app: unknown): unknown { return app; } +`, + 'src/io.ts': `export function readFile(p: string): string { return p; } +export function chalk(s: string): string { return s; } +export class Worker { constructor(readonly file: string) {} } +`, + 'src/helper.ts': `export function helper(): number { return 1; } +`, + 'src/http.ts': `export async function resolveHttpServer(app: unknown) { + const { createServer } = await import('node:http'); + return createServer(app as never); +} +`, + 'src/wasm.ts': `const instantiateFromFile = async (fileUrl: string) => { + const { readFile: read, readFile } = await import('node:fs/promises'); + await read(fileUrl); + return readFile(fileUrl); +}; +export async function paint(s: string) { + const { default: chalk } = await import('chalk'); + return chalk(s); +} +export function later(p: string) { + return import('node:fs/promises').then(({ readFile }) => readFile(p)); +} +export async function spawn() { + const { Worker } = await import('node:worker_threads'); + return new Worker('x'); +} +export async function wrap() { + const { readFile } = await import('node:fs/promises'); + return [readFile].map((f) => f); +} +export { instantiateFromFile }; +`, + 'src/proxy.ts': `async function createWebSocketProxy(opts: string) { + const { createWebSocketProxy } = await import('crossws'); + return createWebSocketProxy(opts as never); +} +export function outer(o: string) { return createWebSocketProxy(o); } +export async function inner(p: string) { + if (p) { + const { readFile } = await import('node:fs/promises'); + await readFile(p); + } + return readFile(p); +} +export async function project() { + const { helper } = await import('./helper'); + return helper(); +} +`, + 'src/top.mjs': `const { readFile } = await import('node:fs/promises'); +export async function fromTop(p) { return readFile(p); } +`, +}; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-dynamic-import-')); + for (const [rel, content] of Object.entries(FILES)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}, 60_000); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** `file:line name` of every edge of `kinds` leaving `file`. */ +function edgesFrom(file: string, kinds: string[]): string[] { + const out: string[] = []; + for (const node of cg.getNodesInFile(file)) { + for (const e of cg.getOutgoingEdges(node.id)) { + if (!kinds.includes(e.kind)) continue; + const target = cg.getNode(e.target)!; + out.push(`${e.line} ${target.filePath}:${target.name}`); + } + } + return out; +} + +describe('a name a dynamic import binds from outside the project (#2491)', () => { + it("never links to a same-named project function: vite's `await import('node:http')`", () => { + expect(edgesFrom('src/http.ts', ['calls'])).not.toContain('3 src/server/index.ts:createServer'); + }); + + it('holds for renamed and default bindings, `.then` destructuring, constructors and function values', () => { + const edges = edgesFrom('src/wasm.ts', ['calls', 'instantiates', 'references', 'function_ref']); + expect(edges.filter((e) => e.includes('src/io.ts'))).toEqual([]); + }); + + it('holds for a top-level `await import()` binding', () => { + expect(edgesFrom('src/top.mjs', ['calls']).filter((e) => e.includes('src/io.ts'))).toEqual([]); + }); + + it("binds only where it is in scope: nitro's function importing its own name", () => { + const edges = edgesFrom('src/proxy.ts', ['calls']); + // Inside the function, `createWebSocketProxy(opts)` is crossws's: no self-loop. + expect(edges).not.toContain('3 src/proxy.ts:createWebSocketProxy'); + // Outside it, the call still means the file's own function. + expect(edges).toContain('5 src/proxy.ts:createWebSocketProxy'); + // Inside the `if` block the name is node's; after the block it is bound by nothing, as before. + expect(edges).not.toContain('9 src/io.ts:readFile'); + expect(edges).toContain('11 src/io.ts:readFile'); + }); + + it("keeps a dynamic import of the project's own module", () => { + expect(edgesFrom('src/proxy.ts', ['calls'])).toContain('15 src/helper.ts:helper'); + }); +}); + +describe('sync reads the dynamic import an edit adds or removes (#2491)', () => { + it('drops the name match when the import appears and restores it when it goes', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-dynamic-import-sync-')); + const write = (rel: string, content: string): void => { + fs.mkdirSync(path.dirname(path.join(dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(dir, rel), content); + }; + const plain = 'export async function a(p: string) {\n return readFile(p);\n}\n'; + const dynamic = "export async function a(p: string) {\n const { readFile } = await import('node:fs/promises');\n return readFile(p);\n}\n"; + write('src/io.ts', 'export function readFile(p: string): string { return p; }\n'); + write('src/use.ts', plain); + const graph = await CodeGraph.init(dir, { index: true }); + const calls = (): string[] => + graph.getNodesInFile('src/use.ts').flatMap((n) => + graph.getOutgoingEdges(n.id).filter((e) => e.kind === 'calls').map((e) => `${graph.getNode(e.target)!.filePath}:${graph.getNode(e.target)!.name}`)); + try { + expect(calls()).toContain('src/io.ts:readFile'); + write('src/use.ts', dynamic); + await graph.sync(); + expect(calls()).not.toContain('src/io.ts:readFile'); + write('src/use.ts', plain); + await graph.sync(); + expect(calls()).toContain('src/io.ts:readFile'); + } finally { + graph.close(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }, 60_000); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index e3523c96..0862d981 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -11324,12 +11324,17 @@ function jsBindingEnclosesReference(name: string, fn: Node, ref: UnresolvedRef, * declaration starts) is still open at the end of `code`. */ function jsBlockOpenAtEnd(code: string, at: number): boolean { + return jsBlockEnd(code, at) === code.length; +} + +/** Where the block that holds `at` closes: its `}`, or the end of `code`. */ +function jsBlockEnd(code: string, at: number): number { let depth = 0; for (let i = at; i < code.length; i++) { if (code[i] === '{') depth++; - else if (code[i] === '}' && --depth < 0) return false; + else if (code[i] === '}' && --depth < 0) return i; } - return true; + return code.length; } const JS_OPERAND_END = /[\w$)\]}'"`]/; @@ -11343,18 +11348,23 @@ const JS_OPERAND_START = /[\w$'"`]/; * statement where no semicolon ends the last. */ function jsBodyOpenAtEnd(code: string, at: number): boolean { + return jsBodyEnd(code, at) === code.length; +} + +/** Where the body that starts at `at` (see jsBodyOpenAtEnd) ends, or the end of `code`. */ +function jsBodyEnd(code: string, at: number): number { let i = at; while (i < code.length && WHITESPACE.test(code[i]!)) i++; - if (code[i] === '{') return jsBlockOpenAtEnd(code, i + 1); + if (code[i] === '{') return jsBlockEnd(code, i + 1); let depth = 0; for (; i < code.length; i++) { const c = code[i]!; if (c === '(' || c === '[' || c === '{') { depth++; } else if (c === ')' || c === ']' || c === '}') { - if (--depth < 0) return false; + if (--depth < 0) return i; } else if (depth === 0 && (c === ',' || c === ';')) { - return false; + return i; } else if (depth === 0 && c === '\n') { // `const f = (x) => x + 1` with no semicolon, above the reference: the // line before ends an operand (a name, a closing bracket, a quote) and @@ -11364,11 +11374,11 @@ function jsBodyOpenAtEnd(code: string, at: number): boolean { while (before >= 0 && WHITESPACE.test(code[before]!)) before--; let after = i + 1; while (after < code.length && WHITESPACE.test(code[after]!)) after++; - if (JS_OPERAND_END.test(code[before] ?? '') && (after === code.length || JS_OPERAND_START.test(code[after]!))) return false; + if (JS_OPERAND_END.test(code[before] ?? '') && (after === code.length || JS_OPERAND_START.test(code[after]!))) return i; i = after - 1; } } - return true; + return code.length; } /** @@ -17815,6 +17825,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { SEALED_MODULES.delete(context); LOCAL_BINDING_MEMO.delete(context); LOCAL_BINDING_SITES.delete(context); + DYNAMIC_IMPORT_SCANS.delete(context); SELECTOR_NAMES.delete(context); GET_STATE_FILES.delete(context); DESTRUCTURED_FILES.delete(context); @@ -19881,14 +19892,113 @@ export function reExportSource(name: string, ref: UnresolvedRef, context: Resolu /** * Does the file bind `name` by importing it from outside the repository? So * does a barrel's `export { name } from 'pkg'`: its `imports` reference names - * the package's export. + * the package's export. So does a dynamic import in scope at the reference + * (dynamicImportSource), which wins over the file's static one, as an inner + * binding does. */ function isOutOfRepoBinding(name: string, ref: UnresolvedRef, context: ResolutionContext): boolean { const binding = context.getImportMappings?.(ref.filePath, ref.language)?.find((m) => m.localName === name); - const source = binding?.source ?? (ref.referenceKind === 'imports' ? reExportSource(name, ref, context) : undefined); + const source = dynamicImportSource(name, ref, context) ?? binding?.source ?? + (ref.referenceKind === 'imports' ? reExportSource(name, ref, context) : undefined); return source !== undefined && context.isOutOfRepoImport?.(source, ref.filePath, ref.language) === true; } +/** + * A file's dynamic `import()` bindings: what each one binds, from which + * module, and the stretch of the file it is in scope for (offsets in the + * file's text). Read once per file; kept for the last few files, since + * references arrive file by file. + */ +interface DynamicImportScan { + names: ReadonlySet; + lineStarts: number[]; + bindings: Array<{ names: string[]; source: string; at: number; scopeEnd: number }>; +} +const DYNAMIC_IMPORT_SCANS = new WeakMap>(); +const DYNAMIC_IMPORT_SCANS_KEEP = 64; +const NO_DYNAMIC_IMPORTS: DynamicImportScan = { names: new Set(), lineStarts: [], bindings: [] }; +/** `const { a, b: c, default: d } = await import('x')`. */ +const DYNAMIC_IMPORT_DECLARATION = /\b(?:const|let|var)\s*\{([^{}]*)\}\s*=\s*await\s+import\s*\(\s*(['"])([^'"\n]+)\2\s*\)/g; +/** `import('x').then(({ a }) => …)`, or a `function ({ a }) {…}` callback. */ +const DYNAMIC_IMPORT_CALLBACK = + /\bimport\s*\(\s*(['"])([^'"\n]+)\1\s*\)\s*\.\s*then\s*\(\s*(?:async\s+)?(?:function\s*[\w$]*\s*)?\(\s*\{([^{}]*)\}\s*\)\s*(=>|\{)/g; + +/** The local names a destructuring list binds: `a`, `b: c` (c), `default: d` (d), `e = 1` (e). */ +function destructuredLocals(list: string): string[] { + const out: string[] = []; + for (const part of list.split(',')) { + const binding = part.replace(/=[\s\S]*$/, '').trim(); + if (binding.startsWith('...')) continue; + const colon = binding.indexOf(':'); + const local = (colon >= 0 ? binding.slice(colon + 1) : binding).trim(); + if (/^[A-Za-z_$][\w$]*$/.test(local)) out.push(local); + } + return out; +} + +function dynamicImportScan(filePath: string, context: ResolutionContext): DynamicImportScan { + let cache = DYNAMIC_IMPORT_SCANS.get(context); + if (!cache) DYNAMIC_IMPORT_SCANS.set(context, (cache = new Map())); + let scan = cache.get(filePath); + if (scan) return scan; + scan = NO_DYNAMIC_IMPORTS; + const source = context.readFile(filePath) ?? ''; + const dynamic = /\bimport\s*\(/.test(source); + const declarations = dynamic ? [...source.matchAll(DYNAMIC_IMPORT_DECLARATION)] : []; + const callbacks = dynamic ? [...source.matchAll(DYNAMIC_IMPORT_CALLBACK)] : []; + if (declarations.length > 0 || callbacks.length > 0) { + // Offsets survive both passes. A match in a comment or a string binds + // nothing, and brackets in strings, template literals and regex literals + // open no scope. + const regexes: number[] = []; + const chars = blankStringContents(stripCommentsForRegex(source, 'javascript'), regexes).split(''); + for (let r = 0; r + 1 < regexes.length; r += 2) chars.fill(' ', regexes[r]! + 1, regexes[r + 1]!); + const code = chars.join(''); + const bindings: DynamicImportScan['bindings'] = []; + for (const m of declarations) { + if (code[m.index!] !== source[m.index!]) continue; + bindings.push({ names: destructuredLocals(m[1]!), source: m[3]!, at: m.index!, scopeEnd: jsBlockEnd(code, m.index!) }); + } + for (const m of callbacks) { + if (code[m.index!] !== source[m.index!]) continue; + const body = m.index! + m[0].length; + const scopeEnd = m[4] === '{' ? jsBlockEnd(code, body) : jsBodyEnd(code, body); + bindings.push({ names: destructuredLocals(m[3]!), source: m[2]!, at: m.index!, scopeEnd }); + } + if (bindings.length > 0) { + const lineStarts = [0]; + for (let i = source.indexOf('\n'); i !== -1; i = source.indexOf('\n', i + 1)) lineStarts.push(i + 1); + scan = { names: new Set(bindings.flatMap((b) => b.names)), lineStarts, bindings }; + } + } + if (cache.size >= DYNAMIC_IMPORT_SCANS_KEEP) cache.delete(cache.keys().next().value!); + cache.set(filePath, scan); + return scan; +} + +/** + * The module a dynamic import binds `name` from, where the reference sits: + * vite's `const { createServer } = await import('node:http'); return + * createServer(app)` means node's `createServer`, never the project's. Only a + * binding in scope there counts — declared earlier in a block that is still + * open (a top-level `await import()` binds the whole module), or a parameter + * of the `import('x').then(({ a }) => …)` callback the reference is in: + * nitro's `async function createWebSocketProxy()` imports crossws's + * `createWebSocketProxy` inside itself, while the file's other calls mean + * the function. Of several in scope, the innermost: the last to start. + */ +function dynamicImportSource(name: string, ref: UnresolvedRef, context: ResolutionContext): string | undefined { + if (!JS_FAMILY.has(ref.language)) return undefined; + const scan = dynamicImportScan(ref.filePath, context); + if (!scan.names.has(name)) return undefined; + const at = (scan.lineStarts[ref.line - 1] ?? Number.POSITIVE_INFINITY) + ref.column; + let found: DynamicImportScan['bindings'][number] | undefined; + for (const binding of scan.bindings) { + if (binding.at < at && at <= binding.scopeEnd && binding.names.includes(name) && (!found || binding.at > found.at)) found = binding; + } + return found?.source; +} + /** A module a bundler loads as data, not code: a stylesheet, an image, a GraphQL document, JSON, a font. */ const ASSET_MODULE = /\.(?:css|scss|sass|less|styl|stylus|pcss|graphql|gql|png|jpe?g|gif|svg|webp|avif|ico|bmp|tiff?|json5?|ya?ml|toml|md|mdx|txt|html?|xml|csv|woff2?|ttf|otf|eot|mp3|mp4|webm|ogg|wav|flac|pdf|wasm)$/i;