feat(lsp)!: change handler signature

Previously, the handler signature was:

  function(err, method, params, client_id, bufnr, config)

In order to better support external plugins that wish to extend the
protocol, there is other information which would be advantageous to
forward to the client, such as the original params of the request that
generated the callback.

In order to do this, we would need to break symmetry of the handlers, to
add an additional "params" as the 7th argument.

Instead, this PR changes the signature of the handlers to:

  function(err, result, ctx, config)

where ctx (the context) includes params, client_id, and bufnr. This also leaves
flexibility for future use-cases.

BREAKING_CHANGE: changes the signature of the built-in client handlers, requiring
updating handler calls

1 files changed, 102 insertions, 101 deletions

diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua
index 3c4d01ae74..aa2d8db206 100644
--- a/test/functional/plugin/lsp_spec.lua
+++ b/test/functional/plugin/lsp_spec.lua
@@ -217,7 +217,7 @@ describe('LSP', function()
 
     it('should run correctly', function()
       local expected_handlers = {
-        {NIL, "test", {}, 1};
+        {NIL, {}, {method="test", client_id=1}};
       }
       test_rpc_server {
         test_name = "basic_init";
@@ -242,7 +242,7 @@ describe('LSP', function()
 
     it('should fail', function()
       local expected_handlers = {
-        {NIL, "test", {}, 1};
+        {NIL, {}, {method="test", client_id=1}};
       }
       test_rpc_server {
         test_name = "basic_init";
@@ -270,8 +270,8 @@ describe('LSP', function()
         return
       end
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1, NIL};
-        {NIL, "test", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="test", client_id=1}};
       }
       test_rpc_server {
         test_name = "basic_init";
@@ -293,12 +293,12 @@ describe('LSP', function()
 
     it('client should return settings via workspace/configuration handler', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "workspace/configuration", { items = {
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, { items = {
               { section = "testSetting1" };
               { section = "testSetting2" };
-          }}, 1};
-        {NIL, "start", {}, 1};
+          }}, { method="workspace/configuration", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -310,9 +310,9 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'start' then
             exec_lua([=[
               local client = vim.lsp.get_client_by_id(TEST_RPC_CLIENT_ID)
               client.config.settings = {
@@ -320,13 +320,13 @@ describe('LSP', function()
                 testSetting2 = false;
             }]=])
           end
-          if method == 'workspace/configuration' then
-            local result = exec_lua([=[
+          if ctx.method == 'workspace/configuration' then
+            local server_result = exec_lua([=[
               local method, params = ...
-              return require'vim.lsp.handlers'['workspace/configuration'](err, method, params, TEST_RPC_CLIENT_ID)]=], method, params)
-            client.notify('workspace/configuration', result)
+              return require'vim.lsp.handlers'['workspace/configuration'](err, params, {method=method, client_id=TEST_RPC_CLIENT_ID})]=], ctx.method, result)
+            client.notify('workspace/configuration', server_result)
           end
-          if method == 'shutdown' then
+          if ctx.method == 'shutdown' then
             client.stop()
           end
         end;
@@ -336,19 +336,19 @@ describe('LSP', function()
       clear_notrace()
       fake_lsp_server_setup('workspace/configuration no settings')
       eq({ NIL, NIL, }, exec_lua [[
-        local params = {
+        local result = {
           items = {
             {section = 'foo'},
             {section = 'bar'},
           }
         }
-        return vim.lsp.handlers['workspace/configuration'](nil, nil, params, TEST_RPC_CLIENT_ID)
+        return vim.lsp.handlers['workspace/configuration'](nil, result, {client_id=TEST_RPC_CLIENT_ID})
       ]])
     end)
 
     it('should verify capabilities sent', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
       }
       test_rpc_server {
         test_name = "basic_check_capabilities";
@@ -372,7 +372,7 @@ describe('LSP', function()
 
     it('client.supports_methods() should validate capabilities', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
       }
       test_rpc_server {
         test_name = "capabilities_for_client_supports_method";
@@ -406,7 +406,7 @@ describe('LSP', function()
 
     it('should call unsupported_method when trying to call an unsupported method', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
       }
       test_rpc_server {
         test_name = "capabilities_for_client_supports_method";
@@ -414,7 +414,8 @@ 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, method)
+              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)
@@ -447,7 +448,7 @@ describe('LSP', function()
 
     it('shouldn\'t call unsupported_method when no client and trying to call an unsupported method', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
       }
       test_rpc_server {
         test_name = "capabilities_for_client_supports_method";
@@ -480,8 +481,8 @@ describe('LSP', function()
 
     it('should not send didOpen if the buffer closes before init', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -512,9 +513,9 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+        on_handler = function(err, result, ctx)
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -523,9 +524,9 @@ describe('LSP', function()
 
     it('should check the body sent attaching before init', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -555,12 +556,12 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          if ctx.method == 'start' then
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -569,9 +570,9 @@ describe('LSP', function()
 
     it('should check the body sent attaching after init', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -598,12 +599,12 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          if ctx.method == 'start' then
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -612,9 +613,9 @@ describe('LSP', function()
 
     it('should check the body and didChange full', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -641,8 +642,8 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          if ctx.method == 'start' then
             exec_lua [[
               vim.api.nvim_buf_set_lines(BUFFER, 1, 2, false, {
                 "boop";
@@ -650,8 +651,8 @@ describe('LSP', function()
             ]]
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -660,9 +661,9 @@ describe('LSP', function()
 
     it('should check the body and didChange full with noeol', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -690,8 +691,8 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          if ctx.method == 'start' then
             exec_lua [[
               vim.api.nvim_buf_set_lines(BUFFER, 1, 2, false, {
                 "boop";
@@ -699,8 +700,8 @@ describe('LSP', function()
             ]]
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -709,9 +710,9 @@ describe('LSP', function()
 
     it('should check the body and didChange incremental', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -739,8 +740,8 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          if ctx.method == 'start' then
             exec_lua [[
               vim.api.nvim_buf_set_lines(BUFFER, 1, 2, false, {
                 "123boop";
@@ -748,8 +749,8 @@ describe('LSP', function()
             ]]
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -759,9 +760,9 @@ describe('LSP', function()
     -- TODO(askhan) we don't support full for now, so we can disable these tests.
     pending('should check the body and didChange incremental normal mode editing', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -788,13 +789,13 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          if ctx.method == 'start' then
             helpers.command("normal! 1Go")
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -803,9 +804,9 @@ describe('LSP', function()
 
     it('should check the body and didChange full with 2 changes', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -832,8 +833,8 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result, ctx)
+          if ctx.method == 'start' then
             exec_lua [[
               vim.api.nvim_buf_set_lines(BUFFER, 1, 2, false, {
                 "321";
@@ -844,8 +845,8 @@ describe('LSP', function()
             ]]
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -854,9 +855,9 @@ describe('LSP', function()
 
     it('should check the body and didChange full lifecycle', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -883,8 +884,8 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          if method == 'start' then
+        on_handler = function(err, result,ctx)
+          if ctx.method == 'start' then
             exec_lua [[
               vim.api.nvim_buf_set_lines(BUFFER, 1, 2, false, {
                 "321";
@@ -896,8 +897,8 @@ describe('LSP', function()
             ]]
             client.notify('finish')
           end
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -908,9 +909,9 @@ describe('LSP', function()
   describe("parsing tests", function()
     it('should handle invalid content-length correctly', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "start", {}, 1};
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -925,22 +926,22 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
+        on_handler = function(err, result, ctx)
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
         end;
       }
     end)
 
     it('should not trim vim.NIL from the end of a list', function()
       local expected_handlers = {
-        {NIL, "shutdown", {}, 1};
-        {NIL, "finish", {}, 1};
-        {NIL, "workspace/executeCommand", {
+        {NIL, {}, {method="shutdown", client_id=1}};
+        {NIL, {}, {method="finish", client_id=1}};
+        {NIL,{
           arguments = { "EXTRACT_METHOD", {metadata = {}}, 3, 0, 6123, NIL },
           command = "refactor.perform",
           title = "EXTRACT_METHOD"
-        }, 1};
-        {NIL, "start", {}, 1};
+        },  {method="workspace/executeCommand", client_id=1}};
+        {NIL, {}, {method="start", client_id=1}};
       }
       local client
       test_rpc_server {
@@ -964,9 +965,9 @@ describe('LSP', function()
           eq(0, code, "exit code", fake_lsp_logfile)
           eq(0, signal, "exit signal", fake_lsp_logfile)
         end;
-        on_handler = function(err, method, params, client_id)
-          eq(table.remove(expected_handlers), {err, method, params, client_id}, "expected handler")
-          if method == 'finish' then
+        on_handler = function(err, result, ctx)
+          eq(table.remove(expected_handlers), {err, result, ctx}, "expected handler")
+          if ctx.method == 'finish' then
             client.stop()
           end
         end;
@@ -1980,7 +1981,7 @@ describe('LSP', function()
   describe('vim.lsp.buf.outgoing_calls', function()
     it('does nothing for an empty response', function()
       local qflist_count = exec_lua([=[
-        require'vim.lsp.handlers'['callHierarchy/outgoingCalls']()
+        require'vim.lsp.handlers'['callHierarchy/outgoingCalls'](nil, nil, {}, nil)
         return #vim.fn.getqflist()
       ]=])
       eq(0, qflist_count)
@@ -2027,7 +2028,7 @@ describe('LSP', function()
           }
         } }
         local handler = require'vim.lsp.handlers'['callHierarchy/outgoingCalls']
-        handler(nil, nil, rust_analyzer_response)
+        handler(nil, rust_analyzer_response, {})
         return vim.fn.getqflist()
       ]=])
 
@@ -2053,7 +2054,7 @@ describe('LSP', function()
   describe('vim.lsp.buf.incoming_calls', function()
     it('does nothing for an empty response', function()
       local qflist_count = exec_lua([=[
-        require'vim.lsp.handlers'['callHierarchy/incomingCalls']()
+        require'vim.lsp.handlers'['callHierarchy/incomingCalls'](nil, nil, {})
         return #vim.fn.getqflist()
       ]=])
       eq(0, qflist_count)
@@ -2101,7 +2102,7 @@ describe('LSP', function()
         } }
 
         local handler = require'vim.lsp.handlers'['callHierarchy/incomingCalls']
-        handler(nil, nil, rust_analyzer_response)
+        handler(nil, rust_analyzer_response, {})
         return vim.fn.getqflist()
       ]=])