fix(AIP-4232): correct repeated field check (#1337)

Author: noahdietz

rules/aip4232/repeated_fields.go

@@ -33,17 +33,21 @@ var repeatedFields = &lint.MethodRule{
 		in := m.GetInputType()
 
 		for _, sig := range sigs {
-			for i, name := range sig {
-				// Skip the last field in the signature, it can be repeated.
-				if i == len(sig)-1 {
-					break
+			for _, name := range sig {
+				if !strings.Contains(name, ".") {
+					continue
 				}
+				chain := strings.Split(name, ".")
 
-				if f := in.FindFieldByName(name); f != nil && f.IsRepeated() {
-					problems = append(problems, lint.Problem{
-						Message:    fmt.Sprintf("Field %q in method_signature %q: only the last field can be repeated.", name, strings.Join(sig, ",")),
-						Descriptor: m,
-					})
+				// Exclude the last one from the look up since it can be repeated.
+				for i := range chain[:len(chain)-1] {
+					n := strings.Join(chain[:i+1], ".")
+					if f := utils.FindFieldDotNotation(in, n); f != nil && f.IsRepeated() {
+						problems = append(problems, lint.Problem{
+							Message:    fmt.Sprintf("Field %q in method_signature %q: only the last field in a signature argument can be repeated.", name, strings.Join(sig, ",")),
+							Descriptor: m,
+						})
+					}
 				}
 			}
 		}

rules/aip4232/repeated_fields_test.go

@@ -27,10 +27,10 @@ func TestRepeatedFields(t *testing.T) {
 		SecondSig string
 		problems  testutils.Problems
 	}{
-		{"Valid", "name,paperback_only,editions", "name,editions", nil},
-		{"InvalidFirstSignature", "name,editions,paperback_only", "name,editions", testutils.Problems{{Message: "only the last"}}},
-		{"InvalidSecondSignature", "name,editions", "name,editions,paperback_only", testutils.Problems{{Message: "only the last"}}},
-		{"BothInvalid", "name,editions,paperback_only", "editions,name", testutils.Problems{{Message: "only the last"}, {Message: "only the last"}}},
+		{"Valid", "name,paperback_only,book.editions", "name,editions", nil},
+		{"InvalidFirstSignature", "name,book.shelves.shelf,paperback_only", "name,editions", testutils.Problems{{Message: "only the last"}}},
+		{"InvalidSecondSignature", "name,book.editions", "name,book.shelves.shelf,paperback_only", testutils.Problems{{Message: "only the last"}}},
+		{"BothInvalid", "name,book.shelves.shelf,paperback_only", "name,book.shelves.shelf,paperback_only", testutils.Problems{{Message: "only the last"}, {Message: "only the last"}}},
 	}
 
 	for _, test := range tests {
@@ -47,8 +47,18 @@ func TestRepeatedFields(t *testing.T) {
 					string name = 1;
 
 					bool paperback_only = 2;
-					
-					repeated int32 editions = 3;
+
+					Book book = 3;
+				}
+				message Book {
+					string name = 1;
+
+					repeated int32 editions = 2;
+
+					repeated Shelf shelves = 3;
+				}
+				message Shelf {
+					string shelf = 1;
 				}
 				message ArchiveBookResponse {}
 			`, test)

rules/internal/utils/find.go

@@ -134,13 +134,14 @@ func GetRepeatedMessageFields(m *desc.MessageDescriptor) []*desc.FieldDescriptor
 // google.api.method_signature annotations.
 func FindFieldDotNotation(msg *desc.MessageDescriptor, ref string) *desc.FieldDescriptor {
 	path := strings.Split(ref, ".")
-	for _, seg := range path {
+	end := len(path) - 1
+	for i, seg := range path {
 		field := msg.FindFieldByName(seg)
 		if field == nil {
 			return nil
 		}
 
-		if m := field.GetMessageType(); m != nil {
+		if m := field.GetMessageType(); m != nil && i != end {
 			msg = m
 			continue
 		}