Skip to content

Polish: inlay hints and codelens performance #634

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.

Sign up for GitHub

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 4 commits into from
Nov 30, 2022
Merged

Polish: inlay hints and codelens performance #634

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ebdcfbe
polish: better performance for hints and codelens
aspeddro Nov 14, 2022
be67d21
update changelog
aspeddro Nov 14, 2022
0f26053
rename Cmt.fullFromPath to Cmt.loadFullCmtFromPath
aspeddro Nov 30, 2022
de5fdf2
sync master
aspeddro Nov 30, 2022
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

- Remove spacing between type definition in clients that do not support markdown links. https://github.com/rescript-lang/rescript-vscode/pull/619
- Rename custom LSP methods names. https://github.com/rescript-lang/rescript-vscode/pull/611
- Better performace for Inlay Hints and Codelens.

#### :bug: Bug Fix

Expand Down
12 changes: 10 additions & 2 deletions analysis/src/Commands.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,19 @@ let completion ~debug ~path ~pos ~currentFile =
|> Protocol.array)

let inlayhint ~path ~pos ~maxLength ~debug =
let result = Hint.inlay ~path ~pos ~maxLength ~debug |> Protocol.array in
let result =
match Hint.inlay ~path ~pos ~maxLength ~debug with
| Some hints -> hints |> Protocol.array
| None -> Protocol.null
in
print_endline result

let codeLens ~path ~debug =
let result = Hint.codeLens ~path ~debug |> Protocol.array in
let result =
match Hint.codeLens ~path ~debug with
| Some lens -> lens |> Protocol.array
| None -> Protocol.null
in
print_endline result

let hover ~path ~pos ~currentFile ~debug ~supportsMarkdownLinks =
Expand Down
141 changes: 65 additions & 76 deletions analysis/src/Hint.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ let inlay ~path ~pos ~maxLength ~debug =
if start_line <= range.end_.line && end_line >= range.start.line then
hints := (range, kind) :: !hints
in
let rec processFunction (exp : Parsetree.expression) =
match exp.pexp_desc with
| Pexp_fun (_, _, pat_exp, e) -> (
match pat_exp with
| {ppat_desc = Ppat_var _} ->
push pat_exp.ppat_loc Type;
processFunction e
| _ -> processFunction e)
| _ -> ()
in
let rec processPattern (pat : Parsetree.pattern) =
match pat.ppat_desc with
| Ppat_tuple pl -> pl |> List.iter processPattern
Expand All @@ -77,14 +67,6 @@ let inlay ~path ~pos ~maxLength ~debug =
push vb.pvb_pat.ppat_loc Type
| {pvb_pat = {ppat_desc = Ppat_tuple _}} -> processPattern vb.pvb_pat
| {pvb_pat = {ppat_desc = Ppat_record _}} -> processPattern vb.pvb_pat
| {
pvb_pat = {ppat_desc = Ppat_var _};
pvb_expr = {pexp_desc = Pexp_fun (_, _, pat, e)};
} ->
(match pat with
| {ppat_desc = Ppat_var _} -> push pat.ppat_loc Type
| _ -> ());
processFunction e
| _ -> ());
Ast_iterator.default_iterator.value_binding iterator vb
in
Expand All @@ -95,38 +77,41 @@ let inlay ~path ~pos ~maxLength ~debug =
in
let {Res_driver.parsetree = structure} = parser ~filename:path in
iterator.structure iterator structure |> ignore);
!hints
|> List.filter_map (fun ((range : Protocol.range), hintKind) ->
match Cmt.fullFromPath ~path with
| None -> None
| Some full -> (
match
References.getLocItem ~full
~pos:(range.start.line, range.start.character + 1)
~debug
with
| None -> None
| Some locItem -> (
let position : Protocol.position =
{line = range.start.line; character = range.end_.character}
in
match locItemToTypeHint locItem ~full with
| Some label -> (
let result =
Protocol.stringifyHint
{
kind = inlayKindToNumber hintKind;
position;
paddingLeft = true;
paddingRight = false;
label = ": " ^ label;
}
match Cmt.fullFromPath ~path with
| None -> None
| Some full ->
let result =
!hints
|> List.filter_map (fun ((range : Protocol.range), hintKind) ->
match
References.getLocItem ~full
~pos:(range.start.line, range.start.character + 1)
~debug
with
| None -> None
| Some locItem -> (
let position : Protocol.position =
{line = range.start.line; character = range.end_.character}
in
match maxlen with
| Some value ->
if String.length label > value then None else Some result
| None -> Some result)
| None -> None)))
match locItemToTypeHint locItem ~full with
| Some label -> (
let result =
Protocol.stringifyHint
{
kind = inlayKindToNumber hintKind;
position;
paddingLeft = true;
paddingRight = false;
label = ": " ^ label;
}
in
match maxlen with
| Some value ->
if String.length label > value then None else Some result
| None -> Some result)
| None -> None))
in
Some result

let codeLens ~path ~debug =
let lenses = ref [] in
Expand Down Expand Up @@ -156,30 +141,34 @@ let codeLens ~path ~debug =
in
let {Res_driver.parsetree = structure} = parser ~filename:path in
iterator.structure iterator structure |> ignore);
!lenses
|> List.filter_map (fun (range : Protocol.range) ->
match Cmt.fullFromPath ~path with
| None -> None
| Some full -> (
match
References.getLocItem ~full
~pos:(range.start.line, range.start.character + 1)
~debug
with
| Some {locType = Typed (_, typeExpr, _)} ->
Some
(Protocol.stringifyCodeLens
{
range;
command =
Some
{
(* Code lenses can run commands. An empty command string means we just want the editor
to print the text, not link to running a command. *)
command = "";
(* Print the type with a huge line width, because the code lens always prints on a
single line in the editor. *)
title = typeExpr |> Shared.typeToString ~lineWidth:400;
};
})
| _ -> None))
match Cmt.fullFromPath ~path with
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is great. Question for the future: does it make sense to have some sort of cache inside of Cmt.fullFromPath directly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The change looks good. Perhaps it's possible to rename fullFromPath to something that makes it clear it's an expensive operation. E.g. loadFullCmtFromPath.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 0f26053

| None -> None
| Some full ->
let result =
!lenses
|> List.filter_map (fun (range : Protocol.range) ->
match
References.getLocItem ~full
~pos:(range.start.line, range.start.character + 1)
~debug
with
| Some {locType = Typed (_, typeExpr, _)} ->
Some
(Protocol.stringifyCodeLens
{
range;
command =
Some
{
(* Code lenses can run commands. An empty command string means we just want the editor
to print the text, not link to running a command. *)
command = "";
(* Print the type with a huge line width, because the code lens always prints on a
single line in the editor. *)
title =
typeExpr |> Shared.typeToString ~lineWidth:400;
};
})
| _ -> None)
in
Some result
24 changes: 0 additions & 24 deletions analysis/tests/src/expected/InlayHint.res.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,36 +35,12 @@ Inlay Hint src/InlayHint.res 1:34
"kind": 1,
"paddingLeft": true,
"paddingRight": false
}, {
"position": {"line": 14, "character": 17},
"label": ": string",
"kind": 1,
"paddingLeft": true,
"paddingRight": false
}, {
"position": {"line": 10, "character": 24},
"label": ": int",
"kind": 1,
"paddingLeft": true,
"paddingRight": false
}, {
"position": {"line": 8, "character": 10},
"label": ": int",
"kind": 1,
"paddingLeft": true,
"paddingRight": false
}, {
"position": {"line": 6, "character": 15},
"label": ": int",
"kind": 1,
"paddingLeft": true,
"paddingRight": false
}, {
"position": {"line": 6, "character": 12},
"label": ": int",
"kind": 1,
"paddingLeft": true,
"paddingRight": false
}, {
"position": {"line": 4, "character": 8},
"label": ": char",
Expand Down