Elixir: `@enforce_keys` should be checked against struct fields when `defstruct` is called

Created on 20 Sep 2020  路  13Comments  路  Source: elixir-lang/elixir

The following code, you can see :foo is misspelled.

defmodule Foo do
  @enforce_keys [:fo]
  defstruct foo: nil
end

iex(4)> %Foo{foo: true}
** (ArgumentError) the following keys must also be given when building struct Foo: [:fo]
    (bug_defstruct 0.1.0) expanding struct: Foo.__struct__/1
    iex:4: (file)

Additionally, @enforce_keys accepts an atom. I don't know if this the intended behaviour but since it is named "keys" it should only accept a list of atoms IMO.

I can contribute a PR

All 13 comments

A PR is definitely welcome. Also note that by checking that @enforce_keys is a subset of the struct keys, we will by definition guarantee their are atoms, as the struct keys are atoms. :) So we effectively need only one check!

Ok. I will tackle this one then.
Just to be clear: Should this raise? @enforce_keys :foo

Considering it would be a breaking change, it will have to go through a deprecation phase with a warning until v2.0 if this change is desired.

@xire28 it is not a breaking change because you can see it is not possible to build the struct if you give the wrong keys. This is just about raising it earlier.

@eksperimental @enforce_keys :foo already raises today so we can improve the error message here too.

@eksperimental @enforce_keys :foo already raises today so we can improve the error message here too.

It is unintentionally supported, thus my question

defmodule EnforceKeys do
  @enforce_keys :foo
  defstruct foo: nil, bar: nil
end

Interactive Elixir (1.11.0-rc.0) - press Ctrl+C to exit (type h() ENTER for help)

iex(1)> %EnforceKeys{}
** (ArgumentError) the following keys must also be given when building struct EnforceKeys: [:foo]
    (bug_defstruct 0.1.0) expanding struct: EnforceKeys.__struct__/1
    iex:1: (file)
iex(1)> %EnforceKeys{bar: 1}
** (ArgumentError) the following keys must also be given when building struct EnforceKeys: [:foo]
    (bug_defstruct 0.1.0) expanding struct: EnforceKeys.__struct__/1
    iex:1: (file)
iex(1)> %EnforceKeys{foo: 1} 
%EnforceKeys{bar: nil, foo: 1}

Considering it would be a breaking change, it will have to go through a deprecation phase with a warning until v2.0 if this change is desired.

@xire28 I cannot see how this would be a breaking change. Do you have an example?

@eksperimental I see. In the code, we are calling List.wrap, so @enforce_keys :foo is intentionally supported. We should not change it.

@eksperimental I see. In the code, we are calling List.wrap, so @enforce_keys :foo is intentionally supported. We should not change it.

I will document this in the PR.

I was hoping this was not intended, and eventually we could support :all as a special value, so we could easily enforce all keys, instead of doing

  @struct config: false
  @enforce_keys Keyword.keys(@struct)
  defstruct @struct

Sorry, my answer was a reply to:

Just to be clear: Should this raise? @enforce_keys :foo

Sorry, my answer was a reply to:

Just to be clear: Should this raise? @enforce_keys :foo

Oh... I see. Thank you

Hello @eksperimental, are you still working on this? If not, I can open a PR. Thanks! :)

Hello @eksperimental, are you still working on this? If not, I can open a PR. Thanks! :)

Yes, I will be working on it, it just needs to wait for a bit. Thank you

Closing in favor of the PR.

Was this page helpful?
0 / 5 - 0 ratings