feat(graindoc)!: Improve docgen for labeled & default arguments (#1776)

Author: alex-snezhko

compiler/graindoc/docblock.re

@@ -4,7 +4,7 @@ open Grain_utils;
 open Grain_diagnostics;
 
 type param = {
-  param_name: string,
+  param_id: string,
   param_type: string,
   param_msg: string,
 };
@@ -77,7 +77,8 @@ exception
     attr: string,
   });
 
-exception MissingParamType({name: string});
+exception MissingLabeledParamType({name: string});
+exception MissingUnlabeledParamType({idx: int});
 exception MissingReturnType;
 exception
   InvalidAttribute({
@@ -96,11 +97,19 @@ let () =
           attr,
         );
       Some(msg);
-    | MissingParamType({name}) =>
+    | MissingLabeledParamType({name}) =>
       let msg =
         Printf.sprintf(
-          "Unable to find a type for %s. Did you specify too many @param attributes?",
+          "Unable to find a matching function parameter for %s. Make sure a parameter exists with this label or use `@param <param_index> %s` for unlabeled parameters.",
           name,
+          name,
+        );
+      Some(msg);
+    | MissingUnlabeledParamType({idx}) =>
+      let msg =
+        Printf.sprintf(
+          "Unable to find a type for parameter at index %d. Make sure a parameter exists at this index in the parameter list.",
+          idx,
         );
       Some(msg);
     | MissingReturnType =>
@@ -161,8 +170,8 @@ let output_for_params = params => {
   Markdown.table(
     ~headers=["param", "type", "description"],
     List.map(
-      ({param_name, param_type, param_msg}) => {
-        [Markdown.code(param_name), Markdown.code(param_type), param_msg]
+      ({param_id, param_type, param_msg}) => {
+        [Markdown.code(param_id), Markdown.code(param_type), param_msg]
       },
       params,
     ),
@@ -207,14 +216,25 @@ let output_for_throws = throws => {
 
 let types_for_function = (~ident, vd: Types.value_description) => {
   switch (Ctype.repr(vd.val_type).desc) {
-  | TTyArrow(args, returns, _) => (
-      Some(List.map(snd, args)),
-      Some(returns),
-    )
+  | TTyArrow(args, returns, _) => (Some(args), Some(returns))
   | _ => (None, None)
   };
 };
 
+let lookup_arg_by_label = (name, args_opt) => {
+  Option.bind(args_opt, args =>
+    List.find_opt(
+      ((label: Grain_parsing.Asttypes.argument_label, _)) =>
+        switch (label) {
+        | Default(l)
+        | Labeled(l) => l.txt == name
+        | _ => false
+        },
+      args,
+    )
+  );
+};
+
 let lookup_type_expr = (~idx, type_exprs) => {
   Option.bind(type_exprs, te => List.nth_opt(te, idx));
 };
@@ -253,7 +273,7 @@ let for_value_description =
     | None => (None, [])
     };
 
-  let (arg_types, return_type) = types_for_function(~ident, vd);
+  let (args, return_type) = types_for_function(~ident, vd);
 
   let (deprecations, since, history, params, returns, throws, examples) =
     List.fold_left(
@@ -291,19 +311,37 @@ let for_value_description =
             throws,
             examples,
           )
-        | Param({attr_name: param_name, attr_desc: param_msg}) =>
-          // TODO: Use label lookups when labeled parameters are introduced
-          let param_type =
-            switch (lookup_type_expr(~idx=List.length(params), arg_types)) {
-            | Some(typ) => Printtyp.string_of_type_sch(typ)
-            | None => raise(MissingParamType({name: param_name}))
+        | Param({attr_id: param_id, attr_desc: param_msg}) =>
+          let (param_id, param_type) =
+            switch (param_id) {
+            | PositionalParam(idx) =>
+              switch (lookup_type_expr(~idx, args)) {
+              | Some((_, typ)) => (
+                  string_of_int(idx),
+                  Printtyp.string_of_type_sch(typ),
+                )
+              | None => raise(MissingUnlabeledParamType({idx: idx}))
+              }
+            | LabeledParam(name) =>
+              switch (lookup_arg_by_label(name, args)) {
+              | Some((Labeled(_), typ)) => (
+                  name,
+                  Printtyp.string_of_type_sch(typ),
+                )
+              // Default parameters have the type Option<a>; extract the type from the Option
+              | Some((Default(_), {desc: TTyConstr(_, [typ], _)})) => (
+                  "?" ++ name,
+                  Printtyp.string_of_type_sch(typ),
+                )
+              | _ => raise(MissingLabeledParamType({name: name}))
+              }
             };
 
           (
             deprecations,
             since,
             history,
-            [{param_name, param_type, param_msg}, ...params],
+            [{param_id, param_type, param_msg}, ...params],
             returns,
             throws,
             examples,
@@ -402,7 +440,7 @@ let for_type_declaration =
             [{history_version, history_msg}, ...history],
             examples,
           )
-        | Param({attr_name: param_name, attr_desc: param_msg}) =>
+        | Param({attr_id: param_id, attr_desc: param_msg}) =>
           raise(InvalidAttribute({name, attr: "param"}))
         | Returns({attr_desc: returns_msg}) =>
           raise(InvalidAttribute({name, attr: "returns"}))
@@ -529,7 +567,7 @@ and for_signature_items =
             [{history_version, history_msg}, ...history],
             examples,
           )
-        | Param({attr_name: param_name, attr_desc: param_msg}) =>
+        | Param({attr_id: param_id, attr_desc: param_msg}) =>
           raise(InvalidAttribute({name, attr: "param"}))
         | Returns({attr_desc: returns_msg}) =>
           raise(InvalidAttribute({name, attr: "returns"}))

compiler/src/diagnostics/comment_attributes.re

@@ -1,9 +1,13 @@
 exception InvalidAttribute(string);
 exception MalformedAttribute(string, string);
 
+type param_id =
+  | LabeledParam(string)
+  | PositionalParam(int);
+
 type t =
   | Param({
-      attr_name: string,
+      attr_id: param_id,
       attr_desc: string,
     })
   | Returns({attr_desc: string})

compiler/src/diagnostics/graindoc_lexer.re

@@ -52,6 +52,9 @@ let newline = [%sedlex.regexp? 0x0A | 0x0C | 0x0D | 0x85 | 0x2028 | 0x2029];
 // A colon eats whitespace after it
 let colon = [%sedlex.regexp? (':', Star(blank))];
 
+let dec_digit = [%sedlex.regexp? '0' .. '9'];
+let dec_int = [%sedlex.regexp? (dec_digit, Star(dec_digit))];
+
 // A asterisk eats whitespace before it and up to 1 after it
 let asterisk = [%sedlex.regexp? (Star(blank), '*', Opt(blank))];
 
@@ -131,6 +134,7 @@ and param = (state, lexbuf) => {
   switch%sedlex (lexbuf) {
   | blank => param(state, lexbuf)
   | ident => IDENT(Sedlexing.Utf8.lexeme(lexbuf))
+  | dec_int => INT(Sedlexing.Utf8.lexeme(lexbuf))
   | colon =>
     state.lexer_mode = Default;
     COLON;

compiler/src/diagnostics/graindoc_parser.mly

@@ -1,5 +1,9 @@
+%{
+open Comment_attributes
+%}
+
 %token PARAM SECTION SINCE HISTORY THROWS RETURNS EXAMPLE DEPRECATED COLON EOL EOF
-%token <string> TEXT IDENT SEMVER CONSTRUCTOR
+%token <string> TEXT IDENT INT SEMVER CONSTRUCTOR
 
 %right EOL
 
@@ -26,8 +30,12 @@ description:
 attribute_text:
   | ioption(eols) multiline_text ioption(eols) %prec EOL { $2 }
 
+param_id:
+  | IDENT { LabeledParam $1 }
+  | INT { PositionalParam (int_of_string $1) }
+
 attribute:
-  | PARAM IDENT COLON attribute_text { Param({ attr_name=$2; attr_desc=$4 }) }
+  | PARAM param_id COLON attribute_text { Param({ attr_id=$2; attr_desc=$4 }) }
   | RETURNS attribute_text { Returns({attr_desc=$2}) }
   | EXAMPLE attribute_text { Example({attr_desc=$2}) }
   | DEPRECATED attribute_text { Deprecated({attr_desc=$2}) }

compiler/src/typed/typedtree.re

@@ -40,6 +40,7 @@ type provide_flag =
   Asttypes.provide_flag = | NotProvided | Provided | Abstract;
 type rec_flag = Asttypes.rec_flag = | Nonrecursive | Recursive;
 type mut_flag = Asttypes.mut_flag = | Mutable | Immutable;
+[@deriving sexp]
 type argument_label =
   Asttypes.argument_label =
     | Unlabeled | Labeled(loc(string)) | Default(loc(string));

stdlib/array.gr

@@ -83,7 +83,7 @@ provide let length = array => {
  * @since v0.1.0
  */
 @unsafe
-provide let make: (Number, a) => Array<a> = (length: Number, item: a) => {
+provide let make = (length: Number, item: a) => {
   from WasmI32 use { (+), (*), (<) }
   checkLength(length)
   let length = coerceNumberToWasmI32(length)
@@ -116,11 +116,7 @@ provide let make: (Number, a) => Array<a> = (length: Number, item: a) => {
  * @since v0.1.0
  */
 @unsafe
-provide let init: (Number, Number => a) => Array<a> =
-  (
-    length: Number,
-    fn: Number => a,
-  ) => {
+provide let init = (length: Number, fn: Number => a) => {
   from WasmI32 use { (+), (*), (<) }
   checkLength(length)
   let length = coerceNumberToWasmI32(length)
@@ -1821,8 +1817,8 @@ provide module Immutable {
    * @history v0.5.4: Originally in `"immutablearray"` module
    */
 
-  provide let contains = (value, array) => {
-    reduce((acc, x) => acc || x == value, false, array)
+  provide let contains = (search, array) => {
+    reduce((acc, x) => acc || x == search, false, array)
   }
 
   /**

stdlib/array.md

@@ -66,7 +66,7 @@ No other changes yet.
 </details>
 
 ```grain
-make : (Number, a) => Array<a>
+make : (length: Number, item: a) => Array<a>
 ```
 
 Creates a new array of the specified length with each element being
@@ -106,7 +106,7 @@ No other changes yet.
 </details>
 
 ```grain
-init : (Number, (Number => a)) => Array<a>
+init : (length: Number, fn: (Number => a)) => Array<a>
 ```
 
 Creates a new array of the specified length where each element is
@@ -1216,7 +1216,7 @@ Parameters:
 |param|type|description|
 |-----|----|-----------|
 |`start`|`Number`|The index of the array where the slice will begin (inclusive)|
-|`end`|`Option<Number>`|The index of the array where the slice will end (exclusive)|
+|`?end`|`Number`|The index of the array where the slice will end (exclusive)|
 |`array`|`Array<a>`|The array to be sliced|
 
 Returns:
@@ -2150,7 +2150,7 @@ Returns:
 </details>
 
 ```grain
-contains : (value: a, array: ImmutableArray<a>) => Bool
+contains : (search: a, array: ImmutableArray<a>) => Bool
 ```
 
 Checks if the value is an element of the input array.
@@ -2528,7 +2528,7 @@ Parameters:
 |param|type|description|
 |-----|----|-----------|
 |`start`|`Number`|The index of the array where the slice will begin (inclusive)|
-|`end`|`Option<Number>`|The index of the array where the slice will end (exclusive)|
+|`?end`|`Number`|The index of the array where the slice will end (exclusive)|
 |`array`|`ImmutableArray<a>`|The array to be sliced|
 
 Returns:

stdlib/bigint.md

@@ -37,7 +37,7 @@ No other changes yet.
 </details>
 
 ```grain
-fromNumber : (x: Number) => BigInt
+fromNumber : (number: Number) => BigInt
 ```
 
 Converts a Number to a BigInt.
@@ -62,7 +62,7 @@ No other changes yet.
 </details>
 
 ```grain
-toNumber : (x: BigInt) => Number
+toNumber : (num: BigInt) => Number
 ```
 
 Converts a BigInt to a Number.

stdlib/bytes.gr

@@ -166,9 +166,9 @@ provide let length = (bytes: Bytes) => {
  * @since v0.3.2
  */
 @unsafe
-provide let copy = (b: Bytes) => {
+provide let copy = (bytes: Bytes) => {
   from WasmI32 use { (+) }
-  let src = WasmI32.fromGrain(b)
+  let src = WasmI32.fromGrain(bytes)
   let size = getSize(src)
   let dst = allocateBytes(size)
   Memory.copy(dst + _VALUE_OFFSET, src + _VALUE_OFFSET, size)

stdlib/bytes.md

@@ -188,7 +188,7 @@ No other changes yet.
 </details>
 
 ```grain
-copy : (b: Bytes) => Bytes
+copy : (bytes: Bytes) => Bytes
 ```
 
 Creates a new byte sequence that contains the same bytes as the input byte sequence.