diff --git a/docs/rules/0158/response-repeated-first-field.md b/docs/rules/0158/response-repeated-field.md similarity index 74% rename from docs/rules/0158/response-repeated-first-field.md rename to docs/rules/0158/response-repeated-field.md index b330b48..0bcb9ea 100644 --- a/docs/rules/0158/response-repeated-first-field.md +++ b/docs/rules/0158/response-repeated-field.md @@ -1,25 +1,24 @@ --- rule: aep: 158 - name: [core, '0158', response-repeated-first-field] + name: [core, '0158', response-repeated-field] summary: | - First field (by both position and field number) of Paginated RPCs' response + One field of a paginated RPCs' response should be repeated. -permalink: /158/response-repeated-first-field +permalink: /158/response-repeated-field redirect_from: - - /0158/response-repeated-first-field + - /0158/response-repeated-field --- # Paginated methods: Page token field This rule enforces that all `List` and `Search` methods have a repeatable field -as a first field in the response message, as mandated in [AEP-158][]. +in the response message, as mandated in [AEP-158][]. ## Details This rule looks at any message matching `List*Response` or `Search*Response` -that has `next_page_token` field and complains if the first field (by both -position and field number) is not repeated. +that has `next_page_token` field and complains if there does not exist a field that is repeated. ## Examples diff --git a/rules/aep0158/response_repeated_first_field.go b/rules/aep0158/response_repeated_field.go similarity index 61% rename from rules/aep0158/response_repeated_first_field.go rename to rules/aep0158/response_repeated_field.go index 2d45bf2..7818ca9 100644 --- a/rules/aep0158/response_repeated_first_field.go +++ b/rules/aep0158/response_repeated_field.go @@ -20,29 +20,21 @@ import ( ) var responseRepeatedFirstField = &lint.MessageRule{ - Name: lint.NewRuleName(158, "response-repeated-first-field"), + Name: lint.NewRuleName(158, "response-repeated-field"), RuleType: lint.NewRuleType(lint.MustRule), OnlyIf: func(m *desc.MessageDescriptor) bool { return isPaginatedResponseMessage(m) && len(m.GetFields()) > 0 }, LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { - // Sanity check: Is the first field (positionally) and the field with - // an ID of 1 actually the same field? - if m.GetFields()[0] != m.FindFieldByNumber(1) { - return []lint.Problem{{ - Message: "The first field of paginated RPCs must have a protobuf ID of 1.", - Descriptor: m.GetFields()[0], - }} + for _, f := range m.GetFields() { + if(f.IsRepeated()) { + return nil; + } } - // Make sure the field is repeated. - if !m.GetFields()[0].IsRepeated() { - return []lint.Problem{{ - Message: "The first field of a paginated response should be repeated.", - Descriptor: m.GetFields()[0], - }} - } - - return nil + return []lint.Problem{{ + Message: "There does not exist a repeated field for pagination results.", + Descriptor: m, + }} }, } diff --git a/rules/aep0158/response_repeated_first_field_test.go b/rules/aep0158/response_repeated_field_test.go similarity index 80% rename from rules/aep0158/response_repeated_first_field_test.go rename to rules/aep0158/response_repeated_field_test.go index ee0e13c..3ae6665 100644 --- a/rules/aep0158/response_repeated_first_field_test.go +++ b/rules/aep0158/response_repeated_field_test.go @@ -29,7 +29,7 @@ func TestResponseRepeatedFirstField(t *testing.T) { }{ {"Valid", "repeated Book books = 1;", "string next_page_token = 2;", nil}, {"SkippedZeroFields", "", "", nil}, - {"InvalidFirstFieldsNoMatch", "string next_page_token = 2;", "repeated Book books = 1;", testutils.Problems{{Message: "protobuf ID"}}}, + {"ProtobufIdIsNotOne", "string next_page_token = 1;", "repeated Book books = 2;", nil}, {"InvalidNotRepeated", "Book book = 1;", "string next_page_token = 2;", testutils.Problems{{Message: "repeated"}}}, } for _, test := range tests { @@ -45,14 +45,9 @@ func TestResponseRepeatedFirstField(t *testing.T) { } `, test) - // Determine the descriptor that a failing test will attach to. - if m := f.GetMessageTypes()[1]; len(m.GetFields()) > 0 { - test.problems.SetDescriptor(m.GetFields()[0]) - } - // Run the lint rule and establish we get the correct problems. problems := responseRepeatedFirstField.Lint(f) - if diff := test.problems.Diff(problems); diff != "" { + if diff := test.problems.SetDescriptor(f.GetMessageTypes()[1]).Diff(problems); diff != "" { t.Errorf(diff) } })