Repository: googleapis/api-linter
Pull request #496 (2020-02-17 18:13:36)

[AIP-158] Response's first field must be plural
Issue #490 

For all messages that:
- end in Response
- Contain a next_page_token field.

Throw a linter warning if:
The first field in the message is not named according to plural(message_name.to_snake().split('_')[1:-1])

Dos-Ph: Could someone merge it? I don't have write access.

Patch:
diff --git a/docs/rules/0158/response-plural-first-field.md b/docs/rules/0158/response-plural-first-field.md
new file mode 100644
index 0000000..8383c52
--- /dev/null
+++ b/docs/rules/0158/response-plural-first-field.md
@@ -0,0 +1,65 @@
+---
+rule:
+  aip: 158
+  name: [core, '0158', response-plural-first-field]
+  summary: First field of Paginated RPCs' response should be plural.
+permalink: /158/response-plural-first-field
+redirect_from:
+  - /0158/response-plural-first-field
+---
+
+# Paginated methods: Page token field
+
+This rule enforces that all `List` and `Search` methods have a plural name
+repeatable field as a first field in the response message, as mandated in
+[AIP-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's name is not
+plural.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect
+message ListBooksResponse {
+  // Field name should be `books`.
+  repeated Book book = 1;
+  string next_page_token = 2;
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message ListBooksResponse {
+  repeated Book books = 1;
+  string next_page_token = 2;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the message or
+above the field. Remember to also include an [aip.dev/not-precedent][] comment
+explaining why.
+
+```proto
+// (-- api-linter: core::0158::response-plural-first-field=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+message ListBooksResponse {
+  repeated Book book = 1;
+  string next_page_token = 2;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-158]: https://aip.dev/158
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0158/aip0158.go b/rules/aip0158/aip0158.go
index dbde76f..d006cc4 100644
--- a/rules/aip0158/aip0158.go
+++ b/rules/aip0158/aip0158.go
@@ -29,6 +29,7 @@ func AddRules(r lint.RuleRegistry) error {
 		requestPaginationPageSize,
 		requestPaginationPageToken,
 		responsePaginationNextPageToken,
+		responsePluralFirstField,
 	)
 }
 
@@ -37,10 +38,10 @@ var paginatedRes = regexp.MustCompile("^(List|Search)[A-Za-z0-9]*Response$")
 
 // Return true if this is an AIP-158 List request message, false otherwise.
 func isPaginatedRequestMessage(m *desc.MessageDescriptor) bool {
-	return paginatedReq.MatchString(m.GetName())
+	return paginatedReq.MatchString(m.GetName()) || m.FindFieldByName("page_size") != nil || m.FindFieldByName("page_token") != nil
 }
 
 // Return true if this is an AIP-158 List response message, false otherwise.
 func isPaginatedResponseMessage(m *desc.MessageDescriptor) bool {
-	return paginatedRes.MatchString(m.GetName())
+	return paginatedRes.MatchString(m.GetName()) || m.FindFieldByName("next_page_token") != nil
 }
diff --git a/rules/aip0158/response_plural_first_field.go b/rules/aip0158/response_plural_first_field.go
new file mode 100644
index 0000000..7700c66
--- /dev/null
+++ b/rules/aip0158/response_plural_first_field.go
@@ -0,0 +1,44 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0158
+
+import (
+	"fmt"
+
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/locations"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var responsePluralFirstField = &lint.MessageRule{
+	Name:   lint.NewRuleName(158, "response-plural-first-field"),
+	OnlyIf: isPaginatedResponseMessage,
+	LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
+		// Throw a linter warning if, the first field in the message is not named according to plural(message_name.to_snake().split('_')[1:-1]).
+		firstField := m.GetFields()[0]
+		want := utils.ToPlural(firstField.GetName())
+		if want != firstField.GetName() {
+			return []lint.Problem{{
+				Message:    fmt.Sprintf("First field of Paginated RPCs' response should be plural."),
+				Suggestion: want,
+				Descriptor: firstField,
+				Location:   locations.DescriptorName(m),
+			}}
+		}
+
+		return nil
+	},
+}
diff --git a/rules/internal/utils/string_pluralize.go b/rules/internal/utils/string_pluralize.go
new file mode 100644
index 0000000..93beef6
--- /dev/null
+++ b/rules/internal/utils/string_pluralize.go
@@ -0,0 +1,28 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 		https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"github.com/gertd/go-pluralize"
+)
+
+var pluralizeClient = pluralize.NewClient()
+
+func ToPlural(s string) string {
+	// Need to convert name to singular first to support none standard case such as persons, cactuses.
+	// persons -> person -> people
+
+	return pluralizeClient.Plural(pluralizeClient.Singular(s))
+}


Test patch:
diff --git a/rules/aip0158/response_plural_first_field_test.go b/rules/aip0158/response_plural_first_field_test.go
new file mode 100644
index 0000000..e29091a
--- /dev/null
+++ b/rules/aip0158/response_plural_first_field_test.go
@@ -0,0 +1,57 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0158
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestResponsePluralFirstField(t *testing.T) {
+	tests := []struct {
+		testName    string
+		MessageName string
+		problems    testutils.Problems
+	}{
+		{"Valid", "student_profiles", testutils.Problems{}},
+		{"InvalidWrongSuffix", "student_profile", testutils.Problems{{Suggestion: "student_profiles"}}},
+		{"ValidLatin", "cacti", testutils.Problems{}},
+		{"InvalidLatin", "cactuses", testutils.Problems{{Suggestion: "cacti"}}},
+		{"ValidNonstandard", "people", testutils.Problems{}},
+		{"InvalidNonstandard", "persons", testutils.Problems{{Suggestion: "people"}}},
+	}
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			// Create the proto message.
+			f := testutils.ParseProto3Tmpl(t, `
+				message Profile {
+					string name = 1;
+				}
+
+				message ListStudentProfilesResponse {
+					repeated Profile {{.MessageName}} = 1;
+					string next_page_token = 2;
+				}
+			`, test)
+
+			// Run the lint rule and establish we get the correct problems.
+			problems := responsePluralFirstField.Lint(f)
+			if diff := test.problems.SetDescriptor(f.GetMessageTypes()[1].FindFieldByNumber(1)).Diff(problems); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}
diff --git a/rules/internal/utils/string_pluralize_test.go b/rules/internal/utils/string_pluralize_test.go
new file mode 100644
index 0000000..8cc3887
--- /dev/null
+++ b/rules/internal/utils/string_pluralize_test.go
@@ -0,0 +1,43 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"testing"
+)
+
+func TestPluralize(t *testing.T) {
+	tests := []struct {
+		name           string
+		word           string
+		pluralizedWord string
+	}{
+		{"PluralizeSingularWord", "person", "people"},
+		{"PluralizePluralWord", "people", "people"},
+		{"PluralizeNonstandardPluralWord", "persons", "people"},
+		{"PluralizeNoPluralFormWord", "moose", "moose"},
+		{"PluralizePluralLatinWord", "cacti", "cacti"},
+		{"PluralizeNonstandardPluralLatinWord", "cactuses", "cacti"},
+		{"PluralizePluralCamelCaseWord", "student_profiles", "student_profiles"},
+		{"PluralizeSingularCamelCaseWord", "student_profile", "student_profiles"},
+	}
+	for _, test := range tests {
+		t.Run(test.word, func(t *testing.T) {
+			if got := ToPlural(test.word); got != test.pluralizedWord {
+				t.Errorf("Plural(%s) got %s, but want %s", test.word, got, test.pluralizedWord)
+			}
+		})
+	}
+}