Add type validator.

[danhper]

Hi,

I would like to validate the types of the values in a struct, but I could not find any easy
way to do it, so I added a validator to check for types.
It works with all builtin types and can be used to check complex types %{:atom => binary}, or things like this.
Take a look at the docs examples and test cases for more info.

Thank you!

[benwilson512]

Can you elaborate a bit about what this function is doing? I get what the result is but the method you need to achieve it is confusing to me.

[danhper]

Given something like [:string, :atom, [list: :string]] it should return ":string, :atom or [list: :string]".
So given types t1, t2, ..., tn it should join t1...t(n-1) with a comma and join t1...t(n-1) and tn with or.
As types do not all implement String.Chars, I am using inspect to get a string.
I am not particularly happy with this implementation so if you have improvements idea I am all ears.

[sascha-wolf]

I have some suggestions:

{last_type, but_last_types} =
  acceptable_types
  |> Enum.map(&inspect/1)
  |> List.pop_at(-1)

but_last = Enum.join(but_last_types, ", ")

"#{but_last} or #{last_type}"

or alternatively:

defp acceptable_type_str([type]) do
  inspect(type)
end

defp acceptable_type_str([type, last_type]) do
  "#{inspect(type)} or #{inspect(last_type)}"
end

defp acceptable_type_str([type | tail]) do
  "#{inspect(type)}, " <> acceptable_type_string(tail)
end

defp acceptable_type_str(type) do
  inspect(type)
end

or:

defp acceptable_type_str([type | tail]) do
  "#{inspect(type)}" <>
    case tail do
      [] -> ""
      [last] -> " or #{inspect(last)}"
      more -> ", " <> acceptable_type_str(more)
    end
end

defp acceptable_type_str(type) do
  inspect(type)
end

I might have a went a bit overboard here... 😄

[benwilson512]

Hey there!

First I want to say that this is an impressive endeavor to embark upon. You're basically retro-fitting static type analysis into the language, and that can be a real challenge. You've done a very good job so far.

I'll tell you that I am at the moment a little skeptical that we can make this work in a sufficiently generic way. We know a thing or two about type systems in elixir from our work on doing GraphQL (http://absinthe-graphql.org/), and it can be a rather hairy process.

For now, the first roadblock is nil. At the moment nil will simply get treated as an atom, but that isn't quite right. You probably need to allow nil to be a value in a type list, that indicates that the type can be nil. IE %{:any => [:binary, nil]} would allow nil values, whereas %{:any => :binary} would not.

From there the next thing people are going to want is the ability to specify particular values. You can do it in elixir typespec with atoms e.g @type foo :: :foo | :bar | :baz and people will no doubt want it here if we're doing type support. This gets a bit challenging because at the moment we're representing types via atoms, but those same atoms might also get used as values.

Then we're in trouble because the moment we allow values into the system we gotta worry about mandatory vs optional values for our container types, and so on.

You see why I'm a bit concerned about trying to support this.

What are your thoughts @bruce ?

[danhper]

Hi @benwilson512,

Thank you very much for the detailed feedback!
I know that type checking is not an easy task, but in the case of this library,
I think it could be very useful.
I also think it should be enough for a good percentage of most people use cases,
the most common one IMO clearly being to check if a top level value is or not of the right type.

For now, the first roadblock is nil. At the moment nil will simply get treated as an atom, but that isn't quite right. You probably need to allow nil to be a value in a type list, that indicates that the type can be nil. IE %{:any => [:binary, nil]} would allow nil values, whereas %{:any => :binary} would not.

You are perfectly right. I was using allow_nil for my particular case, but this will clearly
not work for nested types. Making nil a special case should be easy enough.

From there the next thing people are going to want is the ability to specify particular values. You can do it in elixir typespec with atoms e.g @type foo :: :foo | :bar | :baz and people will no doubt want it here if we're doing type support. This gets a bit challenging because at the moment we're representing types via atoms, but those same atoms might also get used as values.

The current approach will clearly not fit this case. For top level values, the inclusion validator
will be good enough, but this will not work for nested values either.
I however think this limitation is acceptable, and if someone needs this level of control on nested values,
the best bet should still be to use another validator.

Then we're in trouble because the moment we allow values into the system we gotta worry about mandatory vs optional values for our container types, and so on.

Wouldn't a proper nil support be enough to handle this? I would be glad if you could give me an example.

I'll update the PR with proper nil handling for now, and let's continue the discussion.

[danhper]

I just updated the PR with proper nil handling.
I also removed the possibility of using the allow_nil option, as the nil type should be used instead in this validator.

Let me know what you think.

[benwilson512]

Then we're in trouble because the moment we allow values into the system we gotta worry about mandatory vs optional values for our container types, and so on.

Wouldn't a proper nil support be enough to handle this? I would be glad if you could give me an example.

Not if we allow values. Allowing values would let us say something like "not, only must you supply a map with atom keys, the only valid keys :foo, :bar". This is a lot like @type config :: [foo: any, bar: any]. Of course at this point we end up in a discussion of how we say "keys :foo, :bar are REQUIRED to be present, and :baz is optional. This isn't quite as simple as allowing :bar to point to a nil value because pointing to nil isn't the same as the key being absent.

I do agree that trying to deal with values is probably beyond us here.

[danhper]

@benwilson512 Thanks for the feedback!

I get your point about allowing values, but I really think it would make things too complex
if we try to handle this in this validator.
For map or keyword, which is I think the most common place to use values,
a specialized validator would be much simpler from an implementation point of view,
and easy enough to use IMO.
What do you think?

By the way, to give a real use case, I am currently planning to use the type validator
to avoid sending invalid JSON data.
https://github.com/tuvistavie/pushex/blob/master/lib/pushex/gcm/request.ex

[benwilson512]

I'm inclined to think btw that this should be handled as just a regular struct. Regexes, Ranges, and so on are merely structs.

[danhper]

I thought Regex.regex? was doing something but after checking the source code it did not 😄

[benwilson512]

Heh yeah just go with pattern matching on %Regex{}.

[benwilson512]

Alternatively, consider removing :regex altogether. They're just structs afterall, so people could just say Regex and it would work as is.

[danhper]

Thanks, I removed :regex completely, the struct matcher works well enough!

[benwilson512]

As a total side note, if you used GraphQL not only would your JSON output be automatically type checked, but your JSON input from clients would be too ;)

[bruce]

😆 that's a shameless Absinthe plug right there, @benwilson512.

[danhper]

@benwilson512 Thank you for the review!
For my above use case a validator will be good enough, but I have some JSON API I am working on, so I will give GraphQL there 👍
I will update the PR with your feedback and post again!

[danhper]

@benwilson512 I just updated the PR!

[danhper]

@benwilson512 ping 😃

[theduke]

Any progress on this?

I would really loves this functionality.

[sascha-wolf]

Is there still interest in merging this? I could review and merge this.

[danhper]

I have been using this as a custom validator in one of my libraries [1] for quite a while now, so if you could review and merge it, it would be great!
Thanks
[1]: https://github.com/tuvistavie/pushex

[sascha-wolf]

Some documentation suggestions. Also I would ask you, if you could update the README with some simple overview over the new validator.

[sascha-wolf]

should implies that it's merely recommended to be the same size but the validator enforces them to be the same. I would suggest must.

  defp do_validate(tuple, tuple: types)
      when is_tuple(tuple) and is_tuple(types) and tuple_size(tuple) == tuple_size(types) do

[sascha-wolf]

Convention would suggest to name this with a trailing ? (e.g. is_valid_type?). Just a thought though.

[sascha-wolf]

I think this is missing from the @moduledoc. If I got that right you can also do this?

Vex.Validators.Type.validate(%MyStruct{}, is: MyStruct)

[sascha-wolf]

Any life signs here?

[danhper]

Won't have much time in the next few weeks, sorry.
I'll update when I can, but it might take a little while.