From cc338835cc0097d99fa3eacfde378fc03c190216 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Mon, 4 Jan 2021 15:43:04 +0100 Subject: [PATCH 1/2] Fix hovering on record fields. Fixes https://github.com/rescript-lang/rescript-vscode/issues/52 Continuation of https://github.com/rescript-lang/rescript-editor-support/pull/47 Show the correct type information, including type definition for record fields. Also, fix issue in the docstring shown. Before it was either misssing (entire record type) or wrong (for record field, it was showing the doc comment of the containing record type). Clean up code in References. --- Changes.md | 1 + examples/example-project/src/ZZ.res | 31 ++++++--- src/rescript-editor-support/Hover.re | 76 ++++++++++++----------- src/rescript-editor-support/References.re | 28 +++------ 4 files changed, 70 insertions(+), 66 deletions(-) diff --git a/Changes.md b/Changes.md index d84186da..ae6aa531 100644 --- a/Changes.md +++ b/Changes.md @@ -1,4 +1,5 @@ ## master +- Fix type shown when hovering on record fields (see https://github.com/rescript-lang/rescript-vscode/issues/52), and doc comments for records. - Fix issue where type variables are printed with global renaming when hovering or autocompleting a module (see https://github.com/rescript-lang/rescript-editor-support/issues/38). - Fix issue where a log file was always created (see https://github.com/rescript-lang/rescript-vscode/issues/47). - Add support for hover on the id of toplevel module definitions (```module Id = ...```). diff --git a/examples/example-project/src/ZZ.res b/examples/example-project/src/ZZ.res index 39e03083..671c39b1 100644 --- a/examples/example-project/src/ZZ.res +++ b/examples/example-project/src/ZZ.res @@ -38,22 +38,20 @@ type inline = | E({x: int, y: string}) | F -module MSig -: { - type rec t = | A (list) +module MSig: { + type rec t = A(list) and s = list - let x : int -} -= { - type rec t = | A (list) + let x: int +} = { + type rec t = A(list) and s = list let x = 14 } module Impl = { - type rec t = | A (list) + type rec t = A(list) and s = list type w = int @@ -61,8 +59,21 @@ module Impl = { let x = 14 } -module Impl2 = { include Impl}; +module Impl2 = { + include Impl +} module D = MSig module E = Impl -module F = Impl2 \ No newline at end of file +module F = Impl2 + +@ocaml.doc("str docstring") +type str = string + +@ocaml.doc("gr docstring") +type gr = {x: int, s: str} + +let testRecordFields = (gr: gr) => { + let str = gr.s + str +} diff --git a/src/rescript-editor-support/Hover.re b/src/rescript-editor-support/Hover.re index 8dcada60..e5e3820b 100644 --- a/src/rescript-editor-support/Hover.re +++ b/src/rescript-editor-support/Hover.re @@ -49,7 +49,6 @@ let showModule = }; }; -open Infix; let newHover = (~rootUri, ~file: SharedTypes.file, ~getModule, loc) => { switch (loc) { | SharedTypes.Explanation(text) => Some(text) @@ -98,43 +97,46 @@ let newHover = (~rootUri, ~file: SharedTypes.file, ~getModule, loc) => { | Const_nativeint(_) => "int" }, ) - | Typed(t, _) => - let typeString = t |> Shared.typeToString; - let extraTypeInfo = { - let env = {Query.file, exported: file.contents.exported}; - let%opt path = t |> Shared.digConstructor; - let%opt (_env, {name: {txt}, item: {decl}}) = - digConstructor(~env, ~getModule, path); - Some(decl |> Shared.declToString(txt)); - /* TODO type declaration */ - /* None */ - /* Some(typ.toString()) */ - }; - - let typeString = codeBlock(typeString); - let typeString = - typeString - ++ ( + | Typed(t, locKind) => + let fromType = (~docstring, typ) => { + let typeString = codeBlock(typ |> Shared.typeToString); + let extraTypeInfo = { + let env = {Query.file, exported: file.contents.exported}; + let%opt path = typ |> Shared.digConstructor; + let%opt (_env, {docstring, name: {txt}, item: {decl}}) = + digConstructor(~env, ~getModule, path); + Some((decl |> Shared.declToString(txt), docstring)); + }; + let (typeString, docstring) = switch (extraTypeInfo) { - | None => "" - | Some(extra) => "\n\n" ++ codeBlock(extra) - } - ); - - Some( - { - let%opt ({docstring}, {uri}, res) = - References.definedForLoc(~file, ~getModule, loc); + | None => (typeString, docstring) + | Some((extra, extraDocstring)) => ( + typeString ++ "\n\n" ++ codeBlock(extra), + extraDocstring, + ) + }; + (Some(typeString), docstring); + }; + let parts = + switch (References.definedForLoc(~file, ~getModule, locKind)) { + | None => + let (typeString, docstring) = t |> fromType(~docstring=None); + [typeString, docstring]; + | Some((docstring, {uri}, res)) => let uri = Utils.startsWith(uri, rootUri) ? "" ++ Utils.sliceToEnd(uri, String.length(rootUri)) : uri; let parts = switch (res) { - | `Declared => [Some(typeString), docstring] - | `Constructor({cname: {txt}, args}) => [ - Some(typeString), + | `Declared => + let (typeString, docstring) = t |> fromType(~docstring); + [typeString, docstring]; + | `Constructor({cname: {txt}, args}) => + let (typeString, docstring) = t |> fromType(~docstring); + [ + typeString, Some( codeBlock( txt @@ -151,15 +153,15 @@ let newHover = (~rootUri, ~file: SharedTypes.file, ~getModule, loc) => { ), ), docstring, - ] - | `Attribute(_) => [Some(typeString), docstring] + ]; + | `Attribute({typ}) => + let (typeString, docstring) = typ |> fromType(~docstring); + [typeString, docstring]; }; - let parts = parts @ [Some(uri)]; + parts @ [Some(uri)]; + }; - Some(String.concat("\n\n", parts |> Utils.filterMap(x => x))); - } - |? typeString, - ); + Some(String.concat("\n\n", parts |> Utils.filterMap(x => x))); }; }; diff --git a/src/rescript-editor-support/References.re b/src/rescript-editor-support/References.re index 59aa41cf..79edd4e8 100644 --- a/src/rescript-editor-support/References.re +++ b/src/rescript-editor-support/References.re @@ -82,19 +82,19 @@ let localReferencesForLoc = (~file, ~extra, loc) => } }; -let definedForLoc = (~file, ~getModule, loc) => { +let definedForLoc = (~file, ~getModule, locKind) => { let inner = (~file, stamp, tip) => { switch (tip) { | Constructor(name) => let%opt declared = Query.declaredForTip(~stamps=file.stamps, stamp, tip); let%opt constructor = Query.getConstructor(file, stamp, name); - Some((declared, file, `Constructor(constructor))); + Some((declared.docstring, file, `Constructor(constructor))); | Attribute(name) => let%opt declared = Query.declaredForTip(~stamps=file.stamps, stamp, tip); let%opt attribute = Query.getAttribute(file, stamp, name); - Some((declared, file, `Attribute(attribute))); + Some((declared.docstring, file, `Attribute(attribute))); | _ => maybeLog( "Trying for declared " @@ -105,22 +105,15 @@ let definedForLoc = (~file, ~getModule, loc) => { ++ file.uri, ); let%opt x = Query.declaredForTip(~stamps=file.stamps, stamp, tip); - Some((x, file, `Declared)); + Some((x.docstring, file, `Declared)); }; }; - switch (loc) { - | Explanation(_) - | Typed(_, NotFound) - | LModule(NotFound) - | TopLevelModule(_) - | Constant(_) => None - | Typed(_, LocalReference(stamp, tip) | Definition(stamp, tip)) - | LModule(LocalReference(stamp, tip) | Definition(stamp, tip)) => - inner(~file, stamp, tip) - | TypeDefinition(_, _, stamp) => inner(~file, stamp, Type) - | LModule(GlobalReference(moduleName, path, tip)) - | Typed(_, GlobalReference(moduleName, path, tip)) => + switch (locKind) { + | NotFound => None + | LocalReference(stamp, tip) + | Definition(stamp, tip) => inner(~file, stamp, tip) + | GlobalReference(moduleName, path, tip) => { maybeLog("Getting global " ++ moduleName); let%try file = @@ -146,9 +139,6 @@ let definedForLoc = (~file, ~getModule, loc) => { Ok(res); } |> RResult.toOptionAndLog - /* let%try extra = getExtra(moduleName) |> RResult.orError("Failed to get extra for " ++ env.file.uri); */ - /* maybeLog("Finding references for (global) " ++ file.uri ++ " and stamp " ++ string_of_int(stamp) ++ " and tip " ++ tipToString(tip)); */ - /* forLocalStamp(~file, ~extra, ~allModules, ~getModule, ~getExtra, stamp, tip) |> RResult.orError("Could not get for local stamp") */ }; }; From 586a3314fcc12edb55351e4c4da72630ad0bb526 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Mon, 4 Jan 2021 15:52:02 +0100 Subject: [PATCH 2/2] Avoid returning docstring that will be overridden. --- examples/example-project/src/ZZ.res | 5 +++++ src/rescript-editor-support/References.re | 12 ++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/examples/example-project/src/ZZ.res b/examples/example-project/src/ZZ.res index 671c39b1..b56eb60c 100644 --- a/examples/example-project/src/ZZ.res +++ b/examples/example-project/src/ZZ.res @@ -77,3 +77,8 @@ let testRecordFields = (gr: gr) => { let str = gr.s str } + +@ocaml.doc("vr docstring") +type vr = | V1 | V2 + +let v1 = V1 \ No newline at end of file diff --git a/src/rescript-editor-support/References.re b/src/rescript-editor-support/References.re index 79edd4e8..22487079 100644 --- a/src/rescript-editor-support/References.re +++ b/src/rescript-editor-support/References.re @@ -86,15 +86,11 @@ let definedForLoc = (~file, ~getModule, locKind) => { let inner = (~file, stamp, tip) => { switch (tip) { | Constructor(name) => - let%opt declared = - Query.declaredForTip(~stamps=file.stamps, stamp, tip); let%opt constructor = Query.getConstructor(file, stamp, name); - Some((declared.docstring, file, `Constructor(constructor))); + Some((None, file, `Constructor(constructor))); | Attribute(name) => - let%opt declared = - Query.declaredForTip(~stamps=file.stamps, stamp, tip); let%opt attribute = Query.getAttribute(file, stamp, name); - Some((declared.docstring, file, `Attribute(attribute))); + Some((None, file, `Attribute(attribute))); | _ => maybeLog( "Trying for declared " @@ -104,8 +100,8 @@ let definedForLoc = (~file, ~getModule, locKind) => { ++ " in file " ++ file.uri, ); - let%opt x = Query.declaredForTip(~stamps=file.stamps, stamp, tip); - Some((x.docstring, file, `Declared)); + let%opt declared = Query.declaredForTip(~stamps=file.stamps, stamp, tip); + Some((declared.docstring, file, `Declared)); }; };