Fix hovering on record fields.

Fixes rescript-lang/rescript-vscode#52

Continuation of #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.

Author: cristianoc

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 = ...```).

examples/example-project/src/ZZ.res

@@ -38,31 +38,42 @@ type inline =
   | E({x: int, y: string})
   | F
 
-module MSig
-: {
-  type rec t = | A (list<s>)
+module MSig: {
+  type rec t = A(list<s>)
   and s = list<t>
 
-  let x : int
-} 
-= {
-  type rec t = | A (list<s>)
+  let x: int
+} = {
+  type rec t = A(list<s>)
   and s = list<t>
 
   let x = 14
 }
 
 module Impl = {
-  type rec t = | A (list<s>)
+  type rec t = A(list<s>)
   and s = list<t>
 
   type w = int
 
   let x = 14
 }
 
-module Impl2 = { include Impl};
+module Impl2 = {
+  include Impl
+}
 
 module D = MSig
 module E = Impl
-module F = Impl2
+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
+}

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)
             ? "<root>" ++ 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)));
   };
 };

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") */
   };
 };