[DocGen]: Polish (#796)

aspeddro · zth · commit f899f947f8fc · 2023-10-09T17:10:15.000+02:00
* emit items from module alias

* add error msg

* rename fields

* remove double name field

* add ocaml.text attr

* search for all attrs

* fix id module

* update tests

* emit valid id path

* update tests
diff --git a/analysis/src/DocExtraction.ml b/analysis/src/DocExtraction.ml
@@ -13,13 +13,6 @@ type constructorDoc = {
   deprecated: string option;
 }
 
-type docsForModuleAlias = {
-  id: string;
-  docstring: string list;
-  name: string;
-  signature: string;
-}
-
 type docItemDetail =
   | Record of {fieldDocs: fieldDoc list}
   | Variant of {constructorDocs: constructorDoc list}
@@ -41,7 +34,12 @@ type docItem =
           (** Additional documentation for constructors and record fields, if available. *)
     }
   | Module of docsForModule
-  | ModuleAlias of docsForModuleAlias
+  | ModuleAlias of {
+      id: string;
+      docstring: string list;
+      name: string;
+      items: docItem list;
+    }
 and docsForModule = {
   id: string;
   docstring: string list;
@@ -72,13 +70,13 @@ let stringifyDetail ?(indentation = 0) (detail : docItemDetail) =
     stringifyObject ~startOnNewline:true ~indentation
       [
         ("kind", Some (wrapInQuotes "record"));
-        ( "fieldDocs",
+        ( "items",
           Some
             (fieldDocs
             |> List.map (fun fieldDoc ->
                    stringifyObject ~indentation:(indentation + 1)
                      [
-                       ("fieldName", Some (wrapInQuotes fieldDoc.fieldName));
+                       ("name", Some (wrapInQuotes fieldDoc.fieldName));
                        ( "deprecated",
                          match fieldDoc.deprecated with
                          | Some d -> Some (wrapInQuotes d)
@@ -94,14 +92,14 @@ let stringifyDetail ?(indentation = 0) (detail : docItemDetail) =
     stringifyObject ~startOnNewline:true ~indentation
       [
         ("kind", Some (wrapInQuotes "variant"));
-        ( "constructorDocs",
+        ( "items",
           Some
             (constructorDocs
             |> List.map (fun constructorDoc ->
                    stringifyObject ~startOnNewline:true
                      ~indentation:(indentation + 1)
                      [
-                       ( "constructorName",
+                       ( "name",
                          Some (wrapInQuotes constructorDoc.constructorName) );
                        ( "deprecated",
                          match constructorDoc.deprecated with
@@ -170,7 +168,12 @@ let rec stringifyDocItem ?(indentation = 0) ~originalEnv (item : docItem) =
         ("kind", Some (wrapInQuotes "moduleAlias"));
         ("name", Some (wrapInQuotes m.name));
         ("docstrings", Some (stringifyDocstrings m.docstring));
-        ("signature", Some (m.signature |> Json.escape |> wrapInQuotes));
+        ( "items",
+          Some
+            (m.items
+            |> List.map
+                 (stringifyDocItem ~originalEnv ~indentation:(indentation + 1))
+            |> array) );
       ]
 
 and stringifyDocsForModule ?(indentation = 0) ~originalEnv (d : docsForModule) =
@@ -225,8 +228,6 @@ let typeDetail typ ~env ~full =
          })
   | _ -> None
 
-exception Invalid_file_type
-
 let makeId modulePath ~identifier =
   identifier :: modulePath |> List.rev |> SharedTypes.ident
 
@@ -235,7 +236,10 @@ let extractDocs ~path ~debug =
   if
     FindFiles.isImplementation path = false
     && FindFiles.isInterface path = false
-  then raise Invalid_file_type;
+  then (
+    Printf.printf "error: failed to read %s, expected an .res or .resi file\n"
+      path;
+    exit 1);
   let path =
     if FindFiles.isImplementation path then
       let pathAsResi =
@@ -251,7 +255,10 @@ let extractDocs ~path ~debug =
     else path
   in
   match Cmt.loadFullCmtFromPath ~path with
-  | None -> ()
+  | None ->
+    Printf.printf
+      "error: failed to generate doc for %s, try to build the project\n" path;
+    exit 1
   | Some full ->
     let file = full.file in
     let structure = file.structure in
@@ -260,7 +267,7 @@ let extractDocs ~path ~debug =
     let rec extractDocsForModule ?(modulePath = [env.file.moduleName])
         (structure : Module.structure) =
       {
-        id = modulePath |> ident;
+        id = modulePath |> List.rev |> ident;
         docstring = structure.docstring |> List.map String.trim;
         name = structure.name;
         deprecated = structure.deprecated;
@@ -297,15 +304,27 @@ let extractDocs ~path ~debug =
                  | Module (Ident p) ->
                    (* module Whatever = OtherModule *)
                    let aliasToModule = p |> pathIdentToString in
-                   let modulePath = aliasToModule :: modulePath in
+                   let id =
+                     (modulePath |> List.rev |> List.hd) ^ "." ^ item.name
+                   in
+                   let items =
+                     match
+                       ProcessCmt.fileForModule ~package:full.package
+                         aliasToModule
+                     with
+                     | None -> []
+                     | Some file ->
+                       let docs =
+                         extractDocsForModule ~modulePath:[id] file.structure
+                       in
+                       docs.items
+                   in
                    Some
                      (ModuleAlias
                         {
-                          id = modulePath |> List.rev |> SharedTypes.ident;
-                          signature =
-                            Printf.sprintf "module %s = %s" item.name
-                              aliasToModule;
+                          id;
                           name = item.name;
+                          items;
                           docstring = item.docstring |> List.map String.trim;
                         })
                  | Module (Structure m) ->
diff --git a/analysis/src/ProcessAttributes.ml b/analysis/src/ProcessAttributes.ml
@@ -5,7 +5,7 @@ let rec findDocAttribute attributes =
   let open Parsetree in
   match attributes with
   | [] -> None
-  | ( {Asttypes.txt = "ocaml.doc" | "ns.doc" | "res.doc"},
+  | ( {Asttypes.txt = "ocaml.doc" | "ocaml.text" | "ns.doc" | "res.doc"},
       PStr
         [
           {
diff --git a/analysis/src/ProcessCmt.ml b/analysis/src/ProcessCmt.ml
@@ -593,9 +593,11 @@ and forStructure ~name ~env strItems =
       strItems []
   in
   let attributes =
-    match strItems with
-    | {str_desc = Tstr_attribute attribute} :: _ -> [attribute]
-    | _ -> []
+    strItems
+    |> List.filter_map (fun (struc : Typedtree.structure_item) ->
+           match struc with
+           | {str_desc = Tstr_attribute attr} -> Some attr
+           | _ -> None)
   in
   let docstring = attrsToDocstring attributes in
   let deprecated = ProcessAttributes.findDeprecatedAttribute attributes in
diff --git a/analysis/tests/src/expected/DocExtraction2.res.txt b/analysis/tests/src/expected/DocExtraction2.res.txt
@@ -21,7 +21,7 @@ preferring found resi file for impl: src/DocExtraction2.resi
     "docstrings": ["Makerz of stuffz."]
   }, 
   {
-    "id": "InnerModule.DocExtraction2",
+    "id": "DocExtraction2.InnerModule",
     "name": "InnerModule",
     "kind": "module",
     "items": [
diff --git a/analysis/tests/src/expected/DocExtraction2.resi.txt b/analysis/tests/src/expected/DocExtraction2.resi.txt
@@ -20,7 +20,7 @@ extracting docs for src/DocExtraction2.resi
     "docstrings": ["Makerz of stuffz."]
   }, 
   {
-    "id": "InnerModule.DocExtraction2",
+    "id": "DocExtraction2.InnerModule",
     "name": "InnerModule",
     "kind": "module",
     "items": [
diff --git a/analysis/tests/src/expected/DocExtractionRes.res.txt b/analysis/tests/src/expected/DocExtractionRes.res.txt
@@ -14,13 +14,13 @@ extracting docs for src/DocExtractionRes.res
     "detail": 
     {
       "kind": "record",
-      "fieldDocs": [{
-        "fieldName": "name",
+      "items": [{
+        "name": "name",
         "optional": false,
         "docstrings": ["The name of the stuff."],
         "signature": "string"
       }, {
-        "fieldName": "online",
+        "name": "online",
         "optional": false,
         "docstrings": ["Whether stuff is online."],
         "signature": "bool"
@@ -42,7 +42,7 @@ extracting docs for src/DocExtractionRes.res
     "docstrings": ["Stuff goes offline."]
   }, 
   {
-    "id": "SomeInnerModule.DocExtractionRes",
+    "id": "DocExtractionRes.SomeInnerModule",
     "name": "SomeInnerModule",
     "kind": "module",
     "items": [
@@ -55,19 +55,19 @@ extracting docs for src/DocExtractionRes.res
       "detail": 
       {
         "kind": "variant",
-        "constructorDocs": [
+        "items": [
         {
-          "constructorName": "Started",
+          "name": "Started",
           "docstrings": ["If this is started or not"],
           "signature": "Started(t)"
         }, 
         {
-          "constructorName": "Stopped",
+          "name": "Stopped",
           "docstrings": ["Stopped?"],
           "signature": "Stopped"
         }, 
         {
-          "constructorName": "Idle",
+          "name": "Idle",
           "docstrings": ["Now idle."],
           "signature": "Idle"
         }]
@@ -89,16 +89,16 @@ extracting docs for src/DocExtractionRes.res
     }]
   }, 
   {
-    "id": "AnotherModule.DocExtractionRes",
+    "id": "DocExtractionRes.AnotherModule",
     "name": "AnotherModule",
     "kind": "module",
     "items": [
     {
-      "id": "DocExtractionRes.AnotherModule.SomeInnerModule",
+      "id": "DocExtractionRes.LinkedModule",
       "kind": "moduleAlias",
       "name": "LinkedModule",
       "docstrings": ["This links another module. Neat."],
-      "signature": "module LinkedModule = SomeInnerModule"
+      "items": []
     }, 
     {
       "id": "DocExtractionRes.AnotherModule.callback",
@@ -123,9 +123,9 @@ extracting docs for src/DocExtractionRes.res
       "detail": 
       {
         "kind": "variant",
-        "constructorDocs": [
+        "items": [
         {
-          "constructorName": "SomeStuff",
+          "name": "SomeStuff",
           "docstrings": ["This has inline records..."],
           "signature": "SomeStuff"
         }]
@@ -140,7 +140,7 @@ extracting docs for src/DocExtractionRes.res
     }]
   }, 
   {
-    "id": "ModuleWithThingsThatShouldNotBeExported.DocExtractionRes",
+    "id": "DocExtractionRes.ModuleWithThingsThatShouldNotBeExported",
     "name": "ModuleWithThingsThatShouldNotBeExported",
     "kind": "module",
     "items": [

Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@ let rec findDocAttribute attributes =`
`5`	`5`	`let open Parsetree in`
`6`	`6`	`match attributes with`
`7`	`7`	`\| [] -> None`
`8`		`- \| ( {Asttypes.txt = "ocaml.doc" \| "ns.doc" \| "res.doc"},`
	`8`	`+ \| ( {Asttypes.txt = "ocaml.doc" \| "ocaml.text" \| "ns.doc" \| "res.doc"},`
`9`	`9`	`PStr`
`10`	`10`	`[`
`11`	`11`	`{`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ preferring found resi file for impl: src/DocExtraction2.resi`
`21`	`21`	`"docstrings": ["Makerz of stuffz."]`
`22`	`22`	`},`
`23`	`23`	`{`
`24`		`- "id": "InnerModule.DocExtraction2",`
	`24`	`+ "id": "DocExtraction2.InnerModule",`
`25`	`25`	`"name": "InnerModule",`
`26`	`26`	`"kind": "module",`
`27`	`27`	`"items": [`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ extracting docs for src/DocExtraction2.resi`
`20`	`20`	`"docstrings": ["Makerz of stuffz."]`
`21`	`21`	`},`
`22`	`22`	`{`
`23`		`- "id": "InnerModule.DocExtraction2",`
	`23`	`+ "id": "DocExtraction2.InnerModule",`
`24`	`24`	`"name": "InnerModule",`
`25`	`25`	`"kind": "module",`
`26`	`26`	`"items": [`