node:module: list node:test in builtinModules

.github/workflows/source-lints.yml

@@ -17,6 +17,7 @@ on:
       - "src/codegen/class-definitions.ts"
       - "src/js/builtins.d.ts"
       - "src/jsc/bindings/**"
+      - "src/jsc/modules/NodeModuleModule.cpp"
       - "packages/bun-types/redis.d.ts"
       - "scripts/build/**"
       - "scripts/glob-sources.ts"
@@ -38,6 +39,7 @@ on:
       - "src/codegen/class-definitions.ts"
       - "src/js/builtins.d.ts"
       - "src/jsc/bindings/**"
+      - "src/jsc/modules/NodeModuleModule.cpp"
       - "packages/bun-types/redis.d.ts"
       - "scripts/build/**"
       - "scripts/glob-sources.ts"

src/jsc/modules/NodeModuleModule.cpp

@@ -42,11 +42,12 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionWrap);
 JSC_DECLARE_CUSTOM_GETTER(getterRequireFunction);
 JSC_DECLARE_CUSTOM_SETTER(setterRequireFunction);
 
-// This is a list of builtin module names that do not have the node prefix. It
-// also includes Bun's builtin modules, as well as Bun's thirdparty overrides.
-// The reason for overstuffing this list is so that uses that use these as the
-// 'external' option to a bundler will properly exclude things like 'ws' which
-// only work with Bun's native 'ws' implementation and not the JS one on NPM.
+// module.builtinModules. As in Node, a builtin is listed without the "node:"
+// prefix unless it only resolves with the prefix ("node:sqlite", "node:test").
+// Bun's own modules and Bun's thirdparty overrides are listed too, so that
+// users who pass this list as a bundler's 'external' option exclude things
+// like 'ws', which only works with Bun's native implementation and not the JS
+// one on NPM. builtin-module-tables.test.ts checks it against isBuiltinModule.cpp.
 static constexpr ASCIILiteral builtinModuleNames[] = {
     "_http_agent"_s,
     "_http_client"_s,
@@ -92,6 +93,7 @@ static constexpr ASCIILiteral builtinModuleNames[] = {
     "module"_s,
     "net"_s,
     "node:sqlite"_s,
+    "node:test"_s,
     "os"_s,
     "path"_s,
     "path/posix"_s,

test/internal/source-lints/builtin-module-tables.test.ts

@@ -0,0 +1,73 @@
+import { expect, test } from "bun:test";
+import { readFileSync } from "node:fs";
+import path from "node:path";
+
+// The names of the builtin modules live in three hand-written tables:
+//
+// - builtinModuleNames in src/jsc/modules/NodeModuleModule.cpp is module.builtinModules.
+// - builtinModuleNamesSortedLength in src/jsc/bindings/isBuiltinModule.cpp is what module.isBuiltin()
+//   accepts (and what Bun.plugin refuses to override).
+// - node_entry_only_prefix!() in src/resolve_builtins/HardcodedModule.rs marks the builtins that the module
+//   loader resolves with the "node:" prefix only. Node lists those in builtinModules with the prefix.
+//
+// Nothing else compares them. A new module tends to land in the last two and not in the first: "node:test"
+// was missing from builtinModules for over a year while isBuiltin("node:test") was true. This lint requires
+// every name isBuiltin() accepts to be listed in builtinModules, unless notListed says why it is not.
+
+// isBuiltin() names that builtinModules leaves out on purpose, with the reason. The lint fails while an entry
+// here is listed after all, or is no longer accepted by isBuiltin().
+const internalPlumbing =
+  "internal plumbing, not a public module (#31831). isBuiltin() accepts it so that Bun.plugin refuses to override it.";
+const notListed: Record<string, string> = {
+  "bun:main": internalPlumbing,
+  "bun:wrap": internalPlumbing,
+  "node:quic": "experimental. Stock Node 26 builds do not list it either: QUIC is compiled out of them.",
+};
+
+const builtinModulesFile = "src/jsc/modules/NodeModuleModule.cpp";
+const isBuiltinFile = "src/jsc/bindings/isBuiltinModule.cpp";
+const resolverFile = "src/resolve_builtins/HardcodedModule.rs";
+const root = path.resolve(import.meta.dir, "..", "..", "..");
+
+function asciiLiteralTable(file: string, name: string): Set<string> {
+  const source = readFileSync(path.join(root, file), "utf8");
+  const table = new RegExp(String.raw`\b${name}\[\] = \{([^}]*)\};`).exec(source);
+  if (table === null) throw new Error(`${file} no longer defines the table ${name}[]`);
+  const names = new Set(Array.from(table[1].matchAll(/"([^"]+)"_s/g), m => m[1]));
+  if (names.size === 0) throw new Error(`${file}: the table ${name}[] has no "..."_s entries`);
+  return names;
+}
+
+const builtinModules = asciiLiteralTable(builtinModulesFile, "builtinModuleNames");
+const isBuiltin = asciiLiteralTable(isBuiltinFile, "builtinModuleNamesSortedLength");
+const onlyPrefix = Array.from(
+  readFileSync(path.join(root, resolverFile), "utf8").matchAll(/\bnode_entry_only_prefix!\("([^"]+)"\)/g),
+  m => m[1],
+);
+if (onlyPrefix.length === 0) throw new Error(`${resolverFile} has no node_entry_only_prefix!() entries`);
+
+test(`${builtinModulesFile} lists every name ${isBuiltinFile} accepts, unless notListed says why not`, () => {
+  const missing = [...isBuiltin].filter(name => !builtinModules.has(name) && !Object.hasOwn(notListed, name)).sort();
+  expect(missing).toEqual([]);
+});
+
+test(`${isBuiltinFile} accepts every name ${builtinModulesFile} lists`, () => {
+  const unknown = [...builtinModules].filter(name => !isBuiltin.has(name)).sort();
+  expect(unknown).toEqual([]);
+});
+
+test(`${isBuiltinFile} accepts every prefix-only builtin of ${resolverFile} under its prefixed name`, () => {
+  const unknown = onlyPrefix.filter(name => !isBuiltin.has(name)).sort();
+  expect(unknown).toEqual([]);
+});
+
+test("notListed names only entries that isBuiltin() still accepts and builtinModules still leaves out", () => {
+  const stale = Object.keys(notListed)
+    .flatMap(name => {
+      if (!isBuiltin.has(name)) return [`${name} is no longer in ${isBuiltinFile}; delete its entry`];
+      if (builtinModules.has(name)) return [`${name} is listed in ${builtinModulesFile} now; delete its entry`];
+      return [];
+    })
+    .sort();
+  expect(stale).toEqual([]);
+});

test/js/node/module/node-module-module.test.js

@@ -5,11 +5,20 @@ import Module, { _nodeModulePaths, builtinModules, createRequire, isBuiltin, wra
 import path from "path";
 
 describe.concurrent("node-module-module", () => {
-  test("builtinModules exists", () => {
+  // test/internal/source-lints/builtin-module-tables.test.ts checks the table behind builtinModules against the
+  // table behind isBuiltin(), so this only checks what the array looks like at runtime.
+  test("builtinModules holds requireable builtins and lists prefix-only modules with their prefix, as Node does", () => {
     expect(Array.isArray(builtinModules)).toBe(true);
-    // "bun:wrap" is no longer listed: it is internal transpiler plumbing,
-    // not a requireable public module.
-    expect(builtinModules).toHaveLength(76);
+    expect(builtinModules).toContain("node:test");
+    expect(builtinModules).not.toContain("test");
+    // "bun:wrap" is internal transpiler plumbing, not a requireable public module.
+    expect(builtinModules).not.toContain("bun:wrap");
+    expect(builtinModules.filter(name => !isBuiltin(name))).toEqual([]);
+    // A prefixed entry is listed that way because it is requireable only with its prefix.
+    for (const name of builtinModules.filter(name => name.startsWith("node:"))) {
+      expect(isBuiltin(name.slice("node:".length))).toBe(false);
+      expect(process.getBuiltinModule(name)).toBe(require(name));
+    }
   });
 
   test("isBuiltin() works", () => {