fix(spawn): always expand cmd if PATH is not modified (#773)

fix(spawn): always expand cmd on Windows

Closes #720.

2 files changed, 66 insertions, 36 deletions

diff --git a/lua/mason-core/spawn.lua b/lua/mason-core/spawn.lua
index 814ef8f7..bfc6ea29 100644
--- a/lua/mason-core/spawn.lua
+++ b/lua/mason-core/spawn.lua
@@ -8,15 +8,6 @@ local log = require "mason-core.log"
 ---@alias JobSpawn table<string, async fun(opts: SpawnArgs): Result>
 ---@type JobSpawn
 local spawn = {
-    _aliases = {
-        npm = platform.is.win and "npm.cmd" or "npm",
-        gem = platform.is.win and "gem.cmd" or "gem",
-        composer = platform.is.win and "composer.bat" or "composer",
-        gradlew = platform.is.win and "gradlew.bat" or "gradlew",
-        -- for hererocks installations
-        luarocks = (platform.is.win and vim.fn.executable "luarocks.bat" == 1) and "luarocks.bat" or "luarocks",
-        rebar3 = platform.is.win and "rebar3.cmd" or "rebar3",
-    },
     _flatten_cmd_args = _.compose(_.filter(_.complement(_.equals(vim.NIL))), _.flatten),
 }
 
@@ -33,11 +24,16 @@ local function Failure(err, cmd)
     }))
 end
 
-local is_executable = _.memoize(function(cmd)
+local exepath = _.memoize(function(cmd)
     if vim.in_fast_event() then
         a.scheduler()
     end
-    return vim.fn.executable(cmd) == 1
+    local exepath = vim.fn.exepath(cmd)
+    if exepath == "" then
+        return nil
+    else
+        return exepath
+    end
 end, _.identity)
 
 ---@class SpawnArgs
@@ -47,11 +43,10 @@ end, _.identity)
 ---@field stdio_sink StdioSink? If provided, will be used to write to stdout and stderr.
 ---@field cwd string?
 ---@field on_spawn (fun(handle: luv_handle, stdio: luv_pipe[], pid: integer))? Will be called when the process successfully spawns.
----@field check_executable boolean? Whether to check if the provided command is executable (defaults to true).
 
 setmetatable(spawn, {
-    ---@param normalized_cmd string
-    __index = function(self, normalized_cmd)
+    ---@param cmd string
+    __index = function(self, cmd)
         ---@param args SpawnArgs
         return function(args)
             local cmd_args = self._flatten_cmd_args(args)
@@ -76,13 +71,16 @@ setmetatable(spawn, {
                 spawn_args.stdio_sink = stdio.sink
             end
 
-            local cmd = self._aliases[normalized_cmd] or normalized_cmd
-
-            if (env and env.PATH) == nil and args.check_executable ~= false and not is_executable(cmd) then
-                log.fmt_debug("%s is not executable", cmd)
-                return Failure({
-                    stderr = ("%s is not executable"):format(cmd),
-                }, cmd)
+            -- Ensure that the cmd is executable (only if PATH is not modified).
+            if (env and env.PATH) == nil then
+                local expanded_cmd = exepath(cmd)
+                if expanded_cmd == nil then
+                    log.fmt_debug("%s is not executable", cmd)
+                    return Failure({
+                        stderr = ("%s is not executable"):format(cmd),
+                    }, cmd)
+                end
+                cmd = expanded_cmd
             end
 
             local _, exit_code, signal = a.wait(function(resolve)
diff --git a/tests/mason-core/spawn_spec.lua b/tests/mason-core/spawn_spec.lua
index f9f90411..59ccd533 100644
--- a/tests/mason-core/spawn_spec.lua
+++ b/tests/mason-core/spawn_spec.lua
@@ -66,7 +66,7 @@ describe("async spawn", function()
             assert.equals("", result:get_or_nil().stderr)
             assert.spy(process.spawn).was_called(1)
             assert.spy(process.spawn).was_called_with(
-                "bash",
+                match.matches "bash$",
                 match.tbl_containing {
                     stdio_sink = match.tbl_containing {
                         stdout = match.is_function(),
@@ -139,7 +139,8 @@ describe("async spawn", function()
                 callback(false)
             end)
 
-            local result = spawn.my_cmd {}
+            local result = spawn.bash {}
+            assert.spy(process.spawn).was_called(1)
             assert.is_true(result:is_failure())
             assert.is_nil(result:err_or_nil().exit_code)
         end)
@@ -155,33 +156,40 @@ describe("async spawn", function()
 
             local result = spawn.bash {}
             assert.is_true(result:is_failure())
-            assert.equals(
-                "spawn: bash failed with exit code 127 and signal -. This is an error message for bash!",
-                tostring(result:err_or_nil())
+            assert.is_true(
+                match.matches "spawn: .+bash failed with exit code 127 and signal %-%. This is an error message for .+bash!"(
+                    tostring(result:err_or_nil())
+                )
             )
         end)
     )
 
     it(
-        "should check whether command is executable",
+        "should fail if unable to expand command",
         async_test(function()
-            local result = spawn.my_cmd {}
+            spy.on(process, "spawn")
+            stub(vim.fn, "exepath", function()
+                return ""
+            end)
+
+            local result = spawn.unexpand_cmd {}
             assert.is_true(result:is_failure())
-            assert.equals(
-                "spawn: my_cmd failed with exit code - and signal -. my_cmd is not executable",
-                tostring(result:err_or_nil())
+            assert.is_true(
+                match.matches "spawn: unexpand_cmd failed with exit code %- and signal %-%. unexpand_cmd is not executable"(
+                    tostring(result:err_or_nil())
+                )
             )
         end)
     )
 
     it(
-        "should skip checking whether command is executable",
+        "should not expand cmd if custom PATH is used",
         async_test(function()
             stub(process, "spawn", function(_, _, callback)
                 callback(false, 127)
             end)
 
-            local result = spawn.my_cmd { "arg1", check_executable = false }
+            local result = spawn.my_cmd { "arg1", env = { PATH = "/bin" } }
             assert.is_true(result:is_failure())
             assert.spy(process.spawn).was_called(1)
             assert.spy(process.spawn).was_called_with(
@@ -195,17 +203,17 @@ describe("async spawn", function()
     )
 
     it(
-        "should skip checking whether command is executable if with_paths is provided",
+        "should skip expanding command if with_paths is provided",
         async_test(function()
             stub(process, "spawn", function(_, _, callback)
                 callback(false, 127)
             end)
 
-            local result = spawn.my_cmd { "arg1", with_paths = {} }
+            local result = spawn.custom_path { "arg1", with_paths = {} }
             assert.is_true(result:is_failure())
             assert.spy(process.spawn).was_called(1)
             assert.spy(process.spawn).was_called_with(
-                "my_cmd",
+                "custom_path",
                 match.tbl_containing {
                     args = match.same { "arg1" },
                 },
@@ -213,4 +221,28 @@ describe("async spawn", function()
             )
         end)
     )
+
+    it(
+        "should use expanded command path",
+        async_test(function()
+            stub(vim.fn, "exepath", function()
+                return "/abs/path/to/cmd"
+            end)
+            stub(process, "spawn", function(_, _, callback)
+                callback(false)
+            end)
+
+            spawn.the_command { "arg1", "arg2" }
+            assert.spy(vim.fn.exepath).was_called(1)
+            assert.spy(vim.fn.exepath).was_called_with "the_command"
+            assert.spy(process.spawn).was_called(1)
+            assert.spy(process.spawn).was_called_with(
+                "/abs/path/to/cmd",
+                match.tbl_containing {
+                    args = match.same { "arg1", "arg2" },
+                },
+                match.is_function()
+            )
+        end)
+    )
 end)