From 396fba7cca5bb6cd5774173241e3e606045e1c46 Mon Sep 17 00:00:00 2001 From: w0rp Date: Thu, 27 Aug 2020 08:44:43 +0100 Subject: [PATCH] Fix #3312 - Fix a false positive for auto imports ALE was incorrectly detecting completion results from servers such as rust-analyzer as wanting to add import lines when additionalTextEdits was present, but empty. Now ALE only filters out completion results if the autoimport setting is off, and one of the additionalTextEdits starts on some line other than the current line. If any additionalTextEdits happen to be identical to the change from completion anyway, ALE will skip them. --- autoload/ale/completion.vim | 60 ++++++++++----- .../test_lsp_completion_parsing.vader | 76 +++++++++++++++++++ 2 files changed, 119 insertions(+), 17 deletions(-) diff --git a/autoload/ale/completion.vim b/autoload/ale/completion.vim index a273d4e1..87efd191 100644 --- a/autoload/ale/completion.vim +++ b/autoload/ale/completion.vim @@ -496,6 +496,18 @@ function! ale#completion#NullFilter(buffer, item) abort return 1 endfunction +" Check if additional text edits make changes starting on lines other than the +" one you're asking for completions on. +function! s:TextEditsChangeOtherLines(line, text_edit_list) abort + for l:edit in a:text_edit_list + if l:edit.range.start.line + 1 isnot a:line + return 1 + endif + endfor + + return 0 +endfunction + function! ale#completion#ParseLSPCompletions(response) abort let l:buffer = bufnr('') let l:info = get(b:, 'ale_completion_info', {}) @@ -540,7 +552,9 @@ function! ale#completion#ParseLSPCompletions(response) abort " Don't use LSP items with additional text edits when autoimport for " completions is turned off. - if has_key(l:item, 'additionalTextEdits') && !g:ale_completion_autoimport + if has_key(l:item, 'additionalTextEdits') + \&& !g:ale_completion_autoimport + \&& s:TextEditsChangeOtherLines(l:info.line, l:item.additionalTextEdits) continue endif @@ -562,31 +576,41 @@ function! ale#completion#ParseLSPCompletions(response) abort let l:text_changes = [] for l:edit in l:item.additionalTextEdits - let l:range = l:edit.range + " Don't apply additional text edits that are identical to the + " word we're going to insert anyway. + if l:edit.newText is# l:word + \&& l:edit.range.start.line + 1 is l:info.line + \&& l:edit.range.end.line + 1 is l:info.line + \&& l:edit.range.start.character is l:edit.range.end.character + continue + endif + call add(l:text_changes, { \ 'start': { - \ 'line': l:range.start.line + 1, - \ 'offset': l:range.start.character + 1, + \ 'line': l:edit.range.start.line + 1, + \ 'offset': l:edit.range.start.character + 1, \ }, \ 'end': { - \ 'line': l:range.end.line + 1, - \ 'offset': l:range.end.character + 1, + \ 'line': l:edit.range.end.line + 1, + \ 'offset': l:edit.range.end.character + 1, \ }, \ 'newText': l:edit.newText, \}) endfor - let l:changes = [{ - \ 'fileName': expand('#' . l:buffer . ':p'), - \ 'textChanges': l:text_changes, - \}] - \ - let l:result.user_data = json_encode({ - \ 'codeActions': [{ - \ 'description': 'completion', - \ 'changes': l:changes, - \ }], - \ }) + if !empty(l:text_changes) + let l:result.user_data = json_encode({ + \ 'codeActions': [{ + \ 'description': 'completion', + \ 'changes': [ + \ { + \ 'fileName': expand('#' . l:buffer . ':p'), + \ 'textChanges': l:text_changes, + \ } + \ ], + \ }], + \}) + endif endif call add(l:results, l:result) @@ -900,6 +924,8 @@ function! ale#completion#Done() abort endfunction augroup ALECompletionActions + autocmd! + autocmd CompleteDone * call ale#completion#HandleUserData(v:completed_item) augroup END diff --git a/test/completion/test_lsp_completion_parsing.vader b/test/completion/test_lsp_completion_parsing.vader index 8b8b41c7..395314d7 100644 --- a/test/completion/test_lsp_completion_parsing.vader +++ b/test/completion/test_lsp_completion_parsing.vader @@ -537,6 +537,7 @@ Execute(Should handle completion messages with the deprecated insertText attribu Execute(Should handle completion messages with additionalTextEdits when ale_completion_autoimport is turned on): let g:ale_completion_autoimport = 1 + let b:ale_completion_info = {'line': 30} AssertEqual \ [ @@ -591,6 +592,19 @@ Execute(Should handle completion messages with additionalTextEdits when ale_comp \ { \ 'range': { \ 'start': { + \ 'line': 29, + \ 'character': 10, + \ }, + \ 'end': { + \ 'line': 29, + \ 'character': 10, + \ }, + \ }, + \ 'newText': 'next_callback', + \ }, + \ { + \ 'range': { + \ 'start': { \ 'line': 10, \ 'character': 1, \ }, @@ -609,6 +623,7 @@ Execute(Should handle completion messages with additionalTextEdits when ale_comp Execute(Should not handle completion messages with additionalTextEdits when ale_completion_autoimport is turned off): let g:ale_completion_autoimport = 0 + let b:ale_completion_info = {'line': 30} AssertEqual \ [], @@ -630,6 +645,19 @@ Execute(Should not handle completion messages with additionalTextEdits when ale_ \ { \ 'range': { \ 'start': { + \ 'line': 29, + \ 'character': 10, + \ }, + \ 'end': { + \ 'line': 29, + \ 'character': 10, + \ }, + \ }, + \ 'newText': 'next_callback', + \ }, + \ { + \ 'range': { + \ 'start': { \ 'line': 10, \ 'character': 1, \ }, @@ -645,3 +673,51 @@ Execute(Should not handle completion messages with additionalTextEdits when ale_ \ ], \ }, \ }) + +Execute(Should still handle completion messages with additionalTextEdits with ale_completion_autoimport turned off, if edits all start on the line): + let g:ale_completion_autoimport = 0 + let b:ale_completion_info = {'line': 30} + + AssertEqual + \ [ + \ { + \ 'word': 'next_callback', + \ 'menu': 'PlayTimeCallback', + \ 'info': '', + \ 'kind': 'v', + \ 'icase': 1, + \ } + \ ], + \ ale#completion#ParseLSPCompletions({ + \ 'id': 226, + \ 'jsonrpc': '2.0', + \ 'result': { + \ 'isIncomplete': v:false, + \ 'items': [ + \ { + \ 'detail': 'PlayTimeCallback', + \ 'filterText': 'next_callback', + \ 'insertText': 'next_callback', + \ 'insertTextFormat': 1, + \ 'kind': 6, + \ 'label': ' next_callback', + \ 'sortText': '3ee19999next_callback', + \ 'additionalTextEdits': [ + \ { + \ 'range': { + \ 'start': { + \ 'line': 29, + \ 'character': 10, + \ }, + \ 'end': { + \ 'line': 29, + \ 'character': 10, + \ }, + \ }, + \ 'newText': 'next_callback', + \ }, + \ ], + \ }, + \ ], + \ }, + \ })