diff --git a/compiler/graindoc/docblock.re b/compiler/graindoc/docblock.re index 9415add9fe..6d16263998 100644 --- a/compiler/graindoc/docblock.re +++ b/compiler/graindoc/docblock.re @@ -4,6 +4,8 @@ open Grain_typed; open Grain_utils; open Grain_diagnostics; +module StringSet = Set.Make(String); + type param = { param_id: string, param_type: string, @@ -110,6 +112,7 @@ type error = }) | MissingLabeledParamType({name: string}) | MissingUnlabeledParamType({idx: int}) + | DuplicateLabelParamType({name: string}) | MissingReturnType | AttributeAppearsMultipleTimes({attr: string}) | InvalidAttribute({ @@ -117,6 +120,10 @@ type error = attr: string, }); +type argument = + | LabeledArg(string) + | UnlabeledArg(int); + exception Error(Location.t, error); let report_error = (ppf, err) => { @@ -141,6 +148,12 @@ let report_error = (ppf, err) => { "Unable to find a type for parameter at index %d. Make sure a parameter exists at this index in the parameter list.", idx, ) + | DuplicateLabelParamType({name}) => + Format.fprintf( + ppf, + "Multiple `@param` attributes found for parameter %s. Each parameter may only have one associated `@param` attribute.", + name, + ) | MissingReturnType => Format.fprintf(ppf, "Unable to find a return type. Please file an issue!") | AttributeAppearsMultipleTimes({attr}) => @@ -406,11 +419,119 @@ let for_value_description = }; let (args, return_type) = types_for_function(~ident, vd); + let args = Option.value(args, ~default=[]); + + // Extract parameter documentation from attributes + let param_attributes: list((argument, (string, Location.t, ref(bool)))) = + List.filter_map( + (attr: Comment_attributes.t) => + switch (attr.attr) { + | Comment_attributes.Param({attr_id, attr_desc}) => + switch (attr_id) { + | PositionalParam(idx, loc) => + Some((UnlabeledArg(idx), (attr_desc, loc, ref(false)))) + | LabeledParam(name, loc) => + Some((LabeledArg(name), (attr_desc, loc, ref(false)))) + } + | _ => None + }, + attributes, + ); + let has_param_attributes = List.length(param_attributes) > 0; + + let match_label_to_attr = (label: argument) => { + switch (List.assoc_opt(label, param_attributes)) { + | None => + Location.prerr_warning( + loc, + Grain_utils.Warnings.ParameterDocumentationMissing( + switch (label) { + | LabeledArg(name) => name + | UnlabeledArg(idx) => string_of_int(idx) + }, + ), + ); + ("", Location.dummy_loc); + | Some((attr_desc, attr_loc, used)) => + used := true; + (attr_desc, attr_loc); + }; + }; + + // Precompute parameter documentation + let documented_params = + if (has_param_attributes) { + List.mapi( + (index, (label: Asttypes.argument_label, typ: Types.type_expr)) => { + let (param_id, typ, (param_msg, param_loc)) = + switch (label, typ) { + | (Unlabeled, _) => ( + string_of_int(index), + typ, + match_label_to_attr(UnlabeledArg(index)), + ) + | (Labeled(l), _) => ( + l.txt, + typ, + match_label_to_attr(LabeledArg(l.txt)), + ) + | (Default(l), {desc: TTyConstr(_, [typ], _)}) => ( + "?" ++ l.txt, + typ, + match_label_to_attr(LabeledArg(l.txt)), + ) + | (Default(_), _) => + failwith("Impossible: Default parameter type is not an Option") + }; + { + param_id, + param_type: Printtyp.string_of_type_sch(typ), + param_msg, + param_loc, + }; + }, + args, + ); + } else { + []; + }; + + // Validate that all parameter attributes were used (i.e. no invalid labels, no duplicates) + ignore @@ + List.fold_left( + (acc, (label, (_, loc, used))) => { + let label_name = + switch (label) { + | LabeledArg(name) => name + | UnlabeledArg(idx) => string_of_int(idx) + }; + if (used^ == false) { + raise( + Error( + loc, + if (StringSet.mem(label_name, acc)) { + DuplicateLabelParamType({name: label_name}); + } else { + switch (label) { + | LabeledArg(name) => MissingLabeledParamType({name: name}) + | UnlabeledArg(idx) => MissingUnlabeledParamType({idx: idx}) + }; + }, + ), + ); + }; + StringSet.add(label_name, acc); + }, + StringSet.empty, + param_attributes, + ); + + // Map attributes to their respective fields let (deprecations, since, history, params, returns, throws, examples) = List.fold_left( ( - (deprecations, since, history, params, returns, throws, examples), + (deprecations, since, history, params, returns, throws, examples) as acc, {attr, attr_loc}: Comment_attributes.t, ) => { switch (attr) { @@ -467,55 +588,8 @@ let for_value_description = throws, examples, ) - | 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( - Error(attr_loc, 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; extract the type from the Option - | Some((Default(_), {desc: TTyConstr(_, [typ], _)})) => ( - "?" ++ name, - Printtyp.string_of_type_sch(typ), - ) - | _ => - raise( - Error(attr_loc, MissingLabeledParamType({name: name})), - ) - } - }; - - ( - deprecations, - since, - history, - [ - { - param_id, - param_type, - param_msg, - param_loc: attr_loc, - }, - ...params, - ], - returns, - throws, - examples, - ); + // Do nothing because param documentation is precomputed + | Param(_) => acc | Returns({attr_desc: returns_msg}) => switch (returns) { | Some(_) => @@ -579,7 +653,7 @@ let for_value_description = } }, // deprecations, since, history, params, returns, throws, examples - ([], None, [], [], None, [], []), + ([], None, [], List.rev(documented_params), None, [], []), attributes, ); diff --git a/compiler/src/utils/warnings.re b/compiler/src/utils/warnings.re index fd513fddf2..d89ff03b4d 100644 --- a/compiler/src/utils/warnings.re +++ b/compiler/src/utils/warnings.re @@ -42,9 +42,10 @@ type t = | UselessRecordSpread | PrintUnsafe(string) | ToStringUnsafe(string) - | ArrayIndexNonInteger(string); + | ArrayIndexNonInteger(string) + | ParameterDocumentationMissing(string); -let last_warning_number = 22; +let last_warning_number = 23; let number = fun @@ -69,7 +70,8 @@ let number = | UselessRecordSpread => 19 | PrintUnsafe(_) => 20 | ToStringUnsafe(_) => 21 - | ArrayIndexNonInteger(_) => last_warning_number; + | ArrayIndexNonInteger(_) => 22 + | ParameterDocumentationMissing(_) => last_warning_number; let message = fun @@ -175,7 +177,9 @@ let message = ++ typ ++ " from the `runtime/debugPrint` module instead." | ArrayIndexNonInteger(idx) => - "Array index should be an integer, but found `" ++ idx ++ "`."; + "Array index should be an integer, but found `" ++ idx ++ "`." + | ParameterDocumentationMissing(param) => + "Missing documentation for parameter `" ++ param ++ "`."; let sub_locs = fun @@ -223,6 +227,7 @@ let defaults = [ PrintUnsafe(""), ToStringUnsafe(""), ArrayIndexNonInteger(""), + ParameterDocumentationMissing(""), ]; let _ = List.iter(x => current^.active[number(x)] = true, defaults); diff --git a/compiler/src/utils/warnings.rei b/compiler/src/utils/warnings.rei index 971846cccb..e51bc69a77 100644 --- a/compiler/src/utils/warnings.rei +++ b/compiler/src/utils/warnings.rei @@ -56,7 +56,8 @@ type t = | UselessRecordSpread | PrintUnsafe(string) | ToStringUnsafe(string) - | ArrayIndexNonInteger(string); + | ArrayIndexNonInteger(string) + | ParameterDocumentationMissing(string); let is_active: t => bool; let is_error: t => bool; diff --git a/compiler/test/graindoc/paramDuplicate.input.gr b/compiler/test/graindoc/paramDuplicate.input.gr new file mode 100644 index 0000000000..76eae815b9 --- /dev/null +++ b/compiler/test/graindoc/paramDuplicate.input.gr @@ -0,0 +1,7 @@ +module ParamDuplicate + +/** + * @param a: This is the first + * @param a: This is a duplicate + */ +provide let test1 = a => a + a diff --git a/compiler/test/graindoc/paramUnknown.input.gr b/compiler/test/graindoc/paramUnknown.input.gr new file mode 100644 index 0000000000..2e11c4adb3 --- /dev/null +++ b/compiler/test/graindoc/paramUnknown.input.gr @@ -0,0 +1,6 @@ +module ParamUnknown + +/** + * @param x: This parameter is documented + */ +provide let test2 = a => a diff --git a/compiler/test/graindoc/params.expected.md b/compiler/test/graindoc/params.expected.md new file mode 100644 index 0000000000..6e26c88d3f --- /dev/null +++ b/compiler/test/graindoc/params.expected.md @@ -0,0 +1,93 @@ +--- +title: Params +--- + +## Values + +Functions and constants included in the Params module. + +### Params.**test1** + +```grain +test1: (a: Number, b: Number) => Number +``` + +Fully documented labeled parameters test. + +Parameters: + +| param | type | description | +| ----- | -------- | ------------------ | +| `a` | `Number` | This is the first | +| `b` | `Number` | This is the second | + +### Params.**test2** + +```grain +test2: ((Number, a), (Number, b)) => Number +``` + +Fully documented unlabeled parameters test. + +Parameters: + +| param | type | description | +| ----- | ------------- | ------------------ | +| `0` | `(Number, a)` | This is the first | +| `1` | `(Number, b)` | This is the second | + +### Params.**test3** + +```grain +test3: (a: Number, (Number, a), c: Number) => Number +``` + +Fully documented mixed parameters test. + +Parameters: + +| param | type | description | +| ----- | ------------- | ------------------ | +| `a` | `Number` | This is the first | +| `1` | `(Number, a)` | This is the second | +| `c` | `Number` | This is the third | + +### Params.**test4** + +```grain +test4: (a: Number, ?b: Number) => Number +``` + +Optional parameters test. + +Parameters: + +| param | type | description | +| ----- | -------- | ------------- | +| `a` | `Number` | This is the a | +| `?b` | `Number` | This is the b | + +### Params.**test5** + +```grain +test5: (a: Number, b: Number) => Number +``` + +No documentation test. + +### Params.**test6** + +```grain +test6: (a: Number, b: Number, c: Number) => Number +``` + +Partially documented parameters test. + +Parameters: + +| param | type | description | +| ----- | -------- | ------------- | +| `a` | `Number` | This is the a | +| `b` | `Number` | | +| `c` | `Number` | This is the c | + diff --git a/compiler/test/graindoc/params.input.gr b/compiler/test/graindoc/params.input.gr new file mode 100644 index 0000000000..10d21e7df2 --- /dev/null +++ b/compiler/test/graindoc/params.input.gr @@ -0,0 +1,47 @@ +module Params + +/** + * Fully documented labeled parameters test. + * + * @param a: This is the first + * @param b: This is the second + */ +provide let test1 = (a, b) => a + b + +/** + * Fully documented unlabeled parameters test. + * + * @param 0: This is the first + * @param 1: This is the second + */ +provide let test2 = ((a, _), (b, _)) => a + b + +/** + * Fully documented mixed parameters test. + * + * @param a: This is the first + * @param 1: This is the second + * @param c: This is the third + */ +provide let test3 = (a, (b, _), c) => a + b + c + +/** + * Optional parameters test. + * + * @param a: This is the a + * @param b: This is the b + */ +provide let test4 = (a, b=1) => a + b + +/** + * No documentation test. + */ +provide let test5 = (a, b) => a + b + +/** + * Partially documented parameters test. + * + * @param a: This is the a + * @param c: This is the c + */ +provide let test6 = (a, b, c) => a + b + c diff --git a/compiler/test/runner.re b/compiler/test/runner.re index 17332e6be8..d4c57ded34 100644 --- a/compiler/test/runner.re +++ b/compiler/test/runner.re @@ -287,7 +287,7 @@ let doc = (file, arguments) => { let (code, out, err) = open_process(cmd); - (out ++ err, code); + (out, err, code); }; let lsp = stdin_input => { @@ -636,16 +636,16 @@ let makeFormatterRunner = (test, name, filename) => { ); }; -let makeGrainDocRunner = (test, name, filename, arguments) => { +let makeGrainDocRunner = (test, name, filename, arguments, stderr) => { test( name, ({expect}) => { let infile = graindoc_in_file(filename); - let (result, _) = doc(infile, arguments); + let (result, err, _) = doc(infile, arguments); // we need do a binary content comparison to ensure the EOL is correct - expect.ext.binaryFile(result).toBinaryMatch(graindoc_out_file(name)); + expect.string(err).toMatch(stderr); }, ); }; @@ -655,8 +655,9 @@ let makeGrainDocErrorRunner = (test, name, filename, expected, arguments) => { name, ({expect}) => { let infile = graindoc_in_file(filename); - let (result, _) = doc(infile, arguments); - expect.string(result).toMatch(expected); + let (result, err, _) = doc(infile, arguments); + expect.string(result).toBeEmpty(); + expect.string(err).toMatch(expected); }, ); }; diff --git a/compiler/test/suites/graindoc.re b/compiler/test/suites/graindoc.re index 7152de780a..ed6a6fe9da 100644 --- a/compiler/test/suites/graindoc.re +++ b/compiler/test/suites/graindoc.re @@ -11,20 +11,27 @@ describe("graindoc", ({test, testSkip}) => { let assertGrainDocOutput = makeGrainDocRunner(test_or_skip); let assertGrainDocError = makeGrainDocErrorRunner(test_or_skip); - assertGrainDocOutput("noDoc", "noDoc", [||]); + assertGrainDocOutput("noDoc", "noDoc", [||], ""); assertGrainDocOutput( "descriptions", "descriptions", [|"--current-version=v0.2.0"|], + "", + ); + assertGrainDocOutput("since", "since", [|"--current-version=v0.2.0"|], ""); + assertGrainDocOutput( + "example", + "example", + [|"--current-version=v0.2.0"|], + "", ); - assertGrainDocOutput("since", "since", [|"--current-version=v0.2.0"|]); - assertGrainDocOutput("example", "example", [|"--current-version=v0.2.0"|]); assertGrainDocOutput( "functionDoc", "functionDoc", [|"--current-version=v0.2.0"|], + "", ); - assertGrainDocOutput("types", "types", [|"--current-version=v0.2.0"|]); + assertGrainDocOutput("types", "types", [|"--current-version=v0.2.0"|], ""); assertGrainDocError( "singleSince", "singleSince", @@ -43,4 +50,22 @@ describe("graindoc", ({test, testSkip}) => { "line 5", [|"--current-version=v0.2.0"|], ); + assertGrainDocOutput( + "params", + "params", + [|"--current-version=v0.2.0"|], + "Warning 23: Missing documentation for parameter `b`.", + ); + assertGrainDocError( + "paramDuplicate", + "paramDuplicate", + "Error: Multiple `@param` attributes found for parameter a. Each parameter may only have one associated `@param` attribute.", + [|"--current-version=v0.2.0"|], + ); + assertGrainDocError( + "paramUnknown", + "paramUnknown", + "Error: Unable to find a matching function parameter for x. Make sure a parameter exists with this label or use `@param x` for unlabeled parameters.", + [|"--current-version=v0.2.0"|], + ); }); diff --git a/stdlib/uri.gr b/stdlib/uri.gr index cac6a15710..99bb0de6a2 100644 --- a/stdlib/uri.gr +++ b/stdlib/uri.gr @@ -323,6 +323,8 @@ provide let decode = str => { * Encodes a list of key-value pairs into an query string. * * @param urlVals: A list of key-value pairs + * @param encodeSet: An indication for which characters to percent-encode. + * * @returns A query string * * @since v0.6.0 diff --git a/stdlib/uri.md b/stdlib/uri.md index cb24960937..7a6bd1d95e 100644 --- a/stdlib/uri.md +++ b/stdlib/uri.md @@ -182,9 +182,10 @@ Encodes a list of key-value pairs into an query string. Parameters: -| param | type | description | -| --------- | ------------------------ | ------------------------- | -| `urlVals` | `List<(String, String)>` | A list of key-value pairs | +| param | type | description | +| ------------ | ------------------------ | ----------------------------------------------------- | +| `urlVals` | `List<(String, String)>` | A list of key-value pairs | +| `?encodeSet` | `EncodeSet` | An indication for which characters to percent-encode. | Returns: