move
move copied to clipboard
[move-analyzer] function signature help
Motivation
This is a follow-up PR from https://github.com/move-language/move/pull/338. Providing signature help should help developers when calling a function inside a file.
Have you read the Contributing Guidelines on pull requests?
Yes.
Test Plan
(Share your test plan here. If you changed code, please provide us with clear instructions for verifying that your changes work.)
- Main test cases have been discussed and covered in
signatureHelp.test.ts
- Open the extension host in debug mode. You should see a signature help popup when calling a function.
- There are two key parts of a signature help. The first is the signature label and the second is the position of the active parameter, and you should see the expected signature label and the expected active parameter position when you move the cursor around.
Demo
https://www.loom.com/share/16e5c241c78a4433bae71046089b73b6
I have also discovered a problem with the previous PR (https://github.com/move-language/move/pull/338) underlying this one. I added a comment there even though the PR is already merged: https://github.com/move-language/move/pull/338/files#r948387534
Please provide a fix first along with appropriate test before the current PR is merged.
While it might have not mattered in https://github.com/move-language/move/pull/338 where the only thing we cared about was to see if a given identifier is a function, I am pretty sure it will this the current PR as we may mis-assign signatures (only one of two functions in the file will be in the map).
EDIT:
It seems like the simplest way to support this would be to have a list of TypeIdent
s in the BTreeMap
instead of only one as signature help response supports passing multiple signatures. If you decide to go for this solution (or another one for that matter), please create a separate PR rather than bundling this with the current one.
If we want to be fancy, we may consider figuring out where the cursor is (in terms of enclosing module) and mark a signature for a function defined in this module (if any) as the active one.
ANOTHER EDIT: Actually, let me provide the fix. I need it done in a specific way that will provide some additional data I need for another enhancement.
ONE MORE EDIIT:
I have addressed this in https://github.com/move-language/move/pull/389 (please note that I removed the IdentType
for functions is now available via Symbols::file_mods
.
I have addressed this in #389 (please note that I removed the
IdentType
for functions is now available viaSymbols::file_mods
.
Sounds good, let me push the fixes using the latest changes.
I do not get the parameter name auto-completion
Sorry, what do you mean by parameter name auto-completion? Is this addressed in your #389 PR already?
Given the cursor position of the current line, I need to find out the function name and the corresponding module that it belongs. It seems like I can reuse some of the logic from Symbolicator::mod_call_symbols
, but maybe it's better to do custom string parsing and then extract the correct information from file_mods
?
Edit: I think I will check out on_hover_request
and see if I can reuse any of the logic there.
Given the cursor position of the current line, I need to find out the function name and the corresponding module that it belongs.
I thought that you could simply use Symbols::file_mods
. In most cases there will be one module per file, and even if there isn't, iterating over the list of modules in the set shouldn't be a huge deal (in terms of speed and complexity). Unless I misunderstand what information you need to obtain and how.
I thought that you could simply use
Symbols::file_mods
. In most cases there will be one module per file, and even if there isn't, iterating over the list of modules in the set shouldn't be a huge deal (in terms of speed and complexity). Unless I misunderstand what information you need to obtain and how.
Additionally, I also need to find the current enclosing scope given a cursor position, and determine which module the developer is calling the function from.
Additionally, I also need to find the current enclosing scope given a cursor position, and determine which module the developer is calling the function from.
Good point. Here are some thoughts.
First off, it may make sense to special-case implementation for one-module-per-file - we don't have to worry about multiple modules in this case (which should be the common case).
In terms of figuring out module scope (in terms of file lines), when constructing ModuleDefs
, we iterate over typed AST unique map that gives us location of modules (I think) in pos
:
https://github.com/move-language/move/blob/70b34a66473c34ad30d101290b249f2db3c847a2/language/move-analyzer/src/symbols.rs#L708
We currently only store the starting position of the module but this should give us the ending position as well, which we could use to figure out module's scope within a file. To make it fully robust we'd probably have to make an adjustment for a new line (or lines) being inserted that would change start/end of modules in the file, but I am not sure if this is feasible at the moment.
If this does not work for some reason, then I believe a function call can only be inserted inside another function. If we then remembered both starting line of function definition (we already have this in FunctionDef
) and the ending line (we could conceivably obtain it from the outermost code block in function's body) we could use Symbol::file_mods
to iterate over all functions in a given file and figure out if the cursor is located inside one of them. This is potentially more fragile than the previous suggestion, but I haven't checked if module start/end locations are indeed faithfully represented in the AST.
First off, it may make sense to special-case implementation for one-module-per-file - we don't have to worry about multiple modules in this case (which should be the common case).
I am going with the case where we have one-module-per-file for now. I have added a note in the on_signature_help_request
function in case we want to address this in the future.
I am going with the case where we have one-module-per-file for now. I have added a note in the
on_signature_help_request
function in case we want to address this in the future.
What are you going to to in the short term then if we have two modules in a file and each module defines a function with the same name? Are you going to list them both in the function signature help?
What are you going to to in the short term then if we have two modules in a file and each module defines a function with the same name? Are you going to list them both in the function signature help?
The current implementation just returns the signature help of the function in the first module with that function name that we encounter in file_mods
. However, if this behaviour is not acceptable, I can update the implementation to return the signature help of the function inside the enclosing module only in the short-term.
But feel free to propose an alternative solution.
@awelc can you please review?