From 825f00c24778aaffaef6877d756d851645b0af9c Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 11 Jan 2023 20:01:08 +0100 Subject: [PATCH 1/6] add failing tests for inline records in completing patterns --- analysis/tests/src/CompletionPattern.res | 10 ++++++++ .../src/expected/CompletionPattern.res.txt | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/analysis/tests/src/CompletionPattern.res b/analysis/tests/src/CompletionPattern.res index 26cba1167..dcc496355 100644 --- a/analysis/tests/src/CompletionPattern.res +++ b/analysis/tests/src/CompletionPattern.res @@ -191,3 +191,13 @@ let s = (true, Some(true), [false]) // switch b { | #one | #three({test: true}, true | ) } // ^com + +type variantWithInlineRecord = One({thing: bool, otherThing: option}) + +let r: variantWithInlineRecord = One({thing: true, otherThing: None}) + +// switch r { | One()} +// ^com + +// switch r { | One({})} +// ^com diff --git a/analysis/tests/src/expected/CompletionPattern.res.txt b/analysis/tests/src/expected/CompletionPattern.res.txt index a1e346ef3..e3191eff5 100644 --- a/analysis/tests/src/expected/CompletionPattern.res.txt +++ b/analysis/tests/src/expected/CompletionPattern.res.txt @@ -871,3 +871,26 @@ Completable: Cpattern Value[b]->polyvariantPayload::three($1) "documentation": null }] +Complete src/CompletionPattern.res 198:20 +posCursor:[198:20] posNoWhite:[198:19] Found expr:[198:3->198:22] +posCursor:[198:20] posNoWhite:[198:19] Found pattern:[198:16->198:21] +Ppat_construct One:[198:16->198:19] +posCursor:[198:20] posNoWhite:[198:19] Found pattern:[198:19->198:21] +Ppat_construct ():[198:19->198:21] +Completable: Cpattern Value[r]->variantPayload::One($0) +[] + +Complete src/CompletionPattern.res 201:21 +posCursor:[201:21] posNoWhite:[201:20] Found expr:[201:3->201:24] +posCursor:[201:21] posNoWhite:[201:20] Found pattern:[201:16->201:23] +Ppat_construct One:[201:16->201:19] +posCursor:[201:21] posNoWhite:[201:20] Found pattern:[201:20->201:22] +Completable: Cpattern Value[r]->variantPayload::One($0), recordBody +[{ + "label": "One", + "kind": 4, + "tags": [], + "detail": "One\n\ntype variantWithInlineRecord =\n | One({thing: bool, otherThing: option})", + "documentation": null + }] + From 6718af8c9e2cdf89df73e19dde9003d87860d00e Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Wed, 11 Jan 2023 22:23:06 +0100 Subject: [PATCH 2/6] docstrings for record fields --- analysis/src/CompletionBackEnd.ml | 74 ++++++++++++++--------- analysis/src/Hover.ml | 23 ++++--- analysis/src/ProcessCmt.ml | 12 ++++ analysis/src/References.ml | 7 ++- analysis/src/SharedTypes.ml | 5 +- analysis/tests/src/Hover.res | 15 +++++ analysis/tests/src/expected/Hover.res.txt | 10 +++ 7 files changed, 107 insertions(+), 39 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index ac8dc013b..215b6e5de 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -558,7 +558,8 @@ let completionForExporteds iterExported getDeclared ~prefix ~exact ~env res := { (Completion.create ~name:declared.name.txt ~env - ~kind:(transformContents declared.item)) + ~kind:(transformContents declared.item) + ()) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -600,7 +601,8 @@ let completionsForExportedConstructors ~(env : QueryEnv.t) ~prefix ~exact (Completion.create ~name ~env ~kind: (Completion.Constructor - (c, t.item.decl |> Shared.declToString t.name.txt))) + (c, t.item.decl |> Shared.declToString t.name.txt)) + ()) else None)) @ !res | _ -> ()); @@ -619,10 +621,11 @@ let completionForExportedFields ~(env : QueryEnv.t) ~prefix ~exact ~namesUsed = if not (Hashtbl.mem namesUsed name) then let () = Hashtbl.add namesUsed name () in Some - (Completion.create ~name ~env + (Completion.create ~name ~env ~docstring:f.docstring ~kind: (Completion.Field - (f, t.item.decl |> Shared.declToString t.name.txt))) + (f, t.item.decl |> Shared.declToString t.name.txt)) + ()) else None)) @ !res | _ -> ()); @@ -801,7 +804,7 @@ let processLocalValue name loc ~prefix ~exact ~env localTables.resultRev <- { (Completion.create ~name:declared.name.txt ~env - ~kind:(Value declared.item)) + ~kind:(Value declared.item) ()) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -818,6 +821,7 @@ let processLocalValue name loc ~prefix ~exact ~env (Ctype.newconstr (Path.Pident (Ident.create "Type Not Known")) [])) + () :: localTables.resultRev let processLocalConstructor name loc ~prefix ~exact ~env @@ -836,7 +840,8 @@ let processLocalConstructor name loc ~prefix ~exact ~env (Constructor ( declared.item, snd declared.item.typeDecl - |> Shared.declToString (fst declared.item.typeDecl) ))) + |> Shared.declToString (fst declared.item.typeDecl) )) + ()) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -857,7 +862,7 @@ let processLocalType name loc ~prefix ~exact ~env ~(localTables : LocalTables.t) localTables.resultRev <- { (Completion.create ~name:declared.name.txt ~env - ~kind:(Type declared.item)) + ~kind:(Type declared.item) ()) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -878,7 +883,7 @@ let processLocalModule name loc ~prefix ~exact ~env localTables.resultRev <- { (Completion.create ~name:declared.name.txt ~env - ~kind:(Module declared.item)) + ~kind:(Module declared.item) ()) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -1139,7 +1144,8 @@ let getComplementaryCompletionsForTypedValue ~opens ~allFiles ~scope ~env prefix (String.contains name '-') then Some - (Completion.create ~name ~env ~kind:(Completion.FileModule name)) + (Completion.create ~name ~env ~kind:(Completion.FileModule name) + ()) else None) in localCompletionsWithOpens @ fileModules @@ -1164,7 +1170,7 @@ let getCompletionsForPath ~package ~opens ~allFiles ~pos ~exact ~scope then Some (Completion.create ~name ~env - ~kind:(Completion.FileModule name)) + ~kind:(Completion.FileModule name) ()) else None) in localCompletionsWithOpens @ fileModules @@ -1353,28 +1359,32 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env Completion.create ~name:"string" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "string")) [])); + (Ctype.newconstr (Path.Pident (Ident.create "string")) [])) + (); ] | CPInt -> [ Completion.create ~name:"int" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "int")) [])); + (Ctype.newconstr (Path.Pident (Ident.create "int")) [])) + (); ] | CPFloat -> [ Completion.create ~name:"float" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "float")) [])); + (Ctype.newconstr (Path.Pident (Ident.create "float")) [])) + (); ] | CPArray -> [ Completion.create ~name:"array" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "array")) [])); + (Ctype.newconstr (Path.Pident (Ident.create "array")) [])) + (); ] | CPId (path, completionContext) -> path @@ -1418,7 +1428,10 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env | args, tRet when args <> [] -> let args = processApply args labels in let retType = reconstructFunctionType args tRet in - [Completion.create ~name:"dummy" ~env ~kind:(Completion.Value retType)] + [ + Completion.create ~name:"dummy" ~env ~kind:(Completion.Value retType) + (); + ] | _ -> []) | None -> []) | CPField (CPId (path, Module), fieldName) -> @@ -1441,11 +1454,13 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env if checkName field.fname.txt ~prefix:fieldName ~exact then Some (Completion.create ~name:field.fname.txt ~env + ~docstring:field.docstring ~kind: (Completion.Field ( field, typDecl.item.decl - |> Shared.declToString typDecl.name.txt ))) + |> Shared.declToString typDecl.name.txt )) + ()) else None) | None -> []) | None -> []) @@ -1473,7 +1488,7 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env if checkName field ~prefix:label ~exact then Some (Completion.create ~name:field ~env - ~kind:(Completion.ObjLabel typ)) + ~kind:(Completion.ObjLabel typ) ()) else None) | None -> []) | None -> []) @@ -1656,7 +1671,8 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env if List.length ctxPaths = List.length typeExrps then [ Completion.create ~name:"dummy" ~env - ~kind:(Completion.Value (Ctype.newty (Ttuple typeExrps))); + ~kind:(Completion.Value (Ctype.newty (Ttuple typeExrps))) + (); ] else [] | CJsxPropValue {pathToComponent; propName} -> ( @@ -1675,7 +1691,8 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env | Some (_, typ, env) -> [ Completion.create ~name:"dummy" ~env - ~kind:(Completion.Value (Utils.unwrapIfOption typ)); + ~kind:(Completion.Value (Utils.unwrapIfOption typ)) + (); ]) | CArgument {functionContextPath; argumentLabel} -> ( let labels, env = @@ -1710,7 +1727,8 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env Completion.create ~name:"dummy" ~env ~kind: (Completion.Value - (if expandOption then Utils.unwrapIfOption typ else typ)); + (if expandOption then Utils.unwrapIfOption typ else typ)) + (); ]) let getOpens ~debug ~rawOpens ~package ~env = @@ -1803,10 +1821,10 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix [ Completion.create ~name:"true" ~kind:(Label (t |> Shared.typeToString)) - ~env; + ~env (); Completion.create ~name:"false" ~kind:(Label (t |> Shared.typeToString)) - ~env; + ~env (); ] |> filterItems ~prefix | Some (Tvariant {env; constructors; variantDecl; variantName}) -> @@ -1867,7 +1885,7 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix [ Completion.create ~name:"None" ~kind:(Label (t |> Shared.typeToString)) - ~env; + ~env (); Completion.createWithSnippet ~name:"Some(_)" ~kind:(Label (t |> Shared.typeToString)) ~env ~insertText:"Some(${1:_})" (); @@ -1894,7 +1912,7 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix |> List.map (fun (field : field) -> Completion.create ~name:field.fname.txt ~kind:(Field (field, typeExpr |> Shared.typeToString)) - ~env) + ~env ()) |> filterItems ~prefix | None -> if prefix = "" then @@ -1998,7 +2016,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover | Cjsx ([id], prefix, identsSeen) when String.uncapitalize_ascii id = id -> (* Lowercase JSX tag means builtin *) let mkLabel (name, typString) = - Completion.create ~name ~kind:(Label typString) ~env + Completion.create ~name ~kind:(Label typString) ~env () in let keyLabels = if Utils.startsWith "key" prefix then [mkLabel ("key", "string")] else [] @@ -2012,7 +2030,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover | Cjsx (componentPath, prefix, identsSeen) -> let labels = getJsxLabels ~componentPath ~findTypeOfValue ~package in let mkLabel_ name typString = - Completion.create ~name ~kind:(Label typString) ~env + Completion.create ~name ~kind:(Label typString) ~env () in let mkLabel (name, typ, _env) = mkLabel_ name (typ |> Shared.typeToString) @@ -2031,7 +2049,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover @ keyLabels | Cdecorator prefix -> let mkDecorator (name, docstring) = - {(Completion.create ~name ~kind:(Label "") ~env) with docstring} + {(Completion.create ~name ~kind:(Label "") ~env ()) with docstring} in [ ( "as", @@ -2288,7 +2306,7 @@ Note: The `@react.component` decorator requires the react-jsx config to be set i | None -> [] in let mkLabel (name, typ) = - Completion.create ~name ~kind:(Label (typ |> Shared.typeToString)) ~env + Completion.create ~name ~kind:(Label (typ |> Shared.typeToString)) ~env () in labels |> List.filter (fun (name, _t) -> diff --git a/analysis/src/Hover.ml b/analysis/src/Hover.ml index 2cfeb8348..85dec9f3c 100644 --- a/analysis/src/Hover.ml +++ b/analysis/src/Hover.ml @@ -94,8 +94,7 @@ let findRelevantTypesFromType ~file ~package typ = constructors |> List.filter_map (fromConstructorPath ~env:envToSearch) (* Produces a hover with relevant types expanded in the main type being hovered. *) -let hoverWithExpandedTypes ~docstring ~file ~package ~supportsMarkdownLinks typ - = +let hoverWithExpandedTypes ~file ~package ~supportsMarkdownLinks typ = let typeString = Markdown.codeBlock (typ |> Shared.typeToString) in let types = findRelevantTypesFromType typ ~file ~package in let typeDefinitions = @@ -114,7 +113,7 @@ let hoverWithExpandedTypes ~docstring ~file ~package ~supportsMarkdownLinks typ (SharedTypes.pathIdentToString path)) ^ linkToTypeDefinitionStr ^ "\n") in - (typeString :: typeDefinitions |> String.concat "\n", docstring) + typeString :: typeDefinitions |> String.concat "\n" (* Leverages autocomplete functionality to produce a hover for a position. This makes it (most often) work with unsaved content. *) @@ -151,12 +150,20 @@ let getHoverViaCompletions ~debug ~path ~pos ~currentFile ~forHover @ docstring in Some (Protocol.stringifyHover (String.concat "\n\n" parts)) + | {kind = Field _; docstring} :: _ -> ( + match CompletionBackEnd.completionsGetTypeEnv 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 | Some (typ, _env) -> - let typeString, _docstring = - hoverWithExpandedTypes ~docstring:"" ~file ~package - ~supportsMarkdownLinks typ + let typeString = + hoverWithExpandedTypes ~file ~package ~supportsMarkdownLinks typ in Some (Protocol.stringifyHover typeString) | None -> None)))) @@ -221,8 +228,8 @@ let newHover ~full:{file; package} ~supportsMarkdownLinks locItem = | Const_nativeint _ -> "int")) | Typed (_, t, locKind) -> let fromType ~docstring typ = - hoverWithExpandedTypes ~docstring ~file ~package ~supportsMarkdownLinks - typ + ( hoverWithExpandedTypes ~file ~package ~supportsMarkdownLinks typ, + docstring ) in let parts = match References.definedForLoc ~file ~package locKind with diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index 901c19a12..c72718ee1 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -99,6 +99,12 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t) optional = Res_parsetree_viewer.hasOptionalAttribute ld_attributes; + docstring = + (match + ProcessAttributes.findDocAttribute ld_attributes + with + | None -> [] + | Some docstring -> [docstring]); }))); } ~name ~stamp:(Ident.binding_time ident) ~env type_attributes @@ -230,6 +236,12 @@ let forTypeDeclaration ~env ~(exported : Exported.t) optional = Res_parsetree_viewer.hasOptionalAttribute ld_attributes; + docstring = + (match + ProcessAttributes.findDocAttribute ld_attributes + with + | None -> [] + | Some docstring -> [docstring]); }))); } ~name ~stamp ~env typ_attributes diff --git a/analysis/src/References.ml b/analysis/src/References.ml index 7ef538cb9..c47730c05 100644 --- a/analysis/src/References.ml +++ b/analysis/src/References.ml @@ -170,7 +170,12 @@ let definedForLoc ~file ~package locKind = match getConstructor file stamp name with | None -> None | Some constructor -> Some ([], `Constructor constructor)) - | Field _name -> Some ([], `Field) + | Field name -> + Some + ( (match getField file stamp name with + | None -> [] + | Some field -> field.docstring), + `Field ) | _ -> ( maybeLog ("Trying for declared " ^ Tip.toString tip ^ " " ^ string_of_int stamp diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index 768a7bb7e..f329a074d 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -29,6 +29,7 @@ type field = { fname: string Location.loc; typ: Types.type_expr; optional: bool; + docstring: string list; } module Constructor = struct @@ -309,12 +310,12 @@ module Completion = struct kind: kind; } - let create ~name ~kind ~env = + let create ~name ~kind ~env ?(docstring = []) () = { name; env; deprecated = None; - docstring = []; + docstring; kind; sortText = None; insertText = None; diff --git a/analysis/tests/src/Hover.res b/analysis/tests/src/Hover.res index 51c31fec5..8b6221c22 100644 --- a/analysis/tests/src/Hover.res +++ b/analysis/tests/src/Hover.res @@ -233,3 +233,18 @@ let _ = NotShadowed.xx let _ = Shadowed.xx // ^hov + +type recordWithDocstringField = { + /** Mighty fine field here. */ + someField: bool, +} + +let x: recordWithDocstringField = { + someField: true, +} + +// x.someField +// ^hov + +let someField = x.someField +// ^hov diff --git a/analysis/tests/src/expected/Hover.res.txt b/analysis/tests/src/expected/Hover.res.txt index b1a12b118..db66a3737 100644 --- a/analysis/tests/src/expected/Hover.res.txt +++ b/analysis/tests/src/expected/Hover.res.txt @@ -179,3 +179,13 @@ Hover src/Hover.res 230:20 Hover src/Hover.res 233:17 {"contents": {"kind": "markdown", "value": "```rescript\nint\n```\n\n More Stuff "}} +Hover src/Hover.res 245:6 +Nothing at that position. Now trying to use completion. +posCursor:[245:6] posNoWhite:[245:5] Found expr:[245:3->245:14] +Pexp_field [245:3->245:4] someField:[245:5->245:14] +Completable: Cpath Value[x].someField +{"contents": {"kind": "markdown", "value": "```rescript\nbool\n```\n\n Mighty fine field here. "}} + +Hover src/Hover.res 248:19 +{"contents": {"kind": "markdown", "value": "```rescript\nbool\n```\n\n Mighty fine field here. "}} + From c685169f629aae84b02e7eb4bbe171a3b9fbe0fd Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 12 Jan 2023 10:53:39 +0100 Subject: [PATCH 3/6] wire up constructor docstrings (even though the parser doesnt handle docstrings on constructors yet) --- analysis/src/CompletionBackEnd.ml | 2 +- analysis/src/Hover.ml | 2 +- analysis/src/ProcessCmt.ml | 13 +++++++++++++ analysis/src/References.ml | 3 ++- analysis/src/SharedTypes.ml | 1 + 5 files changed, 18 insertions(+), 3 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index 215b6e5de..b84af9491 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -598,7 +598,7 @@ let completionsForExportedConstructors ~(env : QueryEnv.t) ~prefix ~exact if not (Hashtbl.mem namesUsed name) then let () = Hashtbl.add namesUsed name () in Some - (Completion.create ~name ~env + (Completion.create ~name ~env ~docstring:c.docstring ~kind: (Completion.Constructor (c, t.item.decl |> Shared.declToString t.name.txt)) diff --git a/analysis/src/Hover.ml b/analysis/src/Hover.ml index 85dec9f3c..1a5ed6ff4 100644 --- a/analysis/src/Hover.ml +++ b/analysis/src/Hover.ml @@ -241,7 +241,7 @@ let newHover ~full:{file; package} ~supportsMarkdownLinks locItem = | `Declared -> let typeString, docstring = t |> fromType ~docstring in typeString :: docstring - | `Constructor {cname = {txt}; args} -> + | `Constructor {cname = {txt}; args; docstring} -> let typeString, docstring = t |> fromType ~docstring in let argsString = match args with diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index c72718ee1..5895db82e 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -76,6 +76,13 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t) | Cstr_record _ -> []); res = cd_res; typeDecl = (name, decl); + docstring = + (match + ProcessAttributes.findDocAttribute + cd_attributes + with + | None -> [] + | Some docstring -> [docstring]); } in let declared = @@ -207,6 +214,12 @@ let forTypeDeclaration ~env ~(exported : Exported.t) | None -> None | Some t -> Some t.ctyp_type); typeDecl = (name.txt, typ_type); + docstring = + (match + ProcessAttributes.findDocAttribute cd_attributes + with + | None -> [] + | Some docstring -> [docstring]); } in let declared = diff --git a/analysis/src/References.ml b/analysis/src/References.ml index c47730c05..e19439a20 100644 --- a/analysis/src/References.ml +++ b/analysis/src/References.ml @@ -169,7 +169,8 @@ let definedForLoc ~file ~package locKind = | Constructor name -> ( match getConstructor file stamp name with | None -> None - | Some constructor -> Some ([], `Constructor constructor)) + | Some constructor -> + Some (constructor.docstring, `Constructor constructor)) | Field name -> Some ( (match getField file stamp name with diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index f329a074d..bae5d024a 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -39,6 +39,7 @@ module Constructor = struct args: (Types.type_expr * Location.t) list; res: Types.type_expr option; typeDecl: string * Types.type_declaration; + docstring: string list; } end From 08e4b969dcb81165c296ad021a3b0eb1ab117d86 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 12 Jan 2023 11:01:14 +0100 Subject: [PATCH 4/6] reorder constructor hover details so that the constructor + eventual docstring ends up on top before the full variant definition --- analysis/src/Hover.ml | 2 +- analysis/tests/src/Hover.res | 5 +++++ analysis/tests/src/expected/Hover.res.txt | 3 +++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/analysis/src/Hover.ml b/analysis/src/Hover.ml index 1a5ed6ff4..07cb7a665 100644 --- a/analysis/src/Hover.ml +++ b/analysis/src/Hover.ml @@ -251,7 +251,7 @@ let newHover ~full:{file; package} ~supportsMarkdownLinks locItem = |> List.map (fun (t, _) -> Shared.typeToString t) |> String.concat ", " |> Printf.sprintf "(%s)" in - typeString :: Markdown.codeBlock (txt ^ argsString) :: docstring + [Markdown.codeBlock (txt ^ argsString)] @ docstring @ [typeString] | `Field -> let typeString, docstring = t |> fromType ~docstring in typeString :: docstring) diff --git a/analysis/tests/src/Hover.res b/analysis/tests/src/Hover.res index 8b6221c22..739482f9a 100644 --- a/analysis/tests/src/Hover.res +++ b/analysis/tests/src/Hover.res @@ -248,3 +248,8 @@ let x: recordWithDocstringField = { let someField = x.someField // ^hov + +type variant = | /** Cool variant! */ CoolVariant | /** Other cool variant */ OtherCoolVariant + +let coolVariant = CoolVariant +// ^hov diff --git a/analysis/tests/src/expected/Hover.res.txt b/analysis/tests/src/expected/Hover.res.txt index db66a3737..5b47e8f22 100644 --- a/analysis/tests/src/expected/Hover.res.txt +++ b/analysis/tests/src/expected/Hover.res.txt @@ -189,3 +189,6 @@ Completable: Cpath Value[x].someField Hover src/Hover.res 248:19 {"contents": {"kind": "markdown", "value": "```rescript\nbool\n```\n\n Mighty fine field here. "}} +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"}} + From bfd52f742ef86b8af715da5052a7ef0f4629bbd3 Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 12 Jan 2023 18:59:38 +0100 Subject: [PATCH 5/6] refactor and fixes --- analysis/src/CompletionBackEnd.ml | 111 +++++++----------- analysis/src/Hover.ml | 4 +- analysis/src/ProcessCmt.ml | 46 ++------ analysis/src/SharedTypes.ml | 2 +- analysis/tests/src/CompletionPattern.res | 10 -- .../src/expected/CompletionPattern.res.txt | 23 ---- 6 files changed, 56 insertions(+), 140 deletions(-) diff --git a/analysis/src/CompletionBackEnd.ml b/analysis/src/CompletionBackEnd.ml index b84af9491..34ee9afed 100644 --- a/analysis/src/CompletionBackEnd.ml +++ b/analysis/src/CompletionBackEnd.ml @@ -557,9 +557,8 @@ let completionForExporteds iterExported getDeclared ~prefix ~exact ~env Hashtbl.add namesUsed declared.name.txt (); res := { - (Completion.create ~name:declared.name.txt ~env - ~kind:(transformContents declared.item) - ()) + (Completion.create declared.name.txt ~env + ~kind:(transformContents declared.item)) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -598,11 +597,10 @@ let completionsForExportedConstructors ~(env : QueryEnv.t) ~prefix ~exact if not (Hashtbl.mem namesUsed name) then let () = Hashtbl.add namesUsed name () in Some - (Completion.create ~name ~env ~docstring:c.docstring + (Completion.create name ~env ~docstring:c.docstring ~kind: (Completion.Constructor - (c, t.item.decl |> Shared.declToString t.name.txt)) - ()) + (c, t.item.decl |> Shared.declToString t.name.txt))) else None)) @ !res | _ -> ()); @@ -621,11 +619,10 @@ let completionForExportedFields ~(env : QueryEnv.t) ~prefix ~exact ~namesUsed = if not (Hashtbl.mem namesUsed name) then let () = Hashtbl.add namesUsed name () in Some - (Completion.create ~name ~env ~docstring:f.docstring + (Completion.create name ~env ~docstring:f.docstring ~kind: (Completion.Field - (f, t.item.decl |> Shared.declToString t.name.txt)) - ()) + (f, t.item.decl |> Shared.declToString t.name.txt))) else None)) @ !res | _ -> ()); @@ -803,8 +800,7 @@ let processLocalValue name loc ~prefix ~exact ~env Hashtbl.add localTables.namesUsed name (); localTables.resultRev <- { - (Completion.create ~name:declared.name.txt ~env - ~kind:(Value declared.item) ()) + (Completion.create declared.name.txt ~env ~kind:(Value declared.item)) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -815,13 +811,12 @@ let processLocalValue name loc ~prefix ~exact ~env (Printf.sprintf "Completion Value Not Found %s loc:%s\n" name (Loc.toString loc)); localTables.resultRev <- - Completion.create ~name ~env + Completion.create name ~env ~kind: (Value (Ctype.newconstr (Path.Pident (Ident.create "Type Not Known")) [])) - () :: localTables.resultRev let processLocalConstructor name loc ~prefix ~exact ~env @@ -835,13 +830,12 @@ let processLocalConstructor name loc ~prefix ~exact ~env Hashtbl.add localTables.namesUsed name (); localTables.resultRev <- { - (Completion.create ~name:declared.name.txt ~env + (Completion.create declared.name.txt ~env ~kind: (Constructor ( declared.item, snd declared.item.typeDecl - |> Shared.declToString (fst declared.item.typeDecl) )) - ()) + |> Shared.declToString (fst declared.item.typeDecl) ))) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -861,8 +855,7 @@ let processLocalType name loc ~prefix ~exact ~env ~(localTables : LocalTables.t) Hashtbl.add localTables.namesUsed name (); localTables.resultRev <- { - (Completion.create ~name:declared.name.txt ~env - ~kind:(Type declared.item) ()) + (Completion.create declared.name.txt ~env ~kind:(Type declared.item)) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -882,8 +875,8 @@ let processLocalModule name loc ~prefix ~exact ~env Hashtbl.add localTables.namesUsed name (); localTables.resultRev <- { - (Completion.create ~name:declared.name.txt ~env - ~kind:(Module declared.item) ()) + (Completion.create declared.name.txt ~env + ~kind:(Module declared.item)) with deprecated = declared.deprecated; docstring = declared.docstring; @@ -1144,8 +1137,7 @@ let getComplementaryCompletionsForTypedValue ~opens ~allFiles ~scope ~env prefix (String.contains name '-') then Some - (Completion.create ~name ~env ~kind:(Completion.FileModule name) - ()) + (Completion.create name ~env ~kind:(Completion.FileModule name)) else None) in localCompletionsWithOpens @ fileModules @@ -1169,8 +1161,7 @@ let getCompletionsForPath ~package ~opens ~allFiles ~pos ~exact ~scope (String.contains name '-') then Some - (Completion.create ~name ~env - ~kind:(Completion.FileModule name) ()) + (Completion.create name ~env ~kind:(Completion.FileModule name)) else None) in localCompletionsWithOpens @ fileModules @@ -1356,35 +1347,31 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env match contextPath with | CPString -> [ - Completion.create ~name:"string" ~env + Completion.create "string" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "string")) [])) - (); + (Ctype.newconstr (Path.Pident (Ident.create "string")) [])); ] | CPInt -> [ - Completion.create ~name:"int" ~env + Completion.create "int" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "int")) [])) - (); + (Ctype.newconstr (Path.Pident (Ident.create "int")) [])); ] | CPFloat -> [ - Completion.create ~name:"float" ~env + Completion.create "float" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "float")) [])) - (); + (Ctype.newconstr (Path.Pident (Ident.create "float")) [])); ] | CPArray -> [ - Completion.create ~name:"array" ~env + Completion.create "array" ~env ~kind: (Completion.Value - (Ctype.newconstr (Path.Pident (Ident.create "array")) [])) - (); + (Ctype.newconstr (Path.Pident (Ident.create "array")) [])); ] | CPId (path, completionContext) -> path @@ -1428,10 +1415,7 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env | args, tRet when args <> [] -> let args = processApply args labels in let retType = reconstructFunctionType args tRet in - [ - Completion.create ~name:"dummy" ~env ~kind:(Completion.Value retType) - (); - ] + [Completion.create "dummy" ~env ~kind:(Completion.Value retType)] | _ -> []) | None -> []) | CPField (CPId (path, Module), fieldName) -> @@ -1453,14 +1437,13 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env |> Utils.filterMap (fun field -> if checkName field.fname.txt ~prefix:fieldName ~exact then Some - (Completion.create ~name:field.fname.txt ~env + (Completion.create field.fname.txt ~env ~docstring:field.docstring ~kind: (Completion.Field ( field, typDecl.item.decl - |> Shared.declToString typDecl.name.txt )) - ()) + |> Shared.declToString typDecl.name.txt ))) else None) | None -> []) | None -> []) @@ -1487,8 +1470,7 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env |> Utils.filterMap (fun (field, typ) -> if checkName field ~prefix:label ~exact then Some - (Completion.create ~name:field ~env - ~kind:(Completion.ObjLabel typ) ()) + (Completion.create field ~env ~kind:(Completion.ObjLabel typ)) else None) | None -> []) | None -> []) @@ -1670,9 +1652,8 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env in if List.length ctxPaths = List.length typeExrps then [ - Completion.create ~name:"dummy" ~env - ~kind:(Completion.Value (Ctype.newty (Ttuple typeExrps))) - (); + Completion.create "dummy" ~env + ~kind:(Completion.Value (Ctype.newty (Ttuple typeExrps))); ] else [] | CJsxPropValue {pathToComponent; propName} -> ( @@ -1690,9 +1671,8 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env | None -> [] | Some (_, typ, env) -> [ - Completion.create ~name:"dummy" ~env - ~kind:(Completion.Value (Utils.unwrapIfOption typ)) - (); + Completion.create "dummy" ~env + ~kind:(Completion.Value (Utils.unwrapIfOption typ)); ]) | CArgument {functionContextPath; argumentLabel} -> ( let labels, env = @@ -1724,11 +1704,10 @@ let rec getCompletionsForContextPath ~full ~opens ~rawOpens ~allFiles ~pos ~env | None -> [] | Some (_, typ) -> [ - Completion.create ~name:"dummy" ~env + Completion.create "dummy" ~env ~kind: (Completion.Value - (if expandOption then Utils.unwrapIfOption typ else typ)) - (); + (if expandOption then Utils.unwrapIfOption typ else typ)); ]) let getOpens ~debug ~rawOpens ~package ~env = @@ -1819,12 +1798,8 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix match t |> extractType ~env ~package:full.package with | Some (Tbool env) -> [ - Completion.create ~name:"true" - ~kind:(Label (t |> Shared.typeToString)) - ~env (); - Completion.create ~name:"false" - ~kind:(Label (t |> Shared.typeToString)) - ~env (); + Completion.create "true" ~kind:(Label (t |> Shared.typeToString)) ~env; + Completion.create "false" ~kind:(Label (t |> Shared.typeToString)) ~env; ] |> filterItems ~prefix | Some (Tvariant {env; constructors; variantDecl; variantName}) -> @@ -1883,9 +1858,7 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix }) in [ - Completion.create ~name:"None" - ~kind:(Label (t |> Shared.typeToString)) - ~env (); + Completion.create "None" ~kind:(Label (t |> Shared.typeToString)) ~env; Completion.createWithSnippet ~name:"Some(_)" ~kind:(Label (t |> Shared.typeToString)) ~env ~insertText:"Some(${1:_})" (); @@ -1910,9 +1883,9 @@ let rec completeTypedValue (t : Types.type_expr) ~env ~full ~prefix |> List.filter (fun (field : field) -> List.mem field.fname.txt seenFields = false) |> List.map (fun (field : field) -> - Completion.create ~name:field.fname.txt + Completion.create field.fname.txt ~kind:(Field (field, typeExpr |> Shared.typeToString)) - ~env ()) + ~env) |> filterItems ~prefix | None -> if prefix = "" then @@ -2016,7 +1989,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover | Cjsx ([id], prefix, identsSeen) when String.uncapitalize_ascii id = id -> (* Lowercase JSX tag means builtin *) let mkLabel (name, typString) = - Completion.create ~name ~kind:(Label typString) ~env () + Completion.create name ~kind:(Label typString) ~env in let keyLabels = if Utils.startsWith "key" prefix then [mkLabel ("key", "string")] else [] @@ -2030,7 +2003,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover | Cjsx (componentPath, prefix, identsSeen) -> let labels = getJsxLabels ~componentPath ~findTypeOfValue ~package in let mkLabel_ name typString = - Completion.create ~name ~kind:(Label typString) ~env () + Completion.create name ~kind:(Label typString) ~env in let mkLabel (name, typ, _env) = mkLabel_ name (typ |> Shared.typeToString) @@ -2049,7 +2022,7 @@ let rec processCompletable ~debug ~full ~scope ~env ~pos ~forHover @ keyLabels | Cdecorator prefix -> let mkDecorator (name, docstring) = - {(Completion.create ~name ~kind:(Label "") ~env ()) with docstring} + {(Completion.create name ~kind:(Label "") ~env) with docstring} in [ ( "as", @@ -2306,7 +2279,7 @@ Note: The `@react.component` decorator requires the react-jsx config to be set i | None -> [] in let mkLabel (name, typ) = - Completion.create ~name ~kind:(Label (typ |> Shared.typeToString)) ~env () + Completion.create name ~kind:(Label (typ |> Shared.typeToString)) ~env in labels |> List.filter (fun (name, _t) -> diff --git a/analysis/src/Hover.ml b/analysis/src/Hover.ml index 07cb7a665..deef54ebf 100644 --- a/analysis/src/Hover.ml +++ b/analysis/src/Hover.ml @@ -156,7 +156,7 @@ let getHoverViaCompletions ~debug ~path ~pos ~currentFile ~forHover let typeString = hoverWithExpandedTypes ~file ~package ~supportsMarkdownLinks typ in - let parts = [typeString] @ docstring in + let parts = typeString :: docstring in Some (Protocol.stringifyHover (String.concat "\n\n" parts)) | None -> None) | _ -> ( @@ -251,7 +251,7 @@ let newHover ~full:{file; package} ~supportsMarkdownLinks locItem = |> List.map (fun (t, _) -> Shared.typeToString t) |> String.concat ", " |> Printf.sprintf "(%s)" in - [Markdown.codeBlock (txt ^ argsString)] @ docstring @ [typeString] + (Markdown.codeBlock (txt ^ argsString) :: docstring) @ [typeString] | `Field -> let typeString, docstring = t |> fromType ~docstring in typeString :: docstring) diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml index 5895db82e..96f3b15e9 100644 --- a/analysis/src/ProcessCmt.ml +++ b/analysis/src/ProcessCmt.ml @@ -10,6 +10,11 @@ let addDeclared ~(name : string Location.loc) ~extent ~stamp ~(env : Env.t) addStamp env.stamps stamp declared; declared +let attrsToDocstring attrs = + match ProcessAttributes.findDocAttribute attrs with + | None -> [] + | Some docstring -> [docstring] + let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t) (item : Types.signature_item) = match item with @@ -76,13 +81,7 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t) | Cstr_record _ -> []); res = cd_res; typeDecl = (name, decl); - docstring = - (match - ProcessAttributes.findDocAttribute - cd_attributes - with - | None -> [] - | Some docstring -> [docstring]); + docstring = attrsToDocstring cd_attributes; } in let declared = @@ -106,12 +105,7 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t) optional = Res_parsetree_viewer.hasOptionalAttribute ld_attributes; - docstring = - (match - ProcessAttributes.findDocAttribute ld_attributes - with - | None -> [] - | Some docstring -> [docstring]); + docstring = attrsToDocstring ld_attributes; }))); } ~name ~stamp:(Ident.binding_time ident) ~env type_attributes @@ -214,12 +208,7 @@ let forTypeDeclaration ~env ~(exported : Exported.t) | None -> None | Some t -> Some t.ctyp_type); typeDecl = (name.txt, typ_type); - docstring = - (match - ProcessAttributes.findDocAttribute cd_attributes - with - | None -> [] - | Some docstring -> [docstring]); + docstring = attrsToDocstring cd_attributes; } in let declared = @@ -249,12 +238,7 @@ let forTypeDeclaration ~env ~(exported : Exported.t) optional = Res_parsetree_viewer.hasOptionalAttribute ld_attributes; - docstring = - (match - ProcessAttributes.findDocAttribute ld_attributes - with - | None -> [] - | Some docstring -> [docstring]); + docstring = attrsToDocstring ld_attributes; }))); } ~name ~stamp ~env typ_attributes @@ -334,11 +318,7 @@ let forSignature ~name ~env sigItems = | {sig_desc = Tsig_attribute attribute} :: _ -> [attribute] | _ -> [] in - let docstring = - match ProcessAttributes.findDocAttribute attributes with - | None -> [] - | Some d -> [d] - in + let docstring = attrsToDocstring attributes in {Module.name; docstring; exported; items} let forTreeModuleType ~name ~env {Typedtree.mty_desc} = @@ -508,11 +488,7 @@ and forStructure ~name ~env strItems = | {str_desc = Tstr_attribute attribute} :: _ -> [attribute] | _ -> [] in - let docstring = - match ProcessAttributes.findDocAttribute attributes with - | None -> [] - | Some d -> [d] - in + let docstring = attrsToDocstring attributes in {Module.name; docstring; exported; items} let fileForCmtInfos ~moduleName ~uri diff --git a/analysis/src/SharedTypes.ml b/analysis/src/SharedTypes.ml index bae5d024a..7cc7196c5 100644 --- a/analysis/src/SharedTypes.ml +++ b/analysis/src/SharedTypes.ml @@ -311,7 +311,7 @@ module Completion = struct kind: kind; } - let create ~name ~kind ~env ?(docstring = []) () = + let create ~kind ~env ?(docstring = []) name = { name; env; diff --git a/analysis/tests/src/CompletionPattern.res b/analysis/tests/src/CompletionPattern.res index dcc496355..26cba1167 100644 --- a/analysis/tests/src/CompletionPattern.res +++ b/analysis/tests/src/CompletionPattern.res @@ -191,13 +191,3 @@ let s = (true, Some(true), [false]) // switch b { | #one | #three({test: true}, true | ) } // ^com - -type variantWithInlineRecord = One({thing: bool, otherThing: option}) - -let r: variantWithInlineRecord = One({thing: true, otherThing: None}) - -// switch r { | One()} -// ^com - -// switch r { | One({})} -// ^com diff --git a/analysis/tests/src/expected/CompletionPattern.res.txt b/analysis/tests/src/expected/CompletionPattern.res.txt index e3191eff5..a1e346ef3 100644 --- a/analysis/tests/src/expected/CompletionPattern.res.txt +++ b/analysis/tests/src/expected/CompletionPattern.res.txt @@ -871,26 +871,3 @@ Completable: Cpattern Value[b]->polyvariantPayload::three($1) "documentation": null }] -Complete src/CompletionPattern.res 198:20 -posCursor:[198:20] posNoWhite:[198:19] Found expr:[198:3->198:22] -posCursor:[198:20] posNoWhite:[198:19] Found pattern:[198:16->198:21] -Ppat_construct One:[198:16->198:19] -posCursor:[198:20] posNoWhite:[198:19] Found pattern:[198:19->198:21] -Ppat_construct ():[198:19->198:21] -Completable: Cpattern Value[r]->variantPayload::One($0) -[] - -Complete src/CompletionPattern.res 201:21 -posCursor:[201:21] posNoWhite:[201:20] Found expr:[201:3->201:24] -posCursor:[201:21] posNoWhite:[201:20] Found pattern:[201:16->201:23] -Ppat_construct One:[201:16->201:19] -posCursor:[201:21] posNoWhite:[201:20] Found pattern:[201:20->201:22] -Completable: Cpattern Value[r]->variantPayload::One($0), recordBody -[{ - "label": "One", - "kind": 4, - "tags": [], - "detail": "One\n\ntype variantWithInlineRecord =\n | One({thing: bool, otherThing: option})", - "documentation": null - }] - From 0710281d81f5ac4015fa9ca408da1be3cdd05ead Mon Sep 17 00:00:00 2001 From: Gabriel Nordeborn Date: Thu, 12 Jan 2023 19:00:16 +0100 Subject: [PATCH 6/6] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a054d487c..35dc2d2f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ - Prefer opened `Belt` modules in autocomplete when `-open Belt` is detected in `bsconfig`. https://github.com/rescript-lang/rescript-vscode/pull/673 - Improve precision in signature help. You now do not need to type anything into the argument for it to highlight. https://github.com/rescript-lang/rescript-vscode/pull/675 - Remove redundant function name in signature help, to clean up what's shown to the user some. https://github.com/rescript-lang/rescript-vscode/pull/678 +- Show docstrings in hover for record fields and variant constructors. https://github.com/rescript-lang/rescript-vscode/pull/694 #### :bug: Bug Fix