Fixes to code actions (cursor moving, tests, EOL/EOF corner cases) (#3478)

* code_action: Don't move cursor when change covers entire file
* code_action: Refactor/simplify ApplyChanges
* code_action: Fix EOL at EOF corner cases while performing no changes
* code_action: Fix column around EOL corner cases
* code_action: Handle positions out of bounds
* code_action: Add instructions for verifying corner case tests against vscode

1 files changed, 60 insertions, 35 deletions

diff --git a/autoload/ale/code_action.vim b/autoload/ale/code_action.vim
index 69d40933..be4a25da 100644
--- a/autoload/ale/code_action.vim
+++ b/autoload/ale/code_action.vim
@@ -71,6 +71,11 @@ function! ale#code_action#ApplyChanges(filename, changes, should_save) abort
 
     if l:buffer > 0
         let l:lines = getbufline(l:buffer, 1, '$')
+
+        " Add empty line if there's trailing newline, like readfile() does.
+        if getbufvar(l:buffer, '&eol')
+            let l:lines += ['']
+        endif
     else
         let l:lines = readfile(a:filename, 'b')
     endif
@@ -89,62 +94,82 @@ function! ale#code_action#ApplyChanges(filename, changes, should_save) abort
         let l:end_column = l:code_edit.end.offset
         let l:text = l:code_edit.newText
 
-        " Adjust the ends according to previous edits.
-        if l:end_line > len(l:lines)
-            let l:end_line_len = 0
-        else
-            let l:end_line_len = len(l:lines[l:end_line - 1])
-        endif
-
         let l:insertions = split(l:text, '\n', 1)
 
-        if l:line is 1
-            " Same logic as for column below. Vimscript's slice [:-1] will not
-            " be an empty list.
-            let l:start = []
-        else
-            let l:start = l:lines[: l:line - 2]
-        endif
+        " Fix invalid columns
+        let l:column = l:column > 0 ? l:column : 1
+        let l:end_column = l:end_column > 0 ? l:end_column : 1
 
-        " Special case when text must be added after new line
-        if l:column > len(l:lines[l:line - 1])
-            call extend(l:start, [l:lines[l:line - 1]])
-            let l:column = 1
+        " Clamp start to BOF
+        if l:line < 1
+            let [l:line, l:column] = [1, 1]
         endif
 
-        if l:column is 1
-            " We need to handle column 1 specially, because we can't slice an
-            " empty string ending on index 0.
-            let l:middle = [l:insertions[0]]
-        else
-            let l:middle = [l:lines[l:line - 1][: l:column - 2] . l:insertions[0]]
+        " Clamp start to EOF
+        if l:line > len(l:lines) || l:line == len(l:lines) && l:column > len(l:lines[-1]) + 1
+            let [l:line, l:column] = [len(l:lines), len(l:lines[-1]) + 1]
+        " Special case when start is after EOL
+        elseif l:line < len(l:lines) && l:column > len(l:lines[l:line - 1]) + 1
+            let [l:line, l:column] = [l:line + 1, 1]
         endif
 
-        call extend(l:middle, l:insertions[1:])
+        " Adjust end: clamp if invalid and/or adjust if we moved start
+        if l:end_line < l:line || l:end_line == l:line && l:end_column < l:column
+            let [l:end_line, l:end_column] = [l:line, l:column]
+        endif
 
-        if l:end_line <= len(l:lines)
-            " Only extend the last line if end_line is within the range of
-            " lines.
-            let l:middle[-1] .= l:lines[l:end_line - 1][l:end_column - 1 :]
+        " Clamp end to EOF
+        if l:end_line > len(l:lines) || l:end_line == len(l:lines) && l:end_column > len(l:lines[-1]) + 1
+            let [l:end_line, l:end_column] = [len(l:lines), len(l:lines[-1]) + 1]
+        " Special case when end is after EOL
+        elseif l:end_line < len(l:lines) && l:end_column > len(l:lines[l:end_line - 1]) + 1
+            let [l:end_line, l:end_column] = [l:end_line + 1, 1]
         endif
 
+        " Careful, [:-1] is not an empty list
+        let l:start = l:line is 1 ? [] : l:lines[: l:line - 2]
+        let l:middle = l:column is 1 ? [''] : [l:lines[l:line - 1][: l:column - 2]]
+
+        let l:middle[-1] .= l:insertions[0]
+        let l:middle     += l:insertions[1:]
+        let l:middle[-1] .= l:lines[l:end_line - 1][l:end_column - 1 :]
+
+        let l:end_line_len = len(l:lines[l:end_line - 1])
         let l:lines_before_change = len(l:lines)
         let l:lines = l:start + l:middle + l:lines[l:end_line :]
 
         let l:current_line_offset = len(l:lines) - l:lines_before_change
         let l:column_offset = len(l:middle[-1]) - l:end_line_len
 
-        let l:pos = s:UpdateCursor(l:pos,
-        \ [l:line, l:column],
-        \ [l:end_line, l:end_column],
-        \ [l:current_line_offset, l:column_offset])
+        " Keep cursor where it was (if outside of changes) or move it after
+        " the changed text (if inside), but don't touch it when the change
+        " spans the entire buffer, in which case we have no clue and it's
+        " better to not do anything.
+        if l:line isnot 1 || l:column isnot 1
+        \|| l:end_line < l:lines_before_change
+        \|| l:end_line == l:lines_before_change && l:end_column <= l:end_line_len
+            let l:pos = s:UpdateCursor(l:pos,
+            \ [l:line, l:column],
+            \ [l:end_line, l:end_column],
+            \ [l:current_line_offset, l:column_offset])
+        endif
     endfor
 
-    if l:lines[-1] is# ''
+    if l:buffer > 0
+        " Make sure ale#util#{Writefile,SetBufferContents} add trailing
+        " newline if and only if it should be added.
+        if l:lines[-1] is# '' && getbufvar(l:buffer, '&eol')
+            call remove(l:lines, -1)
+        else
+            call setbufvar(l:buffer, '&eol', 0)
+        endif
+    elseif exists('+fixeol') && &fixeol && l:lines[-1] is# ''
+        " Not in buffer, ale#util#Writefile can't check &eol and always adds
+        " newline if &fixeol: remove to prevent double trailing newline.
         call remove(l:lines, -1)
     endif
 
-    if a:should_save
+    if a:should_save || l:buffer < 0
         call ale#util#Writefile(l:buffer, l:lines, a:filename)
     else
         call ale#util#SetBufferContents(l:buffer, l:lines)