mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-18 22:02:49 +08:00
fix(js): resolve calls through CommonJS require bindings (#2189)
Four gaps left CommonJS calls on the local alias variable - or on the wrong
function:
- `require('../')` up to the project root produced an empty relative path, so
its `/index.js` candidate was never found;
- a default import never followed `module.exports = X` /
`exports = module.exports = X` / `module.exports = require('./lib/x')`;
- only the first declarator of `var a = require('x'), b = require('y')` was a
binding, so `b()` - a package call - stayed on the local variable;
- `require('./utils').member` was read as the module's DEFAULT export: every
such alias in express resolved to utils' first exported function
(`normalizeType`). It is now the named member, and `exports.name =
function impl(...)`, `module.exports.name = impl` and `module.exports = { a,
b: c }` bind names the export index could not see.
express -1087 +727 (707 `express()` calls reach createApplication; supertest /
node:assert calls no longer edge to the local variable), eslint -750 +827
(`new RuleTester()` reaches the class, not another test's variable), mocha
-83 +90 (`run(...)` no longer lands in another spec file), koa 65 wrong
`request` edges -> `response`, body-parser +20. zod, trpc, hono, typeorm,
axios unchanged.
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
|
||||
|
||||
- JavaScript calls through CommonJS `require` bindings now reach the right function. `var express = require('../')` follows `module.exports` through an `index.js` to the function it exports, and every binding in a `var a = require('x'), b = require('y')` list is read. `require('./utils').setCharset` means that module's `setCharset` rather than its first export, and a call to a package from outside the project (`supertest`, `node:assert`) is no longer an edge to the local variable. On express, over 700 `express()` calls now reach `createApplication`.
|
||||
- Lua calls made through a `local` alias now reach the function it names. That covers `local splitn = require("kong.tools.string").splitn`, a module member such as `utils.select_listener`, and a function a module exports under another name (`return { check = check_phase }`). Aliases of Lua's own functions, such as `local type = type` or `local fmt = string.format`, no longer show up as calls into your project. On Kong that is over 1,000 calls that now reach their real function, and about 5,000 fewer dead-end edges.
|
||||
- React Router links and `navigate(…)` calls written through a route-config object, like bulletproof-react's `paths.app.discussion.getHref(id)`, now reach the route they open. A `<Link>` with an arrow-function attribute before its `to`, such as `onMouseEnter={() => …}`, is read too.
|
||||
- C++ classes written around macros and preprocessor conditionals now keep their members. These include an access specifier spelled as a macro (`JSON_PRIVATE_UNLESS_TESTED:`), an attribute macro after a template head or a doc comment, and an `#if` in the middle of a declaration, such as inside a constructor's initializer list. The first branch of a mid-declaration `#if` is the one indexed. nlohmann/json's `basic_json` class used to be lost whole and now has its roughly 390 members. Files with a parse error dropped on redis, curl, pybind11 and leveldb, and no file got worse. Re-index after upgrading.
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
/**
|
||||
* Calls through CommonJS `require` bindings (express's shape):
|
||||
* - `var express = require('../')` from `test/` is the project root's
|
||||
* `index.js`, whose `module.exports = require('./lib/express')` forwards to
|
||||
* `exports = module.exports = createApplication` — `express()` is that call;
|
||||
* - each declarator of a list is a binding: `var express = require('../'),
|
||||
* request = require('supertest')` — `request(app)` is the package's, no edge;
|
||||
* - `require('./utils').setCharset` is the module's `setCharset`, written
|
||||
* `exports.setCharset = function setCharset(…)`, not the module's first
|
||||
* export (every such alias in express went to `normalizeType`).
|
||||
*/
|
||||
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', devDependencies: { supertest: '^6' } }),
|
||||
'index.js': `'use strict';
|
||||
|
||||
module.exports = require('./lib/express');
|
||||
`,
|
||||
'lib/express.js': `'use strict';
|
||||
|
||||
exports = module.exports = createApplication;
|
||||
|
||||
function createApplication() {
|
||||
return {};
|
||||
}
|
||||
`,
|
||||
'lib/utils.js': `'use strict';
|
||||
|
||||
exports.normalizeType = function (type) {
|
||||
return type;
|
||||
};
|
||||
|
||||
exports.setCharset = function setCharset(type, charset) {
|
||||
return type + charset;
|
||||
};
|
||||
`,
|
||||
'lib/response.js': `'use strict';
|
||||
|
||||
var normalizeType = require('./utils').normalizeType;
|
||||
var setCharset = require('./utils').setCharset;
|
||||
|
||||
function send(body) {
|
||||
return setCharset(normalizeType(body), 'utf-8');
|
||||
}
|
||||
|
||||
module.exports = { send };
|
||||
`,
|
||||
'test/app.js': `'use strict';
|
||||
|
||||
var express = require('../')
|
||||
, request = require('supertest');
|
||||
|
||||
describe('app', function () {
|
||||
it('responds', function () {
|
||||
var app = express();
|
||||
request(app);
|
||||
});
|
||||
});
|
||||
`,
|
||||
};
|
||||
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 });
|
||||
});
|
||||
|
||||
/** `file: qualified name` of every call edge out of a file. */
|
||||
function callsFrom(file: string): string[] {
|
||||
const ids = cg.getNodesInFile(file).map((n) => n.id);
|
||||
return cg
|
||||
.getOutgoingEdgesFrom(ids)
|
||||
.filter((e) => e.kind === 'calls')
|
||||
.map((e) => {
|
||||
const t = cg.getNode(e.target)!;
|
||||
return `${t.filePath}: ${t.qualifiedName}`;
|
||||
})
|
||||
.sort();
|
||||
}
|
||||
|
||||
describe('calls through CommonJS require bindings', () => {
|
||||
it('follow `module.exports` through the root index to the exported function; an outside package is no edge', () => {
|
||||
expect(callsFrom('test/app.js')).toEqual(['lib/express.js: createApplication']);
|
||||
});
|
||||
|
||||
it('read a `require(…).member` alias as that member', () => {
|
||||
expect(callsFrom('lib/response.js')).toEqual(['lib/utils.js: normalizeType', 'lib/utils.js: setCharset']);
|
||||
});
|
||||
});
|
||||
@@ -131,5 +131,20 @@ export function extractLocalExportAliases(content: string): Array<{ exportedName
|
||||
}
|
||||
}
|
||||
}
|
||||
// CommonJS: `exports.setCharset = function setCharset(…)`, `module.exports.x = impl`.
|
||||
const memberRe = /^[ \t]*(?:module\.)?exports\.([A-Za-z_$][\w$]*)\s*=\s*(?:(?:async\s+)?function\s*\*?\s*([A-Za-z_$][\w$]*)\s*\(|([A-Za-z_$][\w$]*)\s*;?[ \t]*$)/gm;
|
||||
let member: RegExpExecArray | null;
|
||||
while ((member = memberRe.exec(content)) !== null) {
|
||||
out.push({ exportedName: member[1]!, localName: member[2] ?? member[3]! });
|
||||
}
|
||||
// `module.exports = { parse, format: formatDate }`.
|
||||
const objectRe = /^[ \t]*module\.exports\s*=\s*\{([^{}]*)\}/gm;
|
||||
let object: RegExpExecArray | null;
|
||||
while ((object = objectRe.exec(content)) !== null) {
|
||||
for (const raw of object[1]!.split(',')) {
|
||||
const entry = /^\s*([A-Za-z_$][\w$]*)\s*(?::\s*([A-Za-z_$][\w$]*)\s*)?$/.exec(raw);
|
||||
if (entry) out.push({ exportedName: entry[1]!, localName: entry[2] ?? entry[1]! });
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -119,6 +119,24 @@ function defaultExportBinding(filePath: string, context: ResolutionContext): str
|
||||
}
|
||||
const fileExportIndexes = new WeakMap<ResolutionContext, Map<string, FileExportIndex>>();
|
||||
|
||||
/**
|
||||
* `module.exports = …` (also `exports = module.exports = …`): the name it
|
||||
* binds (`createApplication`, `function name(`, `class Name`) or the module
|
||||
* it forwards (`require('./lib/express')`), or null.
|
||||
*/
|
||||
const COMMONJS_DEFAULT_EXPORT =
|
||||
/^[ \t]*(?:exports\s*=\s*)?module\.exports\s*=\s*(?:exports\s*=\s*)?(?:require\(\s*['"]([^'"]+)['"]\s*\)\s*;?[ \t]*$|(?:async\s+)?function\s*\*?\s*([A-Za-z_$][\w$]*)\s*\(|class\s+([A-Za-z_$][\w$]*)|([A-Za-z_$][\w$]*)\s*;?[ \t]*$)/m;
|
||||
|
||||
function commonJsDefaultExport(filePath: string, context: ResolutionContext): { source?: string; name?: string } | null {
|
||||
if (!JS_FAMILY_FILE.test(filePath)) return null;
|
||||
if (context.fileContains && !context.fileContains(filePath, 'module.exports')) return null;
|
||||
const source = context.readFile(filePath);
|
||||
if (!source || !source.includes('module.exports')) return null;
|
||||
const m = COMMONJS_DEFAULT_EXPORT.exec(source);
|
||||
if (!m) return null;
|
||||
return m[1] ? { source: m[1] } : { name: m[2] ?? m[3] ?? m[4] };
|
||||
}
|
||||
|
||||
function getFileExportIndex(filePath: string, context: ResolutionContext): FileExportIndex {
|
||||
let perFile = fileExportIndexes.get(context);
|
||||
if (!perFile) {
|
||||
@@ -505,9 +523,10 @@ function resolveRelativeImport(
|
||||
const basePath = path.resolve(fromDir, importPath);
|
||||
const relativePath = path.relative(projectRoot, basePath).replace(/\\/g, '/');
|
||||
|
||||
// Try each extension
|
||||
// Try each extension. `require('..')` up to the project root is its `index.js`.
|
||||
for (const ext of extensions) {
|
||||
const candidatePath = relativePath + ext;
|
||||
if (relativePath === '' && !ext.startsWith('/')) continue;
|
||||
const candidatePath = relativePath === '' ? ext.slice(1) : relativePath + ext;
|
||||
if (context.fileExists(candidatePath)) {
|
||||
return candidatePath;
|
||||
}
|
||||
@@ -996,17 +1015,18 @@ function extractJSImports(content: string): ImportMapping[] {
|
||||
}
|
||||
}
|
||||
|
||||
// Require statements
|
||||
const requireRegex = /(?:const|let|var)\s+(?:(\w+)|{([^}]+)})\s*=\s*require\(['"]([^'"]+)['"]\)/g;
|
||||
// Require statements — each declarator of a list too (`var express = require('../'),
|
||||
// request = require('supertest')`), and a member of the module (`require('./utils').methods`).
|
||||
const requireRegex = /(?:\b(?:const|let|var)\s+|,\s*)(?:([A-Za-z_$][\w$]*)|{([^}]+)})\s*=\s*require\(\s*['"]([^'"]+)['"]\s*\)(?:\s*\.\s*([A-Za-z_$][\w$]*))?/g;
|
||||
while ((match = requireRegex.exec(content)) !== null) {
|
||||
const [, defaultName, destructured, source] = match;
|
||||
const [, defaultName, destructured, source, member] = match;
|
||||
|
||||
if (defaultName) {
|
||||
mappings.push({
|
||||
localName: defaultName,
|
||||
exportedName: 'default',
|
||||
exportedName: member ?? 'default',
|
||||
source: source!,
|
||||
isDefault: true,
|
||||
isDefault: member === undefined,
|
||||
isNamespace: false,
|
||||
});
|
||||
}
|
||||
@@ -2423,6 +2443,17 @@ function findExportedSymbolWalk(
|
||||
const direct =
|
||||
exportIndex.defaultComponent ?? defaultExportBindingNode(filePath, exportIndex, context) ?? exportIndex.defaultFnClass;
|
||||
if (direct) return direct;
|
||||
// CommonJS: `module.exports = createApplication`, or `= require('./lib/express')`.
|
||||
const commonJs = commonJsDefaultExport(filePath, context);
|
||||
if (commonJs?.source) {
|
||||
const next = resolveImportPath(commonJs.source, filePath, language, context);
|
||||
if (next) return findExportedSymbol(next, want, language, context, visited, depth + 1);
|
||||
} else if (commonJs?.name) {
|
||||
const bound = nodesInFileNamed(filePath, commonJs.name, context)
|
||||
.filter((n) => DEFAULT_BINDING_KINDS.has(n.kind))
|
||||
.sort((a, b) => a.startLine - b.startLine || a.startColumn - b.startColumn)[0];
|
||||
if (bound) return bound;
|
||||
}
|
||||
} else if (want.isNamespace && want.memberName) {
|
||||
const direct = exportedByName(filePath, exportIndex, want.memberName, context);
|
||||
if (direct) return direct;
|
||||
|
||||
Reference in New Issue
Block a user