Polish: inlay hints and codelens performance (#634)

aspeddro · web-flow · commit 75bccdd4939f · 2022-11-30T21:00:12.000+01:00
* polish: better performance for hints and codelens

* update changelog

* rename Cmt.fullFromPath to Cmt.loadFullCmtFromPath
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
diff --git a/analysis/src/Cmt.ml b/analysis/src/Cmt.ml
@@ -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
diff --git a/analysis/src/Commands.ml b/analysis/src/Commands.ml
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/analysis/src/Hint.ml b/analysis/src/Hint.ml
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/analysis/src/Hover.ml b/analysis/src/Hover.ml
@@ -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
diff --git a/analysis/src/SignatureHelp.ml b/analysis/src/SignatureHelp.ml
@@ -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
diff --git a/analysis/src/Xform.ml b/analysis/src/Xform.ml
@@ -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
diff --git a/analysis/tests/src/expected/InlayHint.res.txt b/analysis/tests/src/expected/InlayHint.res.txt
@@ -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",
