Skip to content

Commit

Permalink
Fix buf lint mistaking proto3 optional fields for oneof fields (#2590)
Browse files Browse the repository at this point in the history
Fixes #2589.

This PR fixes the bug where `buf lint` incorrectly report error on
proto3 optional fields.
  • Loading branch information
oliversun9 authored Nov 14, 2023
1 parent 819a804 commit 61bc015
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
be consulted to resolve an element. This can be useful when the result of an RPC
contains extensions or values in `google.protobuf.Any` messages that are not defined
in the same schema that defines the RPC service.
- Fix issue where `buf lint` incorrectly reports error when `(buf.validate.field).required`
is set for an optional field in proto3.

## [v1.28.0] - 2023-11-10

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ func checkConstraintsForField(
if fieldDescriptor.IsExtension() {
checkConstraintsForExtension(adder, fieldConstraints)
}
if fieldDescriptor.ContainingOneof() != nil && fieldConstraints.GetRequired() {
if fieldDescriptor.ContainingOneof() != nil &&
!protodesc.ToFieldDescriptorProto(fieldDescriptor).GetProto3Optional() &&
fieldConstraints.GetRequired() {
adder.addForPathf(
[]int32{requiredFieldNumber},
"Field %q has %s but is in a oneof (%s). Oneof fields must not have %s.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,7 @@ message OneofTest {
}
// required is OK for non-oneof fields
string f4 = 4 [(buf.validate.field).required = true];
// optional in proto3 is a synthetic oneof, we should not report error.
optional string f5 = 5 [(buf.validate.field).required = true];
optional google.protobuf.Duration f6 = 6 [(buf.validate.field).required = true];
}

0 comments on commit 61bc015

Please sign in to comment.