diff --git a/CHANGELOG.md b/CHANGELOG.md index 988ecbf36..03139363d 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 +- 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 `` 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. diff --git a/__tests__/commonjs-require-calls.test.ts b/__tests__/commonjs-require-calls.test.ts new file mode 100644 index 000000000..a131d329e --- /dev/null +++ b/__tests__/commonjs-require-calls.test.ts @@ -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 = { + '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']); + }); +}); diff --git a/src/resolution/alias-binding.ts b/src/resolution/alias-binding.ts index 4b2ec2380..3251abff0 100644 --- a/src/resolution/alias-binding.ts +++ b/src/resolution/alias-binding.ts @@ -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; } diff --git a/src/resolution/import-resolver.ts b/src/resolution/import-resolver.ts index c4a058711..48e68ef15 100644 --- a/src/resolution/import-resolver.ts +++ b/src/resolution/import-resolver.ts @@ -119,6 +119,24 @@ function defaultExportBinding(filePath: string, context: ResolutionContext): str } const fileExportIndexes = new WeakMap>(); +/** + * `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;