search

uber / prototool

Your Swiss Army Knife for Protocol Buffers
MIT License
5.05k stars 345 forks source link

Support experimental optional fields #571

Open setpill opened 3 years ago

setpill commented 3 years ago

Protobuf allows experimental optional fields since v3.12. However, this needs to be specifically enabled in protoc, and prototool doesn't currently seem to allow this. prototool lint throws an exit code 255 with the following output:

$ prototool lint
2021-01-29T[time]Z  WARN    protoc returned a line we do not understand, please file this as an issue at https://github.com/uber/prototool/issues/new   {"protocLine": "[proto file]: This file contains proto3 optional fields, but --experimental_allow_proto3_optional was not set."}
<input>:1:1:[proto file]: This file contains proto3 optional fields, but --experimental_allow_proto3_optional was not set.

This can be reproduced by adding an optional field to any message and running the linter.

AyushSingh13 commented 3 years ago

@setpill Were you able to find a workaround for this?

AyushSingh13 commented 3 years ago

For anyone stumbling across this, protobuf 3.12 required --experimental_allow_proto3_optional to be passed in for the use of optional but this flag isn't required from 3.15 onwards. If you set the protoc version to 3.15.8 (latest at the time of writing), prototool will automatically download it next time it runs and not throw the error above.

While this doesn't solve the problem of passing flags not supported by prototool to protoc, it does fix the use of optional whilst using prototool.

notxcain commented 2 years ago

@AyushSingh13 do you know any workaround for prototool format?

ccakes commented 2 years ago

@AyushSingh13 do you know any workaround for prototool format?

Also interested in this

sogartar commented 2 years ago

I have noticed that if you specify a newer version (>=3.15) of protoc in prototool.yaml, the verification step that uses protoc will not fail, but the format fixer will remove the optional attributes. This is rather unfortunate if the fields are of non-message type and in this case optional is necessary. If you don't have optional for example The Python generator would rise an error when testing for presence.

ValueError: Can't test non-optional, non-submessage field "MyType.my_filed" for presence in proto3.
ccakes commented 2 years ago

I did a bit more reading on this and based on some comments in the buf repo it seems like the prototool formatter is lossy and probably not a great idea to use anyway. Once of the reasons they haven't implemented a formatter yet.

It looks they're they're planning to work on one soon so hopefully we get a new formatter to use that supports the optional keyword