From b41836130c9977317ba3f3ebc61daf05fc34f0da Mon Sep 17 00:00:00 2001 From: Horacio Sanson Date: Wed, 22 May 2019 10:30:24 +0900 Subject: [PATCH 1/5] Fix HandleLSPDiagnostics buffer match logic. To find the buffer corresponding to URIs reported by LSP the HandleLSPDiagnostics() method uses the built-in bufnr() function. From the documentation we learn that the first parameter of bufnr() is an expression, not a path. EclipseLSP will report project wide errors (e.g. gradle errors) that are not related to any actual source file with an URI that corresponds to the project root folder, e.g: file:///home/username/Projects/gradle-simple This URI will match any open buffer of files within the project root hiearchy, thus project-wide errors appear as part of every file within the project, e.g: file:///home/username/Projects/gradle-simple/src/main/java/Hello.java To fix this, this MR adds '^' to the beginning and '$' at the end of the URI path to force an exact match. This is how is recommended in vim help (see :h bufname). --- autoload/ale/lsp_linter.vim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autoload/ale/lsp_linter.vim b/autoload/ale/lsp_linter.vim index f70042dd..13471ab7 100644 --- a/autoload/ale/lsp_linter.vim +++ b/autoload/ale/lsp_linter.vim @@ -31,7 +31,7 @@ endfunction function! s:HandleLSPDiagnostics(conn_id, response) abort let l:linter_name = s:lsp_linter_map[a:conn_id] let l:filename = ale#path#FromURI(a:response.params.uri) - let l:buffer = bufnr(l:filename) + let l:buffer = bufnr('^' . l:filename . '$') let l:info = get(g:ale_buffer_info, l:buffer, {}) if empty(l:info) From 2f13c2d26390703c591d756f49cd1207b98f3689 Mon Sep 17 00:00:00 2001 From: Horacio Sanson Date: Wed, 22 May 2019 20:19:45 +0900 Subject: [PATCH 2/5] Add fix to HandleTSServerDiagnostics function. --- autoload/ale/lsp_linter.vim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autoload/ale/lsp_linter.vim b/autoload/ale/lsp_linter.vim index 13471ab7..4f439b28 100644 --- a/autoload/ale/lsp_linter.vim +++ b/autoload/ale/lsp_linter.vim @@ -49,7 +49,7 @@ endfunction function! s:HandleTSServerDiagnostics(response, error_type) abort let l:linter_name = 'tsserver' - let l:buffer = bufnr(a:response.body.file) + let l:buffer = bufnr('^' . a:response.body.file . '$') let l:info = get(g:ale_buffer_info, l:buffer, {}) if empty(l:info) From 85b3a4a5c68f9bfd81d09bcd96ce6a26a521dc3d Mon Sep 17 00:00:00 2001 From: Horacio Sanson Date: Thu, 23 May 2019 10:19:07 +0900 Subject: [PATCH 3/5] Add exact file match test to TSServer response handler --- test/test_engine_lsp_response_handling.vader | 58 ++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/test_engine_lsp_response_handling.vader b/test/test_engine_lsp_response_handling.vader index 84febe39..aeb13a86 100644 --- a/test/test_engine_lsp_response_handling.vader +++ b/test/test_engine_lsp_response_handling.vader @@ -92,6 +92,35 @@ Execute(tsserver syntax error responses should be handled correctly): \ ], \ ale#test#GetLoclistWithoutModule() + " Syntax errors on the project root should not populate the LocList. + call ale#lsp_linter#HandleLSPResponse(1, { + \ 'seq': 0, + \ 'type': 'event', + \ 'event': 'syntaxDiag', + \ 'body': { + \ 'file': g:dir, + \ 'diagnostics':[ + \ { + \ 'start': { + \ 'line':2, + \ 'offset':14, + \ }, + \ 'end': { + \ 'line':2, + \ 'offset':15, + \ }, + \ 'text': ''','' expected.', + \ "code":1005 + \ }, + \ ], + \ }, + \}) + + AssertEqual + \ [ + \ ], + \ ale#test#GetLoclistWithoutModule() + Execute(tsserver semantic error responses should be handled correctly): runtime ale_linters/typescript/tsserver.vim call ale#test#SetFilename('filename.ts') @@ -165,6 +194,35 @@ Execute(tsserver semantic error responses should be handled correctly): \ ], \ ale#test#GetLoclistWithoutModule() + " Semantic errors on the project root should not populate the LocList. + call ale#lsp_linter#HandleLSPResponse(1, { + \ 'seq': 0, + \ 'type': 'event', + \ 'event': 'semanticDiag', + \ 'body': { + \ 'file': g:dir, + \ 'diagnostics':[ + \ { + \ 'start': { + \ 'line':2, + \ 'offset':14, + \ }, + \ 'end': { + \ 'line':2, + \ 'offset':15, + \ }, + \ 'text': 'Some semantic error', + \ "code":1005 + \ }, + \ ], + \ }, + \}) + + AssertEqual + \ [ + \ ], + \ ale#test#GetLoclistWithoutModule() + Execute(tsserver errors should mark tsserver no longer active): let b:ale_linters = ['tsserver'] runtime ale_linters/typescript/tsserver.vim From 33b4a905079c0d8ecafc265c0a31b3141b034b87 Mon Sep 17 00:00:00 2001 From: Horacio Sanson Date: Thu, 23 May 2019 10:19:50 +0900 Subject: [PATCH 4/5] Add tests for LSP responses --- test/test_engine_lsp_response_handling.vader | 87 ++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/test/test_engine_lsp_response_handling.vader b/test/test_engine_lsp_response_handling.vader index aeb13a86..34b0de3b 100644 --- a/test/test_engine_lsp_response_handling.vader +++ b/test/test_engine_lsp_response_handling.vader @@ -244,6 +244,93 @@ Execute(tsserver errors should mark tsserver no longer active): AssertEqual [], g:ale_buffer_info[bufnr('')].active_linter_list +Execute(LSP diagnostics responses should be handled correctly): + let b:ale_linters = ['eclipselsp'] + runtime ale_linters/java/eclipselsp.vim + call ale#test#SetFilename('filename.java') + call ale#engine#InitBufferInfo(bufnr('')) + call ale#lsp_linter#SetLSPLinterMap({'1': 'eclipselsp'}) + + call ale#lsp_linter#HandleLSPResponse(1, { + \ 'jsonrpc':'2.0', + \ 'method':'textDocument/publishDiagnostics', + \ 'params': { + \ 'uri':'file://' . g:dir . '/filename.java', + \ 'diagnostics': [ + \ { + \ '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': 3, + \ 'col': 1, + \ 'pattern': '', + \ 'valid': 1, + \ 'vcol': 0, + \ 'nr': -1, + \ 'type': 'W', + \ 'text': 'Missing JRE 1-8' + \ } + \ ], + \ ale#test#GetLoclistWithoutModule() + +Execute(LSP diagnostics responses on project root should not populate loclist): + let b:ale_linters = ['eclipselsp'] + runtime ale_linters/java/eclipselsp.vim + call ale#test#SetFilename('filename.java') + call ale#engine#InitBufferInfo(bufnr('')) + call ale#lsp_linter#SetLSPLinterMap({'1': 'eclipselsp'}) + + call ale#lsp_linter#HandleLSPResponse(1, { + \ 'jsonrpc':'2.0', + \ 'method':'textDocument/publishDiagnostics', + \ 'params': { + \ 'uri':'file://' . g:dir, + \ 'diagnostics': [ + \ { + \ 'range': { + \ 'start': { + \ 'line': 0, + \ 'character':0 + \ }, + \ 'end': { + \ 'line': 0, + \ 'character':0 + \ } + \ }, + \ 'severity': 2, + \ 'code': "", + \ 'source': 'Java', + \ 'message': 'Missing JRE 1-8' + \ } + \ ] + \ } + \}) + + AssertEqual + \ [ + \ ], + \ ale#test#GetLoclistWithoutModule() + Execute(LSP errors should mark linters no longer active): let b:ale_linters = ['pyls'] runtime ale_linters/python/pyls.vim From 36c35d840ba90c5069eb514d54a1281d42c3fb26 Mon Sep 17 00:00:00 2001 From: w0rp Date: Fri, 24 May 2019 01:13:52 +0100 Subject: [PATCH 5/5] Fix LSP tests --- test/lsp/test_did_save_event.vader | 1 + .../test_engine_lsp_response_handling.vader | 25 ++++++++++++++++++- test/test_history_saving.vader | 3 +-- test/test_ignoring_linters.vader | 12 ++++++--- 4 files changed, 35 insertions(+), 6 deletions(-) rename test/{ => lsp}/test_engine_lsp_response_handling.vader (92%) diff --git a/test/lsp/test_did_save_event.vader b/test/lsp/test_did_save_event.vader index 1a60bc9f..bdea6d98 100644 --- a/test/lsp/test_did_save_event.vader +++ b/test/lsp/test_did_save_event.vader @@ -9,6 +9,7 @@ Before: call ale#test#SetFilename('dummy.txt') runtime autoload/ale/lsp.vim + runtime autoload/ale/lsp_linter.vim let g:ale_disable_lsp = 0 unlet! b:ale_disable_lsp diff --git a/test/test_engine_lsp_response_handling.vader b/test/lsp/test_engine_lsp_response_handling.vader similarity index 92% rename from test/test_engine_lsp_response_handling.vader rename to test/lsp/test_engine_lsp_response_handling.vader index 34b0de3b..9abfa087 100644 --- a/test/test_engine_lsp_response_handling.vader +++ b/test/lsp/test_engine_lsp_response_handling.vader @@ -1,19 +1,42 @@ Before: + Save g:ale_set_lists_synchronously Save g:ale_buffer_info Save g:ale_lsp_error_messages + Save g:ale_set_loclist + Save g:ale_set_signs + Save g:ale_set_quickfix + Save g:ale_set_highlights + Save g:ale_echo_cursor + Save g:ale_disable_lsp + Save g:ale_history_enabled + Save g:ale_history_log_output + let g:ale_disable_lsp = 0 + let g:ale_set_lists_synchronously = 1 let g:ale_buffer_info = {} + let g:ale_set_loclist = 1 + " Disable features we don't need for these tests. + let g:ale_set_signs = 0 + let g:ale_set_quickfix = 0 + let g:ale_set_highlights = 0 + let g:ale_echo_cursor = 0 + let g:ale_history_enabled = 1 + let g:ale_history_log_output = 1 unlet! g:ale_lsp_error_messages unlet! b:ale_linters + unlet! b:ale_disable_lsp + call ale#linter#Reset() call ale#test#SetDirectory('/testplugin/test') + call setloclist(0, []) After: Restore unlet! b:ale_linters + call setloclist(0, []) call ale#test#RestoreDirectory() call ale#linter#Reset() call ale#lsp_linter#ClearLSPData() @@ -281,7 +304,7 @@ Execute(LSP diagnostics responses should be handled correctly): \ [ \ { \ 'lnum': 1, - \ 'bufnr': 3, + \ 'bufnr': bufnr(''), \ 'col': 1, \ 'pattern': '', \ 'valid': 1, diff --git a/test/test_history_saving.vader b/test/test_history_saving.vader index 18b64db5..5d81c2a3 100644 --- a/test/test_history_saving.vader +++ b/test/test_history_saving.vader @@ -1,5 +1,6 @@ Before: Save g:ale_max_buffer_history_size + Save g:ale_history_enabled Save g:ale_history_log_output Save g:ale_run_synchronously Save g:ale_enabled @@ -54,8 +55,6 @@ After: " Reset the shell back to what it was before. let &shell = g:current_shell unlet g:current_shell - let g:ale_history_enabled = 1 - let g:ale_history_log_output = 0 unlet g:history call ale#engine#Cleanup(bufnr('')) diff --git a/test/test_ignoring_linters.vader b/test/test_ignoring_linters.vader index f2e9e5c9..d758af5c 100644 --- a/test/test_ignoring_linters.vader +++ b/test/test_ignoring_linters.vader @@ -1,3 +1,11 @@ +Before: + Save g:ale_disable_lsp + +After: + Restore + + unlet! b:ale_disable_lsp + Execute(GetList should ignore some invalid values): AssertEqual [], ale#engine#ignore#GetList('', 'foo') AssertEqual [], ale#engine#ignore#GetList('', 0) @@ -98,7 +106,6 @@ Execute(Exclude should handle Dictionaries): \ ) Execute(Exclude should filter LSP linters when g:ale_disable_lsp is set to 1): - let g:ale_disable_lsp = 1 AssertEqual \ [ \ {'name': 'linter1', 'aliases': [], 'lsp': ''}, @@ -116,7 +123,6 @@ Execute(Exclude should filter LSP linters when g:ale_disable_lsp is set to 1): \ ) Execute(Exclude should filter LSP linters when b:ale_disable_lsp is set to 1): - let b:ale_disable_lsp = 1 AssertEqual \ [ \ {'name': 'linter1', 'aliases': [], 'lsp': ''}, @@ -351,7 +357,7 @@ Execute(ale_disable_lsp should be applied for tsserver): AssertEqual [], g:loclist -Execute(ale_disable_lsp should be applied for LSP linters): +Execute(ale_disable_lsp should be applied for LSP linters): call ale#test#SetFilename('filename.py') call ale#engine#InitBufferInfo(bufnr('')) call ale#lsp_linter#SetLSPLinterMap({'347': 'lsplinter'})