Improve handling of copilot_node_command

lua/copilot/auth/init.lua

@@ -153,7 +153,7 @@ function M.find_config_path()
 end
 
 ---@return table|nil
-M.get_creds = function()
+function M.get_creds()
   local filename = M.find_config_path() .. "/github-copilot/apps.json"
 
   if vim.fn.filereadable(filename) == 0 then

lua/copilot/config/init.lua

@@ -12,7 +12,7 @@ local logger = require("copilot.logger")
 ---@field copilot_model string|nil Model to use for Copilot, LSP server dictates the default
 ---@field root_dir RootDirFuncOrString Root directory for the project, defaults to the nearest .git directory
 ---@field should_attach ShouldAttachFunc Function to determine if Copilot should attach to the buffer
----@field copilot_node_command string Path to the Node.js executable, defaults to "node"
+---@field copilot_node_command string|string[] Path to the Node.js executable, defaults to "node"
 ---@field disable_limit_reached_message boolean Disable the limit reached message, defaults to false
 
 local initialized = false
@@ -68,7 +68,7 @@ function M.validate(config)
   vim.validate("copilot_model", config.copilot_model, { "string", "nil" })
   vim.validate("root_dir", config.root_dir, { "string", "function" })
   vim.validate("should_attach", config.should_attach, "function")
-  vim.validate("copilot_node_command", config.copilot_node_command, "string")
+  vim.validate("copilot_node_command", config.copilot_node_command, {"string", "table"})
 
   require("copilot.config.panel").validate(config.panel)
   require("copilot.config.suggestion").validate(config.suggestion)

lua/copilot/lsp/nodejs.lua

@@ -1,19 +1,21 @@
 local logger = require("copilot.logger")
+local util = require("copilot.util")
 
 local M = {
   ---@class copilot_nodejs_server_info
-  ---@type string
+  ---@type string|string[]
   node_command = nil,
   ---@type string
   server_path = nil,
   initialization_failed = false,
 }
 
+
 ---@return string node_version
 ---@return nil|string node_version_error
 function M.get_node_version()
   if not M.node_version then
-    local version_cmd = { M.node_command, "--version" }
+    local version_cmd = util.append_command(M.node_command, "--version")
 
     local node_version_major = 0
     local node_version = ""
@@ -95,13 +97,10 @@ end
 
 ---@return table
 function M.get_execute_command()
-  local cmd = { M.node_command }
-  table.insert(cmd, M.server_path or M.get_server_path())
-  table.insert(cmd, "--stdio")
-  return cmd
+  return util.append_command(M.node_command, { M.server_path or M.get_server_path(), "--stdio" })
 end
 
----@param node_command? string
+---@param node_command? string|string[]
 ---@param custom_server_path? string
 ---@return boolean
 function M.setup(node_command, custom_server_path)

lua/copilot/util.lua

@@ -110,7 +110,7 @@ function M.get_doc_params(overrides)
   return params
 end
 
-M.get_plugin_path = function()
+function M.get_plugin_path()
   local copilot_path = vim.api.nvim_get_runtime_file("lua/copilot/init.lua", false)[1]
   if vim.fn.filereadable(copilot_path) ~= 0 then
     return vim.fn.fnamemodify(copilot_path, ":h:h:h")
@@ -186,4 +186,35 @@ function M.get_buffer_previous_ft(bufnr)
   return (ok and result) or ""
 end
 
+---@param cmd string|string[]
+---@param append string|string[]
+---@return string[]
+function M.append_command(cmd, append)
+  local full_cmd = {}
+
+  -- first append the base command
+  if type(cmd) == "string" then
+    table.insert(full_cmd, cmd)
+  elseif type(cmd) == "table" then
+    for _, part in ipairs(cmd) do
+      if part ~= nil then
+        table.insert(full_cmd, part)
+      end
+    end
+  end
+
+  -- then append the additional parts
+  if type(append) == "string" then
+    table.insert(full_cmd, append)
+  elseif type(append) == "table" then
+    for _, part in ipairs(append) do
+      if part ~= nil then
+        table.insert(full_cmd, part)
+      end
+    end
+  end
+
+  return full_cmd
+end
+
 return M

tests/stubs/nodejs.lua

@@ -0,0 +1,87 @@
+-- these are helper functions for testing the nodejs LSP integration
+-- they stub out system interactions for verifying access to node and the LSP script
+
+M = {}
+
+M.default_server_path = "copilot/js/language-server.js"
+M.custom_server_path = "custom/path/to/language-server.js"
+
+---@param stdout string the stdout that will be returned by the stubbed vim.system
+---@param code integer the exit code that will be returned by the stubbed vim.system
+---@param fail boolean if true, vim.system will error when called
+---@param callback function the function to call while vim.system is stubbed
+---@return table|nil captured_args -- the arguments vim.system was called with
+function M.process(stdout, code, fail, callback)
+  local captured_args = nil
+  local original_vim_system = vim.system
+
+  ---@diagnostic disable-next-line: duplicate-set-field
+  vim.system = function(cmd)
+    captured_args = cmd
+    if fail then
+      error("Command failed")
+    end
+    return {
+      wait = function()
+        return {
+          stdout = stdout .. "\n",
+          code = code,
+        }
+      end,
+    }
+  end
+  -- wrap callback in pcall to ensure vim.system is restored if callback errors
+  local ok, err = pcall(callback)
+  vim.system = original_vim_system
+  if not ok then
+    error(err)
+  end
+  return captured_args
+end
+
+M.valid_node_version = "20.0.0"
+M.invalid_node_version = "10.0.0"
+
+---Convenience wrapper for Stub.process for a valid Node.js version (>= 20)
+function M.valid_node(callback)
+  return M.process("v" .. M.valid_node_version, 0, false, callback)
+end
+
+---Convenience wrapper for Stub.process for an invalid Node.js version (< 20)
+function M.invalid_node(callback)
+  return M.process("v" .. M.invalid_node_version, 0, false, callback)
+end
+
+---@param callback function the function to call while vim.api.nvim_get_runtime_file is stubbed
+---@return string|nil captured_path -- the path vim.api.nvim_get_runtime_file was called with
+function M.get_runtime_server_path(callback)
+  local captured_path = nil
+
+  local original_get_file = vim.api.nvim_get_runtime_file
+  ---@diagnostic disable-next-line: duplicate-set-field
+  vim.api.nvim_get_runtime_file = function(path)
+    captured_path = path
+    return { vim.fn.expand(M.default_server_path) }
+  end
+
+  local original_filereadable = vim.fn.filereadable
+  ---@diagnostic disable-next-line: duplicate-set-field
+  vim.fn.filereadable = function()
+    return 1
+  end
+
+  -- stub valid node version for callback so setup() succeeds
+  M.valid_node(function()
+    -- wrap callback in pcall to ensure vim.api.nvim_get_runtime_file is restored if callback errors
+    local ok, err = pcall(callback)
+    vim.api.nvim_get_runtime_file = original_get_file
+    vim.fn.filereadable = original_filereadable
+    if not ok then
+      error(err)
+    end
+  end)
+
+  return captured_path
+end
+
+return M

tests/test_nodejs.lua

@@ -0,0 +1,185 @@
+local eq = MiniTest.expect.equality
+local stub = require("tests.stubs.nodejs")
+
+local T = MiniTest.new_set({
+  hooks = {
+    pre_once = function() end,
+    pre_case = function()
+      -- Reset the module state before each test
+      package.loaded["copilot.lsp.nodejs"] = nil
+    end,
+  },
+})
+
+T["get_node_version()"] = MiniTest.new_set()
+
+T["get_node_version()"]["default node command"] = function()
+  local captured_args = stub.valid_node(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup()
+
+    local version, error = nodejs.get_node_version()
+
+    eq(version, stub.valid_node_version)
+    eq(error, nil)
+  end)
+  eq(captured_args, { "node", "--version" })
+end
+
+T["get_node_version()"]["custom node command as string"] = function()
+  local captured_args = stub.valid_node(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("/usr/local/bin/node")
+
+    local version, error = nodejs.get_node_version()
+
+    eq(version, stub.valid_node_version)
+    eq(error, nil)
+  end)
+  eq(captured_args, { "/usr/local/bin/node", "--version" })
+end
+
+T["get_node_version()"]["custom node command as string with spaces"] = function()
+  local captured_args = stub.valid_node(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("/path to/node")
+
+    local version, error = nodejs.get_node_version()
+
+    eq(version, stub.valid_node_version)
+    eq(error, nil)
+  end)
+  eq(captured_args, { "/path to/node", "--version" })
+end
+
+T["get_node_version()"]["custom node command as table"] = function()
+  local captured_args = stub.valid_node(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup({ "mise", "x", "node@lts", "--", "node" })
+
+    local version, error = nodejs.get_node_version()
+
+    eq(version, stub.valid_node_version)
+    eq(error, nil)
+  end)
+  eq(captured_args, { "mise", "x", "node@lts", "--", "node", "--version" })
+end
+
+T["get_node_version()"]["handles vim.system failure"] = function()
+  local captured_args = stub.process("", -1, true, function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("node")
+
+    local _, error = nodejs.get_node_version()
+    error = error or ""
+
+    eq(error:find("Could not determine Node.js version") ~= nil, true)
+  end)
+  eq(captured_args, { "node", "--version" })
+end
+
+T["get_node_version()"]["handles process with non-zero exit code"] = function()
+  local captured_args = stub.process("", 127, false, function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("nonexistent-node")
+
+    local _, error = nodejs.get_node_version()
+    error = error or ""
+
+    eq(error:find("Could not determine Node.js version") ~= nil, true)
+  end)
+  eq(captured_args, { "nonexistent-node", "--version" })
+end
+
+T["get_node_version()"]["validates node version requirement"] = function()
+  local captured_args = stub.invalid_node(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("node")
+
+    local _, error = nodejs.get_node_version()
+    error = error or ""
+
+    eq(error:find("Node.js version 20 or newer required") ~= nil, true)
+  end)
+  eq(captured_args, { "node", "--version" })
+end
+
+T["get_execute_command()"] = MiniTest.new_set()
+
+T["get_execute_command()"]["default node command, default server path"] = function()
+  local captured_path = stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    eq(nodejs.setup(), true)
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "node", vim.fn.expand(stub.default_server_path), "--stdio" })
+  end)
+  eq(captured_path, stub.default_server_path)
+end
+
+T["get_execute_command()"]["default node command, custom server path"] = function()
+  stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    eq(nodejs.setup(nil, vim.fn.expand(stub.custom_server_path)), true)
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "node", vim.fn.expand(stub.custom_server_path), "--stdio" })
+  end)
+end
+
+T["get_execute_command()"]["custom node command as string, default server path"] = function()
+  local captured_path = stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    eq(nodejs.setup("/usr/local/bin/node"), true)
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "/usr/local/bin/node", vim.fn.expand(stub.default_server_path), "--stdio" })
+  end)
+  eq(captured_path, stub.default_server_path)
+end
+
+T["get_execute_command()"]["custom node command as string, custom server path"] = function()
+  stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("/usr/local/bin/node", stub.custom_server_path)
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "/usr/local/bin/node", stub.custom_server_path, "--stdio" })
+  end)
+end
+
+T["get_execute_command()"]["custom node command as string with spaces, default server path"] = function()
+  local captured_path = stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("/path to/node")
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "/path to/node", vim.fn.expand(stub.default_server_path), "--stdio" })
+  end)
+  eq(captured_path, stub.default_server_path)
+end
+
+T["get_execute_command()"]["custom node command as string with spaces, custom server path"] = function()
+  stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup("/path to/node", stub.custom_server_path)
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "/path to/node", stub.custom_server_path, "--stdio" })
+  end)
+end
+
+T["get_execute_command()"]["custom node command as table, default server path"] = function()
+  local captured_path = stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup({ "mise", "x", "node@lts", "--", "node" })
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "mise", "x", "node@lts", "--", "node", vim.fn.expand(stub.default_server_path), "--stdio" })
+  end)
+  eq(captured_path, stub.default_server_path)
+end
+
+T["get_execute_command()"]["custom node command as table, custom server path"] = function()
+  stub.get_runtime_server_path(function()
+    local nodejs = require("copilot.lsp.nodejs")
+    nodejs.setup({ "mise", "x", "node@lts", "--", "node" }, stub.custom_server_path)
+    local cmd = nodejs.get_execute_command()
+    eq(cmd, { "mise", "x", "node@lts", "--", "node", stub.custom_server_path, "--stdio" })
+  end)
+end
+
+return T