prototool icon indicating copy to clipboard operation
prototool copied to clipboard

Support experimental optional fields

Open setpill opened this issue 3 years ago • 6 comments

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.

setpill avatar Jan 29 '21 17:01 setpill

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

AyushSingh13 avatar Apr 22 '21 11:04 AyushSingh13

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.

AyushSingh13 avatar Apr 22 '21 19:04 AyushSingh13

@AyushSingh13 do you know any workaround for prototool format?

notxcain avatar Sep 27 '21 11:09 notxcain

@AyushSingh13 do you know any workaround for prototool format?

Also interested in this

ccakes avatar Dec 28 '21 23:12 ccakes

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.

sogartar avatar Jan 13 '22 21:01 sogartar

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

ccakes avatar Jan 14 '22 00:01 ccakes