diff --git a/.github/workflows/source-lints.yml b/.github/workflows/source-lints.yml index be44e6e90a55..3efd99f2b625 100644 --- a/.github/workflows/source-lints.yml +++ b/.github/workflows/source-lints.yml @@ -13,6 +13,7 @@ on: branches: [main] paths: - "src/**/*.rs" + - "src/codegen/**" - "src/jsc/bindings/**" - "scripts/build/**" - "scripts/glob-sources.ts" @@ -25,6 +26,7 @@ on: pull_request: paths: - "src/**/*.rs" + - "src/codegen/**" - "src/jsc/bindings/**" - "scripts/build/**" - "scripts/glob-sources.ts" diff --git a/src/codegen/builtin-parser.ts b/src/codegen/builtin-parser.ts index eab108b8fae0..d552f64b090c 100644 --- a/src/codegen/builtin-parser.ts +++ b/src/codegen/builtin-parser.ts @@ -9,7 +9,8 @@ function createStopRegex(allow_comma: boolean) { "((?:[(,=;:{]|return|\\=\\>)\\s*)\\/[^\\/\\*]|\\/\\*|\\/\\/|['\"}`\\)" + (allow_comma ? "," : "") + "]|(? escapeRegex(x) + "\\(").join("|") + + // `$macro(` is expanded by applyReplacements; `$macro<` (type arguments) is rejected there. + function_replacements.map(x => escapeRegex(x) + "[(<]").join("|") + ")", ); } diff --git a/src/codegen/replacements.ts b/src/codegen/replacements.ts index af31730db055..97ed7d979377 100644 --- a/src/codegen/replacements.ts +++ b/src/codegen/replacements.ts @@ -184,7 +184,8 @@ export const function_replacements = [ "$isPromisePending", "$bindgenFn", ]; -const function_regexp = new RegExp(`__intrinsic__(${function_replacements.join("|").replaceAll("$", "")})`); +// Anchored: builtin-parser.ts ends the chunk right after the macro being called; any earlier macro name is a plain reference. +const function_regexp = new RegExp(`__intrinsic__(${function_replacements.join("|").replaceAll("$", "")})$`); /** Applies source code replacements as defined in `replacements` */ export function applyReplacements(src: string, length: number) { @@ -197,8 +198,14 @@ export function applyReplacements(src: string, length: number) { replacement.toRaw ?? replacement.to!.replaceAll("$", "__intrinsic__").replaceAll("%", "$"), ); } - let match; - if ((match = slice.match(function_regexp)) && rest.startsWith("(")) { + const match = slice.match(function_regexp); + if (match && rest.startsWith("<")) { + throw new Error( + `$${match[1]} does not accept type arguments; only '$${match[1]}(' is expanded at bundle time. ` + + `Cast the result instead: '$${match[1]}(...) as T'. Found: '$${match[1]}${rest.slice(0, 80)}'`, + ); + } + if (match && rest.startsWith("(")) { const name = match[1]; if (name === "debug") { const innerSlice = sliceSourceCode(rest, true); diff --git a/src/js/private.d.ts b/src/js/private.d.ts index 106355dcb997..4770663cc420 100644 --- a/src/js/private.d.ts +++ b/src/js/private.d.ts @@ -212,7 +212,7 @@ interface JSCommonJSModule { * * @returns whatever the binding function returns. */ -declare function $cpp(filename: NativeFilenameCPP, symbol: string): T; +declare function $cpp(filename: NativeFilenameCPP, symbol: string): any; /** * Call a native Rust binding function, getting whatever it returns. * @@ -230,17 +230,13 @@ declare function $cpp(filename: NativeFilenameCPP, symbol: string): T; * * @returns whatever the binding function returns. */ -declare function $rust(filename: NativeFilenameRust, symbol: string): T; -declare function $newCppFunction any>( - filename: NativeFilenameCPP, - symbol: string, - argCount: number, -): T; -declare function $newRustFunction any>( +declare function $rust(filename: NativeFilenameRust, symbol: string): any; +declare function $newCppFunction(filename: NativeFilenameCPP, symbol: string, argCount: number): (...args: any) => any; +declare function $newRustFunction( filename: NativeFilenameRust, symbol: string, argCount: number, -): T; +): (...args: any) => any; /** * Retrieves a handle to a function defined in native code, defined in a * `.bind.ts` file. For more information on how to define bindgen functions, see @@ -248,7 +244,7 @@ declare function $newRustFunction any>( * @param filename - The basename of the `.bind.ts` file. * @param symbol - The name of the function to call. */ -declare function $bindgenFn any>(filename: string, symbol: string): T; +declare function $bindgenFn(filename: string, symbol: string): (...args: any) => any; // NOTE: $debug, $assert, and $isPromiseFulfilled omitted declare module "node:net" { diff --git a/test/internal/source-lints/codegen-builtin-macros.test.ts b/test/internal/source-lints/codegen-builtin-macros.test.ts new file mode 100644 index 000000000000..ac63a48db7da --- /dev/null +++ b/test/internal/source-lints/codegen-builtin-macros.test.ts @@ -0,0 +1,93 @@ +/** Unit tests for the `$macro(...)` expansion that bundle-modules.ts and + * bundle-functions.ts apply to the builtin JS in src/js + * (src/codegen/builtin-parser.ts + src/codegen/replacements.ts). Nothing here + * needs the built binary: the preprocessor is plain TypeScript run at build time. */ +import { describe, expect, test } from "bun:test"; + +import { sliceSourceCode } from "../../../src/codegen/builtin-parser.ts"; +import { function_replacements } from "../../../src/codegen/replacements.ts"; + +/** One well-formed call per macro and what it expands to. The native macros + * are resolved against the real tree (generate-js2native.ts), so their + * arguments name files that exist in src/. */ +const macros: [macro: string, args: string, expansion: string | RegExp][] = [ + ["$debug", `("x")`, `(IS_BUN_DEVELOPMENT?$debug_log("x"):void 0)`], + ["$assert", `(ok, "x")`, `!(IS_BUN_DEVELOPMENT?$assert(ok,"ok", "x"):void 0)`], + ["$rust", `("node_util_binding.rs", "internalErrorName")`, /^__intrinsic__lazy\(\d+\)$/], + ["$newRustFunction", `("node_util_binding.rs", "internalErrorName", 1)`, /^__intrinsic__lazy\(\d+\)$/], + ["$cpp", `("NodeValidator.cpp", "validateInteger")`, /^__intrinsic__lazy\(\d+\)$/], + ["$newCppFunction", `("NodeValidator.cpp", "validateInteger", 4)`, /^__intrinsic__lazy\(\d+\)$/], + ["$bindgenFn", `("bindgen_test.bind.ts", "add")`, /^__intrinsic__lazy\(\d+\)$/], + ["$isPromisePending", `(p)`, `(__intrinsic__peekPromiseStatus(p) === 0)`], + ["$isPromiseFulfilled", `(p)`, `(__intrinsic__peekPromiseStatus(p) === 1)`], + ["$isPromiseRejected", `(p)`, `(__intrinsic__peekPromiseStatus(p) === 2)`], +]; + +function expand(code: string): string { + const { result, rest } = sliceSourceCode(`{ const v = ${code}; }`, true); + expect(rest).toBe(""); + expect(result).toStartWith("{ const v = "); + expect(result).toEndWith("; }"); + return result.slice("{ const v = ".length, -"; }".length); +} + +test("every macro the preprocessor expands is covered below", () => { + expect(macros.map(([macro]) => macro).sort()).toEqual([...function_replacements].sort()); +}); + +describe.each(macros)("%s", (macro, args, expansion) => { + test("a plain call is expanded", () => { + const expanded = expand(`${macro}${args}`); + if (typeof expansion === "string") { + expect(expanded).toBe(expansion); + } else { + expect(expanded).toMatch(expansion); + } + }); + + // private.d.ts used to declare the native macros as generic, so + // `$newRustFunction<() => number>(...)` type-checked, and the preprocessor + // only looked for `$name(`: the call went into the bundle unexpanded and the + // native side was never registered, which only surfaced later as a dead-code + // error in cargo or as a module that fails to load. + test("a call with type arguments is a bundle-time error", () => { + expect(() => expand(`${macro}<() => number>${args}`)).toThrow( + `${macro} does not accept type arguments; only '${macro}(' is expanded at bundle time. ` + + `Cast the result instead: '${macro}(...) as T'. Found: '${macro}<() => number>${args}`, + ); + }); +}); + +test("type arguments are rejected inside macro arguments and template substitutions too", () => { + // $assert's condition is sliced in end-on-comma mode, which uses the other + // stop regexp; template substitutions re-enter the slicer. + expect(() => expand(`$assert($rust("node_util_binding.rs", "internalErrorName"))`)).toThrow( + "$rust does not accept type arguments", + ); + expect(() => expand(`$debug("x", $newCppFunction("NodeValidator.cpp", "validateInteger", 4))`)).toThrow( + "$newCppFunction does not accept type arguments", + ); + expect(() => expand('`${$cpp("NodeValidator.cpp", "validateInteger")}`')).toThrow( + "$cpp does not accept type arguments", + ); +}); + +test("type arguments inside strings, comments and template text are not code", () => { + const code = `{ const s = "$rust(a, b)" + \`$bindgenFn(a, b) \${1}\`; /* $cpp(a, b) */ // $newRustFunction(a, b, 0)\n}`; + expect(sliceSourceCode(code, true)).toEqual({ result: code, rest: "" }); +}); + +test("a macro name that is not being called passes through as a plain intrinsic", () => { + // `$debug` is also a value: bundle-modules defines `__intrinsic__debug` to the + // debug-logging flag, so `if ($debug)` is a real pattern in src/js. + expect(sliceSourceCode(`{ if ($debug) { x(); } }`, true)).toEqual({ + result: `{ if (__intrinsic__debug) { x(); } }`, + rest: "", + }); + expect(expand(`$debug && !handle`)).toBe(`__intrinsic__debug && !handle`); +}); + +test("a bare reference earlier in the same chunk does not hijack the call being expanded", () => { + expect(expand(`$debug && $debug("x")`)).toBe(`__intrinsic__debug && (IS_BUN_DEVELOPMENT?$debug_log("x"):void 0)`); + expect(expand(`$assert && $debug("x")`)).toBe(`__intrinsic__assert && (IS_BUN_DEVELOPMENT?$debug_log("x"):void 0)`); +});