From 6b6dda63b046832e255e22f9ed36f9f6b54500ed Mon Sep 17 00:00:00 2001 From: Mikhail Pustovalov Date: Mon, 24 Nov 2025 16:51:44 +0400 Subject: [PATCH 1/3] Fix logic in has_missing_options/2: use filter instead of dropwhile The function could report present options as missing: it stopped at the first missing option and returned it with the rest of the list which could contain options that are actually present. --- src/term_validator.erl | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/term_validator.erl b/src/term_validator.erl index 51d584b..2744b3a 100644 --- a/src/term_validator.erl +++ b/src/term_validator.erl @@ -153,14 +153,12 @@ has_dynamic_options(_MandatoryOptions, _OptionalOptions) -> -spec has_missing_options(options(), [option_name()]) -> no | {yes, [option_name()]}. has_missing_options(Options, MandatoryOptions) -> - % We remove mandatory option one by one if they're present. If there's - % remaining mandatory options, then we have missing options. - RemainingOptions = lists:dropwhile(fun(Name) -> - proplists:is_defined(Name, Options) + Missing = lists:filter(fun(Name) -> + not proplists:is_defined(Name, Options) end, MandatoryOptions), - case RemainingOptions of + case Missing of [] -> no; - _ -> {yes, RemainingOptions} + _ -> {yes, Missing} end. -spec has_invalid_options(options(), [option_name()]) -> From 7d028357725aef406240760714c2adf68572c63b Mon Sep 17 00:00:00 2001 From: Mikhail Pustovalov Date: Mon, 24 Nov 2025 16:54:40 +0400 Subject: [PATCH 2/3] Fix pattern matching in validators to catch all errors Previously, validators would crash on {no_validator, _} or {invalid_options, _} tuples because the case clause only expected {invalid, _} or valid. --- src/list_validator.erl | 4 +++- src/map_dynamic_validator.erl | 8 ++++++-- src/map_validator.erl | 4 +++- src/tuple_dynamic_validator.erl | 4 +++- src/tuple_validator.erl | 4 +++- 5 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/list_validator.erl b/src/list_validator.erl index 04bf1ff..275ecaf 100644 --- a/src/list_validator.erl +++ b/src/list_validator.erl @@ -43,7 +43,9 @@ validate(Term, {item, Format}, Validators) -> valid -> Accumulator; {invalid, Reason} -> - [{Index, Reason}|Accumulator] + [{Index, Reason}|Accumulator]; + OtherError -> + [{Index, OtherError}|Accumulator] end end, [], diff --git a/src/map_dynamic_validator.erl b/src/map_dynamic_validator.erl index 73422eb..7d814f3 100644 --- a/src/map_dynamic_validator.erl +++ b/src/map_dynamic_validator.erl @@ -41,7 +41,9 @@ validate(Term, {key, Format}, Validators) -> valid -> Accumulator; {invalid, Reason} -> - [{Key, Reason}|Accumulator] + [{Key, Reason}|Accumulator]; + OtherError -> + [{Key, OtherError}|Accumulator] end end, [], maps:keys(Term)), case Result of @@ -57,7 +59,9 @@ validate(Term, {value, Format}, Validators) -> valid -> Accumulator; {invalid, Reason} -> - [{Key, Reason}|Accumulator] + [{Key, Reason}|Accumulator]; + OtherError -> + [{Key, OtherError}|Accumulator] end end, [], Term), case Result of diff --git a/src/map_validator.erl b/src/map_validator.erl index 400bdf8..84353f2 100644 --- a/src/map_validator.erl +++ b/src/map_validator.erl @@ -59,7 +59,9 @@ validate(Term, {fields, Fields}, Validators) -> valid -> Accumulator; {invalid, Reason} -> - {UnexpectedFields, [{Key, Reason}|InvalidFields]} + {UnexpectedFields, [{Key, Reason}|InvalidFields]}; + OtherError -> + {UnexpectedFields, [{Key, OtherError}|InvalidFields]} end end end, diff --git a/src/tuple_dynamic_validator.erl b/src/tuple_dynamic_validator.erl index 92400fb..07d2309 100644 --- a/src/tuple_dynamic_validator.erl +++ b/src/tuple_dynamic_validator.erl @@ -43,7 +43,9 @@ validate(Term, {element, Format}, Validators) -> valid -> Accumulator; {invalid, Reason} -> - [{Index, Reason}|Accumulator] + [{Index, Reason}|Accumulator]; + OtherError -> + [{Index, OtherError}|Accumulator] end end, [], diff --git a/src/tuple_validator.erl b/src/tuple_validator.erl index 8ecced5..9a7250b 100644 --- a/src/tuple_validator.erl +++ b/src/tuple_validator.erl @@ -39,7 +39,9 @@ validate(Term, {elements, Elements}, Validators) -> valid -> Accumulator; {invalid, Reason} -> - [{Index, Reason}|Accumulator] + [{Index, Reason}|Accumulator]; + OtherError -> + [{Index, OtherError}|Accumulator] end end, [], From 7d06a24a1b2c7686ae370f1aeb8f0864d4cf8d29 Mon Sep 17 00:00:00 2001 From: Mikhail Pustovalov Date: Mon, 24 Nov 2025 17:00:11 +0400 Subject: [PATCH 3/3] map_validator: validate 'fields' option type and improve error handling - Add `is_list(Fields)` guard to `map_validator:validate/3`. - Add a catch-all clause to return `{invalid_option_value, not_list}` if the `fields` option is not a list, preventing a crash. - Add tests for map_validator: - missing mandatory options - invalid/Unknown options - invalid option values (non-list fields) - handling of unknown validators in fields --- src/map_validator.erl | 6 ++++-- test/map_validator_test.erl | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/map_validator.erl b/src/map_validator.erl index 84353f2..b8e1a0a 100644 --- a/src/map_validator.erl +++ b/src/map_validator.erl @@ -27,7 +27,7 @@ pre_validate(Term, Options, _Validators) when is_map(Term) -> pre_validate(_Term, _Options, _Validators) -> {invalid, not_map}. -validate(Term, {fields, Fields}, Validators) -> +validate(Term, {fields, Fields}, Validators) when is_list(Fields) -> % We first compute the missing fields by iterating over the declared % mandatory fields and check if they are all present in the Erlang map. % Then we compute the invalid and extra fields by iterating over the fields @@ -81,7 +81,9 @@ validate(Term, {fields, Fields}, Validators) -> end; _ -> {invalid, {missing_fields, MissingFields}} - end. + end; +validate(_Term, {fields, _Fields}, _Validators) -> + {invalid_option_value, not_list}. post_validate(_Term, _Validators) -> valid. diff --git a/test/map_validator_test.erl b/test/map_validator_test.erl index 094a7b5..ca83058 100644 --- a/test/map_validator_test.erl +++ b/test/map_validator_test.erl @@ -71,3 +71,22 @@ map_validator_fields_test() -> true = lists:member({"bar", not_number}, Fields), ok. + +map_validator_options_test() -> + UnknownValidatorFormat = {map, [{fields, [ + {foo, unknown_validator, mandatory} + ]}]}, + {invalid, {fields, [{foo,{no_validator,unknown_validator}}]}} = term_validator:validate(#{ + foo => true + }, UnknownValidatorFormat), + + MissingOptionFormat = {map, []}, + {missing_options, [fields]} = term_validator:validate(#{}, MissingOptionFormat), + + InvalidOptionsFormat = {map, [{fields, []}, {unknown_option, true}]}, + {invalid_options, [unknown_option]} = term_validator:validate(#{}, InvalidOptionsFormat), + + InvalidOptionValue = {map, [{fields, atom}]}, + {invalid_option_value,{fields,atom},not_list} = term_validator:validate(#{}, InvalidOptionValue), + + ok.