Skip to content
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

Polish: inlay hints and codelens performance #634

Merged
merged 4 commits into from
Nov 30, 2022
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 @@ -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 performance for Inlay Hints and Codelens.
- Accept both `@ns.doc` and the new `@res.doc` for the internal representation of doc comments. And both `@ns.optional` and `@res.optional` for the optional fields. https://github.com/rescript-lang/rescript-vscode/pull/642

#### :bug: Bug Fix
Expand Down
2 changes: 1 addition & 1 deletion analysis/src/Cmt.ml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ let fullFromModule ~package ~moduleName =
fullFromUri ~uri
else None

let fullFromPath ~path =
let loadFullCmtFromPath ~path =
let uri = Uri.fromPath path in
fullFromUri ~uri
24 changes: 16 additions & 8 deletions analysis/src/Commands.ml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ let getCompletions ~debug ~path ~pos ~currentFile ~forHover =
Printf.printf "Completable: %s\n"
(SharedTypes.Completable.toString completable);
(* Only perform expensive ast operations if there are completables *)
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> []
| Some {file; package} ->
let env = SharedTypes.QueryEnv.fromFile file in
Expand All @@ -29,16 +29,24 @@ 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 =
let result =
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> Protocol.null
| Some full -> (
match References.getLocItem ~full ~pos ~debug with
Expand Down Expand Up @@ -94,7 +102,7 @@ let codeAction ~path ~pos ~currentFile ~debug =

let definition ~path ~pos ~debug =
let locationOpt =
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> None
| Some full -> (
match References.getLocItem ~full ~pos ~debug with
Expand Down Expand Up @@ -130,7 +138,7 @@ let definition ~path ~pos ~debug =

let typeDefinition ~path ~pos ~debug =
let maybeLocation =
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> None
| Some full -> (
match References.getLocItem ~full ~pos ~debug with
Expand All @@ -149,7 +157,7 @@ let typeDefinition ~path ~pos ~debug =

let references ~path ~pos ~debug =
let allLocs =
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> []
| Some full -> (
match References.getLocItem ~full ~pos ~debug with
Expand All @@ -175,7 +183,7 @@ let references ~path ~pos ~debug =

let rename ~path ~pos ~newName ~debug =
let result =
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> Protocol.null
| Some full -> (
match References.getLocItem ~full ~pos ~debug with
Expand Down
141 changes: 65 additions & 76 deletions analysis/src/Hint.ml
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.loadFullCmtFromPath ~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.loadFullCmtFromPath ~path with
| 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
2 changes: 1 addition & 1 deletion analysis/src/Hover.ml
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ let getHoverViaCompletions ~debug ~path ~pos ~currentFile ~forHover
Printf.printf "Completable: %s\n"
(SharedTypes.Completable.toString completable);
(* Only perform expensive ast operations if there are completables *)
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> None
| Some {file; package} -> (
let env = SharedTypes.QueryEnv.fromFile file in
Expand Down
2 changes: 1 addition & 1 deletion analysis/src/SignatureHelp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ let findFunctionType ~currentFile ~debug ~path ~pos =
with
| None -> None
| Some (completable, scope) -> (
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| None -> None
| Some {file; package} ->
let env = QueryEnv.fromFile file in
Expand Down
2 changes: 1 addition & 1 deletion analysis/src/Xform.ml
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ let parse ~filename =
(structure, printExpr, printStructureItem)

let extractCodeActions ~path ~pos ~currentFile ~debug =
match Cmt.fullFromPath ~path with
match Cmt.loadFullCmtFromPath ~path with
| Some full when Files.classifySourceFile currentFile = Res ->
let structure, printExpr, printStructureItem =
parse ~filename:currentFile
Expand Down
24 changes: 0 additions & 24 deletions analysis/tests/src/expected/InlayHint.res.txt
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
zth marked this conversation as resolved.
Show resolved Hide resolved
}, {
"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