dashbitco / nimble_options

A tiny library for validating and documenting high-level options. πŸ’½
Apache License 2.0
507 stars 38 forks source link

How to validate a list of keyword lists #77

Closed MikaAK closed 2 years ago

MikaAK commented 2 years ago

When trying to validate a list of keyword lists, I would assume the following would work:

definitions = [
  my_items: [
    type: {:list, :keyword_list}, 
    keys: [key: [type: :string]]
  ]
]

NimbleOptions.validate!([
  my_items: [
    [key: "value"],
    [key: "value"]
  ]
], definitions)

But instead I'm met with the following errors

** (ArgumentError) expected a keyword list, but an entry in the list is not a two-element tuple with an atom as its first element, got: [key: "value"]
    (elixir 1.12.1) lib/keyword.ex:475: Keyword.keys/1
    (nimble_options 0.4.0) lib/nimble_options.ex:388: NimbleOptions.validate_unknown_options/2
    (nimble_options 0.4.0) lib/nimble_options.ex:376: NimbleOptions.validate_options_with_schema_and_path/3
    (nimble_options 0.4.0) lib/nimble_options.ex:409: NimbleOptions.reduce_options/2
    (elixir 1.12.1) lib/enum.ex:4251: Enumerable.List.reduce/3
    (elixir 1.12.1) lib/enum.ex:2402: Enum.reduce_while/3
    (nimble_options 0.4.0) lib/nimble_options.ex:402: NimbleOptions.validate_options/2
    (nimble_options 0.4.0) lib/nimble_options.ex:377: NimbleOptions.validate_options_with_schema_and_path/3
josevalim commented 2 years ago

The docs have an example: https://github.com/dashbitco/nimble_options/blob/main/lib/nimble_options.ex#L136

:concurrency is nested inside :producer.

MikaAK commented 2 years ago

Hey @josevalim i think there's a mixup here, the docs show one layer of nesting

[producer: [consumer: 1]]

I'm talking about

[
  producer: [
    [consumer: 1],
    [consumer: 2]
  ]
]
wojtekmach commented 2 years ago

I believe what @MikaAK wants is not nested keyword lists, but a list of keyword lists. You can achieve that with a custom validator:

Mix.install([:nimble_options])

defmodule Validators do
  def validate(values) do
    spec = [
      key: [type: :string]
    ]

    case NimbleOptions.validate(values, spec) do
      {:ok, value} -> {:ok, value}
      {:error, exception} -> {:error, Exception.message(exception)}
    end
  end
end

spec = [
  my_items: [
    type: {:list, {:custom, Validators, :validate, []}}
  ]
]

NimbleOptions.validate!(
  [
    my_items: [
      [key: "value"],
      [key: 24]
    ]
  ],
  spec
)
|> IO.inspect()
MikaAK commented 2 years ago

Thanks @wojtekmach, I wasn't sure if I was just missing something in the docs πŸ˜†

Would there by any interest in me implementing this? Since :list specifies Available types: :any, :keyword_list, :non_empty_keyword_list this is a bit misleading when dealing with it in iex!

josevalim commented 2 years ago

Sorry @MikaAK for not getting it!

Since :list specifies Available types: :any, :keyword_list, :non_empty_keyword_list this is a bit misleading when dealing with it in iex!

What do you mean?

wojtekmach commented 2 years ago

Well, while :list does allow using any type, if you look at the docs for the :keys option, we have:

:keys - Available for types :keyword_list and :non_empty_keyword_list, it defines (...)

so I'd argue the docs are clear enough.

wojtekmach commented 2 years ago

Just to be clear, I think we could support what you're proposing, I just don't know whether we should special case lists of keywords or instead we should continue with the custom type as shown above, perhaps make it more convenient to use.

MikaAK commented 2 years ago

If you pass in an invalid :list it spits out what parameters are available, for example:

** (ArgumentError) invalid schema given to NimbleOptions.validate/2. Reason: invalid subtype for :list type: invalid option type ...

Available types: :any, :keyword_list, :non_empty_keyword_list, :atom, :integer, :non_neg_integer, :pos_integer, :float, :mfa, :mod_arg, :string, :boolean, :timeout, :pid, {:fun, arity}, {:in, choices}, {:or, subtypes}, {:custom, mod, fun, args}, {:list, subtype} (in options [:mutations])

I guess I'm curious how you would use :keyword_list at all in this context

But yes if there was a more convenient way to use it it would be great, happy to help contribute πŸ˜„

josevalim commented 2 years ago

Thank you! It seems the error message is wrong. It is listing all types but I don't think we can support all of them. A PR to address this is welcome. In your actual code, I suggest going with a custom validation!

wojtekmach commented 2 years ago

could you show the code that produced the error in your last comment? Is it something like: type: {:list, :bad}? If so, I believe the error is correct.

Regarding using list of keywords, you'd do: type: {:list, :keyword_list} and it will work:

Mix.install([:nimble_options])

definitions = [
  my_items: [
    type: {:list, :keyword_list}
  ]
]

NimbleOptions.validate!(
  [
    my_items: [
      [key: "value"],
      [key: "value"]
    ]
  ],
  definitions
)
|> IO.inspect()

it's just that you cannot use the :keys option because it doesn't work with the {:list, subtype} type.

MikaAK commented 2 years ago

I was honestly just trying random things at that point, while I was surfing the docs I found https://hexdocs.pm/nimble_options/0.4.0/NimbleOptions.html#docs/2 which stated you can do {:or, [:string, keyword_list: nested_schema]} so I figured I'd try and see if that'd work with {:list, keword_list: schema}

Seeing how it works now that does make sense, it would be great if you could pass more schema options into it like you can with the {:or!

I'll start digging around and seeing how I can help

PS: Thanks for your guys help, much appreciated!!

whatyouhide commented 2 years ago

@MikaAK yes, what we want is support for {:list, keyword_list: nested_schema} for the :list type too, like we have with {:or, [:string, keyword_list: nested_schema]}. Any help with the code and docs would be appreciated! πŸ™ƒ

MikaAK commented 2 years ago

Perfect! Now that I know the goal I'll take a look and see what I can do πŸ˜„