From 0d10653a7c780ac98fd2b980679676f5e8f231e7 Mon Sep 17 00:00:00 2001 From: w0rp Date: Fri, 15 Feb 2019 08:54:53 +0000 Subject: [PATCH] Fix #2295 - Respond to initialize with an initialized message --- autoload/ale/job.vim | 20 ++++++- autoload/ale/lsp.vim | 55 ++++++++++--------- autoload/ale/lsp_linter.vim | 48 +++++++++------- .../test_lsp_completion_messages.vader | 2 +- test/lsp/test_did_save_event.vader | 2 +- ...st_other_initialize_message_handling.vader | 19 +++++++ test/lsp/test_update_config.vader | 4 ++ test/test_find_references.vader | 18 ++++-- test/test_go_to_definition.vader | 20 +++++-- test/test_hover.vader | 1 + 10 files changed, 132 insertions(+), 57 deletions(-) diff --git a/autoload/ale/job.vim b/autoload/ale/job.vim index 841a6379..14b3e484 100644 --- a/autoload/ale/job.vim +++ b/autoload/ale/job.vim @@ -299,7 +299,11 @@ function! ale#job#SendRaw(job_id, string) abort if has('nvim') call jobsend(a:job_id, a:string) else - call ch_sendraw(job_getchannel(s:job_map[a:job_id].job), a:string) + let l:job = s:job_map[a:job_id].job + + if ch_status(l:job) is# 'open' + call ch_sendraw(job_getchannel(l:job), a:string) + endif endif endfunction @@ -323,6 +327,20 @@ function! ale#job#IsRunning(job_id) abort return 0 endfunction +function! ale#job#HasOpenChannel(job_id) abort + if ale#job#IsRunning(a:job_id) + if has('nvim') + " TODO: Implement a check for NeoVim. + return 1 + endif + + " Check if the Job's channel can be written to. + return ch_status(s:job_map[a:job_id].job) is# 'open' + endif + + return 0 +endfunction + " Given a Job ID, stop that job. " Invalid job IDs will be ignored. function! ale#job#Stop(job_id) abort diff --git a/autoload/ale/lsp.vim b/autoload/ale/lsp.vim index 9dab9b8c..7718b349 100644 --- a/autoload/ale/lsp.vim +++ b/autoload/ale/lsp.vim @@ -21,7 +21,6 @@ function! ale#lsp#Register(executable_or_address, project, init_options) abort " init_options: Options to send to the server. " config: Configuration settings to send to the server. " callback_list: A list of callbacks for handling LSP responses. - " message_queue: Messages queued for sending to callbacks. " capabilities_queue: The list of callbacks to call with capabilities. " capabilities: Features the server supports. let s:connections[l:conn_id] = { @@ -35,7 +34,6 @@ function! ale#lsp#Register(executable_or_address, project, init_options) abort \ 'init_options': a:init_options, \ 'config': {}, \ 'callback_list': [], - \ 'message_queue': [], \ 'init_queue': [], \ 'capabilities': { \ 'hover': 0, @@ -250,13 +248,8 @@ function! ale#lsp#HandleInitResponse(conn, response) abort return endif - " After the server starts, send messages we had queued previously. - for l:message_data in a:conn.message_queue - call s:SendMessageData(a:conn, l:message_data) - endfor - - " Remove the messages now. - let a:conn.message_queue = [] + " The initialized message must be sent before everything else. + call ale#lsp#Send(a:conn.id, ale#lsp#message#Initialized()) " Call capabilities callbacks queued for the project. for l:Callback in a:conn.init_queue @@ -317,14 +310,23 @@ function! ale#lsp#MarkConnectionAsTsserver(conn_id) abort let l:conn.capabilities.symbol_search = 1 endfunction +function! s:SendInitMessage(conn) abort + let [l:init_id, l:init_data] = ale#lsp#CreateMessageData( + \ ale#lsp#message#Initialize(a:conn.root, a:conn.init_options), + \) + let a:conn.init_request_id = l:init_id + call s:SendMessageData(a:conn, l:init_data) +endfunction + " Start a program for LSP servers. " " 1 will be returned if the program is running, or 0 if the program could " not be started. function! ale#lsp#StartProgram(conn_id, executable, command) abort let l:conn = s:connections[a:conn_id] + let l:started = 0 - if !has_key(l:conn, 'job_id') || !ale#job#IsRunning(l:conn.job_id) + if !has_key(l:conn, 'job_id') || !ale#job#HasOpenChannel(l:conn.job_id) let l:options = { \ 'mode': 'raw', \ 'out_cb': {_, message -> ale#lsp#HandleMessage(a:conn_id, message)}, @@ -335,6 +337,8 @@ function! ale#lsp#StartProgram(conn_id, executable, command) abort else let l:job_id = ale#job#Start(a:command, l:options) endif + + let l:started = 1 else let l:job_id = l:conn.job_id endif @@ -343,6 +347,10 @@ function! ale#lsp#StartProgram(conn_id, executable, command) abort let l:conn.job_id = l:job_id endif + if l:started + call s:SendInitMessage(l:conn) + endif + return l:job_id > 0 endfunction @@ -352,11 +360,14 @@ endfunction " not be opened. function! ale#lsp#ConnectToAddress(conn_id, address) abort let l:conn = s:connections[a:conn_id] + let l:started = 0 if !has_key(l:conn, 'channel_id') || !ale#socket#IsOpen(l:conn.channel_id) let l:channel_id = ale#socket#Open(a:address, { \ 'callback': {_, mess -> ale#lsp#HandleMessage(a:conn_id, mess)}, \}) + + let l:started = 1 else let l:channel_id = l:conn.channel_id endif @@ -365,6 +376,10 @@ function! ale#lsp#ConnectToAddress(conn_id, address) abort let l:conn.channel_id = l:channel_id endif + if l:started + call s:SendInitMessage(l:conn) + endif + return l:channel_id >= 0 endfunction @@ -429,26 +444,12 @@ function! ale#lsp#Send(conn_id, message) abort return 0 endif - " If we haven't initialized the server yet, then send the message for it. - if !l:conn.initialized && !l:conn.init_request_id - let [l:init_id, l:init_data] = ale#lsp#CreateMessageData( - \ ale#lsp#message#Initialize(l:conn.root, l:conn.init_options), - \) - - let l:conn.init_request_id = l:init_id - - call s:SendMessageData(l:conn, l:init_data) + if !l:conn.initialized + throw 'LSP server not initialized yet!' endif let [l:id, l:data] = ale#lsp#CreateMessageData(a:message) - - if l:conn.initialized - " Send the message now. - call s:SendMessageData(l:conn, l:data) - else - " Add the message we wanted to send to a List to send later. - call add(l:conn.message_queue, l:data) - endif + call s:SendMessageData(l:conn, l:data) return l:id == 0 ? -1 : l:id endfunction diff --git a/autoload/ale/lsp_linter.vim b/autoload/ale/lsp_linter.vim index 82d4d86f..e04915b0 100644 --- a/autoload/ale/lsp_linter.vim +++ b/autoload/ale/lsp_linter.vim @@ -185,6 +185,31 @@ function! ale#lsp_linter#FindProjectRoot(buffer, linter) abort return ale#util#GetFunction(a:linter.project_root_callback)(a:buffer) endfunction +" This function is accessible so tests can call it. +function! ale#lsp_linter#OnInit(linter, details, Callback) abort + let l:buffer = a:details.buffer + let l:conn_id = a:details.connection_id + let l:command = a:details.command + + let l:config = ale#lsp_linter#GetConfig(l:buffer, a:linter) + let l:language_id = ale#util#GetFunction(a:linter.language_callback)(l:buffer) + + call ale#lsp#UpdateConfig(l:conn_id, l:buffer, l:config) + + if ale#lsp#OpenDocument(l:conn_id, l:buffer, l:language_id) + if g:ale_history_enabled && !empty(l:command) + call ale#history#Add(l:buffer, 'started', l:conn_id, l:command) + endif + endif + + " The change message needs to be sent for tsserver before doing anything. + if a:linter.lsp is# 'tsserver' + call ale#lsp#NotifyForChanges(l:conn_id, l:buffer) + endif + + call a:Callback(a:linter, a:details) +endfunction + " Given a buffer, an LSP linter, start up an LSP linter and get ready to " receive messages for the document. function! ale#lsp_linter#StartLSP(buffer, linter, Callback) abort @@ -207,7 +232,7 @@ function! ale#lsp_linter#StartLSP(buffer, linter, Callback) abort else let l:executable = ale#linter#GetExecutable(a:buffer, a:linter) - if empty(l:executable) || !executable(l:executable) + if !ale#engine#IsExecutable(a:buffer, l:executable) return 0 endif @@ -233,31 +258,16 @@ function! ale#lsp_linter#StartLSP(buffer, linter, Callback) abort call ale#lsp#MarkConnectionAsTsserver(l:conn_id) endif - let l:config = ale#lsp_linter#GetConfig(a:buffer, a:linter) - let l:language_id = ale#util#GetFunction(a:linter.language_callback)(a:buffer) - let l:details = { \ 'buffer': a:buffer, \ 'connection_id': l:conn_id, \ 'command': l:command, \ 'project_root': l:root, - \ 'language_id': l:language_id, \} - call ale#lsp#UpdateConfig(l:conn_id, a:buffer, l:config) - - if ale#lsp#OpenDocument(l:conn_id, a:buffer, l:language_id) - if g:ale_history_enabled && !empty(l:command) - call ale#history#Add(a:buffer, 'started', l:conn_id, l:command) - endif - endif - - " The change message needs to be sent for tsserver before doing anything. - if a:linter.lsp is# 'tsserver' - call ale#lsp#NotifyForChanges(l:conn_id, a:buffer) - endif - - call ale#lsp#OnInit(l:conn_id, {-> a:Callback(a:linter, l:details)}) + call ale#lsp#OnInit(l:conn_id, {-> + \ ale#lsp_linter#OnInit(a:linter, l:details, a:Callback) + \}) return 1 endfunction diff --git a/test/completion/test_lsp_completion_messages.vader b/test/completion/test_lsp_completion_messages.vader index 35b167b6..c83ad8e1 100644 --- a/test/completion/test_lsp_completion_messages.vader +++ b/test/completion/test_lsp_completion_messages.vader @@ -23,10 +23,10 @@ Before: call ale#lsp#MarkDocumentAsOpen(g:conn_id, a:buffer) let l:details = { + \ 'command': 'foobar', \ 'buffer': a:buffer, \ 'connection_id': g:conn_id, \ 'project_root': '/foo/bar', - \ 'language_id': 'python', \} call add(g:init_callback_list, {-> a:Callback(a:linter, l:details)}) diff --git a/test/lsp/test_did_save_event.vader b/test/lsp/test_did_save_event.vader index 1d6065ec..423138af 100644 --- a/test/lsp/test_did_save_event.vader +++ b/test/lsp/test_did_save_event.vader @@ -38,10 +38,10 @@ Before: let g:conn_id = ale#lsp#Register('executable', '/foo/bar', {}) call ale#lsp#MarkDocumentAsOpen(g:conn_id, a:buffer) let l:details = { + \ 'command': 'foobar', \ 'buffer': a:buffer, \ 'connection_id': g:conn_id, \ 'project_root': '/foo/bar', - \ 'language_id': 'foobar', \} call a:Callback(a:linter, l:details) diff --git a/test/lsp/test_other_initialize_message_handling.vader b/test/lsp/test_other_initialize_message_handling.vader index 8ae63e91..4150b9d7 100644 --- a/test/lsp/test_other_initialize_message_handling.vader +++ b/test/lsp/test_other_initialize_message_handling.vader @@ -1,5 +1,9 @@ Before: + runtime autoload/ale/lsp.vim + + let g:message_list = [] let b:conn = { + \ 'id': 1, \ 'is_tsserver': 0, \ 'data': '', \ 'root': '/foo/bar', @@ -21,8 +25,17 @@ Before: \ }, \} + function! ale#lsp#Send(conn_id, message) abort + call add(g:message_list, a:message) + + return 42 + endfunction + After: unlet! b:conn + unlet! g:message_list + + runtime autoload/ale/lsp.vim Execute(Messages with no method and capabilities should initialize projects): call ale#lsp#HandleInitResponse(b:conn, { @@ -30,15 +43,18 @@ Execute(Messages with no method and capabilities should initialize projects): \}) AssertEqual 1, b:conn.initialized + AssertEqual [[1, 'initialized']], g:message_list Execute(Other messages should not initialize projects): call ale#lsp#HandleInitResponse(b:conn, {'method': 'lolwat'}) AssertEqual 0, b:conn.initialized + AssertEqual [], g:message_list call ale#lsp#HandleInitResponse(b:conn, {'result': {'x': {}}}) AssertEqual 0, b:conn.initialized + AssertEqual [], g:message_list Execute(Capabilities should bet set up correctly): call ale#lsp#HandleInitResponse(b:conn, { @@ -86,6 +102,7 @@ Execute(Capabilities should bet set up correctly): \ 'symbol_search': 1, \ }, \ b:conn.capabilities + AssertEqual [[1, 'initialized']], g:message_list Execute(Disabled capabilities should be recognised correctly): call ale#lsp#HandleInitResponse(b:conn, { @@ -128,6 +145,7 @@ Execute(Disabled capabilities should be recognised correctly): \ 'symbol_search': 0, \ }, \ b:conn.capabilities + AssertEqual [[1, 'initialized']], g:message_list Execute(Results that are not dictionaries should be handled correctly): call ale#lsp#HandleInitResponse(b:conn, { @@ -135,3 +153,4 @@ Execute(Results that are not dictionaries should be handled correctly): \ 'id': 1, \ 'result': v:null, \}) + AssertEqual [], g:message_list diff --git a/test/lsp/test_update_config.vader b/test/lsp/test_update_config.vader index 07068bc8..698477ec 100644 --- a/test/lsp/test_update_config.vader +++ b/test/lsp/test_update_config.vader @@ -3,6 +3,10 @@ Before: let g:conn_id = ale#lsp#Register('executable', '/foo/bar', {}) + " Stub out this function, so we test updating configs. + function! ale#lsp#Send(conn_id, message) abort + endfunction + After: Restore diff --git a/test/test_find_references.vader b/test/test_find_references.vader index 078333bb..eb06e3bc 100644 --- a/test/test_find_references.vader +++ b/test/test_find_references.vader @@ -13,7 +13,7 @@ Before: let g:conn_id = v:null let g:InitCallback = v:null - runtime autoload/ale/linter.vim + runtime autoload/ale/lsp_linter.vim runtime autoload/ale/lsp.vim runtime autoload/ale/util.vim runtime autoload/ale/preview.vim @@ -21,14 +21,19 @@ Before: function! ale#lsp_linter#StartLSP(buffer, linter, Callback) abort let g:conn_id = ale#lsp#Register('executable', '/foo/bar', {}) call ale#lsp#MarkDocumentAsOpen(g:conn_id, a:buffer) + + if a:linter.lsp is# 'tsserver' + call ale#lsp#MarkConnectionAsTsserver(g:conn_id) + endif + let l:details = { + \ 'command': 'foobar', \ 'buffer': a:buffer, \ 'connection_id': g:conn_id, \ 'project_root': '/foo/bar', - \ 'language_id': 'python', \} - let g:InitCallback = {-> a:Callback(a:linter, l:details)} + let g:InitCallback = {-> ale#lsp_linter#OnInit(a:linter, l:details, a:Callback)} endfunction function! ale#lsp#HasCapability(conn_id, capability) abort @@ -167,6 +172,8 @@ Execute(The preview window should not be opened for empty tsserver responses): AssertEqual ['echom ''No references found.'''], g:expr_list Execute(tsserver reference requests should be sent): + call ale#linter#Reset() + runtime ale_linters/typescript/tsserver.vim call setpos('.', [bufnr(''), 2, 5, 0]) @@ -183,7 +190,10 @@ Execute(tsserver reference requests should be sent): \ 'function(''ale#references#HandleTSServerResponse'')', \ string(g:Callback) AssertEqual - \ [[0, 'ts@references', {'file': expand('%:p'), 'line': 2, 'offset': 5}]], + \ [ + \ ale#lsp#tsserver_message#Change(bufnr('')), + \ [0, 'ts@references', {'file': expand('%:p'), 'line': 2, 'offset': 5}] + \ ], \ g:message_list AssertEqual {'42': {'use_relative_paths': 0}}, ale#references#GetMap() diff --git a/test/test_go_to_definition.vader b/test/test_go_to_definition.vader index 880d7123..452b7692 100644 --- a/test/test_go_to_definition.vader +++ b/test/test_go_to_definition.vader @@ -11,20 +11,26 @@ Before: let g:InitCallback = v:null runtime autoload/ale/linter.vim + runtime autoload/ale/lsp_linter.vim runtime autoload/ale/lsp.vim runtime autoload/ale/util.vim function! ale#lsp_linter#StartLSP(buffer, linter, Callback) abort let g:conn_id = ale#lsp#Register('executable', '/foo/bar', {}) call ale#lsp#MarkDocumentAsOpen(g:conn_id, a:buffer) + + if a:linter.lsp is# 'tsserver' + call ale#lsp#MarkConnectionAsTsserver(g:conn_id) + endif + let l:details = { + \ 'command': 'foobar', \ 'buffer': a:buffer, \ 'connection_id': g:conn_id, \ 'project_root': '/foo/bar', - \ 'language_id': 'python', \} - let g:InitCallback = {-> a:Callback(a:linter, l:details)} + let g:InitCallback = {-> ale#lsp_linter#OnInit(a:linter, l:details, a:Callback)} endfunction function! ale#lsp#HasCapability(conn_id, capability) abort @@ -215,7 +221,10 @@ Execute(tsserver definition requests should be sent): \ 'function(''ale#definition#HandleTSServerResponse'')', \ string(g:Callback) AssertEqual - \ [[0, 'ts@definition', {'file': expand('%:p'), 'line': 2, 'offset': 5}]], + \ [ + \ ale#lsp#tsserver_message#Change(bufnr('')), + \ [0, 'ts@definition', {'file': expand('%:p'), 'line': 2, 'offset': 5}] + \ ], \ g:message_list AssertEqual {'42': {'open_in': 'current-buffer'}}, ale#definition#GetMap() @@ -236,7 +245,10 @@ Execute(tsserver tab definition requests should be sent): \ 'function(''ale#definition#HandleTSServerResponse'')', \ string(g:Callback) AssertEqual - \ [[0, 'ts@definition', {'file': expand('%:p'), 'line': 2, 'offset': 5}]], + \ [ + \ ale#lsp#tsserver_message#Change(bufnr('')), + \ [0, 'ts@definition', {'file': expand('%:p'), 'line': 2, 'offset': 5}] + \ ], \ g:message_list AssertEqual {'42': {'open_in': 'tab'}}, ale#definition#GetMap() diff --git a/test/test_hover.vader b/test/test_hover.vader index 13cfdbab..917694a2 100644 --- a/test/test_hover.vader +++ b/test/test_hover.vader @@ -15,6 +15,7 @@ Before: let g:Callback = a:callback return { + \ 'command': 'foobar', \ 'connection_id': 347, \ 'project_root': '/foo/bar', \}