fix(lsp): do not invoke handlers for unsupported methods (#15926)

Closes https://github.com/neovim/neovim/issues/15174

Instead of invoking handlers with unsupported methods, pre-compute which
clients support a given method and only notify the user if no clients
support the given method.

2 files changed, 40 insertions, 50 deletions

diff --git a/runtime/lua/vim/lsp.lua b/runtime/lua/vim/lsp.lua
index a9e27cf6ac..66a8356735 100644
--- a/runtime/lua/vim/lsp.lua
+++ b/runtime/lua/vim/lsp.lua
@@ -86,7 +86,7 @@ end
 function lsp._unsupported_method(method)
   local msg = string.format("method %s is not supported by any of the servers registered for the current buffer", method)
   log.warn(msg)
-  return lsp.rpc_response_error(protocol.ErrorCodes.MethodNotFound, msg)
+  return msg
 end
 
 ---@private
@@ -131,15 +131,24 @@ local all_client_active_buffers = {}
 ---@param bufnr (Number) of buffer
 ---@param fn (function({client}, {client_id}, {bufnr}) Function to run on
 ---each client attached to that buffer.
-local function for_each_buffer_client(bufnr, fn)
+---@param restrict_client_ids table list of client ids on which to restrict function application.
+local function for_each_buffer_client(bufnr, fn, restrict_client_ids)
   validate {
     fn = { fn, 'f' };
+    restrict_client_ids = { restrict_client_ids, 't' , true};
   }
   bufnr = resolve_bufnr(bufnr)
   local client_ids = all_buffer_active_clients[bufnr]
   if not client_ids or tbl_isempty(client_ids) then
     return
   end
+
+  if restrict_client_ids and #restrict_client_ids > 0 then
+    client_ids = vim.tbl_filter(function(item)
+        return vim.tbl_contains(restrict_client_ids, item)
+      end, vim.tbl_keys(client_ids))
+  end
+
   for client_id in pairs(client_ids) do
     local client = active_clients[client_id]
     if client then
@@ -1255,33 +1264,33 @@ function lsp.buf_request(bufnr, method, params, handler)
     method   = { method, 's' };
     handler  = { handler, 'f', true };
   }
-  local client_request_ids = {}
 
+  local supported_clients = {}
   local method_supported = false
-  for_each_buffer_client(bufnr, function(client, client_id, resolved_bufnr)
+  for_each_buffer_client(bufnr, function(client, client_id)
     if client.supports_method(method) then
       method_supported = true
-      local request_success, request_id = client.request(method, params, handler, resolved_bufnr)
-
-      -- This could only fail if the client shut down in the time since we looked
-      -- it up and we did the request, which should be rare.
-      if request_success then
-        client_request_ids[client_id] = request_id
-      end
+      table.insert(supported_clients, client_id)
     end
   end)
 
-  -- if has client but no clients support the given method, call the callback with the proper
-  -- error message.
+  -- if has client but no clients support the given method, notify the user
   if not tbl_isempty(all_buffer_active_clients[resolve_bufnr(bufnr)] or {}) and not method_supported then
-    local unsupported_err = lsp._unsupported_method(method)
-    handler = handler or lsp.handlers[method]
-    if handler then
-      handler(unsupported_err, nil, {method=method, bufnr=bufnr})
-    end
+    vim.notify(lsp._unsupported_method(method), vim.log.levels.ERROR)
+    vim.api.nvim_command("redraw")
     return
   end
 
+  local client_request_ids = {}
+  for_each_buffer_client(bufnr, function(client, client_id, resolved_bufnr)
+      local request_success, request_id = client.request(method, params, handler, resolved_bufnr)
+      -- This could only fail if the client shut down in the time since we looked
+      -- it up and we did the request, which should be rare.
+      if request_success then
+        client_request_ids[client_id] = request_id
+      end
+  end, supported_clients)
+
   local function _cancel_all_requests()
     for client_id, request_id in pairs(client_request_ids) do
       local client = active_clients[client_id]
@@ -1309,12 +1318,13 @@ function lsp.buf_request_all(bufnr, method, params, callback)
   local request_results = {}
   local result_count = 0
   local expected_result_count = 0
-  local cancel, client_request_ids
 
-  local set_expected_result_count = once(function()
-    for _ in pairs(client_request_ids) do
-      expected_result_count = expected_result_count + 1
-    end
+  local set_expected_result_count = once(function ()
+    for_each_buffer_client(bufnr, function(client)
+      if client.supports_method(method) then
+        expected_result_count = expected_result_count + 1
+      end
+    end)
   end)
 
   local function _sync_handler(err, result, ctx)
@@ -1327,7 +1337,7 @@ function lsp.buf_request_all(bufnr, method, params, callback)
     end
   end
 
-  client_request_ids, cancel = lsp.buf_request(bufnr, method, params, _sync_handler)
+  local _, cancel = lsp.buf_request(bufnr, method, params, _sync_handler)
 
   return cancel
 end
diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua
index 026e6815dc..81ef8a9733 100644
--- a/test/functional/plugin/lsp_spec.lua
+++ b/test/functional/plugin/lsp_spec.lua
@@ -405,7 +405,7 @@ describe('LSP', function()
       }
     end)
 
-    it('should call unsupported_method when trying to call an unsupported method', function()
+    it('should not call unsupported_method when trying to call an unsupported method', function()
       local expected_handlers = {
         {NIL, {}, {method="shutdown", client_id=1}};
       }
@@ -415,24 +415,12 @@ describe('LSP', function()
             exec_lua([=[
               BUFFER = vim.api.nvim_get_current_buf()
               lsp.buf_attach_client(BUFFER, TEST_RPC_CLIENT_ID)
-              vim.lsp.handlers['textDocument/typeDefinition'] = function(err, result, ctx)
-                local method = ctx.method
-                vim.lsp._last_lsp_handler = { err = err; method = method }
-              end
-              vim.lsp._unsupported_method = function(method)
-                vim.lsp._last_unsupported_method = method
-                return 'fake-error'
-              end
-              vim.lsp.buf.type_definition()
+              vim.lsp.handlers['textDocument/typeDefinition'] = function() end
             ]=])
         end;
         on_init = function(client)
           client.stop()
-          local method = exec_lua("return vim.lsp._last_unsupported_method")
-          eq("textDocument/typeDefinition", method)
-          local lsp_cb_call = exec_lua("return vim.lsp._last_lsp_handler")
-          eq("fake-error", lsp_cb_call.err)
-          eq("textDocument/typeDefinition", lsp_cb_call.method)
+          exec_lua("vim.lsp.buf.type_definition()")
           exec_lua [[
             vim.api.nvim_command(BUFFER.."bwipeout")
           ]]
@@ -447,7 +435,7 @@ describe('LSP', function()
       }
     end)
 
-    it('shouldn\'t call unsupported_method when no client and trying to call an unsupported method', function()
+    it('should not call unsupported_method when no client and trying to call an unsupported method', function()
       local expected_handlers = {
         {NIL, {}, {method="shutdown", client_id=1}};
       }
@@ -455,20 +443,12 @@ describe('LSP', function()
         test_name = "capabilities_for_client_supports_method";
         on_setup = function()
             exec_lua([=[
-              vim.lsp.handlers['textDocument/typeDefinition'] = function(err, method)
-                vim.lsp._last_lsp_handler = { err = err; method = method }
-              end
-              vim.lsp._unsupported_method = function(method)
-                vim.lsp._last_unsupported_method = method
-                return 'fake-error'
-              end
-              vim.lsp.buf.type_definition()
+              vim.lsp.handlers['textDocument/typeDefinition'] = function() end
             ]=])
         end;
         on_init = function(client)
           client.stop()
-          eq(NIL, exec_lua("return vim.lsp._last_unsupported_method"))
-          eq(NIL, exec_lua("return vim.lsp._last_lsp_handler"))
+          exec_lua("vim.lsp.buf.type_definition()")
         end;
         on_exit = function(code, signal)
           eq(0, code, "exit code", fake_lsp_logfile)