mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-18 13:53:01 +08:00
fix(js): a CommonJS require and a module path import the file they name (#2263)
* 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 <noreply@anthropic.com>
* fix(js): read require imports inside each extractor, so kernel parity holds
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<string, string> = {
|
||||
'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);
|
||||
});
|
||||
});
|
||||
@@ -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<Language> = 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 = /(?<![\w$.])require\s*\(\s*(?:'([^'\n]+)'|"([^"\n]+)"|`([^`$\n]+)`)\s*\)/g;
|
||||
|
||||
/**
|
||||
* A module of this project: a relative or rooted path, or a project alias
|
||||
* (`@/lib/x`, `~/x`, `#internal`). A package (`express`, `lodash/fp`) is
|
||||
* outside the index — as a name it would only match some same-named symbol.
|
||||
*/
|
||||
const LOCAL_SPECIFIER = /^(?:\.{1,2}(?:\/|$)|\/|[@~]\/|#)/;
|
||||
|
||||
/** `import x = require('y')` is TypeScript's import-equals: an `import_statement` already. */
|
||||
const IMPORT_EQUALS = /\bimport\s+(?:type\s+)?[\w$]+\s*=\s*$/;
|
||||
|
||||
/** The file node both extractors mint: `file:<path>`. */
|
||||
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<string>();
|
||||
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;
|
||||
}
|
||||
@@ -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));
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -30,6 +30,8 @@ export interface KernelStoreBundle {
|
||||
language: Language;
|
||||
buffers: NonNullable<ExtractionResult['kernelBuffers']>;
|
||||
file: FileRecord;
|
||||
/** References read beside the kernel's tables (a CommonJS `require`). */
|
||||
extraRefs?: ExtractionResult['unresolvedReferences'];
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -1529,10 +1529,41 @@ export function resolvePhpImportedStaticCall(
|
||||
return { original: ref, targetNodeId: methods[0]!.id, confidence: 0.95, resolvedBy: 'import' };
|
||||
}
|
||||
|
||||
const JS_MODULE_LANGUAGES: ReadonlySet<string> = 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: <include path>`
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user