Skip to content

Commit 37dd13e

Browse files
committed
fix(cli): harden the createRequire scan against nested args and comment noise
Match createRequire(fileURLToPath(import.meta.url)) by allowing one level of nested parens in the argument, skip hits on commented-out lines, and only scan files that import the module builtin so unrelated functions named createRequire never warn.
1 parent fc80816 commit 37dd13e

2 files changed

Lines changed: 61 additions & 3 deletions

File tree

packages/cli-v3/src/build/createRequireWarnings.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,41 @@ globalThis.require = createRequire(import.meta.url);
128128
expect(scanSourceForCreateRequire(source)).toEqual([]);
129129
});
130130

131+
it("finds calls when the createRequire argument contains a nested call", () => {
132+
const source = `import { createRequire } from "node:module";
133+
import { fileURLToPath } from "node:url";
134+
const mssql = createRequire(fileURLToPath(import.meta.url))("mssql");
135+
const req = createRequire(fileURLToPath(import.meta.url));
136+
const pg = req("pg");
137+
`;
138+
139+
expect(scanSourceForCreateRequire(source).map((r) => r.specifier)).toEqual(["mssql", "pg"]);
140+
});
141+
142+
it("ignores hits inside comments", () => {
143+
const source = `import { createRequire } from "node:module";
144+
// const mssql = createRequire(import.meta.url)("mssql");
145+
/* const pg = createRequire(import.meta.url)("pg"); */
146+
/**
147+
* Example: createRequire(import.meta.url)("sharp")
148+
*/
149+
const real = createRequire(import.meta.url)("bcrypt");
150+
`;
151+
152+
expect(scanSourceForCreateRequire(source).map((r) => r.specifier)).toEqual(["bcrypt"]);
153+
});
154+
155+
it("ignores files that never import the module builtin", () => {
156+
const source = `function createRequire(config: string) {
157+
return (name: string) => registry.get(config, name);
158+
}
159+
const load = createRequire("defaults");
160+
const plugin = load("mssql");
161+
`;
162+
163+
expect(scanSourceForCreateRequire(source)).toEqual([]);
164+
});
165+
131166
it("returns nothing when the source doesn't mention createRequire", () => {
132167
const source = `import mssql from "mssql";
133168
export const pool = mssql.connect();

packages/cli-v3/src/build/createRequireWarnings.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ export type CreateRequireUsage = CreateRequireSpecifier & {
2121

2222
const IDENTIFIER = "[A-Za-z_$][\\w$]*";
2323
const STRING_LITERAL = `(["'])([^"'\\n]+)\\1`;
24+
const NESTED_CALL_ARGS = `(?:[^()]|\\([^()]*\\))*`;
25+
const MODULE_IMPORT_REGEX = /(?:from\s*|require\(\s*|import\(\s*)["'](?:node:)?module["']/;
2426

2527
/**
2628
* Finds string-literal package specifiers loaded through `createRequire`, e.g.
@@ -33,13 +35,13 @@ const STRING_LITERAL = `(["'])([^"'\\n]+)\\1`;
3335
* computed specifiers or a re-exported `createRequire` are not detected.
3436
*/
3537
export function scanSourceForCreateRequire(source: string): CreateRequireSpecifier[] {
36-
if (!source.includes("createRequire")) {
38+
if (!source.includes("createRequire") || !MODULE_IMPORT_REGEX.test(source)) {
3739
return [];
3840
}
3941

4042
const aliases = collectCreateRequireAliases(source);
4143
const aliasPattern = Array.from(aliases).map(escapeRegExp).join("|");
42-
const createRequireCall = `(?:${IDENTIFIER}\\s*\\.\\s*)?(?:${aliasPattern})\\s*\\([^()]*\\)`;
44+
const createRequireCall = `(?:${IDENTIFIER}\\s*\\.\\s*)?(?:${aliasPattern})\\s*\\(${NESTED_CALL_ARGS}\\)`;
4345

4446
const results: CreateRequireSpecifier[] = [];
4547
const seen = new Set<string>();
@@ -51,8 +53,14 @@ export function scanSourceForCreateRequire(source: string): CreateRequireSpecifi
5153
return;
5254
}
5355

56+
const location = locationAt(source, index);
57+
58+
if (isCommentedOut(location.lineText, location.column)) {
59+
return;
60+
}
61+
5462
seen.add(key);
55-
results.push({ specifier, ...locationAt(source, index) });
63+
results.push({ specifier, ...location });
5664
};
5765

5866
const directCallRegex = new RegExp(
@@ -137,6 +145,21 @@ function isWarnableSpecifier(specifier: string): boolean {
137145
return !builtinModules.includes(packageNameForSpecifier(specifier));
138146
}
139147

148+
/**
149+
* Line-level heuristic for hits inside comments (commented-out code is the
150+
* realistic false-positive source). A `//` or `/*` before the hit on the same
151+
* line, or a line shaped like a block-comment continuation, means skip.
152+
*/
153+
function isCommentedOut(lineText: string, column: number): boolean {
154+
const prefix = lineText.slice(0, column);
155+
156+
if (prefix.includes("//") || prefix.includes("/*")) {
157+
return true;
158+
}
159+
160+
return lineText.trimStart().startsWith("*");
161+
}
162+
140163
function locationAt(
141164
source: string,
142165
index: number

0 commit comments

Comments
 (0)