bun test: keep the frame of a matcher call made in tail position

src/js_parser/parse/parse_entry.rs

@@ -2014,6 +2014,14 @@ impl<'a> Parser<'a> {
             }
         }
 
+        // `keep_matcher_call_frame` (visit_stmt.rs) rewrote a returned matcher call. Like the
+        // globals above, this makes the output specific to `bun test`, so it must not be cached.
+        if p.jest.rewrote_matcher_tail_call {
+            if let Some(cache) = p.options.features.runtime_transpiler_cache_mut() {
+                cache.input_hash = None;
+            }
+        }
+
         if p.has_called_runtime {
             let mut runtime_imports: [u8; RuntimeImports::ALL.len()] =
                 [0; RuntimeImports::ALL.len()];

src/js_parser/parser.rs

@@ -362,7 +362,7 @@ pub mod Runtime {
         pub(crate) fn hash_for_runtime_transpiler(&self, hasher: &mut Wyhash) {
             debug_assert!(self.runtime_transpiler_cache.is_some());
 
-            let bools: [bool; 17] = [
+            let bools: [bool; 18] = [
                 self.top_level_await,
                 self.auto_import_jsx,
                 self.allow_runtime,
@@ -380,7 +380,11 @@ pub mod Runtime {
                 self.standard_decorators,
                 self.lower_using,
                 self.repl_mode,
-                // note that we do not include .inject_jest_globals, as we bail out of the cache entirely if this is true
+                // `keep_matcher_call_frame` changes the output of a file with an explicit
+                // `bun:test` import, which stays cacheable. The `input_hash = None` bails in
+                // `_parse` only stop the write: the read has already served `Result::Cached`,
+                // so the hash has to separate the two modes.
+                self.inject_jest_globals,
             ];
 
             // `[bool; N]` is N bytes of 0x00/0x01.
@@ -1600,6 +1604,9 @@ pub struct Jest {
     pub(crate) xit: Ref,
     pub(crate) xtest: Ref,
     pub(crate) xdescribe: Ref,
+    /// `P::keep_matcher_call_frame` rewrote a returned matcher call, so the output is specific
+    /// to `bun test` and must not enter the runtime transpiler cache.
+    pub(crate) rewrote_matcher_tail_call: bool,
 }
 
 impl Jest {
@@ -1642,6 +1649,7 @@ impl Default for Jest {
             xit: Ref::NONE,
             xtest: Ref::NONE,
             xdescribe: Ref::NONE,
+            rewrote_matcher_tail_call: false,
         }
     }
 }

src/js_parser/visit/visit_stmt.rs

@@ -1552,6 +1552,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         if let Some(val) = data.value.as_mut() {
             p.visit_expr(val);
 
+            if p.options.features.inject_jest_globals && !p.is_control_flow_dead {
+                p.keep_matcher_call_frame(val);
+            }
+
             // "return undefined;" can safely just always be "return;"
             if let Some(v) = data.value {
                 if matches!(v.data, js_ast::ExprData::EUndefined(_)) {
@@ -1565,6 +1569,94 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         Ok(())
     }
 
+    /// `bun test` only. A returned matcher call, which is also what `() => expect(x).toBe(1)` is,
+    /// is a proper tail call (test files are modules, so strict mode): JSC replaces the frame that
+    /// contains the call with the matcher's own, so a failure has no location to report and an
+    /// inline snapshot has nowhere to be written. `return [expect(x).toBe(1)][0]` keeps the
+    /// frame. Tail position continues into the branches of `?:` and the right operand of `,`,
+    /// `&&`, `||` and `??`.
+    fn keep_matcher_call_frame(&mut self, value: &mut Expr) {
+        match value.data {
+            js_ast::ExprData::ECall(call) => {
+                if self.is_matcher_call(call.target) {
+                    *value = self.take_out_of_tail_position(*value);
+                }
+            }
+            js_ast::ExprData::EIf(mut e) => {
+                self.keep_matcher_call_frame(&mut e.yes);
+                self.keep_matcher_call_frame(&mut e.no);
+            }
+            js_ast::ExprData::EBinary(mut e)
+                if matches!(
+                    e.op,
+                    js_ast::OpCode::BinComma
+                        | js_ast::OpCode::BinLogicalAnd
+                        | js_ast::OpCode::BinLogicalOr
+                        | js_ast::OpCode::BinNullishCoalescing
+                ) =>
+            {
+                self.keep_matcher_call_frame(&mut e.right);
+            }
+            _ => {}
+        }
+    }
+
+    /// A method call on `expect(...)`, through any chain such as `.not` or `.resolves`, or a call
+    /// to one of the inline snapshot matchers on any receiver.
+    fn is_matcher_call(&self, callee: Expr) -> bool {
+        let Some(dot) = callee.data.e_dot() else {
+            return false;
+        };
+        if dot.name == b"toMatchInlineSnapshot" || dot.name == b"toThrowErrorMatchingInlineSnapshot"
+        {
+            return true;
+        }
+        let mut receiver = dot.target;
+        loop {
+            match receiver.data {
+                js_ast::ExprData::EDot(inner) => receiver = inner.target,
+                js_ast::ExprData::ECall(call) => {
+                    let ref_ = match call.target.data {
+                        js_ast::ExprData::EIdentifier(id) => id.ref_,
+                        js_ast::ExprData::EImportIdentifier(id) => id.ref_,
+                        _ => return false,
+                    };
+                    return self.symbols[ref_.inner_index() as usize]
+                        .original_name
+                        .slice()
+                        == b"expect";
+                }
+                _ => return false,
+            }
+        }
+    }
+
+    /// `[call][0]`. Self-contained on purpose: a generated temporary would turn
+    /// `Function.prototype.toString` output into code with a free identifier, and test code does
+    /// round-trip such text into other modules.
+    fn take_out_of_tail_position(&mut self, call: Expr) -> Expr {
+        self.jest.rewrote_matcher_tail_call = true;
+        let loc = call.loc;
+        let items = js_ast::ExprNodeList::from_arena_slice(self.arena.alloc_slice_copy(&[call]));
+        let array = self.new_expr(
+            E::Array {
+                items,
+                is_single_line: true,
+                ..Default::default()
+            },
+            loc,
+        );
+        let index = self.new_expr(E::Number::new(0.0), loc);
+        self.new_expr(
+            E::Index {
+                target: array,
+                index,
+                optional_chain: None,
+            },
+            loc,
+        )
+    }
+
     fn s_block(
         p: &mut Self,
         stmts: &mut StmtList<'a>,

src/jsc/RuntimeTranspilerCache.rs

@@ -51,7 +51,11 @@ bun_core::declare_scope!(cache, visible);
 /// Version 25: Every ModuleInfo record carries a trailing FetchParameters slot
 /// so ImportEntry/ExportEntry/StarExportEntry moduleRequestType matches JSC's
 /// after WebKit 90b2ecf79ae3 keyed m_loadedModules on (specifier, type).
-const EXPECTED_VERSION: u32 = 25;
+/// Version 26: `inject_jest_globals` participates in the features hash. The
+/// matcher tail-call rewrite changes the output of a file with an explicit
+/// `bun:test` import, which stays cacheable, so `bun run` and `bun test` must
+/// not share entries for it.
+const EXPECTED_VERSION: u32 = 26;
 
 /// Source files smaller than this are not written to / read from the on-disk
 /// transpiler cache. Originally 50 KiB, which excluded almost every file in a

test/cli/test/bun-test.test.ts

@@ -1343,6 +1343,105 @@ describe("bun test", () => {
     expect(stderr).toContain("1 pass");
   });
 
+  // A returned matcher call is a proper tail call, which would otherwise leave the failure with no
+  // frame in the file that contains the call.
+  describe("matcher call in tail position", () => {
+    test("a failure reports the line of the matcher call", () => {
+      const stderr = runTest({
+        input: [
+          {
+            filename: "tail.test.ts",
+            contents: [
+              `import { test, expect } from "bun:test";`,
+              `test("expression-bodied callback", () => expect(1).toBe(2));`,
+              `function check(value: number) { return expect(value).toBe(2); }`,
+              `test("helper", () => { check(1); });`,
+              `test("nullish", () => globalThis.nothing ?? expect(1).toBe(2));`,
+            ].join("\n"),
+          },
+        ],
+        expectExitCode: 1,
+      });
+      expect(stderr).toMatch(/at <anonymous> \(.*tail\.test\.ts:2:\d+\)/);
+      expect(stderr).toMatch(/at check \(.*tail\.test\.ts:3:\d+\)/);
+      expect(stderr).toMatch(/at <anonymous> \(.*tail\.test\.ts:5:\d+\)/);
+      expect(stderr).toContain("3 fail");
+    });
+
+    test("the matcher's return value still reaches the runner", () => {
+      const stderr = runTest({
+        input: `
+          import { test, expect } from "bun:test";
+          expect.extend({
+            async toFailLater() {
+              return { pass: false, message: () => "failed later" };
+            },
+          });
+          test("async matcher", () => expect(1).toFailLater());
+        `,
+        expectExitCode: 1,
+      });
+      expect(stderr).toContain("failed later");
+      expect(stderr).toContain("1 fail");
+    });
+
+    // `bun run` and `bun test` transpile the same file to different output now, so they must not
+    // share a transpiler cache entry. The file needs an explicit `bun:test` import (injected
+    // globals already bail out of the cache) and 4KB of source (the cache floor).
+    test("the rewrite is not lost to a transpiler cache entry from bun run", async () => {
+      const contents = [
+        `import { test, expect } from "bun:test";`,
+        `test("tail", () => expect(1).toBe(2));`,
+        `// ${Buffer.alloc(5000, "x").toString()}`,
+      ].join("\n");
+      using dir = tempDir("tail-cache", { "cached.test.ts": contents });
+      const cacheDir = join(String(dir), "cache");
+      mkdirSync(cacheDir);
+      const env = {
+        ...bunEnv,
+        BUN_RUNTIME_TRANSPILER_CACHE_PATH: cacheDir,
+        // Debug builds read the cache but serve from it only with this set.
+        BUN_DEBUG_ENABLE_RESTORE_FROM_TRANSPILER_CACHE: "1",
+      };
+      await using warm = Bun.spawn({
+        cmd: [bunExe(), "run", "cached.test.ts"],
+        env,
+        cwd: String(dir),
+        stdout: "ignore",
+        stderr: "ignore",
+      });
+      await warm.exited;
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "test", "cached.test.ts"],
+        env,
+        cwd: String(dir),
+        stderr: "pipe",
+      });
+      const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+      expect(stderr).toMatch(/at <anonymous> \(.*cached\.test\.ts:2:\d+\)/);
+      expect(stderr).toContain("1 fail");
+      expect(exitCode).toBe(1);
+    });
+
+    // This arrow is rewritten in this very file, and its toString() text lands in a different
+    // module, so the rewrite must not reference anything from the module it was made in.
+    test("the rewrite survives a Function.toString round-trip into another module", async () => {
+      const fn = () => expect(1).toBe(1);
+      using dir = tempDir("tail-tostring", {
+        "roundtrip.test.ts": `import { test, expect } from "bun:test";\ntest("round-trip", ${fn.toString()});`,
+      });
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "test", "roundtrip.test.ts"],
+        env: bunEnv,
+        cwd: String(dir),
+        stderr: "pipe",
+      });
+      const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+      expect(stderr).toContain("1 pass");
+      expect(exitCode).toBe(0);
+    });
+  });
+
   test("path to a non-test.ts file will work", () => {
     const stderr = runTest({
       args: ["./index.ts"],