When would it make sense to add validation for options using
Keyword.validate (or the backported variant) in Elixir's standard lib, Ecto
or Phoenix?
There's a runtime penalty for doing the validation. Would it make sense to
add it "everywhere" but actually do it only for certain values of Mix.env()?
wt., 14 gru 2021 o 13:26 Wojtek Mach <woj...@wojtekmach.pl> napisał(a):
> I don't think most features should be backported as it increases
> maintenance burden (on features themselves but also more minor stuff like
> @doc since). I definitely agree about the usefulness though. I tend to
> vendor-in the new functionality and at compile-time pick between the
> vendored and upstream implementation. For Keyword.validate/2 it would be:
>
> # TODO: use Keyword.validate when we require Elixir 1.13
> if Version.compare(System.version(), ">= 1.13.0") do
> defp keyword_validate(keyword, values) do
> Keyword.validate(keyword, values)
> end
> else
> defp keyword_validate(keyword, values) when is_list(keyword) and
> is_list(values) do
> validate(keyword, values, [], [], [])
> end
>
> defp validate([{key, _} = pair | keyword], values1, values2, acc,
> bad_keys)
> when is_atom(key) do
> case find_key!(key, values1, values2) do
> {values1, values2} ->
> validate(keyword, values1, values2, [pair | acc], bad_keys)
>
> :error ->
> case find_key!(key, values2, values1) do
> {values1, values2} ->
> validate(keyword, values1, values2, [pair | acc], bad_keys)
>
> :error ->
> validate(keyword, values1, values2, acc, [key | bad_keys])
> end
> end
> end
>
> defp validate([], values1, values2, acc, []) do
> {:ok, move_pairs!(values1, move_pairs!(values2, acc))}
> end
>
> defp validate([], _values1, _values2, _acc, bad_keys) do
> {:error, bad_keys}
> end
>
> defp validate([pair | _], _values1, _values2, _acc, []) do
> raise ArgumentError,
> "expected a keyword list as first argument, got invalid entry:
> #{inspect(pair)}"
> end
>
> defp find_key!(key, [key | rest], acc), do: {rest, acc}
> defp find_key!(key, [{key, _} | rest], acc), do: {rest, acc}
> defp find_key!(key, [head | tail], acc), do: find_key!(key, tail, [head
> | acc])
> defp find_key!(_key, [], _acc), do: :error
>
> defp move_pairs!([key | rest], acc) when is_atom(key),
> do: move_pairs!(rest, acc)
>
> defp move_pairs!([{key, _} = pair | rest], acc) when is_atom(key),
> do: move_pairs!(rest, [pair | acc])
>
> defp move_pairs!([], acc),
> do: acc
>
> defp move_pairs!([other | _], _) do
> raise ArgumentError,
> "expected the second argument to be a list of atoms or tuples,
> got: #{inspect(other)}"
> end
>
> defp keyword_validate!(keyword, values) do
> case keyword_validate(keyword, values) do
> {:ok, kw} ->
> kw
>
> {:error, invalid_keys} ->
> keys =
> for value <- values,
> do: if(is_atom(value), do: value, else: elem(value, 0))
>
> raise ArgumentError,
> "unknown keys #{inspect(invalid_keys)} in
> #{inspect(keyword)}, the allowed keys are: #{inspect(keys)}"
> end
> end
> end
>
>
>
>
> On December 14, 2021, "chrobot.io" <ste...@chrobot.io> wrote:
>
> Hi, I'd like to follow up on this since Keyword.validate has landed in
> 1.13. Is there a will to use the validation in common libraries?
>
> Just recently I've been hit by misspelling "redact: true" with "redacted:
> true" when defining a field in Ecto. Sounds like a perfect use case for
> Keyword.validate. Ideally this would explode, but that's a breaking change,
> so emitting a warning would probably make more sense.
>
> The other thing is that most libraries tend to stay on the oldest Elixir
> version possible, so they can't just call Keyword.validate (unless I'm
> missing something). In our case we're running on Elixir 1.10, but we
> backport small bits because of their usefulness (so for example we have
> Future.Keyword.validate and Future.tap). Is something like that a viable
> way to do this in libraries? I guess the other option is conditional
> compilation, but it sounds like a code that's going to stay there for a
> very long time (what would be the incentive to bump the minimum version as
> high as 1.13?).
>
> Looking forward to your thoughts on this.
>
> Best,
> Stefan
>
> wtorek, 27 lipca 2021 o 10:02:38 UTC+2 polva...@gmail.com napisał(a):
>>
>> The PR has been sent! Link:
>> https://github.com/elixir-lang/elixir/pull/11149
>>
>> Em segunda-feira, 26 de julho de 2021 às 15:07:18 UTC-3, José Valim
>> escreveu:
>>
>>> In this case, I would call Keyword.validate! And then call Map.new. I
>>> don’t see a benefit in the generic API as it won’t be faster and not
>>> necessarily clearer either. So I would stick with my last proposal. :)
>>>
>>> On Mon, Jul 26, 2021 at 20:01 Paulo Valente <polva...@gmail.com> wrote:
>>>
>>>> I was considering making it only accept keywords, and return keywords
>>>> as well.
>>>>
>>>> However, I can see benefits in accepting Enumerables. What do you think
>>>> about using another verb such as cast or coerce, instead of validate, for
>>>> this typing
>>>> (e.g. @spec Keyword.cast!(Enumerable.t(), keyword) :: keyword |
>>>> no_return and @spec Map.cast!(Enumerable.t(), keyword) :: map | no_return)?
>>>>
>>>> Should I also implement it for Map as well, or maybe add it in a second
>>>> PR?
>>>>
>>>> Thanks for the idea on Enumerables!
>>>> Em segunda-feira, 26 de julho de 2021 às 12:47:31 UTC-3, Wojtek Mach
>>>> escreveu:
>>>>
>>>>> Keyword.validate will accept just a keyword, right? Did you consider
>>>>> making it accept any enumerable of pairs, just like Keyword.new does? Same
>>>>> for Map.
>>>>>
>>>>> I think one particular scenario is something like this:
>>>>>
>>>>> def foo(opts) when is_list(opts) do
>>>>> config = Map.validate!(opts, …)
>>>>> config.name # the assertive map.key is really convenient here
>>>>>
>>>>> what do you think? The only concern, besides feature creep :), is
>>>>> perhaps validate is not the best name given it accepts broader set of
>>>>> inputs. But then again, Keyword.keyword and in particular Map.map are
>>>>> maybe
>>>>> a bit awkward.
>>>>>
>>>>> On July 26, 2021, josevalim <jose....@gmail.com> wrote:
>>>>>
>>>>> I debated this with the Elixir team and we agreed on the following API:
>>>>> Keyword.validate! :: keyword | no_return Keyword.validate ::
>>>>> {:ok, keyword} | {:error, unknown_keys, missing_keys}
>>>>> The same functionality would have to be defined for Map.
>>>>> The API for Keyword.validate! and Map.validate! would be the exact
>>>>> same as the one found in Nx.Defn.Kernel.keyword!.
>>>>> Could you please send a PR? Or open up an issue if you can't submit it
>>>>> at the moment.
>>>>>
>>>>> Thanks!On Monday, July 12, 2021 at 12:46:38 PM UTC+2
>>>>> polva...@gmail.com wrote:
>>>>>
>>>>> > I would propose to add this to the Kernel module, to mirror the
>>>>>> struct! helper. But adding to Keyword also works. Thoughts?
>>>>>>
>>>>>> Makes sense! I suggested Keyword mostly because there's the module
>>>>>> already, but since we have struct! in Kernel, it might be better to have
>>>>>> keyword! there too. Usage would also be nicer this way.
>>>>>>
>>>>>> > Should the bang version really raise for redundant options?
>>>>>>
>>>>>> As José said, it would help clean out stray options being passed. I
>>>>>> can think of a few cases in which I thought one option was being used,
>>>>>> but
>>>>>> had either a typo or wrong name altogether (think "end" vs "end_datetime"
>>>>>> sort of situation).
>>>>>> However, I can see a less strict version being useful as well.
>>>>>> Perhaps there could be a keyword!/3 which accepted options like:
>>>>>> keyword!(opts, [extra_keys: true], key1: :default1, key2: :default2)
>>>>>>
>>>>>> --
>>>>> You received this message because you are subscribed to the Google
>>>>> Groups "elixir-lang-core" group.
>>>>> To unsubscribe from this group and stop receiving emails from it,
>>>>> send an email to elixir-lang-co...@googlegroups.com.
>>>>>
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/d/msgid/elixir-lang-core/9b83a803-c2c1-49e7-a531-56791607bcaan%40googlegroups.com
>>>>> <https://groups.google.com/d/msgid/elixir-lang-core/9b83a803-c2c1-49e7-a531-56791607bcaan%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>> .
>>>>>
>>>>> --
>>>> You received this message because you are subscribed to the Google
>>>> Groups "elixir-lang-core" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>> an email to elixir-lang-co...@googlegroups.com.
>>>>
>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/elixir-lang-core/a96fb19a-8148-41dc-8b45-0e2b9fe755b8n%40googlegroups.com
>>>> <https://groups.google.com/d/msgid/elixir-lang-core/a96fb19a-8148-41dc-8b45-0e2b9fe755b8n%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>> .
>>>>
>>>
> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to elixir-lang-core+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/elixir-lang-core/43f206e4-6b22-4622-b8d9-1ade01556039n%40googlegroups.com
> <https://groups.google.com/d/msgid/elixir-lang-core/43f206e4-6b22-4622-b8d9-1ade01556039n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
> --
> You received this message because you are subscribed to the Google Groups
> "elixir-lang-core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to elixir-lang-core+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/elixir-lang-core/b8beea46f50e401bf00961fff49a76f68c360bb3%40hey.com
> <https://groups.google.com/d/msgid/elixir-lang-core/b8beea46f50e401bf00961fff49a76f68c360bb3%40hey.com?utm_medium=email&utm_source=footer>
> .
>
--
You received this message because you are subscribed to the Google Groups
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/elixir-lang-core/CACzMe7aTSK%3Dpni6gM2xDhXoUKFjJTTZ9cY4TqXniDUtddohA3Q%40mail.gmail.com.
