Skip to content

Commit

Permalink
Improve support for navigating to definitions in external modules (#3263
Browse files Browse the repository at this point in the history
)

I rewrote most of `definition_handler` to account for edge cases related to finding definitions in external files. 

- It's now possible to jump to the definition of an import identifier (rather than having to click on a qualified member name). For example, `import M|ap "HashMap"` (where `|` is the cursor) jumps to the top of `HashMap.mo` instead of doing nothing (fixes dfinity/vscode-motoko#17). 

- The new implementation also has a more permissive default behavior for missing declarations. In cases where the module path is known but the qualified symbol is unknown, the language server attempts to direct the user to the relevant module. This is mostly intended as a fallback for situations where the index is missing an expected declaration, such as when the other file has a syntax error. 

Possible future improvements:

- Using the start/end position of the `module` or equivalent keyword (when possible) rather than the top of the file
- Providing the same functionality when the cursor is on the import string literal (and perhaps even the `import` keyword)
- Supporting object pattern-style imports as mentioned in #3078 (currently experimenting with this)

Passes all tests on my WSL setup and works as expected in VSCode with the [official Motoko extension](https://github.com/dfinity/vscode-motoko). 

(@matthewhammer)
  • Loading branch information
rvanasa authored Jun 3, 2022
1 parent 9ad095f commit 4067378
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 10 deletions.
45 changes: 35 additions & 10 deletions src/languageServer/definition.ml
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,42 @@ let definition_handler index position file_contents project_root file_path =
Source_file.identifier_at_pos project_root file_path file_contents
position
in
let* path, region =
(* Find the relevant path for the declaration *)
let* path =
match ident with
| Alias _ -> None
| Alias (_, path) -> Some path
| Unresolved _ -> None
| Resolved resolved ->
let* module_ = DI.lookup_module project_root resolved.path index in
find_named resolved.ident module_
| Ident ident ->
let* uri = Lib.FilePath.relative_to project_root file_path in
let* module_ = DI.lookup_module project_root uri index in
find_named ident module_
| Resolved { path; _ } -> Some path
| Ident _ -> Lib.FilePath.relative_to project_root file_path
in
let* module_ = DI.lookup_module project_root path index in
(* Update `path` from the relevant module *)
let path, _ = module_ in
(* Try to find the start/end cursor range in the relevant module *)
let* decl_range =
match
let* decl_ident =
match ident with
| Alias _ -> None
| Unresolved { ident; _ } -> Some ident (* Currently unreachable *)
| Resolved { ident; _ } -> Some ident
| Ident ident -> Some ident
in
(* Note: ignoring `path` output value from `find_named` *)
let* _, region = find_named decl_ident module_ in
Some (range_of_region region)
with
| Some range -> Some range
| None -> (
match ident with
| Ident _ -> None (* Unresolved identifiers in the same file *)
| _ ->
(* By default, point to the top of the relevant file *)
(* TODO: use the module declaration start/end positions when possible *)
let file_start =
Lsp.{ position_line = 0; position_character = 0 }
in
Some Lsp.{ range_start = file_start; range_end_ = file_start })
in
Some
Lsp.
Expand All @@ -55,7 +80,7 @@ let definition_handler index position file_contents project_root file_path =
(if Source_file.is_non_file_path path then
Option.get (Source_file.uri_for_package path)
else Vfs.uri_from_file path);
location_range = range_of_region region;
location_range = decl_range;
}
in
`TextDocumentDefinitionResponse (Option.to_list location)
7 changes: 7 additions & 0 deletions test/lsp-int/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,13 @@ main = do
(Position 5 31)
[("mydependency/lib.mo", Range (Position 5 17) (Position 5 24))]

log "Definition for an imported module alias"
definitionsTestCase
project
doc
(Position 1 10)
[("lib/list.mo", Range (Position 0 0) (Position 0 0))]

log "Completion tests"
log "Completing top level definitions"
withDoc "ListClient.mo" \doc -> do
Expand Down

0 comments on commit 4067378

Please sign in to comment.