From cc2c65eb77ca43c2845faff81a73f5082e38a805 Mon Sep 17 00:00:00 2001
From: Noah Dietz <noahdietz@users.noreply.github.com>
Date: Fri, 26 May 2023 18:46:30 -0700
Subject: [PATCH] fix(AIP-158): reference resource plural for field name
 (#1158)

---
 rules/aip0158/response_plural_first_field.go  | 12 +++++-
 .../response_plural_first_field_test.go       | 38 +++++++++++++++++++
 rules/internal/utils/resource.go              | 14 ++++++-
 rules/internal/utils/resource_test.go         | 35 ++++++++++++++++-
 4 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/rules/aip0158/response_plural_first_field.go b/rules/aip0158/response_plural_first_field.go
index b35ce3efb..97f6777f1 100644
--- a/rules/aip0158/response_plural_first_field.go
+++ b/rules/aip0158/response_plural_first_field.go
@@ -19,6 +19,7 @@ import (
 	"github.com/googleapis/api-linter/locations"
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
+	"github.com/stoewer/go-strcase"
 )
 
 var responsePluralFirstField = &lint.MessageRule{
@@ -30,7 +31,16 @@ var responsePluralFirstField = &lint.MessageRule{
 		// 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 the field is a resource, use the `plural` annotation to decide the
+		// appropriate plural field name.
+		want := utils.GetResourcePlural(utils.GetResource(firstField.GetMessageType()))
+		if want != "" {
+			want = strcase.SnakeCase(want)
+		} else {
+			want = utils.ToPlural(firstField.GetName())
+		}
+
 		if want != firstField.GetName() {
 			return []lint.Problem{{
 				Message:    "First field of Paginated RPCs' response should be plural.",
diff --git a/rules/aip0158/response_plural_first_field_test.go b/rules/aip0158/response_plural_first_field_test.go
index 42a1ad633..3db6334e0 100644
--- a/rules/aip0158/response_plural_first_field_test.go
+++ b/rules/aip0158/response_plural_first_field_test.go
@@ -55,6 +55,44 @@ func TestResponsePluralFirstField(t *testing.T) {
 		})
 	}
 
+	tests = []struct {
+		testName    string
+		MessageName string
+		problems    testutils.Problems
+	}{
+		{"ValidResourcePlural", "library_books", testutils.Problems{}},
+		{"InvalidResourcePlural", "books", testutils.Problems{{Suggestion: "library_books"}}},
+	}
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			// Create the proto message.
+			f := testutils.ParseProto3Tmpl(t, `
+				import "google/api/resource.proto";
+
+				message LibraryBook {
+					option (google.api.resource) = {
+						type: "example.com/LibraryBook"
+						pattern: "libraryBooks/{libraryBook}"
+						singular: "libraryBook"
+						plural: "libraryBooks"
+					};
+					string name = 1;
+				}
+
+				message ListLibraryBooksResponse {
+					repeated LibraryBook {{.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)
+			}
+		})
+	}
+
 	t.Run("ValidNoFields", func(t *testing.T) {
 		f := testutils.ParseProto3Tmpl(t, `
 			message ListStudentProfilesResponse {}
diff --git a/rules/internal/utils/resource.go b/rules/internal/utils/resource.go
index 72191cf8c..8e6e8bee6 100644
--- a/rules/internal/utils/resource.go
+++ b/rules/internal/utils/resource.go
@@ -14,7 +14,9 @@
 
 package utils
 
-import apb "google.golang.org/genproto/googleapis/api/annotations"
+import (
+	apb "google.golang.org/genproto/googleapis/api/annotations"
+)
 
 // GetResourceSingular returns the resource singular. The
 // empty string is returned if the singular cannot be found.
@@ -37,3 +39,13 @@ func GetResourceSingular(r *apb.ResourceDescriptor) string {
 	}
 	return ""
 }
+
+// GetResourcePlural is a convenience method for getting the `plural` field of a
+// resource.
+func GetResourcePlural(r *apb.ResourceDescriptor) string {
+	if r == nil {
+		return ""
+	}
+
+	return r.Plural
+}
diff --git a/rules/internal/utils/resource_test.go b/rules/internal/utils/resource_test.go
index 5f276733b..cc38b2e59 100644
--- a/rules/internal/utils/resource_test.go
+++ b/rules/internal/utils/resource_test.go
@@ -20,7 +20,7 @@ import (
 	apb "google.golang.org/genproto/googleapis/api/annotations"
 )
 
-func TestMatches(t *testing.T) {
+func TestGetResourceSingular(t *testing.T) {
 	for _, test := range []struct {
 		name     string
 		resource *apb.ResourceDescriptor
@@ -69,3 +69,36 @@ func TestMatches(t *testing.T) {
 		})
 	}
 }
+
+func TestGetResourcePlural(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		resource *apb.ResourceDescriptor
+		want     string
+	}{
+		{
+			name: "PluralSpecified",
+			resource: &apb.ResourceDescriptor{
+				Plural: "bookShelves",
+			},
+			want: "bookShelves",
+		},
+		{
+			name:     "NothingSpecified",
+			resource: &apb.ResourceDescriptor{},
+			want:     "",
+		},
+		{
+			name:     "Nil",
+			resource: nil,
+			want:     "",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			got := GetResourcePlural(test.resource)
+			if got != test.want {
+				t.Errorf("GetResourcePlural: expected %v, got %v", test.want, got)
+			}
+		})
+	}
+}