From f90e72ae1f368ccbec3cb57ef0153936fdb530cd Mon Sep 17 00:00:00 2001 From: w0rp Date: Sun, 23 Mar 2025 14:37:04 +0000 Subject: [PATCH] Close #3600 - Implement pull diagnostics in VimL Implement pull diagnostics in the VimL implementation so ALE is able to track when servers are busy checking files. Only servers that support this feature will return diagnostics these ways. --- autoload/ale/lsp.vim | 12 ++ autoload/ale/lsp/message.vim | 8 ++ autoload/ale/lsp/response.vim | 6 +- autoload/ale/lsp_linter.vim | 65 ++++++++- .../test_engine_lsp_response_handling.vader | 124 ++++++++++++++++++ test/lsp/test_lsp_client_messages.vader | 13 ++ test/lsp/test_lsp_startup.vader | 4 + ...st_other_initialize_message_handling.vader | 7 + test/lsp/test_read_lsp_diagnostics.vader | 36 ++--- 9 files changed, 248 insertions(+), 27 deletions(-) diff --git a/autoload/ale/lsp.vim b/autoload/ale/lsp.vim index 439c912b..a83a7bea 100644 --- a/autoload/ale/lsp.vim +++ b/autoload/ale/lsp.vim @@ -47,6 +47,7 @@ function! ale#lsp#Register(executable_or_address, project, language, init_option \ 'definition': 0, \ 'typeDefinition': 0, \ 'implementation': 0, + \ 'pull_model': 0, \ 'symbol_search': 0, \ 'code_actions': 0, \ 'did_save': 0, @@ -276,6 +277,13 @@ function! ale#lsp#UpdateCapabilities(conn_id, capabilities) abort let l:conn.capabilities.implementation = 1 endif + " Check if the language server supports pull model diagnostics. + if type(get(a:capabilities, 'diagnosticProvider')) is v:t_dict + if type(get(a:capabilities.diagnosticProvider, 'interFileDependencies')) is v:t_bool + let l:conn.capabilities.pull_model = 1 + endif + endif + if get(a:capabilities, 'workspaceSymbolProvider') is v:true let l:conn.capabilities.symbol_search = 1 endif @@ -486,6 +494,10 @@ function! s:SendInitMessage(conn) abort \ 'dynamicRegistration': v:false, \ 'linkSupport': v:false, \ }, + \ 'diagnostic': { + \ 'dynamicRegistration': v:true, + \ 'relatedDocumentSupport': v:true, + \ }, \ 'publishDiagnostics': { \ 'relatedInformation': v:true, \ }, diff --git a/autoload/ale/lsp/message.vim b/autoload/ale/lsp/message.vim index 9d5b6228..72ed7d59 100644 --- a/autoload/ale/lsp/message.vim +++ b/autoload/ale/lsp/message.vim @@ -200,6 +200,14 @@ function! ale#lsp#message#CodeAction(buffer, line, column, end_line, end_column, \}] endfunction +function! ale#lsp#message#Diagnostic(buffer) abort + return [0, 'textDocument/diagnostic', { + \ 'textDocument': { + \ 'uri': ale#util#ToURI(expand('#' . a:buffer . ':p')), + \ }, + \}] +endfunction + function! ale#lsp#message#ExecuteCommand(command, arguments) abort return [0, 'workspace/executeCommand', { \ 'command': a:command, diff --git a/autoload/ale/lsp/response.vim b/autoload/ale/lsp/response.vim index 498ec508..85ac9e69 100644 --- a/autoload/ale/lsp/response.vim +++ b/autoload/ale/lsp/response.vim @@ -21,11 +21,11 @@ let s:SEVERITY_WARNING = 2 let s:SEVERITY_INFORMATION = 3 let s:SEVERITY_HINT = 4 -" Parse the message for textDocument/publishDiagnostics -function! ale#lsp#response#ReadDiagnostics(response) abort +" Convert Diagnostic[] data from a language server to an ALE loclist. +function! ale#lsp#response#ReadDiagnostics(diagnostics) abort let l:loclist = [] - for l:diagnostic in a:response.params.diagnostics + for l:diagnostic in a:diagnostics let l:severity = get(l:diagnostic, 'severity', 0) let l:loclist_item = { \ 'text': substitute(l:diagnostic.message, '\(\r\n\|\n\|\r\)', ' ', 'g'), diff --git a/autoload/ale/lsp_linter.vim b/autoload/ale/lsp_linter.vim index 72801e43..9936263d 100644 --- a/autoload/ale/lsp_linter.vim +++ b/autoload/ale/lsp_linter.vim @@ -23,6 +23,26 @@ function! ale#lsp_linter#SetLSPLinterMap(replacement_map) abort let s:lsp_linter_map = a:replacement_map endfunction +" A map for tracking URIs for diagnostic request IDs +if !has_key(s:, 'diagnostic_uri_map') + let s:diagnostic_uri_map = {} +endif + +" For internal use only. +function! ale#lsp_linter#ClearDiagnosticURIMap() abort + let s:diagnostic_uri_map = {} +endfunction + +" For internal use only. +function! ale#lsp_linter#GetDiagnosticURIMap() abort + return s:diagnostic_uri_map +endfunction + +" Just for tests. +function! ale#lsp_linter#SetDiagnosticURIMap(replacement_map) abort + let s:diagnostic_uri_map = a:replacement_map +endfunction + " Get all enabled LSP linters. " This list still includes linters ignored with `ale_linters_ignore`. " @@ -77,14 +97,17 @@ function! s:ShouldIgnoreDiagnostics(buffer, linter) abort return 0 endfunction -function! s:HandleLSPDiagnostics(conn_id, response) abort +" Handle LSP diagnostics for a given URI. +" The special value 'unchanged' can be used for diagnostics to indicate +" that diagnostics haven't changed since we last checked. +function! s:HandleLSPDiagnostics(conn_id, uri, diagnostics) abort let l:linter = get(s:lsp_linter_map, a:conn_id) if empty(l:linter) return endif - let l:filename = ale#util#ToResource(a:response.params.uri) + let l:filename = ale#util#ToResource(a:uri) let l:escaped_name = escape( \ fnameescape(l:filename), \ has('win32') ? '^' : '^,}]' @@ -100,9 +123,12 @@ function! s:HandleLSPDiagnostics(conn_id, response) abort return endif - let l:loclist = ale#lsp#response#ReadDiagnostics(a:response) - - call ale#engine#HandleLoclist(l:linter.name, l:buffer, l:loclist, 0) + if a:diagnostics is# 'unchanged' + call ale#engine#MarkLinterInactive(l:info, l:linter) + else + let l:loclist = ale#lsp#response#ReadDiagnostics(a:diagnostics) + call ale#engine#HandleLoclist(l:linter.name, l:buffer, l:loclist, 0) + endif endfunction function! s:HandleTSServerDiagnostics(response, error_type) abort @@ -204,7 +230,17 @@ function! ale#lsp_linter#HandleLSPResponse(conn_id, response) abort call s:HandleLSPErrorMessage(l:linter, a:response) elseif l:method is# 'textDocument/publishDiagnostics' - call s:HandleLSPDiagnostics(a:conn_id, a:response) + let l:uri = a:response.params.uri + let l:diagnostics = a:response.params.diagnostics + + call s:HandleLSPDiagnostics(a:conn_id, l:uri, l:diagnostics) + elseif has_key(s:diagnostic_uri_map, get(a:response, 'id')) + let l:uri = remove(s:diagnostic_uri_map, a:response.id) + let l:diagnostics = a:response.result.kind is# 'unchanged' + \ ? 'unchanged' + \ : a:response.result.items + + call s:HandleLSPDiagnostics(a:conn_id, l:uri, l:diagnostics) elseif l:method is# 'window/showMessage' call ale#lsp_window#HandleShowMessage( \ s:lsp_linter_map[a:conn_id].name, @@ -530,6 +566,23 @@ function! s:CheckWithLSP(linter, details) abort let l:save_message = ale#lsp#message#DidSave(l:buffer, l:include_text) let l:notified = ale#lsp#Send(l:id, l:save_message) != 0 endif + + let l:diagnostic_request_id = 0 + + " If the document is updated and we can pull diagnostics, try to. + if ale#lsp#HasCapability(l:id, 'pull_model') + let l:diagnostic_message = ale#lsp#message#Diagnostic(l:buffer) + + let l:diagnostic_request_id = ale#lsp#Send(l:id, l:diagnostic_message) + endif + + " If we are going to pull diagnostics, then mark the linter as active, + " and remember the URI we sent the request for. + if l:diagnostic_request_id + call ale#engine#MarkLinterActive(l:info, a:linter) + let s:diagnostic_uri_map[l:diagnostic_request_id] = + \ l:diagnostic_message[2].textDocument.uri + endif endif endfunction diff --git a/test/lsp/test_engine_lsp_response_handling.vader b/test/lsp/test_engine_lsp_response_handling.vader index b00a6942..7b18befc 100644 --- a/test/lsp/test_engine_lsp_response_handling.vader +++ b/test/lsp/test_engine_lsp_response_handling.vader @@ -410,6 +410,130 @@ Execute(LSP errors should mark linters no longer active): AssertEqual [], g:ale_buffer_info[bufnr('')].active_linter_list +Execute(LSP pull model diagnostic responses should be handled): + let b:ale_linters = ['eclipselsp'] + runtime ale_linters/java/eclipselsp.vim + + if has('win32') + call ale#test#SetFilename('filename,[]^$.ts') + else + call ale#test#SetFilename('filename*?,{}[]^$.java') + endif + + call ale#engine#InitBufferInfo(bufnr('')) + let g:ale_buffer_info[bufnr('')].active_linter_list = ale#linter#Get('eclipselsp') + call ale#lsp_linter#SetLSPLinterMap({'1': {'name': 'eclipselsp', 'aliases': [], 'lsp': 'stdio'}}) + call ale#lsp_linter#SetDiagnosticURIMap({'347': ale#util#ToURI(expand('%:p'))}) + + if has('win32') + AssertEqual 'filename,[]^$.ts', expand('%:p:t') + else + AssertEqual 'filename*?,{}[]^$.java', expand('%:p:t') + endif + + call ale#lsp_linter#HandleLSPResponse(1, { + \ 'jsonrpc':'2.0', + \ 'id': 347, + \ 'result': { + \ 'kind': 'full', + \ 'items': [ + \ { + \ 'range': { + \ 'start': { + \ 'line': 0, + \ 'character':0 + \ }, + \ 'end': { + \ 'line': 0, + \ 'character':0 + \ } + \ }, + \ 'severity': 2, + \ 'code': "", + \ 'source': 'Java', + \ 'message': 'Missing JRE 1-8' + \ } + \ ] + \ }, + \}) + + AssertEqual + \ [ + \ { + \ 'lnum': 1, + \ 'bufnr': bufnr(''), + \ 'col': 1, + \ 'pattern': '', + \ 'valid': 1, + \ 'vcol': 0, + \ 'nr': -1, + \ 'type': 'W', + \ 'text': 'Missing JRE 1-8' + \ } + \ ], + \ ale#test#GetLoclistWithoutNewerKeys() + AssertEqual [], g:ale_buffer_info[bufnr('')].active_linter_list + +Execute(LSP pull model diagnostic responses that are 'unchanged' should be handled): + let b:ale_linters = ['eclipselsp'] + runtime ale_linters/java/eclipselsp.vim + + if has('win32') + call ale#test#SetFilename('filename,[]^$.ts') + else + call ale#test#SetFilename('filename*?,{}[]^$.java') + endif + + call ale#engine#InitBufferInfo(bufnr('')) + let g:ale_buffer_info[bufnr('')].active_linter_list = ale#linter#Get('eclipselsp') + let g:ale_buffer_info[bufnr('')].loclist = [ + \ { + \ 'lnum': 1, + \ 'bufnr': bufnr(''), + \ 'col': 1, + \ 'pattern': '', + \ 'valid': 1, + \ 'vcol': 0, + \ 'nr': -1, + \ 'type': 'W', + \ 'text': 'Missing JRE 1-8' + \ }, + \] + + call ale#lsp_linter#SetLSPLinterMap({'1': {'name': 'eclipselsp', 'aliases': [], 'lsp': 'stdio'}}) + call ale#lsp_linter#SetDiagnosticURIMap({'347': ale#util#ToURI(expand('%:p'))}) + + if has('win32') + AssertEqual 'filename,[]^$.ts', expand('%:p:t') + else + AssertEqual 'filename*?,{}[]^$.java', expand('%:p:t') + endif + + call ale#lsp_linter#HandleLSPResponse(1, { + \ 'jsonrpc':'2.0', + \ 'id': 347, + \ 'result': { + \ 'kind': 'unchanged', + \ }, + \}) + + AssertEqual + \ [ + \ { + \ 'lnum': 1, + \ 'bufnr': bufnr(''), + \ 'col': 1, + \ 'pattern': '', + \ 'valid': 1, + \ 'vcol': 0, + \ 'nr': -1, + \ 'type': 'W', + \ 'text': 'Missing JRE 1-8' + \ } + \ ], + \ g:ale_buffer_info[bufnr('')].loclist + AssertEqual [], g:ale_buffer_info[bufnr('')].active_linter_list + Execute(LSP errors should be logged in the history): call ale#lsp_linter#SetLSPLinterMap({'347': {'name': 'foobar', 'aliases': [], 'lsp': 'stdio'}}) call ale#lsp_linter#HandleLSPResponse(347, { diff --git a/test/lsp/test_lsp_client_messages.vader b/test/lsp/test_lsp_client_messages.vader index 3af67393..8a71dd05 100644 --- a/test/lsp/test_lsp_client_messages.vader +++ b/test/lsp/test_lsp_client_messages.vader @@ -248,6 +248,19 @@ Execute(ale#lsp#message#DidChangeConfiguration() should return correct messages) \ ], \ ale#lsp#message#DidChangeConfiguration(bufnr(''), g:ale_lsp_configuration) +Execute(ale#lsp#message#Diagnostic() should return correct messages): + AssertEqual + \ [ + \ 0, + \ 'textDocument/diagnostic', + \ { + \ 'textDocument': { + \ 'uri': ale#path#ToFileURI(g:dir . '/foo/bar.ts'), + \ }, + \ } + \ ], + \ ale#lsp#message#Diagnostic(bufnr('')) + Execute(ale#lsp#tsserver_message#Open() should return correct messages): AssertEqual \ [ diff --git a/test/lsp/test_lsp_startup.vader b/test/lsp/test_lsp_startup.vader index f7407437..ef36028e 100644 --- a/test/lsp/test_lsp_startup.vader +++ b/test/lsp/test_lsp_startup.vader @@ -193,6 +193,10 @@ Before: \ 'dynamicRegistration': v:false, \ 'linkSupport': v:false, \ }, + \ 'diagnostic': { + \ 'dynamicRegistration': v:true, + \ 'relatedDocumentSupport': v:true, + \ }, \ 'publishDiagnostics': { \ 'relatedInformation': v:true, \ }, diff --git a/test/lsp/test_other_initialize_message_handling.vader b/test/lsp/test_other_initialize_message_handling.vader index b86aa1fa..4a6d89e0 100644 --- a/test/lsp/test_other_initialize_message_handling.vader +++ b/test/lsp/test_other_initialize_message_handling.vader @@ -90,6 +90,7 @@ Execute(Capabilities should be set up correctly): \ 'includeText': 0, \ 'references': 1, \ 'rename': 1, + \ 'pull_model': 0, \ 'symbol_search': 1, \ 'typeDefinition': 0, \ }, @@ -122,6 +123,7 @@ Execute(Disabled capabilities should be recognised correctly): \ 'definitionProvider': v:false, \ 'experimental': {}, \ 'documentHighlightProvider': v:true, + \ 'diagnosticProvider': {}, \ }, \ }, \}) @@ -140,6 +142,7 @@ Execute(Disabled capabilities should be recognised correctly): \ 'includeText': 0, \ 'references': 0, \ 'rename': 0, + \ 'pull_model': 0, \ 'symbol_search': 0, \ 'typeDefinition': 0, \ }, @@ -182,6 +185,9 @@ Execute(Capabilities should be enabled when sent as Dictionaries): \ 'implementationProvider': {}, \ 'experimental': {}, \ 'documentHighlightProvider': v:true, + \ 'diagnosticProvider': { + \ 'interFileDependencies': v:false, + \ }, \ 'workspaceSymbolProvider': {} \ }, \ }, @@ -201,6 +207,7 @@ Execute(Capabilities should be enabled when sent as Dictionaries): \ 'includeText': 1, \ 'references': 1, \ 'rename': 1, + \ 'pull_model': 1, \ 'symbol_search': 1, \ 'typeDefinition': 1, \ }, diff --git a/test/lsp/test_read_lsp_diagnostics.vader b/test/lsp/test_read_lsp_diagnostics.vader index 61ffc73f..80f2e4f0 100644 --- a/test/lsp/test_read_lsp_diagnostics.vader +++ b/test/lsp/test_read_lsp_diagnostics.vader @@ -21,14 +21,14 @@ Execute(ale#lsp#response#ReadDiagnostics() should handle errors): \ 'code': 'some-error', \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'severity': 1, \ 'range': Range(2, 10, 4, 15), \ 'code': 'some-error', \ 'message': 'Something went wrong!', \ }, - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should handle warnings): AssertEqual [ @@ -42,14 +42,14 @@ Execute(ale#lsp#response#ReadDiagnostics() should handle warnings): \ 'code': 'some-warning', \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'severity': 2, \ 'range': Range(1, 3, 1, 3), \ 'code': 'some-warning', \ 'message': 'Something went wrong!', \ }, - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should treat messages with missing severity as errors): AssertEqual [ @@ -63,13 +63,13 @@ Execute(ale#lsp#response#ReadDiagnostics() should treat messages with missing se \ 'code': 'some-error', \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'range': Range(2, 10, 4, 15), \ 'code': 'some-error', \ 'message': 'Something went wrong!', \ }, - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should handle messages without codes): AssertEqual [ @@ -82,12 +82,12 @@ Execute(ale#lsp#response#ReadDiagnostics() should handle messages without codes) \ 'end_col': 15, \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'range': Range(2, 10, 4, 15), \ 'message': 'Something went wrong!', \ }, - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should include sources in detail): AssertEqual [ @@ -101,13 +101,13 @@ Execute(ale#lsp#response#ReadDiagnostics() should include sources in detail): \ 'end_col': 22, \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'range': Range(9, 14, 11, 22), \ 'message': 'Something went wrong!', \ 'source': 'tslint', \ } - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should keep detail with line breaks but replace with spaces in text): AssertEqual [ @@ -121,13 +121,13 @@ Execute(ale#lsp#response#ReadDiagnostics() should keep detail with line breaks b \ 'end_col': 22, \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'range': Range(9, 14, 11, 22), \ 'message': "cannot borrow `cap` as mutable\r\nmore than once at a time\n\nmutable borrow starts here\rin previous iteration of loop", \ 'source': 'rustc', \ } - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should consider -1 to be a meaningless code): AssertEqual [ @@ -140,13 +140,13 @@ Execute(ale#lsp#response#ReadDiagnostics() should consider -1 to be a meaningles \ 'end_col': 15, \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'range': Range(2, 10, 4, 15), \ 'message': 'Something went wrong!', \ 'code': -1, \ }, - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should handle multiple messages): AssertEqual [ @@ -167,7 +167,7 @@ Execute(ale#lsp#response#ReadDiagnostics() should handle multiple messages): \ 'end_col': 4, \ }, \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'range': Range(0, 2, 0, 2), \ 'message': 'Something went wrong!', @@ -177,7 +177,7 @@ Execute(ale#lsp#response#ReadDiagnostics() should handle multiple messages): \ 'range': Range(1, 4, 1, 4), \ 'message': 'A warning', \ }, - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadDiagnostics() should use relatedInformation for detail): AssertEqual [ @@ -191,7 +191,7 @@ Execute(ale#lsp#response#ReadDiagnostics() should use relatedInformation for det \ 'detail': "Something went wrong!\n/tmp/someotherfile.txt:43:80:\n\tmight be this" \ } \ ], - \ ale#lsp#response#ReadDiagnostics({'params': {'uri': 'filename.ts', 'diagnostics': [ + \ ale#lsp#response#ReadDiagnostics([ \ { \ 'range': Range(0, 2, 0, 2), \ 'message': 'Something went wrong!', @@ -206,7 +206,7 @@ Execute(ale#lsp#response#ReadDiagnostics() should use relatedInformation for det \ } \ }] \ } - \ ]}}) + \ ]) Execute(ale#lsp#response#ReadTSServerDiagnostics() should handle tsserver responses): AssertEqual