diff --git a/CHANGELOG.md b/CHANGELOG.md index 1815c4150..ea72a2972 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -201,6 +201,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). #### Symbols, tests and the viewer +- **A binding in a module that exports nothing is no longer a cross-file target.** On vite, every `import { defineConfig } from 'vite'` across the playground resolved onto a `const vite = await createServer(…)` sitting at module scope in `playground/ssr-html/test-stacktrace.js` — a file with an import and no export, so that binding is reachable from nowhere but itself. Name matching commits as soon as one candidate survives, and nothing asked whether an import could reach the survivor; that one binding took 157 edges. A JS/TS file holding an `import` and no export of any kind now offers its locals to no other file. Classic scripts, CommonJS (including `exports["x"] = …`), a later `export { … }`, and names contributed through `declare global` are all unaffected. Across vite this removed 320 wrong edges and added 18, each addition a reference that was previously ambiguous rather than newly invented. Re-index after upgrading. + - **Files under an `e2e/` directory count as tests.** Their calls no longer appear as production callers in Steps, dead-code and test badges. - **Production code under a `samples` or `examples` package path is no longer treated as test code.** A Kotlin or Java project whose package path runs through `com/google/samples/…` (Now in Android, for one) had nearly every file counted as a fixture, so the Map opened on `build-logic`, the entry points hid the app, and dead-code and test badges were wrong. Only the project layout above a `src/` folder decides now; the package path below it never does. diff --git a/__tests__/frameworks-integration.test.ts b/__tests__/frameworks-integration.test.ts index 3df4f2d88..be7e3aea8 100644 --- a/__tests__/frameworks-integration.test.ts +++ b/__tests__/frameworks-integration.test.ts @@ -3,6 +3,10 @@ import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; import { CodeGraph } from '../src'; +import { DatabaseConnection, getDatabasePath } from '../src/db'; +import { QueryBuilder } from '../src/db/queries'; +import { createResolver } from '../src/resolution'; +import type { Node } from '../src/types'; import { initGrammars, loadAllGrammars } from '../src/extraction/grammars'; beforeAll(async () => { @@ -10,6 +14,54 @@ beforeAll(async () => { await loadAllGrammars(); }); +describe('Express middleware imports', () => { + it('does not resolve package imports into license headings', async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-express-doc-import-')); + let cg: CodeGraph | undefined; + try { + fs.writeFileSync(path.join(tmpDir, 'package.json'), JSON.stringify({ dependencies: { express: '*', cors: '*' } })); + fs.writeFileSync(path.join(tmpDir, 'LICENSE.md'), '# cors\n\n# host-validation-middleware\n'); + fs.writeFileSync(path.join(tmpDir, 'local.js'), 'export function localMiddleware() {}\n'); + fs.writeFileSync(path.join(tmpDir, 'server.js'), [ + "import corsMiddleware from 'cors'", + "import { hostValidationMiddleware as originalHostValidationMiddleware } from 'host-validation-middleware'", + "import { localMiddleware } from './local.js'", + 'localMiddleware()', + ].join('\n')); + cg = await CodeGraph.init(tmpDir, { index: true }); + const local = cg.getNodesByKind('function').find((n) => n.name === 'localMiddleware'); + expect(local).toBeDefined(); + expect(cg.getIncomingEdges(local!.id).some((e) => e.kind === 'imports')).toBe(true); + expect(cg.getIncomingEdges(local!.id).some((e) => e.kind === 'calls')).toBe(true); + cg.close(); + cg = undefined; + const db = DatabaseConnection.open(getDatabasePath(tmpDir)); + try { + const queries = new QueryBuilder(db.getDb()); + for (const name of ['cors', 'host-validation-middleware']) { + queries.insertNode({ + id: `heading:${name}`, name, qualifiedName: `LICENSE.md#${name}`, + kind: 'module', language: 'markdown' as Node['language'], filePath: 'LICENSE.md', + startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0, + }); + } + const resolver = createResolver(tmpDir, queries); + for (const referenceName of ['cors', 'corsMiddleware', 'host-validation-middleware']) { + expect(resolver.resolveOne({ + fromNodeId: 'file:server.js', referenceName, referenceKind: 'imports', + filePath: 'server.js', language: 'javascript', line: 1, column: 0, + })).toBeNull(); + } + } finally { + db.close(); + } + } finally { + cg?.close(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); +}); + describe('Django end-to-end framework extraction', () => { let tmpDir: string | undefined; afterEach(() => { diff --git a/__tests__/resolution.test.ts b/__tests__/resolution.test.ts index decaadee5..cb9062019 100644 --- a/__tests__/resolution.test.ts +++ b/__tests__/resolution.test.ts @@ -11,7 +11,7 @@ import * as os from 'os'; import { CodeGraph } from '../src'; import { Node, UnresolvedReference } from '../src/types'; import { ReferenceResolver, createResolver, ResolutionContext } from '../src/resolution'; -import { matchReference, resolveMethodOnType, matchByQualifiedName, preferCallSiteFile, matchMethodCall } from '../src/resolution/name-matcher'; +import { matchReference, resolveMethodOnType, matchByQualifiedName, matchByExactName, preferCallSiteFile, matchMethodCall } from '../src/resolution/name-matcher'; import { resolveImportPath, extractImportMappings, resolveJvmImport, loadCppIncludeDirs, clearCppIncludeDirCache, isPhpIncludePathRef } from '../src/resolution/import-resolver'; import type { UnresolvedRef } from '../src/resolution/types'; import { detectFrameworks, getAllFrameworkResolvers } from '../src/resolution/frameworks'; @@ -5386,4 +5386,269 @@ in expect(importedFilePaths('main.nix')).toEqual([]); }); }); + + describe('Bindings in a module that exports nothing (#1719)', () => { + it('does not treat documentation headings as package imports', () => { + // Inject the planned Markdown node shape without depending on its extractor. + const heading: Node = { + id: 'heading:vite', name: 'vite', qualifiedName: 'guide.md#vite', + kind: 'module', language: 'markdown' as Node['language'], filePath: 'guide.md', + startLine: 1, endLine: 1, startColumn: 0, endColumn: 0, updatedAt: 0, + }; + const context = { + getNodesByName: () => [heading], getNodesInFile: () => [], + getNodesByQualifiedName: () => [], getNodesByKind: () => [], + fileExists: () => false, readFile: () => null, + getProjectRoot: () => tempDir, getAllFiles: () => [], + } as ResolutionContext; + const ref: UnresolvedRef = { + fromNodeId: 'file:consumer.ts', referenceName: 'vite', referenceKind: 'imports', + filePath: 'consumer.ts', language: 'typescript', line: 1, column: 0, + }; + expect(matchByExactName(ref, context)).toBeNull(); + expect(matchByExactName({ ...ref, language: 'markdown' as Node['language'] }, context)?.targetNodeId).toBe(heading.id); + context.getNodesByName = () => [{ ...heading, id: 'fn:vite', kind: 'function', language: 'typescript', filePath: 'vite.ts' }]; + expect(matchByExactName(ref, context)?.targetNodeId).toBe('fn:vite'); + }); + + it('ignores export examples in strings and comments when checking module visibility', async () => { + fs.mkdirSync(path.join(tempDir, 'src')); + fs.writeFileSync(path.join(tempDir, 'src/private.js'), [ + "import fs from 'node:fs'", + 'const example = `', + 'export const example = 1', + '`', + '/*', + 'export { hidden }', + '*/', + 'function hidden() { return fs }', + 'hidden()', + ].join('\n')); + fs.writeFileSync(path.join(tempDir, 'src/consumer.js'), 'hidden()'); + fs.mkdirSync(path.join(tempDir, 'legacy')); + fs.writeFileSync(path.join(tempDir, 'legacy/global.js'), 'function hidden() { return 1 }'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const hidden = cg.getNodesByKind('function').find((n) => n.name === 'hidden' && n.filePath === 'src/private.js'); + expect(hidden).toBeDefined(); + const callers = cg.getIncomingEdges(hidden!.id).filter((e) => e.kind === 'calls'); + expect(callers.some((e) => cg.getNode(e.source)?.filePath === 'src/consumer.js')).toBe(false); + expect(callers.some((e) => cg.getNode(e.source)?.filePath === 'src/private.js')).toBe(true); + const consumer = cg.getNodesByKind('file').find((n) => n.filePath === 'src/consumer.js'); + expect(cg.getOutgoingEdges(consumer!.id).filter((e) => e.kind === 'calls')).toEqual([]); + }); + + it('does not name-match a method call to another file\'s JSON value', async () => { + fs.writeFileSync(path.join(tempDir, 'data.json'), '{"content": "hello"}'); + fs.writeFileSync(path.join(tempDir, 'data.js'), "const content = require('./data.json')\nmodule.exports = { content }\n"); + fs.writeFileSync(path.join(tempDir, 'consumer.js'), 'export async function read(page) { return page.frame("main").content() }'); + fs.writeFileSync(path.join(tempDir, 'use-data.js'), "import { content } from './data'\nconsole.log(content)\n"); + fs.writeFileSync(path.join(tempDir, 'callback.js'), "const callback = require('./handler.js')\nmodule.exports = { callback }\n"); + fs.writeFileSync(path.join(tempDir, 'call.js'), 'callback()'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const content = cg.getNodesByKind('constant').find((n) => n.name === 'content'); + expect(content).toBeDefined(); + expect(cg.getIncomingEdges(content!.id).filter((e) => e.kind === 'calls')).toEqual([]); + expect(cg.getIncomingEdges(content!.id).some((e) => e.kind === 'imports')).toBe(true); + const callback = cg.getNodesByKind('constant').find((n) => n.name === 'callback'); + expect(callback).toBeDefined(); + expect(cg.getIncomingEdges(callback!.id).some((e) => e.kind === 'calls')).toBe(true); + }); + + it('keeps a local file dependency import when a closer private name collides', async () => { + fs.writeFileSync(path.join(tempDir, 'package.json'), JSON.stringify({ dependencies: { 'local-dep': 'file:./dep' } })); + fs.mkdirSync(path.join(tempDir, 'dep')); + fs.mkdirSync(path.join(tempDir, 'src')); + fs.writeFileSync(path.join(tempDir, 'dep/package.json'), JSON.stringify({ name: 'local-dep', main: 'index.js' })); + fs.writeFileSync(path.join(tempDir, 'dep/index.js'), "export const msg = 'local'\n"); + fs.writeFileSync(path.join(tempDir, 'src/private.js'), "import fs from 'node:fs'\nconst msg = 'private'\n"); + fs.writeFileSync(path.join(tempDir, 'src/consumer.js'), "import { msg } from 'local-dep'\nconsole.log(msg)\n"); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const msg = cg.getNodesByKind('constant').find((n) => n.name === 'msg' && n.filePath === 'dep/index.js'); + expect(msg).toBeDefined(); + expect(cg.getIncomingEdges(msg!.id).some((e) => e.kind === 'imports')).toBe(true); + }); + + it('preserves executable CommonJS exports inside nested template interpolations', async () => { + fs.writeFileSync(path.join(tempDir, 'cjs.js'), [ + "import fs from 'node:fs'", + 'function helper() { return fs }', + 'const text = `outer ${`inner ${module.exports = { helper }}`}`', + ].join('\n')); + fs.writeFileSync(path.join(tempDir, 'consumer.js'), 'helper()'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const helper = cg.getNodesByKind('function').find((n) => n.name === 'helper'); + expect(helper).toBeDefined(); + expect(cg.getIncomingEdges(helper!.id).some((e) => + e.kind === 'calls' && cg.getNode(e.source)?.filePath === 'consumer.js')).toBe(true); + }); + + it.each(['export function visible() { return fs }', 'function visible() { return fs }\nexport { visible }'])('preserves real exports after a regex containing a backtick: %s', async (declaration) => { + fs.writeFileSync(path.join(tempDir, 'exported.js'), "import fs from 'node:fs'\nconst re = /`/\nif (fs) /`/.test('text')\nelse /`/.test('other')\nconst make = () => /`/\n" + declaration + '\n'); + fs.writeFileSync(path.join(tempDir, 'consumer.js'), 'visible()'); + cg = await CodeGraph.init(tempDir, { index: true }); + cg.resolveReferences(); + const visible = cg.getNodesByKind('function').find((n) => n.name === 'visible'); + expect(visible).toBeDefined(); + expect(cg.getIncomingEdges(visible!.id).some((e) => e.kind === 'calls')).toBe(true); + }); + + // On vitejs/vite, every `import { defineConfig } from 'vite'` across the + // playground resolved onto `playground/ssr-html/test-stacktrace.js::vite` + // — `const vite = await createServer(…)` at module scope in a file with + // zero exports — because exact-match commits whenever one candidate + // survives, and nothing asked whether an import could reach it. Only + // `sealed.js` may be filtered; every other file here is a class that must + // NOT be — a classic script (a top-level binding really is a reachable + // global), a CommonJS module, one exporting through `exports["x"]`, an ESM + // file whose export is a later `export { … }` statement (which leaves + // `isExported` false on the declaration's node), and one contributing a + // name through `declare global` while exporting nothing of its own. + let tmpDir: string; + let cg: CodeGraph; + + afterEach(() => { + cg?.close(); + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('drops them as cross-file candidates, and keeps scripts, CJS and later exports', async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1719-')); + fs.writeFileSync( + path.join(tmpDir, 'sealed.js'), + `import fsp from 'node:fs/promises' + +function widget() { + return fsp +} + +widget() +` + ); + fs.writeFileSync( + path.join(tmpDir, 'script.js'), + `function gadget() { + return 1 +} +` + ); + fs.writeFileSync( + path.join(tmpDir, 'cjs.js'), + `import osp from 'node:os' + +function helper() { + return osp +} + +module.exports = { helper } +` + ); + fs.writeFileSync( + path.join(tmpDir, 'later.js'), + `import pathp from 'node:path' + +function parser() { + return pathp +} + +export { parser } +` + ); + // `exports["x"]` is a CommonJS export too, and a file declaring globals + // offers them to every other file whether or not it exports anything of + // its own. Both would read as sealed on a test that looked only for + // `export …`, `module.exports` and `exports.x`. + fs.writeFileSync( + path.join(tmpDir, 'bracket.js'), + `import urlp from 'node:url' + +function bracketed() { + return urlp +} + +exports["bracketed"] = bracketed +` + ); + // A module with imports and no export of its own still contributes every + // name in `declare global` to every other file. `plain.ts` is the control + // that makes the assertion mean something: it is the same "import, no + // export" shape holding the same kind of declaration, so the pair differs + // only by the `declare global`, and an assertion on StrayFace alone would + // pass whatever the guard did. + fs.writeFileSync( + path.join(tmpDir, 'ambient.ts'), + `import './later' + +declare global { + interface StrayFace { + a: number + } +} +` + ); + fs.writeFileSync( + path.join(tmpDir, 'plain.ts'), + `import './later' + +interface HiddenFace { + a: number +} + +const unused: HiddenFace = { a: 1 } +` + ); + // A type annotation is the reference here, so this consumer must be .ts. + fs.writeFileSync( + path.join(tmpDir, 'consumer.ts'), + `const face: StrayFace = { a: 1 } +const hidden: HiddenFace = { a: 2 } + +export function use(): number { + return face.a + hidden.a +} +` + ); + // Nothing here is bound by an import, so every name is a free reference + // that falls through to exact name matching — the path this rule sits on. + // A bare import would reach that path too, but a bare specifier names a + // package outside the graph, so no project node is the right target for + // it and such a fixture would assert a resolution nothing should make. + fs.writeFileSync( + path.join(tmpDir, 'consumer.js'), + `widget() +gadget() +helper() +parser() +bracketed() +` + ); + + cg = await CodeGraph.init(tmpDir, { index: true }); + cg.resolveReferences(); + + // Incoming edges rather than callers, so the interfaces are asked the + // same question as the functions: a type annotation is a reference, not + // a call. + const reachedFrom = (consumer: string, name: string): boolean => { + const target = cg + .searchNodes(name, { limit: 10 }) + .find((r) => r.node.name === name && r.node.filePath !== consumer); + expect(target, `no node named ${name}`).toBeDefined(); + return cg + .getIncomingEdges(target!.node.id) + .some((e) => cg.getNode(e.source)?.filePath === consumer); + }; + + expect(reachedFrom('consumer.js', 'widget')).toBe(false); + expect(reachedFrom('consumer.js', 'gadget')).toBe(true); + expect(reachedFrom('consumer.js', 'helper')).toBe(true); + expect(reachedFrom('consumer.js', 'parser')).toBe(true); + expect(reachedFrom('consumer.js', 'bracketed')).toBe(true); + expect(reachedFrom('consumer.ts', 'StrayFace')).toBe(true); + expect(reachedFrom('consumer.ts', 'HiddenFace')).toBe(false); + }, 30000); + }); }); diff --git a/src/mcp/dynamic-boundaries.ts b/src/mcp/dynamic-boundaries.ts index 2378ddaeb..7307bd3fe 100644 --- a/src/mcp/dynamic-boundaries.ts +++ b/src/mcp/dynamic-boundaries.ts @@ -21,7 +21,8 @@ * inside a string is a false positive, so {@link blankStringContents} blanks * them too, quotes preserved.) */ -import { stripCommentsForRegex, type CommentLang } from '../resolution/strip-comments'; +import { blankStringContents, stripCommentsForRegex, type CommentLang } from '../resolution/strip-comments'; +export { blankStringContents } from '../resolution/strip-comments'; export interface BoundaryMatch { /** Stable form id, e.g. 'computed-call' — used for per-form dedupe. */ @@ -222,42 +223,6 @@ function commentLang(language: string): CommentLang | null { const MAX_MATCHES_PER_BODY = 3; const MAX_BODY_CHARS = 60_000; // a god-function tail is still scannable; beyond this, truncate -/** - * Blank the CONTENTS of string literals (quotes preserved, offsets preserved) - * so dispatch-shaped prose — docs, error messages, template text — can't fire - * a matcher. Run AFTER comment stripping (comments are already spaces). - * Backslash escapes are honored; `'`/`"` strings end at a newline (treated as - * unterminated, matching the comment stripper); backticks span lines, and - * `${...}` interpolations inside them are blanked too — missing a dispatch - * inside a template literal is acceptable, false-firing on prose is not. - */ -export function blankStringContents(text: string): string { - const out = text.split(''); - let i = 0; - const n = text.length; - while (i < n) { - const c = text[i]!; - if (c === '"' || c === "'" || c === '`') { - const quote = c; - i++; - while (i < n && text[i] !== quote) { - if (text[i] === '\\' && i + 1 < n) { - out[i] = ' '; - out[i + 1] = ' '; - i += 2; - continue; - } - if (quote !== '`' && text[i] === '\n') break; // unterminated — stop blanking - if (text[i] !== '\n') out[i] = ' '; // keep newlines for line math - i++; - } - if (i < n && text[i] === quote) i++; - continue; - } - i++; - } - return out.join(''); -} /** * Scan one symbol's body for dynamic-dispatch sites. diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 7988c1bcf..f9d90923a 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -2452,6 +2452,8 @@ export class ReferenceResolver { if (!result) return result; if (ref.referenceKind !== 'references' && ref.referenceKind !== 'imports') return result; const tgt = this.getLanguageFromNodeId(result.targetNodeId); + // Package imports cannot target prose found by a framework's name lookup. + if (ref.referenceKind === 'imports' && (tgt as string) === 'markdown' && (ref.language as string) !== 'markdown') return null; if (tgt && ref.language && crossesKnownFamily(tgt, ref.language)) return null; return result; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index c74d8f272..712abe21e 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -6,6 +6,7 @@ import { Language, Node } from '../types'; import { UnresolvedRef, ResolvedRef, ResolutionContext } from './types'; +import { blankStringContents, stripCommentsForRegex } from './strip-comments'; /** * Ceiling on how many same-named definitions a FUZZY name-match strategy will @@ -388,6 +389,108 @@ function isLexicallyReachable( ); } +/** Languages whose module boundary is `import`/`export` (or CommonJS). */ +const ESM_FAMILY = new Set(['typescript', 'tsx', 'javascript', 'jsx', 'arkts']); + +/** + * A line-initial `import` statement — the marker that a JS/TS file is a MODULE + * rather than a classic script. Line-anchored and followed by a name, brace, + * star or quote, so a dynamic `import(` and the word inside a comment or string + * do not match. + */ +const HAS_IMPORT_STATEMENT = /^[ \t]*import[\s{*'"]/m; + +/** + * Anything the file could offer another file, in every form the extractor's own + * `isExported` flag misses. `^export` covers the declaration and later forms + * (`export const`, `export { x }`, `export default x`, `export *`); the + * CommonJS shapes cover files that never use ESM syntax at all, in both the dot + * and the bracket form; and `declare global` contributes names to every file + * whether or not the module exports anything of its own. Kept as a source test + * rather than a node scan precisely because `isExported` is set only from an + * `export_statement` ancestor, so `const x = …; export { x }` and + * `module.exports = { x }` both read as unexported on the node. + */ +const HAS_ESM_EXPORT = /^[ \t]*export[\s{*]|^[ \t]*declare\s+global\b/m; +const HAS_CJS_EXPORT = /\bmodule\.exports\b|\bexports\s*[.[]/; + +/** + * Per-context memo of "this file is a module that exports nothing", asked once + * per candidate FILE rather than once per reference. Derived from file source, + * so it drops with the context's file caches — clearNameMatcherMemos deletes it + * alongside INFER_SCAN_STATES. + */ +const SEALED_MODULES = new WeakMap>(); + +/** + * Whether `filePath` is a JS/TS module that exports NOTHING — an import + * statement present, no export of any form. No reference from another file can + * reach any binding in such a file, so every one of its symbols is a false + * candidate for a cross-file name match. + * + * This is the general case behind a package name capturing a same-named local: + * on `vitejs/vite`, 157 cross-file `imports` refs — every `import { defineConfig + * } from 'vite'` in the playground and the create-vite templates — resolved onto + * `playground/ssr-html/test-stacktrace.js::vite`, which is `const vite = await + * createServer(…)` at module scope in a file with zero exports. The existing + * guards cannot see it: `isLexicallyReachable` returns early for any candidate + * that is not a `function`, and the bare-import guard correctly declines because + * `vite` IS a workspace member, so the specifier really is project-local. What + * is wrong is only which node the name lands on. + * + * Deliberately narrow on three axes, because each is a class this would + * otherwise resolve wrongly in the opposite direction: + * + * - **A classic script is exempt.** Requiring an `import` statement means a + * non-module `.js` file — concatenated globals, a browser `