dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.53k stars 1.43k forks source link

Add support for LSP/tsserver Code Actions #1466

Closed rlerm closed 3 years ago

rlerm commented 6 years ago

Right now, there is no support for using :ALEFix for LSP-based linters. The protocol supports this (via codeAction), although it might be somewhat complex.

w0rp commented 6 years ago

I'm interested in supporting this, and also something similar for tsserver, which I consider close enough to LSP servers to support both. Both offer "quick fix" actions for fixing a specific problem in a number of ways, probably across multiple files. In the first instance, I'm interested in seeing if there's a way to fix all known problems automatically for a given buffer with both protocols.

w0rp commented 6 years ago

After giving this some thought, I think the best way to support fixing files via Language Server Protocol is to not apply fixes to files in the pipeline of fixes for :ALEFix, but to handle code actions separately. Code actions will likely need to write the files they change to disk in many cases, and will also support actions on ranges. What ALE ought to do is ask the server what actions are available for either a range in a document or the whole document, and then have you selection an option to apply.

ALE should support code actions primarily via some commands, and secondarily via the GVim mouse-controlled context menu. There should be some way to issue code actions without first loading the list of actions.

jeffwillette commented 5 years ago

Is there any update on the feasibility of this. I am not happy with any of the vim typescript plugins which offer this functionality and ALE does everything except auto fixes on typescript.

Is there a way to add an optionality interface to ALEfix and then people can write requests to the LS in fixers and have a callback that sends the options to the user?

w0rp commented 5 years ago

If there's something to update this issue with, I'll post a comment. What I said before about code actions still applies.

jazmit commented 5 years ago

+1. Code actions are the last blocker before I can dispense with VSCode. Maybe it would make sense to rename this issue to 'Support code actions'? Are there any known workarounds/hacks to get code actions working?

w0rp commented 5 years ago

https://github.com/dense-analysis/ale/commit/ca2026bd13346cb7e1afeecb54dbec3da0bd5151

I was playing around with adding support for code actions with popup menu support, so you can just right click on regions of code to perform action. GVim supports updating the popup menu while it's still open, and you can trigger requesting some actions whenever the menu is opened, so it will be pretty good.

I'll try to get back on this when I can. I plan to get the popup menu support to be pretty good first, and then add console menu support second for terminals. The hardest part is just applying edits to files that may or may not be open in Vim already.

w0rp commented 4 years ago

Parts of the code required for implementing code actions were previously added to the codebase to support rename operations and organising imports. See here: #2709

ta3pks commented 4 years ago

:+1:

ta3pks commented 4 years ago

just waiting for this feature to go back to ale from vim-lsp I love ale's plug-n-play approach

daliusd commented 3 years ago

If would appreciate if somebody would test my tsserver code actions pull request. In order to do that you need to:

  1. git clone https://github.com/daliusd/ale.git

  2. git checkout codefix

  3. Add in your .vimrc (with assumption that you are using Plug): Plug '~/projects/ale' (with assumption that you have cloned ale into projects folder).

  4. Add mappings for ALECodeAction:

nnoremap <leader>qf :ALECodeAction<CR>
vnoremap <leader>qf :ALECodeAction<CR>

With this configuration you should be able to import missing modules in normal mode and extract functions / methods and more in visual mode.

ta3pks commented 3 years ago

@daliusd you mention particularly about the tsserver arent all language servers supposed to comply with the same protocol hence this pr could be applied to all without changing anything ?

polyzen commented 3 years ago

tsserver does not yet implement the LSP.

daliusd commented 3 years ago

@daliusd you mention particularly about the tsserver arent all language servers supposed to comply with the same protocol hence this pr could be applied to all without changing anything ?

@NikosEfthias I have implemented tsserver support only as this is my main work tool (tsserver is not LSP as @polyzen has mentioned). I think I can implement LSP Code Actions as well but I personally don't know any LSP server that has code actions (except one tsserver wrapper as LSP server). Are you aware of any or have need of any?

polyzen commented 3 years ago

A couple that support code actions are jedi-language-server and rust-analyzer.

daliusd commented 3 years ago

rust-analyzer is in alpha status and that shines in code actions. It seems they have implemented custom code actions (not exactly LSP) and require custom code to support them. coc-rust-analyzer uses custom code to support them: https://github.com/fannheyward/coc-rust-analyzer/blob/c7899dd3901c2202516f9c70f97a7531bcf56aa0/src/client.ts#L71

Sublime has stumbled on this as well: https://github.com/sublimelsp/LSP/issues/1085

It seems rust-analyzer has issue related to that https://github.com/rust-analyzer/rust-analyzer/issues/6368

In result I have failed to implement code actions support while using rust-analyzer. Let's try jedi-language-server.

daliusd commented 3 years ago

I have created https://github.com/daliusd/ale/commits/codeactions branch based on codefix branch mentioned above in this discussions. The last commit is relevant one https://github.com/daliusd/ale/commit/f09e7d5e0491b9cf1b3468c07473a40d411c6694 . The problem is that with jedi-language-server the result looks bad. I have tried to reuse functions from rename LSP and that might be the problem. If there are people willing to have code actions support via Ale feel free to continue from this point. I doubt that I will do more while I don't need LSP code actions myself.

daliusd commented 3 years ago

Information for whoever would like to continue implementing LSP Code Actions:

  1. There is another good server for testing typescript-language-server. Create tls.vim file in ale_linters/typescript/tls.vim with following content:
" Description: typescript-language-server

call ale#Set('typescript_tls_executable', 'typescript-language-server')
call ale#Set('typescript_tls_use_global', get(g:, 'ale_use_global_executables', 0))
call ale#Set('typescript_tls_auto_pipenv', 0)

call ale#linter#Define('typescript', {
\   'name': 'tls',
\   'lsp': 'stdio',
\   'executable': {b -> ale#node#FindExecutable(b, 'typescript_tls', [])},
\   'command': '%e --stdio',
\   'project_root': function('ale#handlers#tsserver#GetProjectRoot'),
\   'language': '',
\})

Add in your vimrc following line:

let g:ale_linters = {
\   'typescript': ['tls'],
\}

My codeactions branch is incomplete as it is not handling several things:

  1. [DONE] Not passing known diagnostic to ale#lsp#message#CodeAction (some code actions might not work because of that)
  2. [DONE] Not handling commands returned by "textDocument/codeAction". It is assumed now that only documentEdits can be returned.
  3. [DONE] Potentially wrong positions are sent for code actions (must be checked).
daliusd commented 3 years ago

One more push to codeactions and at least with typescript-language-server code actions are partially working. There are some nuances and it is not perfect but it is really something what can be used to finish this. I have reported issue to jedi-language-server as well https://github.com/pappasam/jedi-language-server/issues/47

ta3pks commented 3 years ago

I am using rust-analyzer, rls, typescript-language-server, on a daily basis and all of them works fine with code actions using prabirshrestha/vim-lsp

daliusd commented 3 years ago

I am using rust-analyzer, rls, typescript-language-server, on a daily basis and all of them works fine with code actions using prabirshrestha/vim-lsp

Yes, I think existing code is ignoring existing end-of-lines. E.g. it overwrites them instead of writing after them (writing in new line)

daliusd commented 3 years ago

Problem with end-of-lines fixed. jedi-language-server works better now while not perfect in some situations (deeper analysis required to check if it is problem with jedi-language-server or in function ale#code_action#ApplyChanges in Ale).

daliusd commented 3 years ago

OK. I have found some time to work on codeactions and now https://github.com/daliusd/ale/commits/codeactions branch has working solution. It is only missing some tests and refactorings (from my point of view). I have tested the code with typescript-language-server and jedi-langauge-server and the most common code actions seem to work (like import, extract to function and etc.). I had no luck with rust-analyzer however and I have no idea what might be missing.

w0rp commented 3 years ago

Thanks to @daliusd's great work, ALE now has code action support. :+1:

I'm going to see if I can set up the code actions in menus for GVim users like myself.

leo60228 commented 3 years ago

clangd diagnostics that say "fix available" have fixes applied via code actions, right? That isn't working for me.

leo60228 commented 3 years ago

This seems to be an ALE issue. Looking at a trace of the LSP communication:

{
  "jsonrpc": "2.0",
  "method": "textDocument/publishDiagnostics",
  "params": {
    "diagnostics": [
      {
        "code": "undeclared_var_use_suggest",
        "message": "Use of undeclared identifier 'telesmary'; did you mean 'telesummary'? (fix available)",
        "range": {
          "end": {
            "character": 17,
            "line": 309
          },
          "start": {
            "character": 8,
            "line": 309
          }
        },
        "relatedInformation": [
          {
            "location": {
              "range": {
                "end": {
                  "character": 27,
                  "line": 352
                },
                "start": {
                  "character": 16,
                  "line": 352
                }
              },
              "uri": "file:///home/leo60228/VVVVVV/desktop_version/src/Game.h"
            },
            "message": "'telesummary' declared here"
          }
        ],
        "severity": 1,
        "source": "clang"
      }
    ],
    "uri": "file:///home/leo60228/VVVVVV/desktop_version/src/Game.cpp"
  }
}
{
  "method": "textDocument/codeAction",
  "jsonrpc": "2.0",
  "id": 2,
  "params": {
    "context": {
      "diagnostics": [
        {
          "range": {
            "end": {
              "character": 16,
              "line": 309
            },
            "start": {
              "character": 8,
              "line": 309
            }
          },
          "code": "undeclared_var_use_suggest",
          "message": "Use of undeclared identifier 'telesmary'; did you mean 'telesummary'? (fix available)"
        }
      ]
    },
    "range": {
      "end": {
        "character": 2,
        "line": 309
      },
      "start": {
        "character": 1,
        "line": 309
      }
    },
    "textDocument": {
      "uri": "file:///home/leo60228/VVVVVV/desktop_version/src/Game.cpp"
    }
  }
}

Notice that ALE's params.context.diagnostics[0].range.end.character is 16, while the value in the diagnostic is 17.

leo60228 commented 3 years ago

I'm not sure if this is correct, but it fixes the issue:

diff --git a/autoload/ale/codefix.vim b/autoload/ale/codefix.vim
index b58f5e4..54ccfb3 100644
--- a/autoload/ale/codefix.vim
+++ b/autoload/ale/codefix.vim
@@ -304,7 +304,7 @@ function! s:OnReady(line, column, end_line, end_column, linter, lsp_details) abo
                 \ 'message': l:nearest_error.text,
                 \ 'range': {
                 \     'start': { 'line': l:nearest_error.lnum - 1, 'character': l:nearest_error.col - 1 },
-                \     'end': { 'line': l:nearest_error.end_lnum - 1, 'character': l:nearest_error.end_col - 1 }
+                \     'end': { 'line': l:nearest_error.end_lnum - 1, 'character': l:nearest_error.end_col }
                 \}
                 \}]
             endif
daliusd commented 3 years ago

Oh I forgot that LSP indexing starts with zero and LSP with 1 so fix might be correct. I recommend creating pull request and add test to test_codefix.vader file.

daliusd commented 3 years ago

I will test your fix tomorrow with two other LSP servers.

daliusd commented 3 years ago

@leo60228 I have created pull request with your proposed fix and adjusted tests. I have tested your change with some other language servers and they seem to work fine. Thank you for trying it out and actually suggesting fix.

w0rp commented 3 years ago

I've added context menu support for GUI Vim now. Code actions will appear in a Refactor... menu, and :ALERename will appear as Rename. I'll incorporate that bug fix now.

WooterTheTroubleshooter commented 1 year ago

I made a vimscript function that puts a Vim 8.2 popup with the code-actions under the line of the cursor when called.

func! s:codeActionMenu(data, menu_items)                                              
    let l:options = []                                                                
    let l:cmds = []                                                                   
    for [l:type, l:item] in a:menu_items                                              
        let l:name = l:type is# 'tsserver' ? l:item.name : l:item.title               
        let l:func_name = l:type is# 'tsserver'                                       
        \   ? 'ale#codefix#ApplyTSServerCodeAction'                                   
        \   : 'ale#codefix#ApplyLSPCodeAction'                                        

        call add(l:options, l:name)                                                   
        call add(l:cmds, printf(                                                      
        \   "call %s(%s, %s)",                                                        
        \   l:func_name,                                                              
        \   string(a:data),                                                           
        \   string(l:item)                                                            
        \))                                                                           
    endfor                                                                            

    func! s:selectedCommand(id, cmd) closure                                          
        if a:cmd == -1  " menu was canceled                                           
            return                                                                    
        endif                                                                         
        if a:cmd <= len(cmds) && a:cmd > 0                                            
            echo "exe"                                                                
            exe cmds[a:cmd-1]                                                         
        endif                                                                         
    endfunc                                                                           

    call popup_menu(l:options, #{                                                     
    \   zindex: 10,                                                                   
    \   title: "Code actions",                                                        
    \   pos: 'topleft',                                                               
    \   line: 'cursor+1',                                                             
    \   col: 'cursor',                                                                
    \   callback: function('s:selectedCommand'),                                      
    \   filter: function('s:codeActionMenuFilter'),                                   
    \})                                                                               
endfunc                                                                               
func! s:codeActionMenuFilter(id, key)                                                 
    if a:key =~# '\d'                                                                 
        return popup_close(a:id, str2nr(a:key))                                                                                                                                                                                                                    
    else                                                                              
        return popup_filter_menu(a:id, a:key)                                         
    endif                                                                             
endfunc                                                                         
func! MyCodeActionMenu()                                                              
    call ale#codefix#Execute(                                                         
    \   mode() is# 'v' || mode() is# "\<C-V>",                                        
    \   function('s:codeActionMenu')                                                  
    \)                                                                                
endfunc                             

Edit: Added number hotkeys to quickly perform an action.

w0rp commented 1 year ago

@WooterTheTroubleshooter It would be interesting to try and put that in a pull request. It's worth exploring that as something we could support.