Skip to content

distinguish regular modules from module types #925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions analysis/src/ProcessCmt.ml
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
open SharedTypes

let isModuleType (declared : Module.t Declared.t) =
match declared.modulePath with
| ExportedModule {isType} -> isType
| _ -> false

let addDeclared ~(name : string Location.loc) ~extent ~stamp ~(env : Env.t)
~item attributes addExported addStamp =
let isExported = addExported name.txt stamp in
Expand Down Expand Up @@ -150,7 +155,8 @@ let rec forTypeSignatureItem ~(env : SharedTypes.Env.t) ~(exported : Exported.t)
in
[
{
Module.kind = Module declared.item;
Module.kind =
Module {type_ = declared.item; isModuleType = isModuleType declared};
name = declared.name.txt;
docstring = declared.docstring;
deprecated = declared.deprecated;
Expand Down Expand Up @@ -367,7 +373,8 @@ let rec forSignatureItem ~env ~(exported : Exported.t)
in
[
{
Module.kind = Module declared.item;
Module.kind =
Module {type_ = declared.item; isModuleType = isModuleType declared};
name = declared.name.txt;
docstring = declared.docstring;
deprecated = declared.deprecated;
Expand Down Expand Up @@ -481,7 +488,8 @@ let rec forStructureItem ~env ~(exported : Exported.t) item =
in
[
{
Module.kind = Module declared.item;
Module.kind =
Module {type_ = declared.item; isModuleType = isModuleType declared};
name = declared.name.txt;
docstring = declared.docstring;
deprecated = declared.deprecated;
Expand Down Expand Up @@ -513,7 +521,8 @@ let rec forStructureItem ~env ~(exported : Exported.t) item =
in
[
{
Module.kind = Module modTypeItem;
Module.kind =
Module {type_ = declared.item; isModuleType = isModuleType declared};
name = declared.name.txt;
docstring = declared.docstring;
deprecated = declared.deprecated;
Expand Down
2 changes: 1 addition & 1 deletion analysis/src/SharedTypes.ml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ module Module = struct
type kind =
| Value of Types.type_expr
| Type of Type.t * Types.rec_status
| Module of t
| Module of {type_: t; isModuleType: bool}

and item = {
kind: kind;
Expand Down
9 changes: 9 additions & 0 deletions tools/npm/Tools_Docgen.res
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ type rec item =
source: source,
items: array<item>,
})
| @as("moduleType")
ModuleType({
id: string,
docstrings: array<string>,
deprecated?: string,
name: string,
source: source,
items: array<item>,
})
| @as("moduleAlias")
ModuleAlias({
id: string,
Expand Down
9 changes: 9 additions & 0 deletions tools/npm/Tools_Docgen.resi
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ type rec item =
source: source,
items: array<item>,
})
| @as("moduleType")
ModuleType({
id: string,
docstrings: array<string>,
deprecated?: string,
name: string,
source: source,
items: array<item>,
})
| @as("moduleAlias")
ModuleAlias({
id: string,
Expand Down
44 changes: 41 additions & 3 deletions tools/src/tools.ml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type docItem =
(** Additional documentation for constructors and record fields, if available. *)
}
| Module of docsForModule
| ModuleType of docsForModule
| ModuleAlias of {
id: string;
docstring: string list;
Expand Down Expand Up @@ -204,6 +205,26 @@ let rec stringifyDocItem ?(indentation = 0) ~originalEnv (item : docItem) =
(stringifyDocItem ~originalEnv ~indentation:(indentation + 1))
|> array) );
]
| ModuleType m ->
stringifyObject ~startOnNewline:true ~indentation
[
("id", Some (wrapInQuotes m.id));
("name", Some (wrapInQuotes m.name));
("kind", Some (wrapInQuotes "moduleType"));
( "deprecated",
match m.deprecated with
| Some d -> Some (wrapInQuotes d)
| None -> None );
("docstrings", Some (stringifyDocstrings m.docstring));
( "source",
Some (stringifySource ~indentation:(indentation + 1) m.source) );
( "items",
Some
(m.items
|> List.map
(stringifyDocItem ~originalEnv ~indentation:(indentation + 1))
|> array) );
]
| ModuleAlias m ->
stringifyObject ~startOnNewline:true ~indentation
[
Expand Down Expand Up @@ -379,7 +400,7 @@ let extractDocs ~entryPointFile ~debug =
detail = typeDetail typ ~full ~env;
source;
})
| Module (Ident p) ->
| Module {type_ = Ident p; isModuleType = false} ->
(* module Whatever = OtherModule *)
let aliasToModule = p |> pathIdentToString in
let id =
Expand Down Expand Up @@ -409,7 +430,7 @@ let extractDocs ~entryPointFile ~debug =
item.docstring @ internalDocstrings
|> List.map String.trim;
})
| Module (Structure m) ->
| Module {type_ = Structure m; isModuleType = false} ->
(* module Whatever = {} in res or module Whatever: {} in resi. *)
let modulePath = m.name :: modulePath in
let docs = extractDocsForModule ~modulePath m in
Expand All @@ -423,8 +444,25 @@ let extractDocs ~entryPointFile ~debug =
source;
items = docs.items;
})
| Module {type_ = Structure m; isModuleType = true} ->
(* module type Whatever = {} *)
let modulePath = m.name :: modulePath in
let docs = extractDocsForModule ~modulePath m in
Some
(ModuleType
{
id = modulePath |> List.rev |> ident;
name = m.name;
docstring = item.docstring @ m.docstring;
deprecated = item.deprecated;
source;
items = docs.items;
})
| Module
(Constraint (Structure _impl, Structure interface)) ->
{
type_ =
Constraint (Structure _impl, Structure interface);
} ->
(* module Whatever: { <interface> } = { <impl> }. Prefer the interface. *)
Some
(Module
Expand Down
34 changes: 33 additions & 1 deletion tools/tests/src/DocExtractionRes.res
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ let make = name => {
let asOffline = (t: t) => {...t, online: false}

/** exotic identifier */
let \"SomeConstant\" = 12
let \"SomeConstant" = 12

module SomeInnerModule = {
/*** Another module level docstring here.*/
Expand Down Expand Up @@ -96,4 +96,36 @@ module ModuleWithThingsThatShouldNotBeExported: {
}
}

module type Example = {
/***
this is an example module type
*/

/**
main type of this module
*/
type t

/**
function from t to t
*/
let f: t => t
}

module M: Example = {
/***
implementation of Example module type
*/

/**
main type
*/
type t = int

/**
identity function
*/
let f = (x: int) => x
}

// ^dex
42 changes: 39 additions & 3 deletions tools/tests/src/expected/DocExtractionRes.res.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@
}
},
{
"id": "DocExtractionRes.SomeConstant\\",
"id": "DocExtractionRes.SomeConstant",
"kind": "value",
"name": "SomeConstant\\",
"signature": "let SomeConstant\\: int",
"name": "SomeConstant",
"signature": "let SomeConstant: int",
"docstrings": ["exotic identifier"],
"source": {
"filepath": "src/DocExtractionRes.res",
Expand Down Expand Up @@ -270,5 +270,41 @@
"col": 3
}
}]
},
{
"id": "DocExtractionRes.Example",
"name": "Example",
"kind": "moduleType",
"docstrings": [],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstrings shouldn't be empty, since the module type contains:

module type Example = {
  /***
  this is an example module type 
  */
...
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe /*** is for the top level only. If you want a comment on an in-file module you need to use /** just above the module definition, like regular docstrings.

Copy link
Contributor

@woeps woeps Mar 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You confused me for a minute there.. ;)
I don't think so.
See my simple example:
https://github.com/woeps/rescript-tools-doc-md/blob/54b93c3da527074be767227c057e5b10dbdf7b85/test/ExampleModule.res#L6

https://github.com/woeps/rescript-tools-doc-md/blob/54b93c3da527074be767227c057e5b10dbdf7b85/test/md/Example.md?plain=1#L166

The difference in my example is the usage inside a submodule. - but not a module type.

"source": {
"filepath": "src/DocExtractionRes.res",
"line": 99,
"col": 13
},
"items": [
{
"id": "DocExtractionRes.Example.t",
"kind": "type",
"name": "t",
"signature": "type t",
"docstrings": ["main type of this module"],
"source": {
"filepath": "src/DocExtractionRes.res",
"line": 107,
"col": 3
}
},
{
"id": "DocExtractionRes.Example.f",
"kind": "value",
"name": "f",
"signature": "let f: t => t",
"docstrings": ["function from t to t"],
"source": {
"filepath": "src/DocExtractionRes.res",
"line": 109,
"col": 3
}
}]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also expect "id": "DocExtractionRes.M" to be present. - Which is missing here?

}]
}