shell: return which's PATH and cwd lookup as a struct instead of a tuple

mordant-baseline.toml

@@ -90,7 +90,6 @@
 "same_match_twice:src/runtime/webcore/Blob.rs" = 2
 "same_match_twice:src/runtime/webcore/Body.rs" = 2
 "tuple_wants_struct:src/runtime/server/ServerWebSocket.rs" = 1
-"tuple_wants_struct:src/runtime/shell/builtin/which.rs" = 1
 "unchecked_construction:src/runtime/api/js_bundle_completion_task.rs" = 1
 "unchecked_construction:src/runtime/cli/pack_command.rs" = 2
 "unchecked_construction:src/runtime/server/HTMLBundle.rs" = 2

src/runtime/shell/builtin/which.rs

@@ -46,11 +46,11 @@ impl Which {
         if Builtin::of(interp, cmd).stdout.needs_io().is_none() {
             // Synchronous path: resolve every arg, write straight to the
             // captured buffer, then finish.
-            let (path_env, cwd) = Self::path_and_cwd(interp, cmd);
+            let search = SearchEnv::load(interp, cmd);
             let mut had_not_found = false;
             for i in 0..argc {
                 let arg = Self::arg(interp, cmd, i);
-                match Self::resolve(&path_env, &cwd, &arg) {
+                match search.resolve(&arg) {
                     Some(resolved) => {
                         let buf = Builtin::fmt_error_arena(
                             interp,
@@ -100,8 +100,7 @@ impl Which {
         }
 
         let arg = Self::arg(interp, cmd, arg_idx);
-        let (path_env, cwd) = Self::path_and_cwd(interp, cmd);
-        let resolved = Self::resolve(&path_env, &cwd, &arg);
+        let resolved = SearchEnv::load(interp, cmd).resolve(&arg);
 
         let child = ChildPtr::new(cmd, WriterTag::Builtin);
         match resolved {
@@ -191,11 +190,21 @@ impl Which {
 
     // ── helpers ────────────────────────────────────────────────────────────
 
-    /// Look up `$PATH` from the export env and the cwd from the shell env.
-    fn path_and_cwd(interp: &Interpreter, cmd: NodeId) -> (Vec<u8>, Vec<u8>) {
+    fn arg(interp: &Interpreter, cmd: NodeId, idx: usize) -> Vec<u8> {
+        Builtin::of(interp, cmd).arg_bytes(idx).to_vec()
+    }
+}
+
+struct SearchEnv {
+    path_env: Vec<u8>,
+    cwd: Vec<u8>,
+}
+
+impl SearchEnv {
+    fn load(interp: &Interpreter, cmd: NodeId) -> Self {
         let shell = Builtin::shell(interp, cmd);
         // `EnvMap::get` refs the returned string; balance it.
-        let path = shell
+        let path_env = shell
             .export_env
             .get(EnvStr::init_slice(b"PATH"))
             .map(|s| {
@@ -204,15 +213,15 @@ impl Which {
                 v
             })
             .unwrap_or_default();
-        (path, shell.cwd().to_vec())
-    }
-
-    fn arg(interp: &Interpreter, cmd: NodeId, idx: usize) -> Vec<u8> {
-        Builtin::of(interp, cmd).arg_bytes(idx).to_vec()
+        Self {
+            path_env,
+            cwd: shell.cwd().to_vec(),
+        }
     }
 
-    fn resolve(path_env: &[u8], cwd: &[u8], arg: &[u8]) -> Option<Vec<u8>> {
+    fn resolve(&self, arg: &[u8]) -> Option<Vec<u8>> {
         let mut path_buf = bun_paths::path_buffer_pool::get();
-        bun_which::which(&mut *path_buf, path_env, cwd, arg).map(|z| z.as_bytes().to_vec())
+        bun_which::which(&mut *path_buf, &self.path_env, &self.cwd, arg)
+            .map(|z| z.as_bytes().to_vec())
     }
 }

test/js/bun/shell/commands/which.test.ts

@@ -1,6 +1,59 @@
 import { $ } from "bun";
 import { expect, test } from "bun:test";
-import { bunEnv, bunExe, isWindows } from "harness";
+import { bunEnv, bunExe, isWindows, tempDir } from "harness";
+import { chmodSync } from "node:fs";
+import { join } from "node:path";
+
+// Two directories each holding an executable of the same name. `which tool`
+// must come from the $PATH directory and `which ./tool` from the shell's cwd,
+// so the output shows which of the two inputs each lookup read.
+const exe = isWindows ? "tool.exe" : "tool";
+const searchTree = {
+  [`path-dir/${exe}`]: "",
+  [`cwd-dir/${exe}`]: "",
+};
+
+function searchDirs(dir: string) {
+  const pathDir = join(dir, "path-dir");
+  const cwdDir = join(dir, "cwd-dir");
+  if (!isWindows) {
+    chmodSync(join(pathDir, exe), 0o755);
+    chmodSync(join(cwdDir, exe), 0o755);
+  }
+  return { pathDir, cwdDir, expected: `${join(pathDir, exe)}\n${join(cwdDir, exe)}\n` };
+}
+
+test.concurrent("which searches $PATH for bare names and the shell cwd for ./ names (captured stdout)", async () => {
+  using dir = tempDir("which-captured", searchTree);
+  const { pathDir, cwdDir, expected } = searchDirs(String(dir));
+
+  const { stdout, exitCode } = await $`which tool ./tool`.env({ PATH: pathDir }).cwd(cwdDir).quiet().nothrow();
+  expect(stdout.toString()).toBe(expected);
+  expect(exitCode).toBe(0);
+});
+
+test.concurrent("which searches $PATH for bare names and the shell cwd for ./ names (streamed stdout)", async () => {
+  using dir = tempDir("which-streamed", searchTree);
+  const { pathDir, cwdDir, expected } = searchDirs(String(dir));
+
+  // Without .quiet() the builtin writes to the process's stdout (the pipe
+  // below), which is the one-argument-per-write path in the builtin.
+  const fixture = /* ts */ `
+    import { $ } from "bun";
+    const { exitCode } = await $\`which tool ./tool\`.env({ PATH: ${JSON.stringify(pathDir)} }).cwd(${JSON.stringify(cwdDir)}).nothrow();
+    process.exitCode = exitCode;
+  `;
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "-e", fixture],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stdout).toBe(expected);
+  expect(stderr).toBe("");
+  expect(exitCode).toBe(0);
+});
 
 test.skipIf(isWindows)("which with an absolute path at the platform path length limit reports not found", async () => {
   const fixture = /* ts */ `