generate types for records about gpb HOT 5 CLOSED

I'd like to learn about your reasons for wanting types also for records. I can see some alternatives why it might be desirable, but I'd like to know a bit more, if you could elaborate?

My own thinking goes like this: at first I was considering this when types for enums was done, but then I thought it it already possible to write just #record_name{} as type. But you would have to include an .hrl file. With an exported type in an erlang module, there is no need to include anything, although, one would probably need to include the .hrl file anyway to be able to make use of the record, but maybe not necessarily(?) depending on code flow.

On the other hand, when generated with the maps option, there would be no #record_name{} type and no .hrl file to include, so an exported type would make more sense.

Generally, I think it might well be a useful feature, though.

from gpb.

Types for records are desirable mainly because of dialyzer, it's a lot cleaner to write record_name() instead of #record_name{} in specs for user methods that use generated records, elvis actually throws an error with it sees this.

As you mentioned, when using records probably makes more sense to keep the type right next to the record in the .hrl, can't think of a situation where you want to use the record definition without including the .hrl

If generated with the maps option the spec in the user app would just be maps() and there would be no need for a type, but i haven't actually worked a lot with maps so i'm just speculating...

from gpb.

About keeping the type right next to the record in the .hrl: wouldn't the name space discussion in #30 apply? It defines a #record_name{}, but that doesn't necessarily mean that a corresponding type record_name() can always be defined without name collisions. So here, too, I think it makes sense to have the type in the .erl instead, and -export_type it.

With maps, it is possible to specify type for different keys, for example:

  -type message_x() :: #{f1 => integer(),
                         f2 => string(),
                         f3 => a | b | c}.

  -export_type([message_x/0]).

So for maps, since there is no .hrl file, I think the natural way is to define the type in the .erl file and -export_type it.

About types for maps: currently, optional fields are always in the map, but with the value undefined if non-present. It's because it is just a straight translation from records, but there's work going on (albeit slowly at the moment) to allow for non-present optional fields to be omitted instead, since this is much more natural choice for maps. See #29. I have some code on a branch for that, but until I merge it, I think type specs for maps should specify optional fields to be always present with type like this some_optional_field => undefined | integer(), and when I merge the other branch, I will have to update the handling of this. Just mentioning this, so you know what's on the horizon.

(By the way, would it work better with elvis if it could see the entire source code -- ie also the generated code -- when it starts? I've never used elvis, so I don't know)

from gpb.

agreed, your reasoning makes sense
elvis processes files separately, but what i think it does is simply check for #record{} definitions in method specs and issue a warning about this

from gpb.

Closing this, since I just merged #34. Thanks!

from gpb.