Gradualizer icon indicating copy to clipboard operation
Gradualizer copied to clipboard

Checking Elixir structs

Open gomoripeti opened this issue 5 years ago • 3 comments

Currently Elixist struct are handled as ordinary maps and they are only checked against the expected map type.

However in Elixir they are quite similar to Erlang records with a definition including types, so theoretically when creating or updating a struct it could be checked against its definition as well (even if the expected type is allowing eg term())

Of course some info is lost while Elixir source code is converted to Erlang AST but an indicator could be the presence of a __struct__ field.

gomoripeti avatar Apr 17 '19 08:04 gomoripeti

I don't understand completely. A known_problem example would be useful.

I think looking for a __struct__ key in the map is unique enough so I don't think there's a risk of misusing this logic. If there is a risk of ambiguity or similar, then an option {elixir_structs, boolean()} could be added.

zuiderkwast avatar Apr 17 '19 12:04 zuiderkwast

the idea came up on ElixirConf EU.

Now that I'm checking, it does not seem to be possible to specify types of fields when defining a struct (unlike typed_record_def). The convention is to define a separate type t() in the module defining the struct (https://hexdocs.pm/elixir/Kernel.html#defstruct/1-types).

I think we are all good if there is a type or spec using that t() type.

The request was to also check that the struct below matches the type User.t() (although IO.inspect can take any term)

IO.inspect %User{name: :john}

Struct creation compiles to a map creation with the literal __struct__ field being assigned with a literal module name, so this seems to be doable.

#{'__struct__' => 'Elixir.User', ...}

I totally agree to introduce an option for this (maybe generic --elixir) if this is implemented.

I would also love to hear the opinion of Elixir developers how useful this extra check would be as checking against the User.t() type when given in specs might also cover most of the cases. @tim2CF what do you think?

gomoripeti avatar Apr 18 '19 07:04 gomoripeti

Just checking on the *.t() would be fine I'd think?

OvermindDL1 avatar Jun 20 '19 21:06 OvermindDL1