Dispatch textDocument/didChange after rename (2) (#4049)

* Dispatch textDocument/didChange after rename

Previously whenever we renamed a symbol that was referenced from other
files we'd just edit those files in the background, and the LSP wouldn't
know about these changes. If we tried to rename the same symbol again,
the renaming would fail. In some scenarios, the operation would just be
wrong. Here is an attempt to fix this issue.

I also noticed another bug when using Go with `gopls` LSP and the `gofmt`
fixer. Whenever the file was saved, the `gofmt` would run and reformat
the file. But it seems there was some kind of a race condition so I
disabled saving for now, and all of the modified files will be unsaved,
so the user should call `:wa` to save them. I personally like this even
better because I can inspect exactly what changes happened, and I
instantly see them in the other opened buffers, which was previously not
the case.

Fixes #3343, #3642, #3781.

* Address PR comments

* Remove mode tests in corner case tests

* Address PR comments

* Save after ALERename and ALEOrganizeImports

Also provide options to disable automatic saving, as well as instructions to
enable `set hidden` before doing that.

* Fix broken test

* Save only when !&hidden

* Update doc

* Update doc

* Add silent

1 files changed, 216 insertions, 19 deletions

diff --git a/test/test_code_action.vader b/test/test_code_action.vader
index c613222c..80e2b1d8 100644
--- a/test/test_code_action.vader
+++ b/test/test_code_action.vader
@@ -1,4 +1,15 @@
 Before:
+  let g:notified_changes = []
+
+  runtime autoload/ale/lsp.vim
+
+  function! ale#lsp#NotifyForChanges(conn_id, buffer) abort
+    call add(g:notified_changes, {
+    \ 'conn_id': a:conn_id,
+    \ 'buffer': a:buffer
+    \})
+  endfunction
+
   Save g:ale_enabled
   let g:ale_enabled = 0
 
@@ -36,10 +47,10 @@ Before:
 
 After:
   " Close the extra buffers if we opened it.
-  if bufnr(g:file1) != -1
+  if bufnr(g:file1) != -1 && buflisted(bufnr(g:file1))
     execute ':bp! | :bd! ' . bufnr(g:file1)
   endif
-  if bufnr(g:file2) != -1
+  if bufnr(g:file2) != -1 && buflisted(bufnr(g:file2))
     execute ':bp! | :bd! ' . bufnr(g:file2)
   endif
 
@@ -50,12 +61,16 @@ After:
     call delete(g:file2)
   endif
 
+  unlet! g:notified_changes
+  " unlet! g:expected_notified_changes
   unlet! g:file1
   unlet! g:file2
   unlet! g:test
   unlet! g:changes
   delfunction WriteFileAndEdit
 
+  runtime autoload/ale/lsp.vim
+
   Restore
 
 
@@ -118,7 +133,7 @@ Execute(It should modify and save multiple files):
   \     }]
   \   }],
   \ },
-  \ {'should_save': 1},
+  \ {'should_save': 1, 'conn_id': 'test_conn'},
   \)
 
   AssertEqual [
@@ -140,6 +155,13 @@ Execute(It should modify and save multiple files):
   \ '',
   \], readfile(g:file2, 'b')
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(g:file1),
+  \}, {
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(g:file2),
+  \}], g:notified_changes
 
 Execute(Beginning of file can be modified):
   let g:test.text = [
@@ -166,7 +188,7 @@ Execute(Beginning of file can be modified):
   \      }],
   \   }]
   \ },
-  \ {'should_save': 1},
+  \ {'should_save': 1, 'conn_id': 'test_conn'},
   \)
 
   AssertEqual [
@@ -174,6 +196,11 @@ Execute(Beginning of file can be modified):
   \  'type B: number',
   \] + g:test.text + [''], readfile(g:file1, 'b')
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(g:file1),
+  \}], g:notified_changes
+
 
 Execute(End of file can be modified):
   let g:test.text = [
@@ -200,7 +227,7 @@ Execute(End of file can be modified):
   \     }],
   \   }]
   \ },
-  \ {'should_save': 1},
+  \ {'should_save': 1, 'conn_id': 'test_conn'},
   \)
 
   AssertEqual g:test.text + [
@@ -209,6 +236,11 @@ Execute(End of file can be modified):
   \  '',
   \], readfile(g:file1, 'b')
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(g:file1),
+  \}], g:notified_changes
+
 
 Execute(Current buffer contents will be reloaded):
   let g:test.text = [
@@ -238,7 +270,7 @@ Execute(Current buffer contents will be reloaded):
   \      }],
   \   }]
   \ },
-  \ {'should_save': 1},
+  \ {'should_save': 1, 'conn_id': 'test_conn'},
   \)
 
   AssertEqual [
@@ -251,6 +283,55 @@ Execute(Current buffer contents will be reloaded):
   \  'type B: number',
   \] + g:test.text, getbufline(g:test.buffer, 1, '$')
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(g:file1),
+  \}], g:notified_changes
+
+
+Execute(Unlisted buffer contents will be modified correctly):
+  let g:test.text = [
+  \ 'class Name {',
+  \ '  value: string',
+  \ '}',
+  \]
+  call writefile(g:test.text, g:file1, 'S')
+
+  execute 'edit ' . g:file1
+  let g:test.buffer = bufnr(g:file1)
+
+  execute 'bd'
+  AssertEqual bufnr(g:file1), g:test.buffer
+
+  call ale#code_action#HandleCodeAction(
+  \ {
+  \   'changes': [{
+  \      'fileName': g:file1,
+  \      'textChanges': [{
+  \        'start': {
+  \          'line': 1,
+  \          'offset': 1,
+  \        },
+  \        'end': {
+  \          'line': 1,
+  \          'offset': 1,
+  \        },
+  \        'newText': "type A: string\ntype B: number\n",
+  \      }],
+  \   }]
+  \ },
+  \ {'should_save': 1, 'conn_id': 'test_conn'},
+  \)
+
+  AssertEqual [
+  \  'type A: string',
+  \  'type B: number',
+  \] + g:test.text + [''], readfile(g:file1, 'b')
+
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(g:file1),
+  \}], g:notified_changes
 
 # Tests for cursor repositioning. In comments `=` designates change range, and
 # `C` cursor position
@@ -261,13 +342,32 @@ Execute(Cursor will not move when it is before text change):
   let g:test.changes = g:test.create_change(2, 3, 2, 8, 'value2')
 
   call setpos('.', [0, 1, 1, 0])
-  call ale#code_action#HandleCodeAction(g:test.changes, {'should_save': 1})
+  call ale#code_action#HandleCodeAction(g:test.changes, {
+  \ 'should_save': 1,
+  \ 'conn_id': 'test_conn',
+  \})
   AssertEqual [1, 1], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
   call setpos('.', [0, 2, 2, 0])
-  call ale#code_action#HandleCodeAction(g:test.changes, {'should_save': 1})
+  call ale#code_action#HandleCodeAction(g:test.changes, {
+  \ 'should_save': 1,
+  \ 'conn_id': 'test_conn',
+  \})
   AssertEqual [2, 2], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}, {
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 # ====C====
 Execute(Cursor column will move to the change end when cursor between start/end):
   let g:test.changes = g:test.create_change(2, 3, 2, 8, 'value2')
@@ -276,11 +376,34 @@ Execute(Cursor column will move to the change end when cursor between start/end)
     call WriteFileAndEdit()
     call setpos('.', [0, 2, r, 0])
     AssertEqual '  value: string', getline('.')
-    call ale#code_action#HandleCodeAction(g:test.changes, {'should_save': 1})
+    call ale#code_action#HandleCodeAction(g:test.changes, {
+    \ 'should_save': 1,
+    \ 'conn_id': 'test_conn',
+    \})
     AssertEqual '  value2: string', getline('.')
     AssertEqual [2, 9], getpos('.')[1:2]
   endfor
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}, {
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}, {
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}, {
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}, {
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}, {
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 
 # ====C
 Execute(Cursor column will move back when new text is shorter):
@@ -289,11 +412,18 @@ Execute(Cursor column will move back when new text is shorter):
   AssertEqual '  value: string', getline('.')
   call ale#code_action#HandleCodeAction(
   \ g:test.create_change(2, 3, 2, 8, 'val'),
-  \ {'should_save': 1},
-  \)
+  \ {
+  \   'should_save': 1,
+  \   'conn_id': 'test_conn',
+  \ })
   AssertEqual '  val: string', getline('.')
   AssertEqual [2, 6], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 
 # ==== C
 Execute(Cursor column will move forward when new text is longer):
@@ -302,10 +432,19 @@ Execute(Cursor column will move forward when new text is longer):
   call setpos('.', [0, 2, 8, 0])
   AssertEqual '  value: string', getline('.')
   call ale#code_action#HandleCodeAction(
-  \ g:test.create_change(2, 3, 2, 8, 'longValue'), {'should_save': 1})
+  \ g:test.create_change(2, 3, 2, 8, 'longValue'),
+  \ {
+  \   'should_save': 1,
+  \   'conn_id': 'test_conn',
+  \ })
   AssertEqual '  longValue: string', getline('.')
   AssertEqual [2, 12], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 # =========
 # =
 # C
@@ -314,10 +453,19 @@ Execute(Cursor line will move when updates are happening on lines above):
   call setpos('.', [0, 3, 1, 0])
   AssertEqual '}', getline('.')
   call ale#code_action#HandleCodeAction(
-  \ g:test.create_change(1, 1, 2, 1, "test\ntest\n"), {'should_save': 1})
+  \ g:test.create_change(1, 1, 2, 1, "test\ntest\n"),
+  \ {
+  \   'should_save': 1,
+  \   'conn_id': 'test_conn',
+  \ })
   AssertEqual '}', getline('.')
   AssertEqual [4, 1], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 
 # =========
 # =C
@@ -326,10 +474,19 @@ Execute(Cursor line and column will move when change on lines above and just bef
   call setpos('.', [0, 2, 2, 0])
   AssertEqual '  value: string', getline('.')
   call ale#code_action#HandleCodeAction(
-  \ g:test.create_change(1, 1, 2, 1, "test\ntest\n123"), {'should_save': 1})
+  \ g:test.create_change(1, 1, 2, 1, "test\ntest\n123"),
+  \ {
+  \   'should_save': 1,
+  \   'conn_id': 'test_conn',
+  \ })
   AssertEqual '123  value: string', getline('.')
   AssertEqual [3, 5], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 # =========
 # ======C==
 # =
@@ -338,10 +495,19 @@ Execute(Cursor line and column will move at the end of changes):
   call setpos('.', [0, 2, 10, 0])
   AssertEqual '  value: string', getline('.')
   call ale#code_action#HandleCodeAction(
-  \ g:test.create_change(1, 1, 3, 1, "test\n"), {'should_save': 1})
+  \ g:test.create_change(1, 1, 3, 1, "test\n"),
+  \ {
+  \   'should_save': 1,
+  \   'conn_id': 'test_conn',
+  \ })
   AssertEqual '}', getline('.')
   AssertEqual [2, 1], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 #      C ==
 # ===
 Execute(Cursor will not move when changes happening on lines >= cursor, but after cursor):
@@ -349,23 +515,42 @@ Execute(Cursor will not move when changes happening on lines >= cursor, but afte
   call setpos('.', [0, 2, 3, 0])
   AssertEqual '  value: string', getline('.')
   call ale#code_action#HandleCodeAction(
-  \ g:test.create_change(2, 10, 3, 1, "number\n"), {'should_save': 1})
+  \ g:test.create_change(2, 10, 3, 1, "number\n"),
+  \ {
+  \   'should_save': 1,
+  \   'conn_id': 'test_conn',
+  \ })
   AssertEqual '  value: number', getline('.')
   AssertEqual [2, 3], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 Execute(Cursor will not move when change covers entire file):
   call WriteFileAndEdit()
   call setpos('.', [0, 2, 3, 0])
   call ale#code_action#HandleCodeAction(
   \ g:test.create_change(1, 1, len(g:test.text) + 1, 1,
   \   join(g:test.text + ['x'], "\n")),
-  \ {'should_save': 1})
+  \ {
+  \   'should_save': 1,
+  \   'conn_id': 'test_conn',
+  \ })
   AssertEqual [2, 3], getpos('.')[1:2]
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 Execute(It should just modify file when should_save is set to v:false):
   call WriteFileAndEdit()
   let g:test.change = g:test.create_change(1, 1, 1, 1, "import { writeFile } from 'fs';\n")
-  call ale#code_action#HandleCodeAction(g:test.change, {})
+  call ale#code_action#HandleCodeAction(g:test.change, {
+  \ 'conn_id': 'test_conn',
+  \})
   AssertEqual 1, getbufvar(bufnr(''), '&modified')
   AssertEqual [
   \ 'import { writeFile } from ''fs'';',
@@ -374,6 +559,11 @@ Execute(It should just modify file when should_save is set to v:false):
   \ '}',
   \], getline(1, '$')
 
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
+
 Given typescript(An example TypeScript file):
   type Foo = {}
 
@@ -393,7 +583,14 @@ Execute():
   \ {'end': {'offset': 14, 'line': 9}, 'newText': 'foo', 'start': {'offset': 3, 'line': 9}},
   \]
 
-  call ale#code_action#ApplyChanges(expand('%:p'), g:changes, 0)
+  call ale#code_action#ApplyChanges(expand('%:p'), g:changes, {
+  \ 'conn_id': 'test_conn',
+  \})
+
+  AssertEqual [{
+  \ 'conn_id': 'test_conn',
+  \ 'buffer': bufnr(''),
+  \}], g:notified_changes
 
 Expect(The changes should be applied correctly):
   type Foo = {}