Raise exceptions if incorrect types are passed into `new`

[keathley]

Currently, no checks are done when passing attributes to new/2. This means that it's possible to pass incorrect types or sub-messages while constructing a new struct. In most cases these errors won't be caught until the message is decoded since the encoder doesn't perform validations while it's encoding. To make this more concrete let's say that we have these definitions:

defmodule Foo do
  @moduledoc false
  use Protobuf, syntax: :proto3

  defstruct [:id]

  field :id, 1, type: :int32
end

defmodule Bar do
  @moduledoc false
  use Protobuf, syntax: :proto3

  defstruct [:id]

  field :id, 1, type: :string
end

defmodule Baz do
  @moduledoc false
  use Protobuf, syntax: :proto3

  defstruct [:foo]

  field :foo, 1, type: Foo
end 

It's currently possible to do this:

Baz.new(foo: Bar.new(id: "test")) |> Baz.encode()
<<10, 6, 10, 4, 116, 101, 115, 116>>

Errors like this won't be caught until the client attempts to decode the message on the other end.

Proposal

I'd like to suggest that we check inputs as part of new and raise an exception if the value doesn't match the correct type. I believe that raising is the correct approach here because calling new with invalid data is equivalent to an ArgumentError. I don't believe that there's any recovery the user could take if they pass the wrong arguments to the function and its better to raise loudly so they're aware of the issue.

Obviously this would only work if the user calls new and doesn't work if they build the struct themselves. I'm not sure if that's an acceptable tradeoff or not.

I'm happy to work on submitting a PR for these changes if you believe that the idea has merit. But I wanted to make sure you thought this was a reasonable addition before I did the work on it.

[whatyouhide]

Obviously this would only work if the user calls new and doesn't work if they build the struct themselves. I'm not sure if that's an acceptable tradeoff or not.

This is how most things in Elixir work, so I strongly suggest that this is an acceptable tradeoff :)

@keathley we also don't check the field names in new/1, FWIW 😄

[haljin]

I always wondered why aren't the defaults filled out when you just make the raw struct? You could feed default values to defstruct based off protobuf types?