Close #1559 - Report errors from LSP servers in :ALEInfo

6 files changed, 237 insertions, 7 deletions

diff --git a/autoload/ale/debugging.vim b/autoload/ale/debugging.vim
index bec79a85..f32e4308 100644
--- a/autoload/ale/debugging.vim
+++ b/autoload/ale/debugging.vim
@@ -168,6 +168,30 @@ function! s:EchoLinterAliases(all_linters) abort
     endfor
 endfunction
 
+function! s:EchoLSPErrorMessages(all_linter_names) abort
+    let l:lsp_error_messages = get(g:, 'ale_lsp_error_messages', {})
+    let l:header_echoed = 0
+
+    for l:linter_name in a:all_linter_names
+        let l:error_list = get(l:lsp_error_messages, l:linter_name, [])
+
+        if !empty(l:error_list)
+            if !l:header_echoed
+                call s:Echo(' LSP Error Messages:')
+                call s:Echo('')
+            endif
+
+            call s:Echo('(Errors for ' . l:linter_name . ')')
+
+            for l:message in l:error_list
+                for l:line in split(l:message, "\n")
+                    call s:Echo(l:line)
+                endfor
+            endfor
+        endif
+    endfor
+endfunction
+
 function! ale#debugging#Info() abort
     let l:filetype = &filetype
 
@@ -200,6 +224,7 @@ function! ale#debugging#Info() abort
     call s:Echo(' Global Variables:')
     call s:Echo('')
     call s:EchoGlobalVariables()
+    call s:EchoLSPErrorMessages(l:all_names)
     call s:Echo('  Command History:')
     call s:Echo('')
     call s:EchoCommandHistory()
diff --git a/autoload/ale/engine.vim b/autoload/ale/engine.vim
index e1c41552..5cd12187 100644
--- a/autoload/ale/engine.vim
+++ b/autoload/ale/engine.vim
@@ -81,6 +81,11 @@ function! ale#engine#ClearLSPData() abort
     let s:lsp_linter_map = {}
 endfunction
 
+" Just for tests.
+function! ale#engine#SetLSPLinterMap(replacement_map) abort
+    let s:lsp_linter_map = a:replacement_map
+endfunction
+
 " This function is documented and part of the public API.
 "
 " Return 1 if ALE is busy checking a given buffer
@@ -270,20 +275,38 @@ function! s:HandleTSServerDiagnostics(response, error_type) abort
     call ale#engine#HandleLoclist('tsserver', l:buffer, l:loclist)
 endfunction
 
-function! s:HandleLSPErrorMessage(error_message) abort
-    execute 'echoerr ''Error from LSP:'''
+function! s:HandleLSPErrorMessage(linter_name, response) abort
+    if !g:ale_history_enabled || !g:ale_history_log_output
+        return
+    endif
+
+    if empty(a:linter_name)
+        return
+    endif
 
-    for l:line in split(a:error_message, "\n")
-        execute 'echoerr l:line'
-    endfor
+    let l:message = ale#lsp#response#GetErrorMessage(a:response)
+
+    if empty(l:message)
+        return
+    endif
+
+    " This global variable is set here so we don't load the debugging.vim file
+    " until someone uses :ALEInfo.
+    let g:ale_lsp_error_messages = get(g:, 'ale_lsp_error_messages', {})
+
+    if !has_key(g:ale_lsp_error_messages, a:linter_name)
+        let g:ale_lsp_error_messages[a:linter_name] = []
+    endif
+
+    call add(g:ale_lsp_error_messages[a:linter_name], l:message)
 endfunction
 
 function! ale#engine#HandleLSPResponse(conn_id, response) abort
     let l:method = get(a:response, 'method', '')
+    let l:linter_name = get(s:lsp_linter_map, a:conn_id, '')
 
     if get(a:response, 'jsonrpc', '') is# '2.0' && has_key(a:response, 'error')
-        " Uncomment this line to print LSP error messages.
-        " call s:HandleLSPErrorMessage(a:response.error.message)
+        call s:HandleLSPErrorMessage(l:linter_name, a:response)
     elseif l:method is# 'textDocument/publishDiagnostics'
         call s:HandleLSPDiagnostics(a:conn_id, a:response)
     elseif get(a:response, 'type', '') is# 'event'
diff --git a/autoload/ale/lsp/response.vim b/autoload/ale/lsp/response.vim
index 5a431287..94794e98 100644
--- a/autoload/ale/lsp/response.vim
+++ b/autoload/ale/lsp/response.vim
@@ -1,6 +1,20 @@
 " Author: w0rp <devw0rp@gmail.com>
 " Description: Parsing and transforming of LSP server responses.
 
+" Constants for error codes.
+" Defined by JSON RPC
+let s:PARSE_ERROR = -32700
+let s:INVALID_REQUEST = -32600
+let s:METHOD_NOT_FOUND = -32601
+let s:INVALID_PARAMS = -32602
+let s:INTERNAL_ERROR = -32603
+let s:SERVER_ERROR_START = -32099
+let s:SERVER_ERROR_END = -32000
+let s:SERVER_NOT_INITIALIZED = -32002
+let s:UNKNOWN_ERROR_CODE = -32001
+" Defined by the protocol.
+let s:REQUEST_CANCELLED = -32800
+
 " Constants for message severity codes.
 let s:SEVERITY_ERROR = 1
 let s:SEVERITY_WARNING = 2
@@ -72,3 +86,31 @@ function! ale#lsp#response#ReadTSServerDiagnostics(response) abort
 
     return l:loclist
 endfunction
+
+function! ale#lsp#response#GetErrorMessage(response) abort
+    if type(get(a:response, 'error', 0)) isnot type({})
+        return ''
+    endif
+
+    let l:code = get(a:response.error, 'code')
+
+    " Only report things for these error codes.
+    if l:code isnot s:INVALID_PARAMS && l:code isnot s:INTERNAL_ERROR
+        return ''
+    endif
+
+    let l:message = get(a:response.error, 'message', '')
+
+    if empty(l:message)
+        return ''
+    endif
+
+    " Include the traceback as details, if it's there.
+    let l:traceback = get(get(a:response.error, 'data', {}), 'traceback', [])
+
+    if type(l:traceback) is type([]) && !empty(l:traceback)
+        let l:message .= "\n" . join(l:traceback, "\n")
+    endif
+
+    return l:message
+endfunction
diff --git a/test/lsp/test_lsp_error_parsing.vader b/test/lsp/test_lsp_error_parsing.vader
new file mode 100644
index 00000000..7464b0e7
--- /dev/null
+++ b/test/lsp/test_lsp_error_parsing.vader
@@ -0,0 +1,65 @@
+Execute(Invalid responses should be handled):
+  AssertEqual '', ale#lsp#response#GetErrorMessage({})
+  AssertEqual '', ale#lsp#response#GetErrorMessage({'error': 0})
+  AssertEqual '', ale#lsp#response#GetErrorMessage({'error': {}})
+  AssertEqual '', ale#lsp#response#GetErrorMessage({
+  \ 'error': {
+  \   'code': 0,
+  \   'message': 'x',
+  \ },
+  \})
+  AssertEqual '', ale#lsp#response#GetErrorMessage({'error': {'code': -32602}})
+  AssertEqual '', ale#lsp#response#GetErrorMessage({'error': {'code': -32603}})
+
+Execute(Messages without tracebacks should be handled):
+  AssertEqual 'xyz', ale#lsp#response#GetErrorMessage({
+  \ 'error': {
+  \   'code': -32602,
+  \   'message': 'xyz',
+  \ },
+  \})
+  AssertEqual 'abc', ale#lsp#response#GetErrorMessage({
+  \ 'error': {
+  \   'code': -32603,
+  \   'message': 'abc',
+  \ },
+  \})
+
+Execute(Invalid traceback data should be tolerated):
+  AssertEqual 'xyz', ale#lsp#response#GetErrorMessage({
+  \ 'error': {
+  \   'code': -32602,
+  \   'message': 'xyz',
+  \   'data': {
+  \   },
+  \ },
+  \})
+  AssertEqual 'xyz', ale#lsp#response#GetErrorMessage({
+  \ 'error': {
+  \   'code': -32602,
+  \   'message': 'xyz',
+  \   'data': {
+  \     'traceback': 0,
+  \   },
+  \ },
+  \})
+  AssertEqual 'xyz', ale#lsp#response#GetErrorMessage({
+  \ 'error': {
+  \   'code': -32602,
+  \   'message': 'xyz',
+  \   'data': {
+  \     'traceback': [],
+  \   },
+  \ },
+  \})
+
+Execute(Messages with tracebacks should be handled):
+  AssertEqual "xyz\n123\n456", ale#lsp#response#GetErrorMessage({
+  \ 'error': {
+  \   'code': -32602,
+  \   'message': 'xyz',
+  \   'data': {
+  \     'traceback': ['123', '456'],
+  \   },
+  \ },
+  \})
diff --git a/test/test_ale_info.vader b/test/test_ale_info.vader
index c1ae5a7d..16c04b76 100644
--- a/test/test_ale_info.vader
+++ b/test/test_ale_info.vader
@@ -12,6 +12,12 @@ Before:
   Save g:ale_pattern_options_enabled
   Save g:ale_set_balloons
   Save g:ale_warn_about_trailing_whitespace
+  Save g:ale_sign_error
+  Save g:ale_sign_warning
+  Save g:ale_sign_info
+  Save g:ale_sign_style_error
+  Save g:ale_sign_style_warning
+  Save g:ale_lsp_error_messages
 
   unlet! b:ale_history
 
@@ -26,6 +32,12 @@ Before:
   let g:ale_pattern_options_enabled = 0
   let g:ale_set_balloons = 0
   let g:ale_warn_about_trailing_whitespace = 1
+  let g:ale_sign_error = '>>'
+  let g:ale_sign_warning = '--'
+  let g:ale_sign_info = '--'
+  let g:ale_sign_style_error = '>>'
+  let g:ale_sign_style_warning = '--'
+  let g:ale_lsp_error_messages = {}
 
   let g:testlinter1 = {'name': 'testlinter1', 'executable': 'testlinter1', 'command': 'testlinter1', 'callback': 'testCB1', 'output_stream': 'stdout'}
   let g:testlinter2 = {'name': 'testlinter2', 'executable': 'testlinter2', 'command': 'testlinter2', 'callback': 'testCB2', 'output_stream': 'stdout'}
@@ -469,3 +481,44 @@ Execute (The option for caching failing executable checks should work):
   \ '(executable check - success) ' . (has('win32') ? 'cmd' : 'echo'),
   \ '(executable check - failure) TheresNoWayThisIsExecutable',
   \])
+
+Given testft (Empty buffer):
+Execute (LSP errors for a linter should be outputted):
+  let g:ale_lsp_error_messages = {'testlinter1': ['foo', 'bar']}
+  call ale#linter#Define('testft', g:testlinter1)
+
+  call CheckInfo(
+  \ [
+  \   ' Current Filetype: testft',
+  \   'Available Linters: [''testlinter1'']',
+  \   '  Enabled Linters: [''testlinter1'']',
+  \   ' Linter Variables:',
+  \   '',
+  \ ]
+  \ + g:globals_lines
+  \ + [
+  \   ' LSP Error Messages:',
+  \   '',
+  \   '(Errors for testlinter1)',
+  \   'foo',
+  \   'bar',
+  \ ]
+  \ + g:command_header
+  \)
+
+Given testft (Empty buffer):
+Execute (LSP errors for other linters shouldn't appear):
+  let g:ale_lsp_error_messages = {'testlinter2': ['foo']}
+  call ale#linter#Define('testft', g:testlinter1)
+
+  call CheckInfo(
+  \ [
+  \   ' Current Filetype: testft',
+  \   'Available Linters: [''testlinter1'']',
+  \   '  Enabled Linters: [''testlinter1'']',
+  \   ' Linter Variables:',
+  \   '',
+  \ ]
+  \ + g:globals_lines
+  \ + g:command_header
+  \)
diff --git a/test/test_engine_lsp_response_handling.vader b/test/test_engine_lsp_response_handling.vader
index b3a45b14..4c6fe637 100644
--- a/test/test_engine_lsp_response_handling.vader
+++ b/test/test_engine_lsp_response_handling.vader
@@ -1,5 +1,9 @@
 Before:
   Save g:ale_buffer_info
+  Save g:ale_lsp_error_messages
+
+  unlet! g:ale_lsp_error_messages
+
   call ale#test#SetDirectory('/testplugin/test')
 
 After:
@@ -7,6 +11,7 @@ After:
 
   call ale#test#RestoreDirectory()
   call ale#linter#Reset()
+  call ale#engine#ClearLSPData()
 
 Execute(tsserver syntax error responses should be handled correctly):
   runtime ale_linters/typescript/tsserver.vim
@@ -153,3 +158,20 @@ Execute(tsserver semantic error responses should be handled correctly):
   \ [
   \ ],
   \ getloclist(0)
+
+Execute(LSP errors should be logged in the history):
+  call ale#engine#SetLSPLinterMap({'347': 'foobar'})
+  call ale#engine#HandleLSPResponse(347, {
+  \ 'jsonrpc': '2.0',
+  \ 'error': {
+  \   'code': -32602,
+  \   'message': 'xyz',
+  \   'data': {
+  \     'traceback': ['123', '456'],
+  \   },
+  \ },
+  \})
+
+  AssertEqual
+  \ {'foobar': ["xyz\n123\n456"]},
+  \ get(g:, 'ale_lsp_error_messages', {})