From c4babfce6afbd5b95ea10ddd9c4ccddb5910b3b7 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Thu, 1 Oct 2026 20:37:37 +0000 Subject: [PATCH] fix(js): a CommonJS require and a module path import the file they name (#2263) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(js): a CommonJS require and a module path import the file they name A `require('./application')` was only a call, so a CommonJS file depended on nothing at file level: Express's lib/express.js showed no imports and impact missed the files it loads. Local require specifiers are now read after extraction (both the wasm extractor and the kernel, including its deferred-decode transport) as `imports` refs from the file, and a JS/TS module path resolves to its file the way the runtime finds it — extensions, index files, aliases — which also lands an extensionless `import './polyfills'` on its file instead of its own import node. Co-Authored-By: Claude Opus 5.5 * fix(js): read require imports inside each extractor, so kernel parity holds Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/commonjs-require-imports.test.ts | 103 +++++++++++++++++++++ src/extraction/commonjs-requires.ts | 68 ++++++++++++++ src/extraction/index.ts | 1 + src/extraction/kernel/index.ts | 5 + src/extraction/parse-worker.ts | 4 +- src/extraction/store-worker.ts | 1 + src/extraction/store-writer.ts | 2 + src/extraction/tree-sitter.ts | 5 + src/resolution/import-resolver.ts | 31 +++++++ src/resolution/index.ts | 8 +- 11 files changed, 225 insertions(+), 4 deletions(-) create mode 100644 __tests__/commonjs-require-imports.test.ts create mode 100644 src/extraction/commonjs-requires.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e9ce9fd79..ded85aa29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- In JavaScript and TypeScript, a CommonJS `require('./application')` now counts as an import of that file, the way an ESM `import` does. A module path also resolves the way Node and bundlers find it: extensions, `index` files and path aliases. Before, a CommonJS file depended on nothing at file level, so Express's own `lib/express.js` showed no imports and impact analysis missed the files it loads. An extensionless side-effect import like `import './polyfills'` also stopped at its own import statement instead of reaching the file. - An Express route whose handler is written inline as `function (req, res) {…}`, the form Express's own examples use, now links to what that handler calls, just like an arrow handler does. The handler is the route's last argument, so an inline middleware before a named handler no longer stands in for it, and a wrapper like `asyncHandler(async function (req, res) {…})` and a trailing comma both work. Before, a route such as `POST /login` linked to nothing. - A Python module member used through the imported module, like `generic.ObjectEditView` after `from netbox.views import generic`, `serializers.ValidationError` or `utils.assert_deserialize(…)`, now also follows the package's re-exports to its definition. Before, netbox's view classes lost their `generic.*` base classes, and mealie's test helpers called through `utils.` went unresolved. - A Python name imported from a package now follows the package's re-exports to where it's defined. That covers both `from .users import *` and `from .tokens import Token` in its `__init__.py`. Before, httpx's own tests' `httpx.Client(…)` and `httpx.URL(…)`, pytest's `pytest.raises(…)` and `pytest.param(…)`, and netbox's `from users.models import User` went unresolved or to a same-named class elsewhere, such as a test's `User` or an Alembic migration's copy of a model. diff --git a/__tests__/commonjs-require-imports.test.ts b/__tests__/commonjs-require-imports.test.ts new file mode 100644 index 000000000..f09f32528 --- /dev/null +++ b/__tests__/commonjs-require-imports.test.ts @@ -0,0 +1,103 @@ +/** + * A CommonJS `require('./x')` is a file import, as ESM's `import` is: Express's + * `lib/express.js` requires `./application`, `./request` and `./response`, and + * the index said it imported nothing — an empty Imports rail in the viewer and + * no file dependency — because those bindings are used as values, never + * called. A module path also resolves the way the runtime finds it, so an + * extensionless ESM side-effect import (`import './polyfills'`) reaches its + * file instead of stopping at its own import statement. A package + * (`require('express')`) stays outside the index. + */ +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; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cjs-require-')); + const files: Record = { + 'package.json': JSON.stringify({ name: 'web', main: 'index.js' }), + 'index.js': `module.exports = require('./lib/express'); +`, + 'lib/express.js': `var EventEmitter = require('events').EventEmitter; +var mixin = require('merge-descriptors'); +var proto = require('./application'); +var req = require('./request'); + +function createApplication() { + var app = function () {}; + mixin(app, EventEmitter.prototype, false); + mixin(app, proto, false); + app.request = Object.create(req); + return app; +} + +module.exports = createApplication; +`, + 'lib/application.js': `var app = exports = module.exports = {}; +app.init = function init() {}; +`, + 'lib/request.js': `var req = Object.create({}); +module.exports = req; +`, + 'test/app.js': `var express = require('..'); +var assert = require("assert"); +`, + 'src/main.ts': `import './polyfills'; +import helper = require('./helper'); +export const value = helper.run(); +`, + 'src/polyfills.ts': `export {}; +`, + 'src/helper.ts': `export function run(): number { + return 1; +} +`, + }; + 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 }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +const importedFiles = (file: string) => { + const ids = cg.getNodesInFile(file).map((n) => n.id); + return [...new Set(cg.getOutgoingEdgesFrom(ids, ['imports']) + .map((e) => cg.getNode(e.target)!.filePath) + .filter((p) => p !== file))].sort(); +}; + +describe('CommonJS require', () => { + it('imports the files a module requires, used as values or not, and no package', () => { + expect(importedFiles('lib/express.js')).toEqual(['lib/application.js', 'lib/request.js']); + expect(cg.getFileDependencies('lib/express.js')).toEqual(expect.arrayContaining(['lib/application.js', 'lib/request.js'])); + }); + + it('follows a directory require to its package entry', () => { + expect(importedFiles('index.js')).toEqual(['lib/express.js']); + expect(importedFiles('test/app.js')).toEqual(['index.js']); + }); +}); + +describe('a module path', () => { + it('reaches an extensionless side-effect import’s file', () => { + expect(importedFiles('src/main.ts')).toContain('src/polyfills.ts'); + }); + + it('reaches a TypeScript import-equals file once', () => { + const ids = cg.getNodesInFile('src/main.ts').map((n) => n.id); + const toHelper = cg.getOutgoingEdgesFrom(ids, ['imports']) + .filter((e) => cg.getNode(e.target)!.filePath === 'src/helper.ts' && cg.getNode(e.target)!.kind === 'file'); + expect(toHelper.length).toBeLessThanOrEqual(1); + }); +}); diff --git a/src/extraction/commonjs-requires.ts b/src/extraction/commonjs-requires.ts new file mode 100644 index 000000000..436a03385 --- /dev/null +++ b/src/extraction/commonjs-requires.ts @@ -0,0 +1,68 @@ +/** + * CommonJS `require('…')` as an import. + * + * An ESM `import … from './application'` is an `import_statement` the + * extractor turns into an `imports` reference from the file, which the import + * resolver lands on the module's file. A `require('./application')` is just a + * call, so a CommonJS file depended on nothing at file level: Express's + * `lib/express.js` requires `./application`, `./request` and `./response` and + * the index said it imports nothing — an empty Imports rail in the viewer, no + * link on the Map, and no file dependency for impact. A binding used only as a + * value (`mixin(app, proto)`) never made a call edge to stand in for it either. + * + * Read from the source after extraction rather than in a walker, so the wasm + * extractor and the native kernel get it alike — including the kernel's + * deferred-decode transport, which carries these beside its buffers. Only the + * module's own file reference is emitted — the names a binding is called + * through already resolve through the require binding (import-resolver's + * CommonJS reader). + */ + +import type { Language, UnresolvedReference } from '../types'; +import { stripCommentsForRegex } from '../resolution/strip-comments'; + +const COMMONJS_LANGUAGES: ReadonlySet = new Set(['javascript', 'jsx', 'typescript', 'tsx']); + +/** + * `require('x')` with a literal specifier, not a member's (`foo.require(…)`) + * or a property (`require.resolve(…)`). A template literal counts only when + * it has no `${…}` hole. + */ +const REQUIRE_CALL = /(?`. */ +export function commonJsRequireRefs(filePath: string, source: string, language: Language): UnresolvedReference[] { + if (!COMMONJS_LANGUAGES.has(language) || !source.includes('require')) return []; + const safe = stripCommentsForRegex(source, language === 'javascript' || language === 'jsx' ? 'javascript' : 'typescript'); + const out: UnresolvedReference[] = []; + const seen = new Set(); + REQUIRE_CALL.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = REQUIRE_CALL.exec(safe)) !== null) { + const specifier = (m[1] ?? m[2] ?? m[3] ?? '').trim(); + if (!LOCAL_SPECIFIER.test(specifier) || seen.has(specifier)) continue; + const lineStart = safe.lastIndexOf('\n', m.index - 1) + 1; + if (IMPORT_EQUALS.test(safe.slice(lineStart, m.index))) continue; + seen.add(specifier); + out.push({ + fromNodeId: `file:${filePath}`, + referenceName: specifier, + referenceKind: 'imports', + line: safe.slice(0, m.index).split('\n').length, + column: m.index - lineStart, + filePath, + language, + }); + } + return out; +} diff --git a/src/extraction/index.ts b/src/extraction/index.ts index e71ffabe6..80016bb59 100644 --- a/src/extraction/index.ts +++ b/src/extraction/index.ts @@ -2229,6 +2229,7 @@ export class ExtractionOrchestrator { language, buffers: result.kernelBuffers, file: this.buildFileRecord(filePath, content, language, stats, nodeCount, result.errors), + ...(result.unresolvedReferences.length > 0 ? { extraRefs: result.unresolvedReferences } : {}), }); } else { storeWriter.send(this.buildFreshStoreBundle(filePath, content, language, stats, result)); diff --git a/src/extraction/kernel/index.ts b/src/extraction/kernel/index.ts index c4337a091..1e71d170c 100644 --- a/src/extraction/kernel/index.ts +++ b/src/extraction/kernel/index.ts @@ -15,6 +15,7 @@ */ import type { ExtractionResult, Language } from '../../types'; +import { commonJsRequireRefs } from '../commonjs-requires'; import { EXTRACTORS } from '../languages'; import { getKernel, kernelSupports } from './loader'; import { decodeExtractBuffers } from './decode'; @@ -269,6 +270,8 @@ export function materializeKernelResult( language ); decoded.durationMs = result.durationMs; + // References read beside the tables (a CommonJS `require`) ride on the transport. + if (result.unresolvedReferences.length > 0) decoded.unresolvedReferences.push(...result.unresolvedReferences); return decoded; } @@ -294,6 +297,8 @@ export function tryKernelExtract( const buffers = kernel.extractFile(filePath, pre, language); const result = decodeExtractBuffers(buffers, filePath, language); POST_PASSES[language]?.(result, source); + // Read beside the tables, as TreeSitterExtractor does (a CommonJS `require`). + result.unresolvedReferences.push(...commonJsRequireRefs(filePath, source, language)); result.durationMs = Date.now() - t0; return result; } catch (err) { diff --git a/src/extraction/parse-worker.ts b/src/extraction/parse-worker.ts index b5be212b3..e1ec6bc28 100644 --- a/src/extraction/parse-worker.ts +++ b/src/extraction/parse-worker.ts @@ -17,6 +17,7 @@ import { parentPort } from 'worker_threads'; import { extractFromSource } from './tree-sitter'; import { detectLanguage, loadGrammarsForLanguages, resetParser } from './grammars'; import { tryKernelExtractRaw } from './kernel'; +import { commonJsRequireRefs } from './commonjs-requires'; import { getAllFrameworkResolvers, getApplicableFrameworks } from '../resolution/frameworks'; import type { Language, ExtractionResult } from '../types'; @@ -103,7 +104,8 @@ parentPort!.on('message', async (msg: { type: string; id?: number; filePath?: st result = { nodes: [], edges: [], - unresolvedReferences: [], + // Read beside the kernel's tables (extractFromSource's own post-step). + unresolvedReferences: commonJsRequireRefs(filePath!, content!, language), errors: raw.errors, durationMs: 0, kernelBuffers: raw.buffers, diff --git a/src/extraction/store-worker.ts b/src/extraction/store-worker.ts index 6493e0040..a562f557a 100644 --- a/src/extraction/store-worker.ts +++ b/src/extraction/store-worker.ts @@ -70,6 +70,7 @@ function decodeKernelBundle(bundle: KernelStoreBundle): StoreBundle { bundle.filePath, bundle.language ); + if (bundle.extraRefs?.length) decoded.unresolvedReferences.push(...bundle.extraRefs); return finalizeStoreBundle(decoded, bundle.filePath, bundle.language, bundle.file); } diff --git a/src/extraction/store-writer.ts b/src/extraction/store-writer.ts index a766bd813..683f09117 100644 --- a/src/extraction/store-writer.ts +++ b/src/extraction/store-writer.ts @@ -30,6 +30,8 @@ export interface KernelStoreBundle { language: Language; buffers: NonNullable; file: FileRecord; + /** References read beside the kernel's tables (a CommonJS `require`). */ + extraRefs?: ExtractionResult['unresolvedReferences']; } /** diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 7c9baa69d..5c27523ac 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -32,6 +32,7 @@ import { VueExtractor } from './vue-extractor'; import { MyBatisExtractor } from './mybatis-extractor'; import { CfmlExtractor } from './cfml-extractor'; import { tryKernelExtract, takeDeferredPreParse } from './kernel'; +import { commonJsRequireRefs } from './commonjs-requires'; import { getAllFrameworkResolvers, getApplicableFrameworks, @@ -657,6 +658,10 @@ export class TreeSitterExtractor { if (packageNodeId) this.nodeStack.pop(); this.nodeStack.pop(); + // A CommonJS `require('./x')` is a file import, like ESM's `import`. The + // kernel reads the same (kernel/index.ts and the parse worker's transport). + this.unresolvedReferences.push(...commonJsRequireRefs(this.filePath, this.source, this.language)); + // hasError is routine for several grammars; warn only when no symbols survived. const symbolCount = this.nodes.filter((n) => n.kind !== 'file').length; if (this.tree?.rootNode.hasError && symbolCount === 0) { diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index d1ca2c1fe..3c908fdd0 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -1529,10 +1529,41 @@ export function resolvePhpImportedStaticCall( return { original: ref, targetNodeId: methods[0]!.id, confidence: 0.95, resolvedBy: 'import' }; } +const JS_MODULE_LANGUAGES: ReadonlySet = new Set(['javascript', 'jsx', 'typescript', 'tsx', 'arkts']); + +/** + * A module specifier written as a path — relative (`./x`, `../x`), rooted, or + * an alias with a folder in it (`@/lib/x`, `~/x`, `$lib/x`) — rather than an + * imported binding's name or a bare package (`react`, `lodash`). + */ +function isJsPathSpecifier(name: string): boolean { + return name.startsWith('./') || name.startsWith('../') || name === '.' || name === '..' || + (name.includes('/') && !/\s/.test(name) && !/^@[\w.-]+\/[\w.-]+$/.test(name)); +} + +/** + * A JS/TS `imports` reference that names a module by path. It names a FILE, not + * a symbol, so it skips the resolver's name-exists pre-filter — a CommonJS + * `require('./x')` has no import node of that name to pass it. + */ +export function isJsPathImportRef(ref: UnresolvedRef): boolean { + return ref.referenceKind === 'imports' && JS_MODULE_LANGUAGES.has(ref.language) && isJsPathSpecifier(ref.referenceName); +} + export function resolveViaImport( ref: UnresolvedRef, context: ResolutionContext ): ResolvedRef | null { + // A JS/TS module specifier — `import './polyfills'`, the module of `import x + // from '../lib/a'`, a CommonJS `require('./application')` — names a FILE, + // found the way the runtime finds it: extensions, `index` files, path + // aliases. Matching the basename instead missed every extensionless one + // (the common spelling) and could land on a same-named file elsewhere. + if (isJsPathImportRef(ref)) { + const file = resolveImportPath(ref.referenceName, ref.filePath, ref.language, context); + const fileNode = file && file !== ref.filePath ? context.getNodesInFile(file).find((n) => n.kind === 'file') : undefined; + if (fileNode) return { original: ref, targetNodeId: fileNode.id, confidence: 0.9, resolvedBy: 'import' }; + } // C/C++ #include references — resolve directly to the included file // (file→file edge), bypassing symbol lookup. The extractor emits these // with `referenceKind: 'imports'` and `referenceName: ` diff --git a/src/resolution/index.ts b/src/resolution/index.ts index d05bc41a4..e12d24979 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -26,7 +26,7 @@ import { isVisibleCppMacro, clearCppMacroVisibility } from './cpp-macro-visibili import { isCppConstructorRef, matchCppConstructor } from './cpp-constructor'; import { gateSwiftTypeTarget, clearSwiftTypeVisibility, swiftExtendedConformances } from './swift-type-visibility'; import { gateTypeParameter, clearTypeParameterMemos } from './type-parameters'; -import { resolveViaImport, resolvePhpImportedStaticCall, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isBoundToOutOfRepoImport, clearImportResolverMemos, resolveImportPath, isExternalImport } from './import-resolver'; +import { resolveViaImport, resolvePhpImportedStaticCall, resolveJvmImport, extractImportMappings, extractReExports, loadCppIncludeDirs, isPhpIncludePathRef, isCobolCopybookRef, isNixPathImportRef, isJsPathImportRef, isBoundToOutOfRepoImport, clearImportResolverMemos, resolveImportPath, isExternalImport } from './import-resolver'; import { ResolverPool, minRefsForPool } from './resolver-pool'; import { resolveAliasBinding } from './alias-binding'; import { detectFrameworks } from './frameworks'; @@ -1100,8 +1100,9 @@ export class ReferenceResolver { // ArkTS chained-attribute refs carry a leading dot (`.titleStyle`) that // routes them to the decorator-gated matcher; the symbol itself is // indexed under the bare name, so the existence check strips the dot. - // Nix static path imports (`import ./x.nix`) name a FILE, not a symbol — - // they bypass the symbol-existence check and resolve via resolveViaImport. + // Nix static path imports (`import ./x.nix`) and JS/TS module paths + // (`require('./x')`) name a FILE, not a symbol — they bypass the + // symbol-existence check and resolve via resolveViaImport. let existenceName = ref.language === 'arkts' && ref.referenceName.startsWith('.') ? ref.referenceName.slice(1) @@ -1112,6 +1113,7 @@ export class ReferenceResolver { const tPre = this.profileStages ? process.hrtime.bigint() : 0n; const preFilterPass = isNixPathImportRef(ref) || + isJsPathImportRef(ref) || this.hasAnyPossibleMatch(existenceName) || // PHP, Pascal, CFML, COBOL and VB.NET names ignore case: `formatprice()` // calls `FormatPrice`, which the exact-name set never lists.