Merge pull request #2823 from jeremija/do-not-save-on-autoimport

Do not save after ALERename and ALEOrganizeImports

10 files changed, 101 insertions, 54 deletions

diff --git a/autoload/ale/code_action.vim b/autoload/ale/code_action.vim
index 0af1bb70..60c3bbef 100644
--- a/autoload/ale/code_action.vim
+++ b/autoload/ale/code_action.vim
@@ -1,7 +1,7 @@
 " Author: Jerko Steiner <jerko.steiner@gmail.com>
 " Description: Code action support for LSP / tsserver
 
-function! ale#code_action#HandleCodeAction(code_action) abort
+function! ale#code_action#HandleCodeAction(code_action, should_save) abort
     let l:current_buffer = bufnr('')
     let l:changes = a:code_action.changes
 
@@ -17,11 +17,14 @@ function! ale#code_action#HandleCodeAction(code_action) abort
 
     for l:file_code_edit in l:changes
         call ale#code_action#ApplyChanges(
-        \ l:file_code_edit.fileName, l:file_code_edit.textChanges)
+        \ l:file_code_edit.fileName,
+        \ l:file_code_edit.textChanges,
+        \ a:should_save,
+        \ )
     endfor
 endfunction
 
-function! ale#code_action#ApplyChanges(filename, changes) abort
+function! ale#code_action#ApplyChanges(filename, changes, should_save) abort
     let l:current_buffer = bufnr('')
     " The buffer is used to determine the fileformat, if available.
     let l:buffer = bufnr(a:filename)
@@ -106,10 +109,17 @@ function! ale#code_action#ApplyChanges(filename, changes) abort
         call remove(l:lines, -1)
     endif
 
-    call ale#util#Writefile(l:buffer, l:lines, a:filename)
+    if a:should_save
+        call ale#util#Writefile(l:buffer, l:lines, a:filename)
+    else
+        call ale#util#SetBufferContents(l:buffer, l:lines)
+    endif
 
     if l:is_current_buffer
-        call ale#util#Execute(':e!')
+        if a:should_save
+            call ale#util#Execute(':e!')
+        endif
+
         call setpos('.', [0, l:pos[0], l:pos[1], 0])
     endif
 endfunction
diff --git a/autoload/ale/completion.vim b/autoload/ale/completion.vim
index 80684a30..2b5756e4 100644
--- a/autoload/ale/completion.vim
+++ b/autoload/ale/completion.vim
@@ -823,7 +823,7 @@ function! ale#completion#HandleUserData(completed_item) abort
     endif
 
     for l:code_action in get(l:user_data, 'codeActions', [])
-        call ale#code_action#HandleCodeAction(l:code_action)
+        call ale#code_action#HandleCodeAction(l:code_action, v:false)
     endfor
 endfunction
 
diff --git a/autoload/ale/fix.vim b/autoload/ale/fix.vim
index dad9e2bc..69817b36 100644
--- a/autoload/ale/fix.vim
+++ b/autoload/ale/fix.vim
@@ -4,40 +4,15 @@ call ale#Set('fix_on_save_ignore', {})
 " Vim doesn't let you modify hidden buffers.
 function! ale#fix#ApplyQueuedFixes(buffer) abort
     let l:data = get(g:ale_fix_buffer_data, a:buffer, {'done': 0})
-    let l:has_bufline_api = exists('*deletebufline') && exists('*setbufline')
 
-    if !l:data.done || (!l:has_bufline_api && a:buffer isnot bufnr(''))
+    if !l:data.done || (!ale#util#HasBuflineApi() && a:buffer isnot bufnr(''))
         return
     endif
 
     call remove(g:ale_fix_buffer_data, a:buffer)
 
     if l:data.changes_made
-        " If the file is in DOS mode, we have to remove carriage returns from
-        " the ends of lines before calling setline(), or we will see them
-        " twice.
-        let l:new_lines = getbufvar(a:buffer, '&fileformat') is# 'dos'
-        \   ? map(copy(l:data.output), 'substitute(v:val, ''\r\+$'', '''', '''')')
-        \   : l:data.output
-        let l:first_line_to_remove = len(l:new_lines) + 1
-
-        " Use a Vim API for setting lines in other buffers, if available.
-        if l:has_bufline_api
-            call setbufline(a:buffer, 1, l:new_lines)
-            call deletebufline(a:buffer, l:first_line_to_remove, '$')
-        " Fall back on setting lines the old way, for the current buffer.
-        else
-            let l:old_line_length = len(l:data.lines_before)
-
-            if l:old_line_length >= l:first_line_to_remove
-                let l:save = winsaveview()
-                silent execute
-                \   l:first_line_to_remove . ',' . l:old_line_length . 'd_'
-                call winrestview(l:save)
-            endif
-
-            call setline(1, l:new_lines)
-        endif
+        let l:new_lines = ale#util#SetBufferContents(a:buffer, l:data.output)
 
         if l:data.should_save
             if a:buffer is bufnr('')
diff --git a/autoload/ale/organize_imports.vim b/autoload/ale/organize_imports.vim
index bc9b829e..e89c832c 100644
--- a/autoload/ale/organize_imports.vim
+++ b/autoload/ale/organize_imports.vim
@@ -15,7 +15,7 @@ function! ale#organize_imports#HandleTSServerResponse(conn_id, response) abort
     call ale#code_action#HandleCodeAction({
     \   'description': 'Organize Imports',
     \   'changes': l:file_code_edits,
-    \})
+    \}, v:false)
 endfunction
 
 function! s:OnReady(linter, lsp_details) abort
diff --git a/autoload/ale/rename.vim b/autoload/ale/rename.vim
index 02b7b579..fbd6c2ad 100644
--- a/autoload/ale/rename.vim
+++ b/autoload/ale/rename.vim
@@ -80,7 +80,7 @@ function! ale#rename#HandleTSServerResponse(conn_id, response) abort
     call ale#code_action#HandleCodeAction({
     \ 'description': 'rename',
     \ 'changes': l:changes,
-    \})
+    \}, v:true)
 endfunction
 
 function! ale#rename#HandleLSPResponse(conn_id, response) abort
@@ -134,7 +134,7 @@ function! ale#rename#HandleLSPResponse(conn_id, response) abort
         call ale#code_action#HandleCodeAction({
         \   'description': 'rename',
         \   'changes': l:changes,
-        \})
+        \}, v:true)
     endif
 endfunction
 
diff --git a/autoload/ale/util.vim b/autoload/ale/util.vim
index 3ca9ea08..ee62af28 100644
--- a/autoload/ale/util.vim
+++ b/autoload/ale/util.vim
@@ -476,3 +476,44 @@ endfunction
 function! ale#util#Input(message, value) abort
     return input(a:message, a:value)
 endfunction
+
+function! ale#util#HasBuflineApi() abort
+    return exists('*deletebufline') && exists('*setbufline')
+endfunction
+
+" Sets buffer contents to lines
+function! ale#util#SetBufferContents(buffer, lines) abort
+    let l:has_bufline_api = ale#util#HasBuflineApi()
+
+    if !l:has_bufline_api && a:buffer isnot bufnr('')
+        return
+    endif
+
+    " If the file is in DOS mode, we have to remove carriage returns from
+    " the ends of lines before calling setline(), or we will see them
+    " twice.
+    let l:new_lines = getbufvar(a:buffer, '&fileformat') is# 'dos'
+    \   ? map(copy(a:lines), 'substitute(v:val, ''\r\+$'', '''', '''')')
+    \   : a:lines
+    let l:first_line_to_remove = len(l:new_lines) + 1
+
+    " Use a Vim API for setting lines in other buffers, if available.
+    if l:has_bufline_api
+        call setbufline(a:buffer, 1, l:new_lines)
+        call deletebufline(a:buffer, l:first_line_to_remove, '$')
+    " Fall back on setting lines the old way, for the current buffer.
+    else
+        let l:old_line_length = line('$')
+
+        if l:old_line_length >= l:first_line_to_remove
+            let l:save = winsaveview()
+            silent execute
+            \   l:first_line_to_remove . ',' . l:old_line_length . 'd_'
+            call winrestview(l:save)
+        endif
+
+        call setline(1, l:new_lines)
+    endif
+
+    return l:new_lines
+endfunction
diff --git a/test/completion/test_completion_events.vader b/test/completion/test_completion_events.vader
index 2ac2b15c..3a7a31d0 100644
--- a/test/completion/test_completion_events.vader
+++ b/test/completion/test_completion_events.vader
@@ -50,7 +50,8 @@ Before:
   let g:handle_code_action_called = 0
   function! MockHandleCodeAction() abort
     " delfunction! ale#code_action#HandleCodeAction
-    function! ale#code_action#HandleCodeAction(action) abort
+    function! ale#code_action#HandleCodeAction(action, should_save) abort
+      AssertEqual v:false, a:should_save
       let g:handle_code_action_called += 1
     endfunction
   endfunction
diff --git a/test/test_code_action.vader b/test/test_code_action.vader
index ffaca630..b47f24ff 100644
--- a/test/test_code_action.vader
+++ b/test/test_code_action.vader
@@ -37,10 +37,10 @@ Before:
 After:
   " Close the extra buffers if we opened it.
   if bufnr(g:file1) != -1
-    execute ':bp | :bd ' . bufnr(g:file1)
+    execute ':bp! | :bd! ' . bufnr(g:file1)
   endif
   if bufnr(g:file2) != -1
-    execute ':bp | :bd ' . bufnr(g:file2)
+    execute ':bp! | :bd! ' . bufnr(g:file2)
   endif
 
   if filereadable(g:file1)
@@ -116,7 +116,7 @@ Execute(It should modify and save multiple files):
   \       'newText': "import {A, B} from 'module'\n\n",
   \     }]
   \  }],
-  \})
+  \}, v:true)
 
   AssertEqual [
   \ 'class Value {',
@@ -161,7 +161,7 @@ Execute(Beginning of file can be modified):
   \        'newText': "type A: string\ntype B: number\n",
   \      }],
   \   }]
-  \})
+  \}, v:true)
 
   AssertEqual [
   \  'type A: string',
@@ -192,7 +192,7 @@ Execute(End of file can be modified):
   \        'newText': "type A: string\ntype B: number\n",
   \      }],
   \   }]
-  \})
+  \}, v:true)
 
   AssertEqual g:test.text + [
   \  'type A: string',
@@ -227,7 +227,7 @@ Execute(Current buffer contents will be reloaded):
   \        'newText': "type A: string\ntype B: number\n",
   \      }],
   \   }]
-  \})
+  \}, v:true)
 
   AssertEqual [
   \  'type A: string',
@@ -249,11 +249,11 @@ 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)
+  call ale#code_action#HandleCodeAction(g:test.changes, v:true)
   AssertEqual [1, 1], getpos('.')[1:2]
 
   call setpos('.', [0, 2, 2, 0])
-  call ale#code_action#HandleCodeAction(g:test.changes)
+  call ale#code_action#HandleCodeAction(g:test.changes, v:true)
   AssertEqual [2, 2], getpos('.')[1:2]
 
 # ====C====
@@ -264,7 +264,7 @@ 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)
+    call ale#code_action#HandleCodeAction(g:test.changes, v:true)
     AssertEqual '  value2: string', getline('.')
     AssertEqual [2, 9], getpos('.')[1:2]
   endfor
@@ -275,7 +275,8 @@ Execute(Cursor column will move back when new text is shorter):
   call WriteFileAndEdit()
   call setpos('.', [0, 2, 8, 0])
   AssertEqual '  value: string', getline('.')
-  call ale#code_action#HandleCodeAction(g:test.create_change(2, 3, 2, 8, 'val'))
+  call ale#code_action#HandleCodeAction(
+  \ g:test.create_change(2, 3, 2, 8, 'val'), v:true)
   AssertEqual '  val: string', getline('.')
   AssertEqual [2, 6], getpos('.')[1:2]
 
@@ -286,7 +287,8 @@ 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'))
+  call ale#code_action#HandleCodeAction(
+  \ g:test.create_change(2, 3, 2, 8, 'longValue'), v:true)
   AssertEqual '  longValue: string', getline('.')
   AssertEqual [2, 12], getpos('.')[1:2]
 
@@ -297,7 +299,8 @@ Execute(Cursor line will move when updates are happening on lines above):
   call WriteFileAndEdit()
   call setpos('.', [0, 3, 1, 0])
   AssertEqual '}', getline('.')
-  call ale#code_action#HandleCodeAction(g:test.create_change(1, 1, 2, 1, "test\ntest\n"))
+  call ale#code_action#HandleCodeAction(
+  \ g:test.create_change(1, 1, 2, 1, "test\ntest\n"), v:true)
   AssertEqual '}', getline('.')
   AssertEqual [4, 1], getpos('.')[1:2]
 
@@ -308,7 +311,8 @@ Execute(Cursor line and column will move when change on lines above and just bef
   call WriteFileAndEdit()
   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"))
+  call ale#code_action#HandleCodeAction(
+  \ g:test.create_change(1, 1, 2, 1, "test\ntest\n123"), v:true)
   AssertEqual '123  value: string', getline('.')
   AssertEqual [3, 5], getpos('.')[1:2]
 
@@ -319,7 +323,8 @@ Execute(Cursor line and column will move at the end of changes):
   call WriteFileAndEdit()
   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"))
+  call ale#code_action#HandleCodeAction(
+  \ g:test.create_change(1, 1, 3, 1, "test\n"), v:true)
   AssertEqual '}', getline('.')
   AssertEqual [2, 1], getpos('.')[1:2]
 
@@ -329,6 +334,19 @@ Execute(Cursor will not move when changes happening on lines >= cursor, but afte
   call WriteFileAndEdit()
   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"))
+  call ale#code_action#HandleCodeAction(
+  \ g:test.create_change(2, 10, 3, 1, "number\n"), v:true)
   AssertEqual '  value: number', getline('.')
   AssertEqual [2, 3], getpos('.')[1:2]
+
+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, v:false)
+  AssertEqual 1, getbufvar(bufnr(''), '&modified')
+  AssertEqual [
+  \ 'import { writeFile } from ''fs'';',
+  \ 'class Name {',
+  \ '  value: string',
+  \ '}',
+  \], getline(1, '$')
diff --git a/test/test_organize_imports.vader b/test/test_organize_imports.vader
index 137326a9..c51ff1c0 100644
--- a/test/test_organize_imports.vader
+++ b/test/test_organize_imports.vader
@@ -57,8 +57,9 @@ Before:
     call add(g:expr_list, a:expr)
   endfunction
 
-  function! ale#code_action#HandleCodeAction(code_action) abort
+  function! ale#code_action#HandleCodeAction(code_action, should_save) abort
     let g:handle_code_action_called = 1
+    AssertEqual v:false, a:should_save
     call add(g:code_actions, a:code_action)
   endfunction
 
diff --git a/test/test_rename.vader b/test/test_rename.vader
index 98e3ef30..3600df59 100644
--- a/test/test_rename.vader
+++ b/test/test_rename.vader
@@ -57,8 +57,9 @@ Before:
     call add(g:expr_list, a:expr)
   endfunction
 
-  function! ale#code_action#HandleCodeAction(code_action) abort
+  function! ale#code_action#HandleCodeAction(code_action, should_save) abort
     let g:handle_code_action_called = 1
+    AssertEqual v:true, a:should_save
     call add(g:code_actions, a:code_action)
   endfunction