Merge branch 'master' into fix-markdown-divider-render

Author: aspeddro

CHANGELOG.md

@@ -13,12 +13,23 @@
 ## master
 
 #### :nail_care: Polish
+
 - 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
+
 #### :bug: Bug Fix
 
 - Fix issue where `-open Some.Path` in `"bsc-flags"` would sometimes be treated differently from `open Some.Path` locally in a file https://github.com/rescript-lang/rescript-vscode/pull/616
 
+- Fix issue where doc comment is not shown on hover in case of shadowed identifier (in particular for JSX V4 components which shadow `make`) https://github.com/rescript-lang/rescript-vscode/issues/621
+
+- Adapt command to create interface files to latest JSX V4 (no key prop, possibly empty record) https://github.com/rescript-lang/rescript-vscode/issues/617
+
+- Fix issue where pipes were not taken into account in the signature help, resulting in the highlighted argument in signature help always being off by one for unlabelled arguments in piped expressions https://github.com/rescript-lang/rescript-vscode/issues/618
+
+- Fix file location in Document Symbols response. https://github.com/rescript-lang/rescript-vscode/issues/629
+
 ## v1.8.2
 
 #### :rocket: New Feature

analysis/src/CompletionBackEnd.ml

@@ -1283,10 +1283,18 @@ let rec getCompletionsForContextPath ~package ~opens ~rawOpens ~allFiles ~pos
       |> completionsGetTypeEnv
     with
     | Some (typ, _envNotUsed) -> (
-      let arrayModulePath = ["Js"; "Array2"] in
-      let listModulePath = ["Belt"; "List"] in
-      let optionModulePath = ["Belt"; "Option"] in
-      let stringModulePath = ["Js"; "String2"] in
+      let {
+        arrayModulePath;
+        optionModulePath;
+        stringModulePath;
+        intModulePath;
+        floatModulePath;
+        promiseModulePath;
+        listModulePath;
+        resultModulePath;
+      } =
+        package.builtInCompletionModules
+      in
       let getModulePath path =
         let rec loop (path : Path.t) =
           match path with
@@ -1296,9 +1304,15 @@ let rec getCompletionsForContextPath ~package ~opens ~rawOpens ~allFiles ~pos
         in
         match path with
         | Path.Pident id when Ident.name id = "array" -> arrayModulePath
-        | Path.Pident id when Ident.name id = "list" -> listModulePath
         | Path.Pident id when Ident.name id = "option" -> optionModulePath
         | Path.Pident id when Ident.name id = "string" -> stringModulePath
+        | Path.Pident id when Ident.name id = "int" -> intModulePath
+        | Path.Pident id when Ident.name id = "float" -> floatModulePath
+        | Path.Pident id when Ident.name id = "promise" -> promiseModulePath
+        | Path.Pident id when Ident.name id = "list" -> listModulePath
+        | Path.Pident id when Ident.name id = "result" -> resultModulePath
+        | Path.Pident id when Ident.name id = "lazy_t" -> ["Lazy"]
+        | Path.Pident id when Ident.name id = "char" -> ["Char"]
         | _ -> (
           match loop path with
           | _ :: rest -> List.rev rest

analysis/src/CreateInterface.ml

@@ -240,16 +240,19 @@ let printSignature ~extractor ~signature =
         ( propsId,
           {
             type_params;
-            type_kind = Type_record (labelDecls, Record_optional_labels optLbls);
+            type_kind = Type_record (labelDecls, recordRepresentation);
           },
           _ )
       :: Sig_value (makeId (* make *), makeValueDesc)
       :: rest
       when Ident.name propsId = "props"
            && getComponentTypeV4 makeValueDesc.val_type <> None
-           && optLbls |> List.mem "key" ->
+           &&
+           match recordRepresentation with
+           | Record_optional_labels _ -> true
+           | _ -> labelDecls = [] (* empty record *) ->
       (* PPX V4 component declaration:
-         type props = {..., key?: _}
+         type props = {...}
          let v = ...
       *)
       let newItemStr =
@@ -268,15 +271,18 @@ let printSignature ~extractor ~signature =
             in
             let lblName = labelDecl.ld_id |> Ident.name in
             let lbl =
+              let optLbls =
+                match recordRepresentation with
+                | Record_optional_labels optLbls -> optLbls
+                | _ -> []
+              in
               if List.mem lblName optLbls then Asttypes.Optional lblName
               else Labelled lblName
             in
-            if lblName = "key" then mkFunType rest
-            else
-              {retType with desc = Tarrow (lbl, propType, mkFunType rest, Cok)}
+            {retType with desc = Tarrow (lbl, propType, mkFunType rest, Cok)}
         in
         let funType =
-          if List.length labelDecls = 1 (* No props: only "key "*) then
+          if List.length labelDecls = 0 (* No props *) then
             let tUnit =
               Ctype.newconstr (Path.Pident (Ident.create "unit")) []
             in

analysis/src/DocumentSymbol.ml

@@ -137,16 +137,9 @@ let command ~path =
   let result =
     !symbols
     |> List.rev_map (fun (name, loc, kind) ->
+           let range = Utils.cmtLocToRange loc in
            Protocol.stringifyDocumentSymbolItem
-             {
-               name;
-               location =
-                 {
-                   uri = Uri.toString (Uri.fromPath path);
-                   range = Utils.cmtLocToRange loc;
-                 };
-               kind = kindNumber kind;
-             })
+             {name; range; selectionRange = range; kind = kindNumber kind})
     |> String.concat ",\n"
   in
   print_endline ("[\n" ^ result ^ "\n]")

analysis/src/Packages.ml

@@ -93,6 +93,36 @@ let newBsPackage ~rootPath =
                pathsForModule;
                opens;
                namespace;
+               builtInCompletionModules =
+                 (if
+                  opens_from_bsc_flags
+                  |> List.find_opt (fun opn ->
+                         match opn with
+                         | ["RescriptStdlib"] -> true
+                         | _ -> false)
+                  |> Option.is_some
+                 then
+                  {
+                    arrayModulePath = ["Array"];
+                    optionModulePath = ["Option"];
+                    stringModulePath = ["String"];
+                    intModulePath = ["Int"];
+                    floatModulePath = ["Float"];
+                    promiseModulePath = ["Promise"];
+                    listModulePath = ["List"];
+                    resultModulePath = ["Result"];
+                  }
+                 else
+                   {
+                     arrayModulePath = ["Js"; "Array2"];
+                     optionModulePath = ["Belt"; "Option"];
+                     stringModulePath = ["Js"; "String2"];
+                     intModulePath = ["Belt"; "Int"];
+                     floatModulePath = ["Belt"; "Float"];
+                     promiseModulePath = ["Js"; "Promise"];
+                     listModulePath = ["Belt"; "List"];
+                     resultModulePath = ["Belt"; "Result"];
+                   });
              })))
     | None -> None)
 

analysis/src/ProcessCmt.ml

@@ -10,18 +10,30 @@ let addDeclared ~(name : string Location.loc) ~extent ~stamp ~(env : Env.t)
   addStamp env.stamps stamp declared;
   declared
 
-let rec forTypeSignatureItem ~env ~(exported : Exported.t)
+let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t)
     (item : Types.signature_item) =
   match item with
   | Sig_value (ident, {val_type; val_attributes; val_loc = loc}) ->
     let item = val_type in
+    let stamp = Ident.binding_time ident in
+    let oldDeclared = Stamps.findValue env.stamps stamp in
     let declared =
       addDeclared
         ~name:(Location.mknoloc (Ident.name ident))
-        ~extent:loc ~stamp:(Ident.binding_time ident) ~env ~item val_attributes
+        ~extent:loc ~stamp ~env ~item val_attributes
         (Exported.add exported Exported.Value)
         Stamps.addValue
     in
+    let declared =
+      (* When an id is shadowed, a module constraint without the doc comment is created.
+         Here the existing doc comment is restored. See https://github.com/rescript-lang/rescript-vscode/issues/621 *)
+      match oldDeclared with
+      | Some oldDeclared when declared.docstring = [] ->
+        let newDeclared = {declared with docstring = oldDeclared.docstring} in
+        Stamps.addValue env.stamps stamp newDeclared;
+        newDeclared
+      | _ -> declared
+    in
     [{Module.kind = Module.Value declared.item; name = declared.name.txt}]
   | Sig_type
       ( ident,

analysis/src/Protocol.ml

@@ -42,7 +42,12 @@ type completionItem = {
 }
 
 type location = {uri: string; range: range}
-type documentSymbolItem = {name: string; kind: int; location: location}
+type documentSymbolItem = {
+  name: string;
+  kind: int;
+  range: range;
+  selectionRange: range;
+}
 type renameFile = {oldUri: string; newUri: string}
 type textEdit = {range: range; newText: string}
 
@@ -103,15 +108,16 @@ let stringifyLocation (h : location) =
   Printf.sprintf {|{"uri": "%s", "range": %s}|} (Json.escape h.uri)
     (stringifyRange h.range)
 
-let stringifyDocumentSymbolItem i =
+let stringifyDocumentSymbolItem (i : documentSymbolItem) =
+  let range = stringifyRange i.range in
   Printf.sprintf
     {|{
         "name": "%s",
         "kind": %i,
-        "location": %s
+        "range": %s,
+        "selectionRange": %s
 }|}
-    (Json.escape i.name) i.kind
-    (stringifyLocation i.location)
+    (Json.escape i.name) i.kind range range
 
 let stringifyRenameFile {oldUri; newUri} =
   Printf.sprintf {|{

analysis/src/SharedTypes.ml

@@ -361,12 +361,24 @@ type file = string
 
 module FileSet = Set.Make (String)
 
+type builtInCompletionModules = {
+  arrayModulePath: string list;
+  optionModulePath: string list;
+  stringModulePath: string list;
+  intModulePath: string list;
+  floatModulePath: string list;
+  promiseModulePath: string list;
+  listModulePath: string list;
+  resultModulePath: string list;
+}
+
 type package = {
   rootPath: filePath;
   projectFiles: FileSet.t;
   dependenciesFiles: FileSet.t;
   pathsForModule: (file, paths) Hashtbl.t;
   namespace: string option;
+  builtInCompletionModules: builtInCompletionModules;
   opens: path list;
 }
 

analysis/src/SignatureHelp.ml

@@ -154,9 +154,68 @@ let signatureHelp ~path ~pos ~currentFile ~debug =
   let posBeforeCursor = Pos.posBeforeCursor pos in
   let supportsMarkdownLinks = true in
   let foundFunctionApplicationExpr = ref None in
-  let setFound r = foundFunctionApplicationExpr := Some r in
+  let setFound r =
+    if !foundFunctionApplicationExpr = None then
+      foundFunctionApplicationExpr := Some r
+  in
+  let searchForArgWithCursor ~isPipeExpr ~args ~exp =
+    let extractedArgs = extractExpApplyArgs ~args in
+    let argAtCursor =
+      let unlabelledArgCount = ref (if isPipeExpr then 1 else 0) in
+      extractedArgs
+      |> List.find_map (fun arg ->
+             match arg.label with
+             | None ->
+               let currentUnlabelledArgCount = !unlabelledArgCount in
+               unlabelledArgCount := currentUnlabelledArgCount + 1;
+               (* An argument without a label is just the expression, so we can use that. *)
+               if arg.exp.pexp_loc |> Loc.hasPos ~pos:posBeforeCursor then
+                 Some (Unlabelled currentUnlabelledArgCount)
+               else None
+             | Some {name; posStart; posEnd} -> (
+               (* Check for the label identifier itself having the cursor *)
+               match
+                 pos |> CursorPosition.classifyPositions ~posStart ~posEnd
+               with
+               | HasCursor -> Some (Labelled name)
+               | NoCursor | EmptyLoc -> (
+                 (* If we're not in the label, check the exp. Either the exp
+                    exists and has the cursor. Or the exp is a parser recovery
+                    node, in which case we assume that the parser recovery
+                    indicates that the cursor was here. *)
+                 match
+                   ( arg.exp.pexp_desc,
+                     arg.exp.pexp_loc
+                     |> CursorPosition.classifyLoc ~pos:posBeforeCursor )
+                 with
+                 | Pexp_extension ({txt = "rescript.exprhole"}, _), _
+                 | _, HasCursor ->
+                   Some (Labelled name)
+                 | _ -> None)))
+    in
+    setFound (argAtCursor, exp, extractedArgs)
+  in
   let expr (iterator : Ast_iterator.iterator) (expr : Parsetree.expression) =
     (match expr with
+    (* Handle pipes, like someVar->someFunc(... *)
+    | {
+     pexp_desc =
+       Pexp_apply
+         ( {pexp_desc = Pexp_ident {txt = Lident "|."}},
+           [
+             _;
+             ( _,
+               {
+                 pexp_desc =
+                   Pexp_apply (({pexp_desc = Pexp_ident _} as exp), args);
+                 pexp_loc;
+               } );
+           ] );
+    }
+      when pexp_loc
+           |> CursorPosition.classifyLoc ~pos:posBeforeCursor
+           == HasCursor ->
+      searchForArgWithCursor ~isPipeExpr:true ~args ~exp
     (* Look for applying idents, like someIdent(...) *)
     | {
      pexp_desc = Pexp_apply (({pexp_desc = Pexp_ident _} as exp), args);
@@ -165,41 +224,7 @@ let signatureHelp ~path ~pos ~currentFile ~debug =
       when pexp_loc
            |> CursorPosition.classifyLoc ~pos:posBeforeCursor
            == HasCursor ->
-      let extractedArgs = extractExpApplyArgs ~args in
-      let argAtCursor =
-        let unlabelledArgCount = ref 0 in
-        extractedArgs
-        |> List.find_map (fun arg ->
-               match arg.label with
-               | None ->
-                 let currentUnlabelledArgCount = !unlabelledArgCount in
-                 unlabelledArgCount := currentUnlabelledArgCount + 1;
-                 (* An argument without a label is just the expression, so we can use that. *)
-                 if arg.exp.pexp_loc |> Loc.hasPos ~pos:posBeforeCursor then
-                   Some (Unlabelled currentUnlabelledArgCount)
-                 else None
-               | Some {name; posStart; posEnd} -> (
-                 (* Check for the label identifier itself having the cursor *)
-                 match
-                   pos |> CursorPosition.classifyPositions ~posStart ~posEnd
-                 with
-                 | HasCursor -> Some (Labelled name)
-                 | NoCursor | EmptyLoc -> (
-                   (* If we're not in the label, check the exp. Either the exp
-                      exists and has the cursor. Or the exp is a parser recovery
-                      node, in which case we assume that the parser recovery
-                      indicates that the cursor was here. *)
-                   match
-                     ( arg.exp.pexp_desc,
-                       arg.exp.pexp_loc
-                       |> CursorPosition.classifyLoc ~pos:posBeforeCursor )
-                   with
-                   | Pexp_extension ({txt = "rescript.exprhole"}, _), _
-                   | _, HasCursor ->
-                     Some (Labelled name)
-                   | _ -> None)))
-      in
-      setFound (argAtCursor, exp, extractedArgs)
+      searchForArgWithCursor ~isPipeExpr:false ~args ~exp
     | _ -> ());
     Ast_iterator.default_iterator.expr iterator expr
   in