Shopify
Definition support for autoloaded constants
Motivation
There are an issue calling for complete this function (https://github.com/Shopify/ruby-lsp/issues/1893).
Implementation
autoload argument is a Prism::CallNode which has name :autoload. I create handle_autoload_definition method and add it to on_call_node_enter method in 'lib/ruby_lsp/listeners/definition.rb'. Then in handle_autoload_definition method, we can use the name of the constant in the Prism::SymbolNode under the Prism::CallNode and pass the name to find_in_index, which can create the response of jumping.
Automated Tests
Completed automated tests in test/requests/definition_expectations_test.rb, including test_jumping_to_autoload_definition_when_declaration_exists, test_jumping_to_autoload_definition_with_two_definition and test_does_nothing_when_autoload_declaration_does_not_exist
Manual Tests
- Start the extension on this branch
- open an
.rbfile which hasautoloadarguments. - move the cursor above the
autoloadarguments, press ctrl & click it - sometimes it doesn't work in original file of ruby-lsp since it's a sorbet project
There is one typecheck failure, it may indicate a missing nil check.
@andyw8 Did you mean this error？ I have fixed it.
Sorry, the development on the LSP is moving fast and I feel like you keep chasing a moving target.
We made changes to allow for more precise jump to definition based on a method call and its arguments. You can see an example for require definition.
The idea is that the event now matches on_string_node_enter, then checks if the enclosing call node is a require.
You will need to do the exact same thing, but for on_symbol_node_enter. Then you check the enclosing call node to verify if it's an autoload.
@Super-Xray Thanks for the PR! Because we want to have autoload's definition support to be released soon-ish to match other code navigation features we made recently, I took the liberty to open https://github.com/Shopify/ruby-lsp/pull/2395 with your commits and built some small improvements on top of it.
(I tried to merge this PR first by resolving the conflicts, but then I found some tests need to be updated as well to account for some recent changes. Since GH UI doesn't allow me to update non-conflicting files, nor can I push to your fork, I needed to open another PR instead).
