fix(AIP-203): no check for on OneOf fields (#1184)

OneOf fields should not be checked for annotations,
as they are implicitly optional.

However, if a sub-message is used in the OneOf,
it should be verified.

docs/rules/0203/field-behavior-required.md

@@ -18,10 +18,10 @@ This rule enforces that each field in a message used in a request has a
 
 ## Details
 
-This rule looks at all fields and ensures they have a
-`google.api.field_behavior` annotation. It also verifies that they have at least
-one of the options `OUTPUT_ONLY`, `REQUIRED`, or `OPTIONAL`: all fields must
-fall into one of these categories.
+This rule looks at all fields except those nested inside a OneOf and ensures
+they have a `google.api.field_behavior` annotation. It also verifies that they
+have at least one of the options `OUTPUT_ONLY`, `REQUIRED`, or `OPTIONAL`: all
+fields must fall into one of these categories.
 
 Although all request messages **must** be annotated, this linter only verifies
 messages that are in the same package as some upstream protos (e.g. common

rules/aip0203/field_behavior_required.go

@@ -49,9 +49,12 @@ func problems(m *desc.MessageDescriptor, pkg string) []lint.Problem {
 			continue
 		}
 
-		p := checkFieldBehavior(f)
-		if p != nil {
-			ps = append(ps, *p)
+		// Ignore a field if it is a OneOf (do not ignore children)
+		if f.AsFieldDescriptorProto().OneofIndex == nil {
+			p := checkFieldBehavior(f)
+			if p != nil {
+				ps = append(ps, *p)
+			}
 		}
 
 		if mt := f.GetMessageType(); mt != nil && !mt.IsMapEntry() && mt.GetFile().GetPackage() == pkg {

rules/aip0203/field_behavior_required_test.go

@@ -49,6 +49,16 @@ func TestFieldBehaviorRequired_SingleFile_SingleMessage(t *testing.T) {
 			"map<string, string> page_count = 1 [(google.api.field_behavior) = OPTIONAL];",
 			nil,
 		},
+		// OneOfs are not required to have an annotation, as they
+		// are implicitly optional.
+		{
+			"ValidOneOfNoAnnotation",
+			`oneof candy_bar {
+				bool snickers = 1;
+				bool chocolate = 3;
+			}`,
+			nil,
+		},
 		{
 			"ValidOutputOnly",
 			"int32 page_count = 1 [(google.api.field_behavior) = OUTPUT_ONLY];",
@@ -181,6 +191,14 @@ func TestFieldBehaviorRequired_NestedMessages_SingleFile(t *testing.T) {
 			"NonAnnotated non_annotated = 1 [(google.api.field_behavior) = REQUIRED];",
 			testutils.Problems{{Message: "must be set"}},
 		},
+		// Children of OneOfs should still be validated.
+		{
+			"InvalidOneOfChildNotAnnotated",
+			`oneof candy_bar {
+				NonAnnotated non_annotated = 1;
+			}`,
+			testutils.Problems{{Message: "must be set"}},
+		},
 	}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {