diff --git a/rules/aip0231/request_names_field.go b/rules/aip0231/request_names_field.go index 92c5b19af..ba2e7e44e 100644 --- a/rules/aip0231/request_names_field.go +++ b/rules/aip0231/request_names_field.go @@ -16,10 +16,11 @@ package aip0231 import ( "fmt" + "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" "github.com/googleapis/api-linter/v2/locations" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -78,10 +79,10 @@ var namesField = &lint.MessageRule{ } // Rule check: Ensure that the standard get request message field is the - // correct type. Note: Use m.Name()[8:len(m.Name())-7]) to retrieve - // the resource name from the batch get request, for example: - // "BatchGetBooksRequest" -> "Books" - rightTypeName := fmt.Sprintf("Get%sRequest", pluralize.NewClient().Singular(string(m.Name())[8:len(m.Name())-7])) + // correct type. Note: Retrieve the resource name from the batch get + // request, for example: "BatchGetBooksRequest" -> "Books" + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Request"), "BatchGet") + rightTypeName := fmt.Sprintf("Get%sRequest", utils.ResourceSingular(pluralName, m)) if getReqMsg != nil && (getReqMsg.Message() == nil || getReqMsg.Message().Name() != protoreflect.Name(rightTypeName)) { problems = append(problems, lint.Problem{ Message: fmt.Sprintf(`The "requests" field on Batch Get Request should be a %q type`, rightTypeName), diff --git a/rules/aip0231/request_names_field_test.go b/rules/aip0231/request_names_field_test.go index 53abc3160..99eda4df4 100644 --- a/rules/aip0231/request_names_field_test.go +++ b/rules/aip0231/request_names_field_test.go @@ -149,3 +149,72 @@ func TestNamesField(t *testing.T) { }) } } + +func TestNamesFieldResourceAnnotation(t *testing.T) { + tests := []struct { + testName string + src string + problems testutils.Problems + problemDesc func(m protoreflect.MessageDescriptor) protoreflect.Descriptor + }{ + { + testName: "Valid-ResourceSingularMetadata", + src: ` + import "google/api/resource.proto"; + + message BatchGetImpressionMetadataRequest { + repeated GetImpressionMetadataRequest requests = 1; + } + message GetImpressionMetadataRequest {} + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + problems: testutils.Problems{}, + }, + { + testName: "Invalid-WrongTypeWithResourceSingular", + src: ` + import "google/api/resource.proto"; + + message BatchGetImpressionMetadataRequest { + repeated string requests = 1; + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + problems: testutils.Problems{{Message: `The "requests" field on Batch Get Request should be a "GetImpressionMetadataRequest" type`}}, + problemDesc: func(m protoreflect.MessageDescriptor) protoreflect.Descriptor { + return m.Fields().ByName("requests") + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3String(t, test.src) + m := file.Messages().Get(0) + + var problemDesc protoreflect.Descriptor = m + if test.problemDesc != nil { + problemDesc = test.problemDesc(m) + } + + problems := namesField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0231/response_resource_field.go b/rules/aip0231/response_resource_field.go index 517ea0bd9..2257c82e6 100644 --- a/rules/aip0231/response_resource_field.go +++ b/rules/aip0231/response_resource_field.go @@ -18,8 +18,8 @@ import ( "fmt" "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -29,8 +29,8 @@ var resourceField = &lint.MessageRule{ OnlyIf: isBatchGetResponseMessage, LintMessage: func(m protoreflect.MessageDescriptor) []lint.Problem { // The singular form the resource message name; the first letter capitalized. - plural := strings.TrimSuffix(strings.TrimPrefix(string(m.Name()), "BatchGet"), "Response") - resourceMsgName := pluralize.NewClient().Singular(plural) + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Response"), "BatchGet") + resourceMsgName := utils.ResourceSingular(pluralName, m) for i := 0; i < m.Fields().Len(); i++ { fieldDesc := m.Fields().Get(i) diff --git a/rules/aip0231/response_resource_field_test.go b/rules/aip0231/response_resource_field_test.go index 3ccc1b575..e8aeca8f5 100644 --- a/rules/aip0231/response_resource_field_test.go +++ b/rules/aip0231/response_resource_field_test.go @@ -93,3 +93,58 @@ func TestResourceField(t *testing.T) { }) } } + +func TestResourceFieldResourceAnnotation(t *testing.T) { + tests := []struct { + testName string + Field string + problems testutils.Problems + problemDesc func(m protoreflect.MessageDescriptor) protoreflect.Descriptor + }{ + { + testName: "Valid-ResourceSingularMetadata", + Field: "repeated ImpressionMetadata impression_metadata", + problems: testutils.Problems{}, + }, + { + testName: "MissingField-ResourceSingularMetadata", + Field: "string response", + problems: testutils.Problems{{Message: `no "ImpressionMetadata" type field`}}, + problemDesc: func(m protoreflect.MessageDescriptor) protoreflect.Descriptor { + return m + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message BatchGetImpressionMetadataResponse { + {{.Field}} = 1; + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, test) + + m := file.Messages().Get(0) + + var problemDesc protoreflect.Descriptor = m.Fields().Get(0) + if test.problemDesc != nil { + problemDesc = test.problemDesc(m) + } + + problems := resourceField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0233/request_requests_field.go b/rules/aip0233/request_requests_field.go index 03f6c5756..4a859af41 100644 --- a/rules/aip0233/request_requests_field.go +++ b/rules/aip0233/request_requests_field.go @@ -18,9 +18,9 @@ import ( "fmt" "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" "github.com/googleapis/api-linter/v2/locations" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -52,8 +52,8 @@ var requestRequestsField = &lint.MessageRule{ // Rule check: Ensure that the standard create request message field is the // correct type. Note: Retrieve the resource name from the the batch create // request, for example: "BatchCreateBooksRequest" -> "Books" - rightTypeName := fmt.Sprintf("Create%sRequest", - pluralize.NewClient().Singular(strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Request"), "BatchCreate"))) + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Request"), "BatchCreate") + rightTypeName := fmt.Sprintf("Create%sRequest", utils.ResourceSingular(pluralName, m)) if requests.Message() == nil || requests.Message().Name() != protoreflect.Name(rightTypeName) { problems = append(problems, lint.Problem{ Message: fmt.Sprintf(`The "requests" field on Batch Create Request should be a %q type`, rightTypeName), diff --git a/rules/aip0233/request_requests_field_test.go b/rules/aip0233/request_requests_field_test.go index f28aca438..afe48be54 100644 --- a/rules/aip0233/request_requests_field_test.go +++ b/rules/aip0233/request_requests_field_test.go @@ -98,3 +98,61 @@ func TestRequestRequestsField(t *testing.T) { }) } } + +func TestRequestRequestsFieldResourceAnnotation(t *testing.T) { + tests := []struct { + testName string + Field string + problems testutils.Problems + problemDesc func(m protoreflect.MessageDescriptor) protoreflect.Descriptor + }{ + { + testName: "Valid-ResourceSingularMetadata", + Field: "repeated CreateImpressionMetadataRequest requests = 1;", + problems: testutils.Problems{}, + }, + { + testName: "Invalid-WrongTypeWithResourceSingular", + Field: "repeated int32 requests = 1;", + problems: testutils.Problems{{ + Suggestion: "CreateImpressionMetadataRequest", + }}, + problemDesc: func(m protoreflect.MessageDescriptor) protoreflect.Descriptor { + return m.Fields().ByName("requests") + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message BatchCreateImpressionMetadataRequest { + {{.Field}} + } + message CreateImpressionMetadataRequest {} + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, test) + + m := file.Messages().Get(0) + + var problemDesc protoreflect.Descriptor = m + if test.problemDesc != nil { + problemDesc = test.problemDesc(m) + } + + problems := requestRequestsField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0233/response_resource_field.go b/rules/aip0233/response_resource_field.go index da99b9bcd..93a90e538 100644 --- a/rules/aip0233/response_resource_field.go +++ b/rules/aip0233/response_resource_field.go @@ -18,8 +18,8 @@ import ( "fmt" "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -31,7 +31,8 @@ var responseResourceField = &lint.MessageRule{ // the singular form the resource name, the first letter is Capitalized. // Note: Retrieve the resource name from the the batch create response, // for example: "BatchCreateBooksResponse" -> "Books" - resourceMsgName := pluralize.NewClient().Singular(strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Response"), "BatchCreate")) + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Response"), "BatchCreate") + resourceMsgName := utils.ResourceSingular(pluralName, m) for i := 0; i < m.Fields().Len(); i++ { fieldDesc := m.Fields().Get(i) diff --git a/rules/aip0233/response_resource_field_test.go b/rules/aip0233/response_resource_field_test.go index a2137a26e..201cd1452 100644 --- a/rules/aip0233/response_resource_field_test.go +++ b/rules/aip0233/response_resource_field_test.go @@ -74,3 +74,58 @@ func TestResponseResourceField(t *testing.T) { }) } } + +func TestResponseResourceFieldResourceAnnotation(t *testing.T) { + tests := []struct { + testName string + Src string + problems testutils.Problems + problemDesc func(m protoreflect.MessageDescriptor) protoreflect.Descriptor + }{ + { + testName: "Valid-ResourceSingularMetadata", + Src: `repeated ImpressionMetadata impression_metadata = 1;`, + problems: testutils.Problems{}, + }, + { + testName: "MissingField-ResourceSingularMetadata", + Src: `string response = 1;`, + problems: testutils.Problems{{Message: `no "ImpressionMetadata" type field`}}, + problemDesc: func(m protoreflect.MessageDescriptor) protoreflect.Descriptor { + return m + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message BatchCreateImpressionMetadataResponse { + {{.Src}} + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, test) + + m := file.Messages().Get(0) + + var problemDesc protoreflect.Descriptor = m.Fields().Get(0) + if test.problemDesc != nil { + problemDesc = test.problemDesc(m) + } + + problems := responseResourceField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0234/request_requests_field.go b/rules/aip0234/request_requests_field.go index 5df64051a..122bdd5a7 100644 --- a/rules/aip0234/request_requests_field.go +++ b/rules/aip0234/request_requests_field.go @@ -18,9 +18,9 @@ import ( "fmt" "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" "github.com/googleapis/api-linter/v2/locations" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -52,8 +52,8 @@ var requestRequestsField = &lint.MessageRule{ // Rule check: Ensure that the standard update request message field is the // correct type. Note: Retrieve the resource name from the the batch update // request, for example: "BatchUpdateBooksRequest" -> "Books" - rightTypeName := fmt.Sprintf("Update%sRequest", - pluralize.NewClient().Singular(strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Request"), "BatchUpdate"))) + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Request"), "BatchUpdate") + rightTypeName := fmt.Sprintf("Update%sRequest", utils.ResourceSingular(pluralName, m)) if requests.Message() == nil || requests.Message().Name() != protoreflect.Name(rightTypeName) { problems = append(problems, lint.Problem{ Message: fmt.Sprintf(`The "requests" field on Batch Update Request should be a %q type`, rightTypeName), diff --git a/rules/aip0234/request_requests_field_test.go b/rules/aip0234/request_requests_field_test.go index 5dcbc6891..bc4e588b1 100644 --- a/rules/aip0234/request_requests_field_test.go +++ b/rules/aip0234/request_requests_field_test.go @@ -87,3 +87,60 @@ func TestRequestRequestsField(t *testing.T) { }) } } + +func TestRequestRequestsFieldResourceAnnotation(t *testing.T) { + // Test that the rule respects google.api.resource singular annotation + // for words that go-pluralize would incorrectly singularize (e.g. + // "Metadata" -> "Metadatum"). + tests := []struct { + testName string + Field string + problems testutils.Problems + }{ + { + testName: "Valid-ResourceSingularMetadata", + Field: "repeated UpdateImpressionMetadataRequest requests", + problems: testutils.Problems{}, + }, + { + testName: "Invalid-WrongTypeWithResourceSingular", + Field: "repeated int32 requests", + problems: testutils.Problems{{ + Suggestion: "UpdateImpressionMetadataRequest", + }}, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message BatchUpdateImpressionMetadataRequest { + {{.Field}} = 1; + } + message UpdateImpressionMetadataRequest {} + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.googleapis.com/ImpressionMetadata" + pattern: "dataProviders/{data_provider}/impressionMetadata/{impression_metadata}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, test) + + var problemDesc protoreflect.Descriptor + if requests := file.Messages().Get(0).Fields().ByName("requests"); requests != nil { + problemDesc = requests + } else { + problemDesc = file.Messages().Get(0) + } + + problems := requestRequestsField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0234/response_resource_field.go b/rules/aip0234/response_resource_field.go index 357bdf855..7ccab8012 100644 --- a/rules/aip0234/response_resource_field.go +++ b/rules/aip0234/response_resource_field.go @@ -18,8 +18,8 @@ import ( "fmt" "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -31,7 +31,8 @@ var responseResourceField = &lint.MessageRule{ // the singular form the resource name, the first letter is Capitalized. // Note: Retrieve the resource name from the the batch update response, // for example: "BatchUpdateBooksResponse" -> "Books" - resourceMsgName := pluralize.NewClient().Singular(strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Response"), "BatchUpdate")) + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Response"), "BatchUpdate") + resourceMsgName := utils.ResourceSingular(pluralName, m) for i := 0; i < m.Fields().Len(); i++ { fieldDesc := m.Fields().Get(i) diff --git a/rules/aip0234/response_resource_field_test.go b/rules/aip0234/response_resource_field_test.go index 185b31d69..3899bf638 100644 --- a/rules/aip0234/response_resource_field_test.go +++ b/rules/aip0234/response_resource_field_test.go @@ -74,3 +74,60 @@ func TestResponseResourceField(t *testing.T) { }) } } + +func TestResponseResourceFieldResourceAnnotation(t *testing.T) { + // Test that the rule respects google.api.resource singular annotation + // for words that go-pluralize would incorrectly singularize. + tests := []struct { + testName string + Field string + problems testutils.Problems + problemDesc func(m protoreflect.MessageDescriptor) protoreflect.Descriptor + }{ + { + testName: "Valid-ResourceSingularMetadata", + Field: "repeated ImpressionMetadata impression_metadata", + problems: testutils.Problems{}, + }, + { + testName: "MissingField-ResourceSingularMetadata", + Field: "string response", + problems: testutils.Problems{{Message: `no "ImpressionMetadata" type field`}}, + problemDesc: func(m protoreflect.MessageDescriptor) protoreflect.Descriptor { + return m + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message BatchUpdateImpressionMetadataResponse { + {{.Field}} = 1; + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.googleapis.com/ImpressionMetadata" + pattern: "dataProviders/{data_provider}/impressionMetadata/{impression_metadata}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, test) + + m := file.Messages().Get(0) + + var problemDesc protoreflect.Descriptor = m.Fields().Get(0) + if test.problemDesc != nil { + problemDesc = test.problemDesc(m) + } + + problems := responseResourceField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0235/request_names_field.go b/rules/aip0235/request_names_field.go index 562eb4212..c8a36d955 100644 --- a/rules/aip0235/request_names_field.go +++ b/rules/aip0235/request_names_field.go @@ -16,10 +16,11 @@ package aip0235 import ( "fmt" + "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" "github.com/googleapis/api-linter/v2/locations" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -78,10 +79,10 @@ var requestNamesField = &lint.MessageRule{ } // Rule check: Ensure that the standard delete request message field is the - // correct type. Note: Use m.Name()[11:len(m.Name())-7]) to retrieve - // the resource name from the batch delete request, for example: - // "BatchDeleteBooksRequest" -> "Books" - rightTypeName := fmt.Sprintf("Delete%sRequest", pluralize.NewClient().Singular(string(m.Name())[11:len(m.Name())-7])) + // correct type. Note: Retrieve the resource name from the batch delete + // request, for example: "BatchDeleteBooksRequest" -> "Books" + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Request"), "BatchDelete") + rightTypeName := fmt.Sprintf("Delete%sRequest", utils.ResourceSingular(pluralName, m)) if deleteReqMsg != nil && (deleteReqMsg.Message() == nil || deleteReqMsg.Message().Name() != protoreflect.Name(rightTypeName)) { problems = append(problems, lint.Problem{ Message: fmt.Sprintf(`The "requests" field on Batch Delete Request should be a %q type`, rightTypeName), diff --git a/rules/aip0235/request_names_field_test.go b/rules/aip0235/request_names_field_test.go index 828f45dc4..6fcb29d32 100644 --- a/rules/aip0235/request_names_field_test.go +++ b/rules/aip0235/request_names_field_test.go @@ -149,3 +149,92 @@ func TestNamesField(t *testing.T) { }) } } + +func TestNamesFieldResourceAnnotation(t *testing.T) { + tests := []struct { + testName string + src string + problems testutils.Problems + problemDesc func(m protoreflect.MessageDescriptor) protoreflect.Descriptor + }{ + { + testName: "Valid-Names-ResourceSingularMetadata", + src: ` + import "google/api/resource.proto"; + + message BatchDeleteImpressionMetadataRequest { + repeated string names = 1; + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + problems: testutils.Problems{}, + }, + { + testName: "Valid-StandardDeleteReq-ResourceSingularMetadata", + src: ` + import "google/api/resource.proto"; + + message BatchDeleteImpressionMetadataRequest { + repeated DeleteImpressionMetadataRequest requests = 1; + } + message DeleteImpressionMetadataRequest {} + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + problems: testutils.Problems{}, + }, + { + testName: "Invalid-DeleteReqFieldWrongType-ResourceSingularMetadata", + src: ` + import "google/api/resource.proto"; + + message BatchDeleteImpressionMetadataRequest { + repeated string requests = 1; + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + problems: testutils.Problems{{Message: `should be a "DeleteImpressionMetadataRequest" type`}}, + problemDesc: func(m protoreflect.MessageDescriptor) protoreflect.Descriptor { + return m.Fields().ByName("requests") + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3String(t, test.src) + + m := file.Messages().Get(0) + + var problemDesc protoreflect.Descriptor = m + if test.problemDesc != nil { + problemDesc = test.problemDesc(m) + } + + problems := requestNamesField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0235/response_resource_field.go b/rules/aip0235/response_resource_field.go index 3f68426c4..f87124838 100644 --- a/rules/aip0235/response_resource_field.go +++ b/rules/aip0235/response_resource_field.go @@ -18,8 +18,8 @@ import ( "fmt" "strings" - "github.com/gertd/go-pluralize" "github.com/googleapis/api-linter/v2/lint" + "github.com/googleapis/api-linter/v2/rules/internal/utils" "google.golang.org/protobuf/reflect/protoreflect" ) @@ -31,7 +31,8 @@ var responseResourceField = &lint.MessageRule{ // the singular form the resource name, the first letter is Capitalized. // Note: Retrieve the resource name from the the batch update response, // for example: "BatchDeleteBooksResponse" -> "Books" - resourceMsgName := pluralize.NewClient().Singular(strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Response"), "BatchDelete")) + pluralName := strings.TrimPrefix(strings.TrimSuffix(string(m.Name()), "Response"), "BatchDelete") + resourceMsgName := utils.ResourceSingular(pluralName, m) for i := 0; i < m.Fields().Len(); i++ { fieldDesc := m.Fields().Get(i) diff --git a/rules/aip0235/response_resource_field_test.go b/rules/aip0235/response_resource_field_test.go index 3d12a6bd2..1baf13a50 100644 --- a/rules/aip0235/response_resource_field_test.go +++ b/rules/aip0235/response_resource_field_test.go @@ -84,3 +84,61 @@ func TestResponseResourceField(t *testing.T) { }) } } + +func TestResponseResourceFieldResourceAnnotation(t *testing.T) { + tests := []struct { + testName string + Message string + Field string + problems testutils.Problems + problemDesc func(m protoreflect.MessageDescriptor) protoreflect.Descriptor + }{ + { + testName: "Valid-ResourceSingularMetadata", + Message: "BatchDeleteImpressionMetadataResponse", + Field: "repeated ImpressionMetadata impression_metadata", + problems: testutils.Problems{}, + }, + { + testName: "MissingField-ResourceSingularMetadata", + Message: "BatchDeleteImpressionMetadataResponse", + Field: "string response", + problems: testutils.Problems{{Message: `no "ImpressionMetadata" type field`}}, + problemDesc: func(m protoreflect.MessageDescriptor) protoreflect.Descriptor { + return m + }, + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3Tmpl(t, ` + import "google/api/resource.proto"; + + message {{.Message}} { + {{.Field}} = 1; + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, test) + + m := file.Messages().Get(0) + + var problemDesc protoreflect.Descriptor = m.Fields().Get(0) + if test.problemDesc != nil { + problemDesc = test.problemDesc(m) + } + + problems := responseResourceField.Lint(file) + if diff := test.problems.SetDescriptor(problemDesc).Diff(problems); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/internal/utils/string_pluralize.go b/rules/internal/utils/string_pluralize.go index b416086f2..499e63642 100644 --- a/rules/internal/utils/string_pluralize.go +++ b/rules/internal/utils/string_pluralize.go @@ -1,4 +1,4 @@ -// Copyright 2019 Google LLC +// Copyright 2026 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package utils import ( "github.com/gertd/go-pluralize" + "github.com/stoewer/go-strcase" + "google.golang.org/protobuf/reflect/protoreflect" ) var pluralizeClient = pluralize.NewClient() @@ -32,3 +34,80 @@ func ToPlural(s string) string { func ToSingular(s string) string { return pluralizeClient.Singular(s) } + +// ResourceSingular returns the singular form of a resource name extracted from +// a batch method message name (e.g. "Books" from "BatchUpdateBooksRequest"). +// +// It searches for a resource message with a google.api.resource annotation +// whose singular matches the expected singular of pluralName. This search +// covers the file's entire import graph (recursively), since the resource +// message may be defined in a separate file from the service and +// request/response messages. +// +// If a matching resource annotation is found, its singular is returned (in +// UpperCamelCase). Otherwise, it falls back to the go-pluralize library. +// +// This avoids incorrect singularization of words like "Metadata" (which +// go-pluralize converts to "Metadatum" using Latin grammar rules). +func ResourceSingular(pluralName string, m protoreflect.MessageDescriptor) string { + if f := m.ParentFile(); f != nil { + if s := findResourceSingularRecursive(pluralName, f, make(map[string]bool)); s != "" { + return s + } + } + return pluralizeClient.Singular(pluralName) +} + +// findResourceSingularRecursive searches a file and its transitive imports +// for a resource whose singular annotation matches the given pluralName. +func findResourceSingularRecursive(pluralName string, f protoreflect.FileDescriptor, visited map[string]bool) string { + if f == nil || visited[string(f.Path())] { + return "" + } + visited[string(f.Path())] = true + + if s := findResourceSingularInFile(pluralName, f); s != "" { + return s + } + imports := f.Imports() + for i := 0; i < imports.Len(); i++ { + if s := findResourceSingularRecursive(pluralName, imports.Get(i).FileDescriptor, visited); s != "" { + return s + } + } + return "" +} + +// findResourceSingularInFile searches all messages in a file for a resource +// whose singular annotation matches the given pluralName. +func findResourceSingularInFile(pluralName string, f protoreflect.FileDescriptor) string { + if f == nil { + return "" + } + for i := 0; i < f.Messages().Len(); i++ { + msg := f.Messages().Get(i) + res := GetResource(msg) + if res == nil { + continue + } + s := GetResourceSingular(res) + if s == "" { + continue + } + // The singular from the annotation is typically lowerCamelCase + // (e.g. "impressionMetadata"). Convert to UpperCamelCase for + // comparison with the message name. + upperSingular := strcase.UpperCamelCase(s) + // Check if the pluralName matches: either the singular itself + // (for uncountable nouns like "Metadata" where plural == singular) + // or the go-pluralize plural of the singular. + if pluralName == upperSingular || pluralName == pluralizeClient.Plural(upperSingular) { + return upperSingular + } + // Also check if the message name matches the plural portion. + if string(msg.Name()) == pluralName { + return upperSingular + } + } + return "" +} diff --git a/rules/internal/utils/string_pluralize_test.go b/rules/internal/utils/string_pluralize_test.go index 8cc3887d9..3b03f0496 100644 --- a/rules/internal/utils/string_pluralize_test.go +++ b/rules/internal/utils/string_pluralize_test.go @@ -16,6 +16,8 @@ package utils import ( "testing" + + "github.com/googleapis/api-linter/v2/rules/internal/testutils" ) func TestPluralize(t *testing.T) { @@ -41,3 +43,182 @@ func TestPluralize(t *testing.T) { }) } } + +func TestResourceSingular(t *testing.T) { + tests := []struct { + testName string + pluralName string + src string + want string + }{ + { + testName: "AnnotationInSameFile", + pluralName: "ImpressionMetadata", + src: ` + import "google/api/resource.proto"; + + message BatchUpdateImpressionMetadataRequest { + string parent = 1; + } + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + want: "ImpressionMetadata", + }, + { + testName: "FallbackToGoPluralizeBooks", + pluralName: "Books", + src: ` + message BatchUpdateBooksRequest { + string parent = 1; + } + `, + want: "Book", + }, + { + testName: "UncountableNounPluralEqualsSingular", + pluralName: "Metadata", + src: ` + import "google/api/resource.proto"; + + message BatchUpdateMetadataRequest { + string parent = 1; + } + message Metadata { + option (google.api.resource) = { + type: "example.com/Metadata" + pattern: "items/{item}/metadata/{metadata}" + singular: "metadata" + plural: "metadata" + }; + } + `, + want: "Metadata", + }, + { + testName: "MessageNameMatchesPluralName", + pluralName: "CursorData", + src: ` + import "google/api/resource.proto"; + + message BatchUpdateCursorDataRequest { + string parent = 1; + } + message CursorData { + option (google.api.resource) = { + type: "example.com/CursorData" + pattern: "items/{item}/cursorData/{cursor_data}" + singular: "cursorDatum" + plural: "cursorData" + }; + } + `, + want: "CursorDatum", + }, + { + testName: "NoAnnotationLatinWord", + pluralName: "Data", + src: ` + message BatchUpdateDataRequest { + string parent = 1; + } + `, + want: "Datum", + }, + } + + for _, test := range tests { + t.Run(test.testName, func(t *testing.T) { + file := testutils.ParseProto3String(t, test.src) + m := file.Messages().Get(0) + got := ResourceSingular(test.pluralName, m) + if got != test.want { + t.Errorf("ResourceSingular(%q) = %q, want %q", test.pluralName, got, test.want) + } + }) + } +} + +func TestResourceSingularImportedFile(t *testing.T) { + // Verify that ResourceSingular finds the resource annotation in a + // directly imported file, not just the same file. + files := testutils.ParseProtoStrings(t, map[string]string{ + "resource.proto": ` + syntax = "proto3"; + import "google/api/resource.proto"; + + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + "service.proto": ` + syntax = "proto3"; + import "resource.proto"; + + message BatchUpdateImpressionMetadataRequest { + string parent = 1; + } + `, + }) + + serviceFile := files["service.proto"] + m := serviceFile.Messages().Get(0) + got := ResourceSingular("ImpressionMetadata", m) + if got != "ImpressionMetadata" { + t.Errorf("ResourceSingular(\"ImpressionMetadata\") = %q, want \"ImpressionMetadata\"", got) + } +} + +func TestResourceSingularTransitiveImport(t *testing.T) { + // Verify that ResourceSingular finds the resource annotation + // through transitive imports (service -> common -> resource). + files := testutils.ParseProtoStrings(t, map[string]string{ + "resource.proto": ` + syntax = "proto3"; + import "google/api/resource.proto"; + + message ImpressionMetadata { + option (google.api.resource) = { + type: "example.com/ImpressionMetadata" + pattern: "dataProviders/{dp}/impressionMetadata/{im}" + singular: "impressionMetadata" + plural: "impressionMetadata" + }; + } + `, + "common.proto": ` + syntax = "proto3"; + import "resource.proto"; + + message CommonFields { + string parent = 1; + } + `, + "service.proto": ` + syntax = "proto3"; + import "common.proto"; + + message BatchUpdateImpressionMetadataRequest { + string parent = 1; + } + `, + }) + + serviceFile := files["service.proto"] + m := serviceFile.Messages().Get(0) + got := ResourceSingular("ImpressionMetadata", m) + if got != "ImpressionMetadata" { + t.Errorf("ResourceSingular(\"ImpressionMetadata\") = %q, want \"ImpressionMetadata\"", got) + } +}