ale icon indicating copy to clipboard operation
ale copied to clipboard

C/C++ ALEGoToDeclaration (or ALEGoToImplementation)

Open paulreimer opened this issue 6 years ago • 23 comments

Since there is ALEGoToDefinition, maybe there should be an ALEGoToDeclaration for languages (like C/C++) which have those separately (e.g. the ALEGoToDefinition should go to the right place in the *.h file, and the ALEGoToDeclaration should go to the right place in the corresponding *.cpp file.)

FWIW, YouCompleteMe has a similar feature: https://github.com/Valloric/YouCompleteMe#the-gotodeclaration-subcommand

paulreimer avatar May 31 '19 20:05 paulreimer

There's :ALEGoToTypeDefinition, does that do what we want?

w0rp avatar Jun 01 '19 15:06 w0rp

I think ALEGoToTypeDefinition already covers this.

w0rp avatar Jun 10 '19 20:06 w0rp

:ALEGoToTypeDefinition doesn't seem to work for me in C++, it does the same thing as ALEGoToDefinition. I use clangd+compile_commands.json to get my type info (I have a lot of other linters installed, but they don't read compile_commands -- I'm not sure how they all combine in ALE).

paulreimer avatar Jul 11 '19 04:07 paulreimer

That might be something that needs to be fixed in the language server. I can't see a "go to declaration" command command in the language server protocol specification. The only options appear to be "go to definition," "go to type definition," and "go to implementation." "Go to type definition" looks like the closest thing to a "go to declaration" command.

w0rp avatar Jul 14 '19 14:07 w0rp

Is there an ALEGoToImplementation command? I think that if that's something a language server provides, then "Go to implementation" sounds like it might work here to go to e.g. the definition of a function in a *.cpp file.

The terms are somewhat confusing and language server seems to call them the opposite of what I'd call them, but here's what I would be interested to try:

  • ALEGoToDefinition -> goes to function declaration in *.h (this is the current behaviour, even though I think it is a declaration)
  • ALEGoToImplementation -> goes to function definition in *.cpp (I don't think this command exists yet in ALE, but I'd be curious what the language server does)
  • ALEGoToTypeDefinition -> maybe this doesn't apply to C++, or maybe e.g. if you had the cursor placed on a function that returns a class, then GoToTypeDefinition would go to the definition of the class?

paulreimer avatar Jul 15 '19 19:07 paulreimer

Since there is ALEGoToDefinition, maybe there should be an ALEGoToDeclaration for languages (like C/C++) which have those separately (e.g. the ALEGoToDefinition should go to the right place in the *.h file, and the ALEGoToDeclaration should go to the right place in the corresponding *.cpp file.)

FWIW, YouCompleteMe has a similar feature: https://github.com/Valloric/YouCompleteMe#the-gotodeclaration-subcommand

I think you got it the other way around. ALEGoToDefinition should bring you to the implementation in the *.cpp file, while ALEGoToDeclaration should bring you to the declaration in the *.h file.

But I agree, this functionality would be really nice to see :) For example, vim-lsp implements this feature.

P.S. Thank you @w0rp for this plugin, it's awesome!

proxict avatar Jul 19 '19 14:07 proxict

I agree with @proxict that terminology would be most correct for C++.

I had hoped it would also handle the case for other languages which only use a single definition+declaration point, so in that case, as long as both GoToDefinition and GoToDeclaration commands do the same thing it should be fine. (For key mapping simplicity, so I wouldn't need per-language mappings e.g. to prefer GoToDefinition in C++ files)

paulreimer avatar Jul 19 '19 15:07 paulreimer

@w0rp, can this issue be reopened? This feature would be so nice to have for languages which require a separate declaration from definition.

proxict avatar Jul 26 '19 14:07 proxict

This can be re-opened as the language server protocol specification has since been updated to include a "go to declaration" command. https://microsoft.github.io/language-server-protocol/specification#textDocument_declaration It might not be implemented by a lot of servers yet.

The "go to implementation" command has been around for longer and has a different purpose. https://microsoft.github.io/language-server-protocol/specification#textDocument_implementation

w0rp avatar Jul 29 '19 21:07 w0rp

Typescript file also has same problem. we need go to implementation to find a implementation of a interface. So far, as I know, the coc has this feature. So far if I use ale and coc, both of them will launch a ts-server, which make two node process to eat up my cpu. I think I can use ale only if has go to implementation. Thanks :)

maple-leaf avatar Jan 11 '20 09:01 maple-leaf

Hi, I'm very late in this but, I was about to open a new ticket to request exactly this. In C/C++ jumping from declaration to implementation is very handy, I'd say indispensable.

I have no idea of how vimscript works, but @w0rp, if this is a low-hanging fruit, I... could try and hack this?

s-marios avatar Dec 24 '21 05:12 s-marios

if this is a low-hanging fruit, I... could try and hack this?

Spent an hour or so, and here are my findings:

Hooking up ALEGoToImplementation was surprisingly easy.

  1. Declare it in plugin/ale.vim
  2. Add one more code path to the ale#definition#GoToCommandHandler with an argument of 'implementation'
  3. Copy-modify the ale#definition#GoToTypeDefinition into GoToImplementation by just changing the capability to 'implementation'

However, using clangd, ale#lsp#HasCapability promptly throws an exception with the message: Exception not caught: Invalid capability implementation

Unless the capability is named something other than implementation, I'm fairly certain then that neither clangd nor ccls for that matter support this capability (sad face).

So, I think that as things stand right now, this cannot really be resolved at the LSP server level (ALE doesn't support the capability yet, C LSPs don't either) and we'll have to rely on other plugins.

However, any interest in the GoToImplementation path for the future? Probably catching the exception and reporting that the lsp doesn't support the capability/silently ignoring it seems good enough to me..

s-marios avatar Dec 24 '21 07:12 s-marios

And for posterity, the formatted patch:

From 981753ff731ad954485ad1cf2e9d319e90a52685 Mon Sep 17 00:00:00 2001
From: Marios Sioutis <[email protected]>
Date: Fri, 24 Dec 2021 16:49:20 +0900
Subject: [PATCH] Support for GoToImplementation

---
 autoload/ale/definition.vim | 10 ++++++++++
 plugin/ale.vim              |  3 +++
 2 files changed, 13 insertions(+)

diff --git a/autoload/ale/definition.vim b/autoload/ale/definition.vim
index 9574017..caaed5a 100644
--- a/autoload/ale/definition.vim
+++ b/autoload/ale/definition.vim
@@ -166,6 +166,14 @@ function! ale#definition#GoToType(options) abort
     endfor
 endfunction
 
+function! ale#definition#GoToImplementation(options) abort
+    for l:linter in ale#linter#Get(&filetype)
+        if !empty(l:linter.lsp)
+            call s:GoToLSPDefinition(l:linter, a:options, 'implementation')
+        endif
+    endfor
+endfunction
+
 function! ale#definition#GoToCommandHandler(command, ...) abort
     let l:options = {}
 
@@ -191,6 +199,8 @@ function! ale#definition#GoToCommandHandler(command, ...) abort
 
     if a:command is# 'type'
         call ale#definition#GoToType(l:options)
+    elseif a:command is# 'implementation'
+        call ale#definition#GoToImplementation(l:options)
     else
         call ale#definition#GoTo(l:options)
     endif
diff --git a/plugin/ale.vim b/plugin/ale.vim
index 423a743..cc4fe4d 100644
--- a/plugin/ale.vim
+++ b/plugin/ale.vim
@@ -246,6 +246,9 @@ command! -bar -nargs=* ALEGoToDefinition :call ale#definition#GoToCommandHandler
 " Go to type definition for tsserver and LSP
 command! -bar -nargs=* ALEGoToTypeDefinition :call ale#definition#GoToCommandHandler('type', <f-args>)
 
+" Go to implementation for tsserver and LSP
+command! -bar -nargs=* ALEGoToImplementation :call ale#definition#GoToCommandHandler('implementation', <f-args>)
+
 " Repeat a previous selection in the preview window
 command! -bar ALERepeatSelection :call ale#preview#RepeatSelection()
 
-- 
2.33.0

s-marios avatar Dec 24 '21 07:12 s-marios

@s-marios can you check the pr i raise does it help with go to implementation

sak96 avatar Apr 19 '22 17:04 sak96

Appreciated if anyone can verify #4160 solves this issue.

hsanson avatar Apr 30 '22 04:04 hsanson

@sak96 @hsanson

Sorry for taking this long to provide feedback, but I think I'm losing my mind here. Let me explain.

  1. Updated ALE and all my plugins
  2. I tried :ALEGoToImplementation on a c header file, expecting to jump to the .c file, but nothing happened. Maybe clangd doesn't support it?
  3. [The most interesting thing] :ALEGoToDefinition now jumps from the .c file to .h, but also from .h back to .c ! And I'm very surprised, this was definitely not the case when I jumped into this discussion.

FWIW, the behavior in 2. is good enough for me. If you have something more specific that you want me to try out I'll be glad to!

s-marios avatar May 18 '22 01:05 s-marios

@s-marios

it seems there are few issue w.r.t. clangd it doesn't seem to work well even in vs code and neovim and ...

can you check this once?

Also creating a mini repo (hello world) which can demonstrate the issue may help.

sak96 avatar May 18 '22 05:05 sak96

@sak96 Had a bit of time and tried your suggestions on a hello_world project. TL;DR: if a compile_commands.json is present, behavior is the same as my previous response. ALEGoToImplementation does not work, regardless of the order of opening the files.

If there's no compile_commands.json, neither ALEGoToDefinition, nor ALEGoToImplementation work for me, and essentially I can't jump around at all. I guess clangd only works well with compile_commands.json present..

s-marios avatar May 20 '22 00:05 s-marios

Could be clang specific issue clangd/clangd#893

sak96 avatar May 21 '22 05:05 sak96

clangd is a language server so it requires a project root to be starterd. You can see what project root is being used by ALE by running this command:

:echo ale#c#FindProjectRoot('')

If this returns empty then no project root was found and clangd won't be started. This means all LSP commands (e..g ALEGoToImplementation) won't work.

From the source I can see ALE looks for these files in this order to figure out the root:

  • compile_commands.json
  • .git/HEAD
  • configure
  • Makefile
  • CMakeLists.txt

Also from clangd documentation https://clangd.llvm.org/features.html#cross-references it seems that these cross-reference functions only work for files you have opened when using clangd-7 and for the whole project when using clangd-9.

hsanson avatar May 22 '22 00:05 hsanson

@hsanson FindProjectRoot reports the project root correctly. However, :ALEGoToImplementation still does not do anything. :ALEGoToDefinition does jump back and forth between declaration and implementation, as in the clangd documentation you linked.

I'm using clangd-13.

s-marios avatar May 23 '22 06:05 s-marios

Maybe relevant information here:

  • https://github.com/clangd/clangd/issues/789#issuecomment-850132259

I do not use clangd so I am not sure what exactly should happen when invoking ALEGoToImplementation. From the discussion I linked it seems that this works on certain cases (derived classes and virtual methods)?

hsanson avatar May 23 '22 12:05 hsanson

Thanks for digging through this. It seems that nothing happening when calling ALEGoToImplementation is a clangd behavior after all.

s-marios avatar May 25 '22 02:05 s-marios