Skip to content

leverage new completion features to provide hover on (some) unsaved code #749

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

Merged
merged 3 commits into from
Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#### :rocket: New Feature

- Greatly extend completion abilities for unsaved code. WARNING: Might be a bit unstable initially. Report any issues you see. https://github.com/rescript-lang/rescript-vscode/pull/712
- Provide hovers for more unsaved code via the new completion features. https://github.com/rescript-lang/rescript-vscode/pull/749

## 1.14.0

Expand Down
2 changes: 1 addition & 1 deletion analysis/src/Commands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ let completion ~debug ~path ~pos ~currentFile =
Completions.getCompletions ~debug ~path ~pos ~currentFile ~forHover:false
with
| None -> []
| Some (completions, _) -> completions
| Some (completions, _, _) -> completions
in
print_endline
(completions
Expand Down
2 changes: 1 addition & 1 deletion analysis/src/Completions.ml
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ let getCompletions ~debug ~path ~pos ~currentFile ~forHover =
|> CompletionBackEnd.processCompletable ~debug ~full ~pos ~scope ~env
~forHover
in
Some (completables, full)))
Some (completables, full, scope)))
25 changes: 19 additions & 6 deletions analysis/src/Hover.ml
Original file line number Diff line number Diff line change
Expand Up @@ -125,31 +125,44 @@ let getHoverViaCompletions ~debug ~path ~pos ~currentFile ~forHover
~supportsMarkdownLinks =
match Completions.getCompletions ~debug ~path ~pos ~currentFile ~forHover with
| None -> None
| Some (completions, {file; package}) -> (
| Some (completions, ({file; package} as full), scope) -> (
let rawOpens = Scope.getRawOpens scope in
let allFiles =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: there's a bit of redundancy.
allFiles comes from package.
There are functions that take both allFiles and package (or full). So they' don't really need all those args.
I guess allFiles can be either a function of package, or directly a field there. So it's computed on the spot when needed instead of being passed around.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix in separate PR.

FileSet.union package.projectFiles package.dependenciesFiles
in
match completions with
| {kind = Label typString; docstring} :: _ ->
let parts =
(if typString = "" then [] else [Markdown.codeBlock typString])
@ docstring
in
Some (Protocol.stringifyHover (String.concat "\n\n" parts))
| {kind = Field _; docstring} :: _ -> (
match CompletionBackEnd.completionsGetTypeEnv completions with
| {kind = Field _; env; docstring} :: _ -> (
let opens = CompletionBackEnd.getOpens ~debug ~rawOpens ~package ~env in
match
CompletionBackEnd.completionsGetTypeEnv2 ~debug ~full ~rawOpens ~opens
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had not noticed much the business of completionsGetTypeEnv vs completionsGetTypeEnv2. Is there ever a reason for using the former given the latter is more powerful?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not. Mostly a bridge for migrating to the new one. Will take care of this as we do our major refactor of all of this new functionality.

~allFiles ~pos ~scope completions
with
| Some (typ, _env) ->
let typeString =
hoverWithExpandedTypes ~file ~package ~supportsMarkdownLinks typ
in
let parts = typeString :: docstring in
Some (Protocol.stringifyHover (String.concat "\n\n" parts))
| None -> None)
| _ -> (
match CompletionBackEnd.completionsGetTypeEnv completions with
| {env} :: _ -> (
let opens = CompletionBackEnd.getOpens ~debug ~rawOpens ~package ~env in
match
CompletionBackEnd.completionsGetTypeEnv2 ~debug ~full ~rawOpens ~opens
~allFiles ~pos ~scope completions
with
| Some (typ, _env) ->
let typeString =
hoverWithExpandedTypes ~file ~package ~supportsMarkdownLinks typ
in
Some (Protocol.stringifyHover typeString)
| None -> None))
| None -> None)
| _ -> None)

let newHover ~full:{file; package} ~supportsMarkdownLinks locItem =
match locItem.locType with
Expand Down
7 changes: 7 additions & 0 deletions analysis/tests/src/Hover.res
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,10 @@ type variant = | /** Cool variant! */ CoolVariant | /** Other cool variant */ Ot

let coolVariant = CoolVariant
// ^hov

// Hover on unsaved
// let fff = "hello"; fff
// ^hov

// switch x { | {someField} => someField }
// ^hov
1 change: 1 addition & 0 deletions analysis/tests/src/expected/Completion.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1593,6 +1593,7 @@ Raw opens: 2 Shadow.B.place holder ... Shadow.A.place holder
Resolved opens 2 Completion.res Completion.res
ContextPath Value[FAO, forAutoObject]
Path FAO.forAutoObject
Raw opens: 2 Shadow.B.place holder ... Shadow.A.place holder
{"contents": {"kind": "markdown", "value": "```rescript\n{\"age\": int, \"forAutoLabel\": FAR.forAutoRecord}\n```"}}

Hover src/Completion.res 352:17
Expand Down
22 changes: 22 additions & 0 deletions analysis/tests/src/expected/Hover.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,25 @@ Hover src/Hover.res 248:19
Hover src/Hover.res 253:20
{"contents": {"kind": "markdown", "value": "```rescript\nCoolVariant\n```\n\n Cool variant! \n\n```rescript\nvariant\n```\n\n---\n\n```\n \n```\n```rescript\ntype variant = CoolVariant | OtherCoolVariant\n```\nGo to: [Type definition](command:rescript-vscode.go_to_location?%5B%22Hover.res%22%2C251%2C0%5D)\n"}}

Hover src/Hover.res 257:23
Nothing at that position. Now trying to use completion.
posCursor:[257:23] posNoWhite:[257:22] Found expr:[257:22->257:25]
Pexp_ident fff:[257:22->257:25]
Completable: Cpath Value[fff]
ContextPath Value[fff]
Path fff
ContextPath string
{"contents": {"kind": "markdown", "value": "```rescript\nstring\n```"}}

Hover src/Hover.res 260:33
Nothing at that position. Now trying to use completion.
posCursor:[260:33] posNoWhite:[260:32] Found expr:[260:31->260:40]
Pexp_ident someField:[260:31->260:40]
Completable: Cpath Value[someField]
ContextPath Value[someField]
Path someField
ContextPath CPatternPath(Value[x])->recordField(someField)
ContextPath Value[x]
Path x
{"contents": {"kind": "markdown", "value": "```rescript\nbool\n```"}}