bundler: warn about direct eval in ES modules, with one guard for weird-code warnings

src/ast/lib.rs

@@ -2068,6 +2068,9 @@ impl Log {
         notes: Box<[Data]>,
         args: fmt::Arguments<'_>,
     ) {
+        if !Kind::Warn.should_print(self.level) {
+            return;
+        }
         let text = alloc_print(args);
         self.add_formatted_msg(Kind::Warn, source, r, text, notes, false)
     }

src/js_parser/p.rs

@@ -1001,6 +1001,60 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         Ok(())
     }
 
+    /// A visit-pass warning about valid but suspect code is silent inside node_modules, as in
+    /// esbuild, and on the minifier's second visit of a statement.
+    fn warns_about_weird_code(&self) -> bool {
+        !self.is_revisit_for_substitution
+            && !bun_paths::strings::path_contains_node_modules_folder(self.source.path.text)
+    }
+
+    /// `problem` is the start of the sentence, such as "Writing to read-only method".
+    #[cold]
+    #[inline(never)]
+    pub(crate) fn warn_private_name(
+        &mut self,
+        problem: &'static str,
+        name: &[u8],
+        loc: bun_ast::Loc,
+    ) {
+        if !self.warns_about_weird_code() {
+            return;
+        }
+
+        let r = bun_ast::Range {
+            loc,
+            len: i32::try_from(name.len()).expect("int cast"),
+        };
+        self.log().add_range_warning_fmt(
+            Some(self.source),
+            r,
+            format_args!("{} \"{}\" will throw", problem, bstr::BStr::new(name)),
+        );
+    }
+
+    /// esbuild's `direct-eval` warning. The caller checks that the file is a bundled ES module.
+    #[cold]
+    #[inline(never)]
+    pub(crate) fn warn_direct_eval(&mut self, loc: bun_ast::Loc) {
+        if !self.warns_about_weird_code() {
+            return;
+        }
+
+        let r = js_lexer::range_of_identifier(self.source, loc);
+        self.log().add_range_warning_fmt_with_notes(
+            Some(self.source),
+            r,
+            Box::new([bun_ast::range_data(
+                None,
+                bun_ast::Range::NONE,
+                "Use globalThis.eval(code) when the code reads only global variables. To give the code a variable, an import or require from this file, pass it as an argument: new Function(\"name\", code)(name)",
+            )]),
+            format_args!(
+                "Using direct eval with a bundler is not recommended and may cause problems"
+            ),
+        );
+    }
+
     // ───────────────────────────────────────────────────────────────────────
     // Inlined ExpressionTransposer recursion.
     // Defined as `&mut self` methods so the only live `&mut` is the caller's

src/js_parser/visit/visit_expr.rs

@@ -941,49 +941,20 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                         && (kind == js_ast::symbol::Kind::PrivateMethod
                             || kind == js_ast::symbol::Kind::PrivateStaticMethod)
                     {
-                        let r = bun_ast::Range {
-                            loc: e_.index.loc,
-                            len: i32::try_from(name.len()).expect("int cast"),
-                        };
-                        p.log().add_range_warning_fmt(
-                            Some(p.source),
-                            r,
-                            format_args!(
-                                "Writing to read-only method \"{}\" will throw",
-                                BStr::new(name)
-                            ),
-                        );
+                        p.warn_private_name("Writing to read-only method", name, e_.index.loc);
                     } else if in_.assign_target != js_ast::AssignTarget::None
                         && (kind == js_ast::symbol::Kind::PrivateGet
                             || kind == js_ast::symbol::Kind::PrivateStaticGet)
                     {
-                        let r = bun_ast::Range {
-                            loc: e_.index.loc,
-                            len: i32::try_from(name.len()).expect("int cast"),
-                        };
-                        p.log().add_range_warning_fmt(
-                            Some(p.source),
-                            r,
-                            format_args!(
-                                "Writing to getter-only property \"{}\" will throw",
-                                BStr::new(name)
-                            ),
-                        );
+                        p.warn_private_name("Writing to getter-only property", name, e_.index.loc);
                     } else if in_.assign_target != js_ast::AssignTarget::Replace
                         && (kind == js_ast::symbol::Kind::PrivateSet
                             || kind == js_ast::symbol::Kind::PrivateStaticSet)
                     {
-                        let r = bun_ast::Range {
-                            loc: e_.index.loc,
-                            len: i32::try_from(name.len()).expect("int cast"),
-                        };
-                        p.log().add_range_warning_fmt(
-                            Some(p.source),
-                            r,
-                            format_args!(
-                                "Reading from setter-only property \"{}\" will throw",
-                                BStr::new(name)
-                            ),
+                        p.warn_private_name(
+                            "Reading from setter-only property",
+                            name,
+                            e_.index.loc,
                         );
                     }
                 }
@@ -1941,12 +1912,18 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 {
                     e_.is_direct_eval = true;
 
-                    // Pessimistically assume that if this looks like a CommonJS module
-                    // (e.g. no "export" keywords), a direct call to "eval" means that
-                    // code could potentially access "module" or "exports".
-                    if p.options.bundle && !p.is_file_considered_to_have_esm_exports {
-                        p.record_usage(p.module_ref);
-                        p.record_usage(p.exports_ref);
+                    if p.options.bundle {
+                        // Pessimistically assume that if this looks like a CommonJS module
+                        // (e.g. no "export" keywords), a direct call to "eval" means that
+                        // code could potentially access "module" or "exports".
+                        if !p.is_file_considered_to_have_esm_exports {
+                            p.record_usage(p.module_ref);
+                            p.record_usage(p.exports_ref);
+                        }
+
+                        if p.is_file_considered_to_have_esm_exports || p.has_es_module_syntax {
+                            p.warn_direct_eval(e_.target.loc);
+                        }
                     }
 
                     // Walk `Scope.parent` (`Option<StoreRef<Scope>>`) via the safe
@@ -1957,8 +1934,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                         scope.contains_direct_eval = true;
                         scope_iter = scope.parent;
                     }
-
-                    // TODO: Log a build note for this like esbuild does
                 }
             }
             Data::EDot(dot) => {

test/bundler/bun-build-api.test.ts

@@ -745,6 +745,105 @@ describe("Bun.build", () => {
     Bun.gc(true);
   });
 
+  test("warns about a direct eval in an ES module", async () => {
+    const evalLine = `console.log(eval("secret + helper()"));`;
+    using dir = tempDir("bun-build-api-direct-eval", {
+      "esm.js": `import { helper } from "./lib.js";\nconst secret = 42;\n${evalLine}\n`,
+      "lib.js": `export function helper() { return 1; }\n`,
+      "commonjs.js": `module.exports = eval("1");\n`,
+    });
+    // `notes` is not in the `BuildMessage` type.
+    const plain = (message: any) => ({ level: message.level, message: message.message, position: message.position });
+    const logsOf = async (entry: string) => {
+      const build = await Bun.build({ entrypoints: [join(String(dir), entry)] });
+      return {
+        success: build.success,
+        logs: build.logs.map((log: any) => ({ ...plain(log), notes: log.notes.map(plain) })),
+      };
+    };
+
+    expect(await logsOf("esm.js")).toEqual({
+      success: true,
+      logs: [
+        {
+          level: "warn",
+          message: "Using direct eval with a bundler is not recommended and may cause problems",
+          position: {
+            file: expect.stringMatching(/esm\.js$/),
+            namespace: "file",
+            line: 3,
+            column: evalLine.indexOf("eval") + 1,
+            length: "eval".length,
+            offset: expect.any(Number),
+            lineText: evalLine,
+          },
+          notes: [
+            {
+              level: "note",
+              message:
+                'Use globalThis.eval(code) when the code reads only global variables. To give the code a variable, an import or require from this file, pass it as an argument: new Function("name", code)(name)',
+              position: null,
+            },
+          ],
+        },
+      ],
+    });
+    expect(await logsOf("commonjs.js")).toEqual({ success: true, logs: [] });
+  });
+
+  test('logLevel = "error" in bunfig.toml drops the direct eval warning', async () => {
+    const project = {
+      "entry.js": `export const value = eval("1");\n`,
+      "build.js": `
+        const build = await Bun.build({ entrypoints: ["./entry.js"] });
+        console.log(JSON.stringify({ success: build.success, logs: build.logs.map(log => log.message) }));
+      `,
+    };
+    using dir = tempDir("bun-build-api-direct-eval-log-level", {
+      default: project,
+      quiet: { ...project, "bunfig.toml": `logLevel = "error"\n` },
+    });
+    const build = async (project: string) => {
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "build.js"],
+        env: bunEnv,
+        cwd: join(String(dir), project),
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      return { stdout: JSON.parse(stdout), stderr, exitCode };
+    };
+
+    expect({ default: await build("default"), quiet: await build("quiet") }).toEqual({
+      default: {
+        stdout: {
+          success: true,
+          logs: ["Using direct eval with a bundler is not recommended and may cause problems"],
+        },
+        stderr: "",
+        exitCode: 0,
+      },
+      quiet: { stdout: { success: true, logs: [] }, stderr: "", exitCode: 0 },
+    });
+  });
+
+  test("a build of an ES module bundle warns about a direct eval that came from node_modules", async () => {
+    using dir = tempDir("bun-build-api-direct-eval-second-build", {
+      "entry.js": `import { run } from "dep";\nexport const value = run("1");\n`,
+      "node_modules/dep/package.json": `{ "name": "dep", "type": "module", "main": "index.js" }`,
+      "node_modules/dep/index.js": `export function run(code) { return eval(code); }\n`,
+    });
+    const messages = (build: Awaited<ReturnType<typeof Bun.build>>) => build.logs.map(log => log.message);
+
+    const first = await Bun.build({ entrypoints: [join(String(dir), "entry.js")], outdir: join(String(dir), "dist") });
+    const second = await Bun.build({ entrypoints: [join(String(dir), "dist", "entry.js")] });
+    expect({ first: messages(first), second: messages(second) }).toEqual({
+      first: [],
+      second: ["Using direct eval with a bundler is not recommended and may cause problems"],
+    });
+  });
+
   test("Bun.write(BuildArtifact)", async () => {
     Bun.gc(true);
     const tmpdir = tempDirWithFiles("bun-build-api-write", {

test/bundler/compile-argv.test.ts

@@ -83,6 +83,9 @@ describe("bundler", () => {
         `,
         "/imported.ts": /* js */ `export default 2;`,
       },
+      bundleWarnings: {
+        "/see.ts": ["Using direct eval with a bundler is not recommended and may cause problems"],
+      },
       entryPointsRaw: ["./entry.ts", "./worker.ts"],
       outfile: "dist/out",
       run: {