Skip to content

Commit

Permalink
chore(AIP-136): PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
liveFreeOrCode committed May 17, 2024
1 parent cfd6056 commit b007953
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 18 deletions.
35 changes: 19 additions & 16 deletions rules/aip0136/response_message_name.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import (
"github.com/jhump/protoreflect/desc"
)

const responseMessageNameErrorMessage = "" +
"Custom methods should return a message matching the RPC name, with a `Response` suffix, " +
"or the resource being operated on, not %q."

// Custom methods should return a response message matching the RPC name,
// with a Response suffix, or the resource being operated on.
var responseMessageName = &lint.MethodRule{
Expand All @@ -36,31 +40,30 @@ var responseMessageName = &lint.MethodRule{
// Reference of the input type's `name` field. This guidance is documented
// in https://google.aip.dev/136#resource-based-custom-methods

response := utils.GetResponseType(m)
requestType := utils.GetResourceReference(m.GetInputType().FindFieldByName("name")).GetType()
resourceOperatedOn := utils.FindResourceMessage(requestType, m.GetFile())

// Output type has `Response` suffix
if len(utils.LintMethodHasMatchingResponseName(m)) == 0 {
suffixFindings := utils.LintMethodHasMatchingResponseName(m)
if len(suffixFindings) == 0 {
return nil
}

// Response is the resource being operated on
if response == resourceOperatedOn {
return nil
}
response := utils.GetResponseType(m)
requestResourceType := utils.GetResourceReference(m.GetInputType().FindFieldByName("name")).GetType()

msg := "Custom methods should return a message matching the RPC name, with a `Response` suffix, or the resource being operated on, not %q."
got := m.GetName()
// If we can't determine if this is a resource-based custom method return the
// findings from the above matching response name lint
if !utils.IsResource(response) || requestResourceType == "" {
return suffixFindings
}

suggestion := got + "Response"
if utils.IsResource(response) && resourceOperatedOn != nil {
suggestion += " or " + resourceOperatedOn.GetName()
// However, if we can determine that this is a resource-based custom method,
// the ensure that they equal
responseResourceType := utils.GetResource(response).GetType()
if responseResourceType == requestResourceType {
return nil
}

return []lint.Problem{{
Message: fmt.Sprintf(msg, got),
Suggestion: suggestion,
Message: fmt.Sprintf(responseMessageNameErrorMessage, response.GetName()),
Descriptor: m,
Location: locations.MethodResponseType(m),
}}
Expand Down
6 changes: 4 additions & 2 deletions rules/aip0136/response_message_name_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ func TestResponseMessageName(t *testing.T) {
}{
{"Valid", "ArchiveBook", "Book", false, testutils.Problems{}},
{"Valid LRO", "ArchiveBook", "Book", true, testutils.Problems{}},
{"Invalid", "ArchiveBook", "Author", false, testutils.Problems{{Suggestion: "ArchiveBookResponse or Book"}}},
{"Invalid LRO", "ArchiveBook", "Author", true, testutils.Problems{{Suggestion: "ArchiveBookResponse or Book"}}},
{"Invalid", "ArchiveBook", "Author", false, testutils.Problems{{
Message: "Custom methods should return a message matching the RPC name, with a `Response` suffix, or the resource being operated on, not \"Author\"."}}},
{"Invalid LRO", "ArchiveBook", "Author", true, testutils.Problems{{
Message: "Custom methods should return a message matching the RPC name, with a `Response` suffix, or the resource being operated on, not \"Author\"."}}},
}

for _, test := range tests {
Expand Down

0 comments on commit b007953

Please sign in to comment.