Skip to content

Commit

Permalink
fix: BatchGet rule now singularizes correctly. (#585)
Browse files Browse the repository at this point in the history
  • Loading branch information
Luke Sneeringer authored Aug 3, 2020
1 parent 910e38f commit dd1810f
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 19 deletions.
9 changes: 4 additions & 5 deletions rules/aip0231/response_resource_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package aip0231

import (
"fmt"
"strings"

"github.com/gertd/go-pluralize"
"github.com/googleapis/api-linter/lint"
Expand All @@ -27,11 +28,9 @@ var resourceField = &lint.MessageRule{
Name: lint.NewRuleName(231, "response-resource-field"),
OnlyIf: isBatchGetResponseMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
// the singular form the resource name, the first letter is Capitalized.
// Note: Use m.GetName()[8 : len(m.GetName())-9] to retrieve the resource
// name from the the batch get response, for example:
// "BatchGetBooksResponse" -> "Books"
resourceMsgName := pluralize.NewClient().Singular(m.GetName()[8 : len(m.GetName())-9])
// The singular form the resource message name; the first letter capitalized.
plural := strings.TrimSuffix(strings.TrimPrefix(m.GetName(), "BatchGet"), "Response")
resourceMsgName := pluralize.NewClient().Singular(plural)

for _, fieldDesc := range m.GetFields() {
if msgDesc := fieldDesc.GetMessageType(); msgDesc != nil && msgDesc.GetName() == resourceMsgName {
Expand Down
37 changes: 23 additions & 14 deletions rules/aip0231/response_resource_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,27 +32,35 @@ func TestResourceField(t *testing.T) {
{
testName: "Valid",
src: `
message BatchGetBooksResponse {
// Books requested.
repeated Book books = 1;
}`,
message BatchGetBooksResponse {
// Books requested.
repeated Book books = 1;
}`,
problems: testutils.Problems{},
},
{
testName: "ValidEs",
src: `
message BatchGetMatchesResponse {
repeated Match matches = 1;
}`,
problems: testutils.Problems{},
},
{
testName: "FieldIsNotRepeated",
src: `
message BatchGetBooksResponse {
// Book requested.
Book book = 1;
}`,
message BatchGetBooksResponse {
// Book requested.
Book book = 1;
}`,
problems: testutils.Problems{{Message: "The \"Book\" type field on Batch Get Response message should be repeated"}},
},
{
testName: "MissingField",
src: `
message BatchGetBooksResponse {
string response = 1;
}`,
message BatchGetBooksResponse {
string response = 1;
}`,
problems: testutils.Problems{{Message: "Message \"BatchGetBooksResponse\" has no \"Book\" type field"}},
problemDesc: func(m *desc.MessageDescriptor) desc.Descriptor {
return m
Expand All @@ -64,9 +72,10 @@ message BatchGetBooksResponse {
for _, test := range tests {
t.Run(test.testName, func(t *testing.T) {
template := `
{{.Src}}
message Book {
}`
{{.Src}}
message Book {}
message Match {}
`
file := testutils.ParseProto3Tmpl(t, template, struct{ Src string }{test.src})

m := file.GetMessageTypes()[0]
Expand Down

0 comments on commit dd1810f

Please sign in to comment.