Fix #2295 - Respond to initialize with an initialized message

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',
     \}