From aef58f598cd66e4be971bf3c9b91af94fbf09858 Mon Sep 17 00:00:00 2001 From: w0rp Date: Tue, 13 Jun 2017 17:53:47 +0100 Subject: [PATCH] Handle LSP responses for different files more consistently --- autoload/ale/engine.vim | 66 +++++------ autoload/ale/lsp.vim | 143 ++++++++++++------------ autoload/ale/lsp/tsserver_message.vim | 2 +- test/lsp/test_lsp_client_messages.vader | 2 +- test/test_loclist_corrections.vader | 16 +-- 5 files changed, 106 insertions(+), 123 deletions(-) diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim index 8c9293f3..d15ab901 100644 --- a/autoload/ale/engine.vim +++ b/autoload/ale/engine.vim @@ -11,15 +11,6 @@ if !has_key(s:, 'job_info_map') let s:job_info_map = {} endif -" Stores information for each LSP command including: -" -" linter: The linter dictionary for the command. -" buffer: The buffer number for the command. -" message: The message we sent, [is_notification, command, params?] -if !has_key(s:, 'lsp_command_info_map') - let s:lsp_command_info_map = {} -endif - let s:executable_cache_map = {} " Check if files are executable, and if they are, remember that they are @@ -52,7 +43,6 @@ function! ale#engine#InitBufferInfo(buffer) abort \ 'temporary_directory_list': [], \ 'history': [], \ 'open_lsp_documents': [], - \ 'lsp_command_list': [], \} endif endfunction @@ -114,13 +104,13 @@ function! s:GatherOutput(job_id, line) abort endif endfunction -function! s:HandleLoclist(linter, buffer, loclist) abort +function! s:HandleLoclist(linter_name, buffer, loclist) abort " Make some adjustments to the loclists to fix common problems, and also " to set default values for loclist items. - let l:linter_loclist = ale#engine#FixLocList(a:buffer, a:linter, a:loclist) + let l:linter_loclist = ale#engine#FixLocList(a:buffer, a:linter_name, a:loclist) " Remove previous items for this linter. - call filter(g:ale_buffer_info[a:buffer].loclist, 'v:val.linter_name !=# a:linter.name') + call filter(g:ale_buffer_info[a:buffer].loclist, 'v:val.linter_name !=# a:linter_name') " Add the new items. call extend(g:ale_buffer_info[a:buffer].loclist, l:linter_loclist) @@ -130,7 +120,7 @@ function! s:HandleLoclist(linter, buffer, loclist) abort call sort(g:ale_buffer_info[a:buffer].loclist, 'ale#util#LocItemCompare') let l:linting_is_done = empty(g:ale_buffer_info[a:buffer].job_list) - \ && empty(g:ale_buffer_info[a:buffer].lsp_command_list) + \ && !get(g:ale_buffer_info[a:buffer], 'waiting_for_tsserver', 0) if l:linting_is_done " Automatically remove all managed temporary files and directories @@ -201,31 +191,37 @@ function! s:HandleExit(job_id, exit_code) abort let l:loclist = ale#util#GetFunction(l:linter.callback)(l:buffer, l:output) - call s:HandleLoclist(l:linter, l:buffer, l:loclist) + call s:HandleLoclist(l:linter.name, l:buffer, l:loclist) endfunction -function! s:HandleLSPResponse(request_id, response) abort - let l:info = get(s:lsp_command_info_map, a:request_id, {}) +function! s:HandleLSPResponse(response) abort + let l:is_diag_response = get(a:response, 'type', '') ==# 'event' + \ && get(a:response, 'event', '') ==# 'semanticDiag' + + if !l:is_diag_response + return + endif + + let l:buffer = bufnr(a:response.body.file) + + let l:info = get(g:ale_buffer_info, l:buffer, {}) if empty(l:info) return endif - call remove(s:lsp_command_info_map, a:request_id) - - let l:command_list = g:ale_buffer_info[l:info.buffer].lsp_command_list - call filter(l:command_list, 'v:val != a:request_id') + let l:info.waiting_for_tsserver = 0 let l:loclist = ale#lsp#response#ReadTSServerDiagnostics(a:response) - call s:HandleLoclist(l:info.linter, l:info.buffer, l:loclist) + call s:HandleLoclist('tsserver', l:buffer, l:loclist) endfunction function! ale#engine#SetResults(buffer, loclist) abort let l:info = get(g:ale_buffer_info, a:buffer, {}) let l:job_list = get(l:info, 'job_list', []) - let l:lsp_command_list = get(l:info, 'lsp_command_list', []) - let l:linting_is_done = empty(l:job_list) && empty(l:lsp_command_list) + let l:waiting_for_tsserver = get(l:info, 'waiting_for_tsserver', 0) + let l:linting_is_done = empty(l:job_list) && !l:waiting_for_tsserver " Set signs first. This could potentially fix some line numbers. " The List could be sorted again here by SetSigns. @@ -261,7 +257,7 @@ function! ale#engine#SetResults(buffer, loclist) abort endif endfunction -function! ale#engine#FixLocList(buffer, linter, loclist) abort +function! ale#engine#FixLocList(buffer, linter_name, loclist) abort let l:new_loclist = [] " Some errors have line numbers beyond the end of the file, @@ -290,7 +286,7 @@ function! ale#engine#FixLocList(buffer, linter, loclist) abort \ 'vcol': get(l:old_item, 'vcol', 0), \ 'type': get(l:old_item, 'type', 'E'), \ 'nr': get(l:old_item, 'nr', -1), - \ 'linter_name': a:linter.name, + \ 'linter_name': a:linter_name, \} if has_key(l:old_item, 'detail') @@ -542,36 +538,28 @@ function! s:CheckWithTSServer(buffer, linter, executable) abort let l:open_documents = l:info.open_lsp_documents let l:is_open = index(l:open_documents, a:linter.name) >= 0 + call ale#lsp#StartProgram(a:executable, a:executable, function('s:HandleLSPResponse')) + if !l:is_open call add(l:open_documents, a:linter.name) call ale#lsp#SendMessageToProgram( \ a:executable, - \ a:executable, \ ale#lsp#tsserver_message#Open(a:buffer), \) endif call ale#lsp#SendMessageToProgram( \ a:executable, - \ a:executable, \ ale#lsp#tsserver_message#Change(a:buffer), \) - let l:message = ale#lsp#tsserver_message#Geterr(a:buffer) let l:request_id = ale#lsp#SendMessageToProgram( \ a:executable, - \ a:executable, - \ l:message, - \ function('s:HandleLSPResponse'), + \ ale#lsp#tsserver_message#Geterr(a:buffer), \) - if l:request_id > 0 - let s:lsp_command_info_map[l:request_id] = { - \ 'buffer': a:buffer, - \ 'linter': a:linter, - \ 'message': l:message, - \} - call add(l:info.lsp_command_list, l:request_id) + if l:request_id != 0 + let l:info.waiting_for_tsserver = 1 endif endfunction diff --git a/autoload/ale/lsp.vim b/autoload/ale/lsp.vim index a8e68a0a..449aa304 100644 --- a/autoload/ale/lsp.vim +++ b/autoload/ale/lsp.vim @@ -8,13 +8,11 @@ let g:ale_lsp_next_message_id = 1 function! s:NewConnection() abort " data: The message data received so far. - " callback_map: A mapping from connections to response callbacks. " address: An address only set for server connections. " executable: An executable only set for program connections. " job: A job ID only set for running programs. let l:conn = { \ 'data': '', - \ 'callback_map': {}, \ 'address': '', \ 'executable': '', \ 'job_id': -1, @@ -135,16 +133,6 @@ function! ale#lsp#ReadMessageData(data) abort return [l:remainder, l:response_list] endfunction -function! s:FindCallbackIDForType(callback_map, type) abort - for l:key in reverse(keys(a:callback_map)) - if a:callback_map[l:key][1][1] ==# a:type - return str2nr(l:key) - endif - endfor - - return 0 -endfunction - function! ale#lsp#HandleMessage(conn, message) abort let a:conn.data .= a:message @@ -153,18 +141,8 @@ function! ale#lsp#HandleMessage(conn, message) abort " Call our callbacks. for l:response in l:response_list - let l:id = has_key(l:response, 'seq') - \ ? l:response.seq - \ : l:response.id - - if get(l:response, 'type', '') ==# 'event' - \&& get(l:response, 'event', '') ==# 'semanticDiag' - let l:id = s:FindCallbackIDForType(a:conn.callback_map, 'ts@geterr') - endif - - if has_key(a:conn.callback_map, l:id) - let [l:Callback, l:message] = remove(a:conn.callback_map, l:id) - call ale#util#GetFunction(l:Callback)(l:id, l:response) + if has_key(a:conn, 'callback') + call ale#util#GetFunction(a:conn.callback)(l:response) endif endfor endfunction @@ -183,34 +161,18 @@ function! s:HandleCommandMessage(job_id, message) abort call ale#lsp#HandleMessage(l:conn, a:message) endfunction -" Send a message to a server with a given executable, and a command for -" running the executable. -" -" A callback can be registered to handle the response. -" Notifications do not need to be handled. -" (executable, command, message, callback?) -" -" Returns 1 when a message is sent, 0 otherwise. -function! ale#lsp#SendMessageToProgram(executable, command, message, ...) abort - if a:0 > 1 - throw 'Too many arguments!' - endif - - if !a:message[0] && a:0 == 0 - throw 'A callback must be set for messages which are not notifications!' - endif - +" Start a program for LSP servers which run with executables. +function! ale#lsp#StartProgram(executable, command, callback) abort if !executable(a:executable) return 0 endif - let [l:id, l:data] = ale#lsp#CreateMessageData(a:message) - let l:matches = filter(s:connections[:], 'v:val.executable ==# a:executable') " Get the current connection or a new one. let l:conn = !empty(l:matches) ? l:matches[0] : s:NewConnection() let l:conn.executable = a:executable + let l:conn.callback = a:callback if !ale#job#IsRunning(l:conn.job_id) let l:options = { @@ -226,37 +188,42 @@ function! ale#lsp#SendMessageToProgram(executable, command, message, ...) abort return 0 endif - " The ID is 0 when the message is a Notification, which is a JSON-RPC - " request for which the server must not return a response. - if l:id != 0 - " Add the callback, which the server will respond to later. - let l:conn.callback_map[l:id] = [a:1, a:message] - endif - - call ale#job#SendRaw(l:job_id, l:data) - let l:conn.job_id = l:job_id - return l:id + return 1 endfunction -" Send a message to a server at a given address. -" A callback can be registered to handle the response. -" Notifications do not need to be handled. -" (address, message, callback?) +" Send a message to a server with a given executable, and a command for +" running the executable. " -" Returns 1 when a message is sent, 0 otherwise. -function! ale#lsp#SendMessageToAddress(address, message, ...) abort - if a:0 > 1 - throw 'Too many arguments!' - endif - - if !a:message[0] && a:0 == 0 - throw 'A callback must be set for messages which are not notifications!' - endif - +" Returns -1 when a message is sent, but no response is expected +" 0 when the message is not sent and +" >= 1 with the message ID when a response is expected. +function! ale#lsp#SendMessageToProgram(executable, message) abort let [l:id, l:data] = ale#lsp#CreateMessageData(a:message) + let l:matches = filter(s:connections[:], 'v:val.executable ==# a:executable') + + " No connection is currently open. + if empty(l:matches) + return 0 + endif + + " Get the current connection or a new one. + let l:conn = l:matches[0] + let l:conn.executable = a:executable + + if get(l:conn, 'job_id', 0) == 0 + return 0 + endif + + call ale#job#SendRaw(l:conn.job_id, l:data) + + return l:id == 0 ? -1 : l:id +endfunction + +" Connect to an address and set up a callback for handling responses. +function! ale#lsp#ConnectToAddress(address, callback) abort let l:matches = filter(s:connections[:], 'v:val.address ==# a:address') " Get the current connection or a new one. let l:conn = !empty(l:matches) ? l:matches[0] : s:NewConnection() @@ -269,19 +236,47 @@ function! ale#lsp#SendMessageToAddress(address, message, ...) abort \}) endif - " The ID is 0 when the message is a Notification, which is a JSON-RPC - " request for which the server must not return a response. - if l:id != 0 - " Add the callback, which the server will respond to later. - let l:conn.callback_map[l:id] = [a:1, a:message] + if ch_status(l:conn.channnel) ==# 'fail' + return 0 endif - if ch_status(l:conn.channnel) ==# 'fail' + let l:conn.callback = a:callback + + return 1 +endfunction + +" Send a message to a server at a given address. +" Notifications do not need to be handled. +" +" Returns -1 when a message is sent, but no response is expected +" 0 when the message is not sent and +" >= 1 with the message ID when a response is expected. +function! ale#lsp#SendMessageToAddress(address, message) abort + if a:0 > 1 + throw 'Too many arguments!' + endif + + if !a:message[0] && a:0 == 0 + throw 'A callback must be set for messages which are not notifications!' + endif + + let [l:id, l:data] = ale#lsp#CreateMessageData(a:message) + + let l:matches = filter(s:connections[:], 'v:val.address ==# a:address') + + " No connection is currently open. + if empty(l:matches) + return 0 + endif + + let l:conn = l:matches[0] + + if ch_status(l:conn.channnel) !=# 'open' return 0 endif " Send the message to the server call ch_sendraw(l:conn.channel, l:data) - return l:id + return l:id == 0 ? -1 : l:id endfunction diff --git a/autoload/ale/lsp/tsserver_message.vim b/autoload/ale/lsp/tsserver_message.vim index dc5a4717..e78b29e3 100644 --- a/autoload/ale/lsp/tsserver_message.vim +++ b/autoload/ale/lsp/tsserver_message.vim @@ -33,5 +33,5 @@ function! ale#lsp#tsserver_message#Change(buffer) abort endfunction function! ale#lsp#tsserver_message#Geterr(buffer) abort - return [0, 'ts@geterr', {'files': [expand('#' . a:buffer . ':p')]}] + return [1, 'ts@geterr', {'files': [expand('#' . a:buffer . ':p')]}] endfunction diff --git a/test/lsp/test_lsp_client_messages.vader b/test/lsp/test_lsp_client_messages.vader index 75f5826a..5decbf6c 100644 --- a/test/lsp/test_lsp_client_messages.vader +++ b/test/lsp/test_lsp_client_messages.vader @@ -139,7 +139,7 @@ Execute(ale#lsp#tsserver_message#Geterr() should return correct messages): AssertEqual \ [ - \ 0, + \ 1, \ 'ts@geterr', \ { \ 'files': [b:dir . '/foo.ts'], diff --git a/test/test_loclist_corrections.vader b/test/test_loclist_corrections.vader index 4e3f543c..e23109ed 100644 --- a/test/test_loclist_corrections.vader +++ b/test/test_loclist_corrections.vader @@ -36,7 +36,7 @@ Execute(FixLocList should set all the default values correctly): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [{'text': 'a', 'lnum': 2}, {'text': 'b', 'lnum': 2}], \ ) @@ -56,7 +56,7 @@ Execute(FixLocList should use the values we supply): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [{ \ 'text': 'a', \ 'lnum': 3, @@ -84,7 +84,7 @@ Execute(FixLocList should set items with lines beyond the end to the last line): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [{'text': 'a', 'lnum': 11}], \ ) @@ -104,7 +104,7 @@ Execute(FixLocList should move line 0 to line 1): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [{'text': 'a', 'lnum': 0}], \ ) @@ -125,7 +125,7 @@ Execute(FixLocList should convert line and column numbers correctly): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [{'text': 'a', 'lnum': '010', 'col': '010'}], \ ) @@ -158,7 +158,7 @@ Execute(FixLocList should pass on end_col values): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [ \ {'text': 'a', 'lnum': '010', 'col': '010', 'end_col': '012'}, \ {'text': 'a', 'lnum': '010', 'col': '011', 'end_col': 12}, @@ -195,7 +195,7 @@ Execute(FixLocList should pass on end_lnum values): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [ \ {'text': 'a', 'lnum': '010', 'col': '010', 'end_col': '012', 'end_lnum': '013'}, \ {'text': 'a', 'lnum': '010', 'col': '011', 'end_col': 12, 'end_lnum': 13}, @@ -220,6 +220,6 @@ Execute(FixLocList should allow subtypes to be set): \], \ ale#engine#FixLocList( \ bufnr('%'), - \ {'name': 'foobar'}, + \ 'foobar', \ [{'text': 'a', 'lnum': 11, 'sub_type': 'style'}], \ )