Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for LSP/tsserver Code Actions #1466

Closed
rlerm opened this issue Apr 2, 2018 · 32 comments
Closed

Add support for LSP/tsserver Code Actions #1466

rlerm opened this issue Apr 2, 2018 · 32 comments
Labels
enhancement LSP Any issue relating to LSP or tsserver

Comments

@rlerm
Copy link
Contributor

rlerm commented Apr 2, 2018

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
Copy link
Member

w0rp commented Apr 5, 2018

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
Copy link
Member

w0rp commented Apr 23, 2018

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.

@w0rp w0rp added the LSP Any issue relating to LSP or tsserver label Apr 23, 2018
@jeffwillette
Copy link
Contributor

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
Copy link
Member

w0rp commented Dec 29, 2018

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

@jazmit
Copy link

jazmit commented Jul 30, 2019

+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
Copy link
Member

w0rp commented Jul 30, 2019

ca2026b

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 w0rp added this to To Do in Old Working List via automation Jul 30, 2019
@w0rp w0rp changed the title Support fixing from LSP linters Add support for LSP/tsserver Code Actions Jul 30, 2019
@w0rp w0rp moved this from To Do to In Progress in Old Working List Jul 30, 2019
@w0rp w0rp added this to In Progress in On the Radar Aug 17, 2019
@w0rp
Copy link
Member

w0rp commented Jan 2, 2020

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

@w0rp w0rp moved this from In Progress to To Do in On the Radar Feb 22, 2020
@ta3pks
Copy link

ta3pks commented Mar 21, 2020

👍

@ta3pks
Copy link

ta3pks commented Jun 30, 2020

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

@daliusd
Copy link
Contributor

daliusd commented Nov 8, 2020

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
Copy link

ta3pks commented Nov 9, 2020

@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
Copy link
Contributor

polyzen commented Nov 9, 2020

@daliusd
Copy link
Contributor

daliusd commented Nov 9, 2020

@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
Copy link
Contributor

polyzen commented Nov 9, 2020

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

@daliusd
Copy link
Contributor

daliusd commented Nov 9, 2020

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: sublimelsp/LSP#1085

It seems rust-analyzer has issue related to that rust-lang/rust-analyzer#6368

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

@daliusd
Copy link
Contributor

daliusd commented Nov 9, 2020

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 daliusd@f09e7d5 . 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
Copy link
Contributor

daliusd commented Nov 10, 2020

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
Copy link
Contributor

daliusd commented Nov 10, 2020

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 pappasam/jedi-language-server#47

@ta3pks
Copy link

ta3pks commented Nov 11, 2020

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
Copy link
Contributor

daliusd commented Nov 11, 2020

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
Copy link
Contributor

daliusd commented Nov 12, 2020

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
Copy link
Contributor

daliusd commented Nov 12, 2020

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
Copy link
Member

w0rp commented Nov 14, 2020

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

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

@leo60228
Copy link

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

@leo60228
Copy link

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
Copy link

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
Copy link
Contributor

daliusd commented Nov 17, 2020

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
Copy link
Contributor

daliusd commented Nov 17, 2020

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

@daliusd
Copy link
Contributor

daliusd commented Nov 17, 2020

@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 w0rp closed this as completed in 7c04ee5 Nov 21, 2020
On the Radar automation moved this from To Do to Done Nov 21, 2020
@w0rp
Copy link
Member

w0rp commented Nov 21, 2020

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.

ivorpeles pushed a commit to ivorpeles/ale that referenced this issue Nov 22, 2020
Code actions and ALERename now appear in the right click context menu
for GVim by default.
benknoble added a commit to benknoble/ale that referenced this issue Nov 30, 2020
* origin/master: (40 commits)
  fix: correct suggested filetype for yamlfix
  feat: add yamlfix fixer
  Use _config for LSP config options
  Add support for R languageserver (dense-analysis#3370)
  Fix 3103 - add shellcheck shell directive detection. (dense-analysis#3216)
  Added the Vundle command in installation instructions (dense-analysis#3400)
  Adds support for Tlint - A Tighten Opinionated PHP Linter (dense-analysis#3291)
  Add php phpcbf options (dense-analysis#3383)
  Use has('gui_running') instead of has('gui')
  Close dense-analysis#2727 - Add a hover-only setting for balloons
  Fix dense-analysis#3332 - Modify everything for rename/actions
  Add a missing blank line in documentation
  Add luafmt fixer (dense-analysis#3289)
  dense-analysis#3442 Fix code fix clangd issue
  Close dense-analysis#1466 - Add GVIM refactor menu support
  Look for node packages in .yarn/sdks as well
  Update documentation for code actions and rename
  cmp forwards, and reverse the code actions
  Support for LSP/tsserver Code Actions (dense-analysis#3437)
  Move the test for buffer-local variables
  ...
jsit added a commit to jsit/ale that referenced this issue Dec 26, 2020
* origin/master: (164 commits)
  fix: correct suggested filetype for yamlfix
  feat: add yamlfix fixer
  Use _config for LSP config options
  Add support for R languageserver (dense-analysis#3370)
  Fix 3103 - add shellcheck shell directive detection. (dense-analysis#3216)
  Added the Vundle command in installation instructions (dense-analysis#3400)
  Adds support for Tlint - A Tighten Opinionated PHP Linter (dense-analysis#3291)
  Add php phpcbf options (dense-analysis#3383)
  Use has('gui_running') instead of has('gui')
  Close dense-analysis#2727 - Add a hover-only setting for balloons
  Fix dense-analysis#3332 - Modify everything for rename/actions
  Add a missing blank line in documentation
  Add luafmt fixer (dense-analysis#3289)
  dense-analysis#3442 Fix code fix clangd issue
  Close dense-analysis#1466 - Add GVIM refactor menu support
  Look for node packages in .yarn/sdks as well
  Update documentation for code actions and rename
  cmp forwards, and reverse the code actions
  Support for LSP/tsserver Code Actions (dense-analysis#3437)
  Move the test for buffer-local variables
  ...
motato1 pushed a commit to motato1/ale that referenced this issue Jan 11, 2021
Code actions and ALERename now appear in the right click context menu
for GVim by default.
jsit added a commit to jsit/ale that referenced this issue Apr 19, 2021
* master: (35 commits)
  fix: correct suggested filetype for yamlfix
  feat: add yamlfix fixer
  Use _config for LSP config options
  Add support for R languageserver (dense-analysis#3370)
  Fix 3103 - add shellcheck shell directive detection. (dense-analysis#3216)
  Added the Vundle command in installation instructions (dense-analysis#3400)
  Adds support for Tlint - A Tighten Opinionated PHP Linter (dense-analysis#3291)
  Add php phpcbf options (dense-analysis#3383)
  Use has('gui_running') instead of has('gui')
  Close dense-analysis#2727 - Add a hover-only setting for balloons
  Fix dense-analysis#3332 - Modify everything for rename/actions
  Add a missing blank line in documentation
  Add luafmt fixer (dense-analysis#3289)
  dense-analysis#3442 Fix code fix clangd issue
  Close dense-analysis#1466 - Add GVIM refactor menu support
  Look for node packages in .yarn/sdks as well
  Update documentation for code actions and rename
  cmp forwards, and reverse the code actions
  Support for LSP/tsserver Code Actions (dense-analysis#3437)
  feat: add autoimport fixer
  ...
@WooterTheTroubleshooter
Copy link
Contributor

WooterTheTroubleshooter commented Feb 9, 2023

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
Copy link
Member

w0rp commented Mar 15, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement LSP Any issue relating to LSP or tsserver
Projects
Development

No branches or pull requests

9 participants