From b007953dcd47b31c661c4f18e1694236715c3e4b Mon Sep 17 00:00:00 2001
From: Matt Birchler <mbirchler@goodrx.com>
Date: Fri, 17 May 2024 17:11:05 -0400
Subject: [PATCH] chore(AIP-136): PR feedback

---
 rules/aip0136/response_message_name.go      | 35 +++++++++++----------
 rules/aip0136/response_message_name_test.go |  6 ++--
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/rules/aip0136/response_message_name.go b/rules/aip0136/response_message_name.go
index c790abe2..d8f4aae3 100644
--- a/rules/aip0136/response_message_name.go
+++ b/rules/aip0136/response_message_name.go
@@ -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{
@@ -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),
 		}}
diff --git a/rules/aip0136/response_message_name_test.go b/rules/aip0136/response_message_name_test.go
index 108cec7f..f37f8a62 100644
--- a/rules/aip0136/response_message_name_test.go
+++ b/rules/aip0136/response_message_name_test.go
@@ -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 {