nilslice / protolock

Protocol Buffer companion tool. Track your .proto files and prevent changes to messages and services which impact API compatibility.
https://protolock.dev
BSD 3-Clause "New" or "Revised" License
596 stars 35 forks source link

Options not parsed at the Enum level #135

Closed johnfishbein closed 2 years ago

johnfishbein commented 2 years ago

Hi, Recently I've been working on creating a custom protolock plugin specific to my codebase. My plugin relies on a few custom options I've defined. Using protolock, custom options defined at the file level and message level are parsed by the tool into the Protolock struct, but options defined inside enums are not parsed by the tool.

The Entry struct contains the Options field which receives all of the file level options after parsing.

type Entry struct {
    Enums    []Enum    `json:"enums,omitempty"`
    Messages []Message `json:"messages,omitempty"`
    Services []Service `json:"services,omitempty"`
    Imports  []Import  `json:"imports,omitempty"`
    Package  Package   `json:"package,omitempty"`
    Options  []Option  `json:"options,omitempty"`
}

Similarly, the Message struct also contains an Options field which the message level options are parsed into.

type Message struct {
    Name          string    `json:"name,omitempty"`
    Fields        []Field   `json:"fields,omitempty"`
    Maps          []Map     `json:"maps,omitempty"`
    ReservedIDs   []int     `json:"reserved_ids,omitempty"`
    ReservedNames []string  `json:"reserved_names,omitempty"`
    Filepath      Protopath `json:"filepath,omitempty"`
    Messages      []Message `json:"messages,omitempty"`
    Options       []Option  `json:"options,omitempty"`
}

However, Enum options are ignored during parse. The Enum struct does not include an Options field, but it does include the AllowAlias option which is one of many possible enum level options.

type Enum struct {
    Name          string      `json:"name,omitempty"`
    EnumFields    []EnumField `json:"enum_fields,omitempty"`
    ReservedIDs   []int       `json:"reserved_ids,omitempty"`
    ReservedNames []string    `json:"reserved_names,omitempty"`
    AllowAlias    bool        `json:"allow_alias,omitempty"`
}

Is there a reason for this decision to not include the Options field for enums? If not, would it be difficult/possible to include enum level options?

An example of what I'm trying to do is:

enum TestEnum {
    option (custom_option) = true;
    DEFAULT = 0;
    FIRST = 1;
    SECOND = 2;
}

Thank you!

nilslice commented 2 years ago

Hi @johnfishbein - apologies for the delayed response. It is certainly possible to collect the Enum options, so long as the parser knows how to do so. I believe it should, but you might want to check the API in https://github.com/emicklei/proto. If we need to bump the dependency version to get this support, I would happily accept a PR for that in addition to the feature you need.

Currently, I don't have bandwidth to add support for this but the code should be relatively straightforward, given other option type data is collected in other data types.

If you are interested in giving the implementation a shot, I can point you in the right direction!

nilslice commented 2 years ago

Closed in #138