Add new MessageOneof rule
#377
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
tools/protovalidate-conformance/internal/cases/cases_message.go
Outdated
Show resolved
Hide resolved
|
Updated to include a test for unknown field name and changed the formatting for the validation errors. |
tools/protovalidate-conformance/internal/cases/cases_message.go
Outdated
Show resolved
Hide resolved
|
|
||
| message MessageOneofRule { | ||
| repeated string fields = 1; | ||
| optional bool required = 2; |
There was a problem hiding this comment.
add comments on the fields too?
| bool bool_field = 2; | ||
| } | ||
|
|
||
| message MessageOneofUnknownFieldName { |
There was a problem hiding this comment.
consider also having oneof with no fields (which should also be an error)
There was a problem hiding this comment.
There are a few rough edges (like this, like if you specify the same field name multiple times, and certainly others we haven't had time to think of yet), but the desire was to get this out fast. We'll keep track of these and hopefully can fix them soon, but I don't want to block this on edge cases.
The faster you can start using this, the faster you can help us find more of those edges cases and we can take care of them!
There was a problem hiding this comment.
In light of this, I removed the MessageOneofNoFields test case in fd2217b and we can address as a follow-up.
|
|
||
| message MessageWith3dInside {} | ||
|
|
||
| message MessageOneofSingleField { |
There was a problem hiding this comment.
Add test case for MessageOneofSingleFieldRequired?
| // 3. This will generate validation errors when unmarshalling, even from the binary | ||
| // format. With a Protobuf oneof, if multiple fields are present in the serialized | ||
| // form, earlier values are usually silently ignored when unmarshalling, with only | ||
| // the last field being present when unmarshalling completes. |
There was a problem hiding this comment.
I know I proposed this copy, but I found some issues in it. Here's a take two 😄
| // 3. This will generate validation errors when unmarshalling, even from the binary | |
| // format. With a Protobuf oneof, if multiple fields are present in the serialized | |
| // form, earlier values are usually silently ignored when unmarshalling, with only | |
| // the last field being present when unmarshalling completes. | |
| // 3. This will always generate validation errors for a message unmarshalled from | |
| // serialized data that sets more than one field. With a Protobuf oneof, when | |
| // multiple fields are present in the serialized form, earlier values are usually | |
| // silently ignored when unmarshalling, with only the last field being set when | |
| // unmarshalling completes. |
There was a problem hiding this comment.
Ah shoot, sorry. Will update this in a follow-up.
This implements the `MessageOneof` rule specified in bufbuild/protovalidate#377 --------- Co-authored-by: Timo Stamm <[email protected]>
This adds new message rule
oneofwhich allows for oneof semantics without the ceremony of Protobufoneof.Consumers can specify a list of field names in a message that constitute the
oneofand can also specify an optionalrequiredflag, which enforces that exactly one of the fields must be set. Ifrequiredis false, it enforces that zero-or-one of specified fields can be set.