fix(AIP-162): handle LRO in response name rules (#1431)

rules/aip0162/commit_response_message_name.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/locations"
+	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
 )
 
@@ -28,8 +29,19 @@ var commitResponseMessageName = &lint.MethodRule{
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
 		// Rule check: Establish that for methods such as `CommitBook`, the response
 		// message is `Book`.
+		response := utils.GetResponseType(m)
+		if response == nil {
+			return nil
+		}
+		got := response.GetName()
 		want := commitMethodRegexp.FindStringSubmatch(m.GetName())[1]
-		got := m.GetOutputType().GetName()
+		loc := locations.MethodResponseType(m)
+		suggestion := want
+
+		if utils.GetOperationInfo(m) != nil {
+			loc = locations.MethodOperationInfo(m)
+			suggestion = "" // We cannot offer a precise enough location to make a suggestion.
+		}
 
 		// Return a problem if we did not get the expected return name.
 		if got != want {
@@ -39,9 +51,9 @@ var commitResponseMessageName = &lint.MethodRule{
 					want,
 					got,
 				),
-				Suggestion: want,
+				Suggestion: suggestion,
 				Descriptor: m,
-				Location:   locations.MethodResponseType(m),
+				Location:   loc,
 			}}
 		}
 		return nil

rules/aip0162/commit_response_message_name_test.go

@@ -33,7 +33,6 @@ func TestCommitResponseMessageName(t *testing.T) {
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			f := testutils.ParseProto3Tmpl(t, `
-				import "google/longrunning/operations.proto";
 				service Library {
 					rpc {{.Method}}(CommitBookRequest) returns ({{.ResponseType}});
 				}
@@ -47,3 +46,37 @@ func TestCommitResponseMessageName(t *testing.T) {
 		})
 	}
 }
+
+func TestCommitResponseMessageNameLRO(t *testing.T) {
+	for _, test := range []struct {
+		name         string
+		Method       string
+		ResponseType string
+		problems     testutils.Problems
+	}{
+		{"Valid", "CommitBook", "Book", nil},
+		{"Invalid", "CommitBook", "CommitBookResponse", testutils.Problems{{Message: "Book"}}},
+		{"Irrelevant", "AcquireBook", "PurgeBooksResponse", nil},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				import "google/longrunning/operations.proto";
+				service Library {
+					rpc {{.Method}}(CommitBookRequest) returns (google.longrunning.Operation) {
+					  option (google.longrunning.operation_info) = {
+					    response_type: "{{.ResponseType}}"
+						metadata_type: "OperationMetadata"
+					  };
+					};
+				}
+				message CommitBookRequest {}
+				message {{.ResponseType}} {}
+				message OperationMetadata{}
+			`, test)
+			m := f.GetServices()[0].GetMethods()[0]
+			if diff := test.problems.SetDescriptor(m).Diff(commitResponseMessageName.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}

rules/aip0162/delete_revision_response_message_name.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/locations"
+	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
 )
 
@@ -28,14 +29,27 @@ var deleteRevisionResponseMessageName = &lint.MethodRule{
 	Name:   lint.NewRuleName(162, "delete-revision-response-message-name"),
 	OnlyIf: isDeleteRevisionMethod,
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		got := m.GetOutputType().GetName()
+		response := utils.GetResponseType(m)
+		if response == nil {
+			return nil
+		}
+		got := response.GetName()
 		want := strings.TrimPrefix(strings.TrimSuffix(m.GetName(), "Revision"), "Delete")
+
+		loc := locations.MethodResponseType(m)
+		suggestion := want
+
+		if utils.GetOperationInfo(m) != nil {
+			loc = locations.MethodOperationInfo(m)
+			suggestion = "" // We cannot offer a precise enough location to make a suggestion.
+		}
+
 		if got != want {
 			return []lint.Problem{{
 				Message:    fmt.Sprintf("Delete Revision methods should return the resource itself (`%s`), not `%s`.", want, got),
-				Suggestion: want,
+				Suggestion: suggestion,
 				Descriptor: m,
-				Location:   locations.MethodResponseType(m),
+				Location:   loc,
 			}}
 		}
 		return nil

rules/aip0162/delete_revision_response_message_name_test.go

@@ -48,3 +48,39 @@ func TestDeleteRevisionResponseMessageName(t *testing.T) {
 		})
 	}
 }
+
+func TestDeleteRevisionResponseMessageNameLRO(t *testing.T) {
+	for _, test := range []struct {
+		name         string
+		MethodName   string
+		ResponseType string
+		problems     testutils.Problems
+	}{
+		{"Valid", "DeleteBookRevision", "Book", nil},
+		{"Invalid", "DeleteBookRevision", "DeleteBookRevisionResponse", testutils.Problems{{Message: "Book"}}},
+		{"Irrelevant", "DeleteBook", "DeleteBookResponse", nil},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+				import "google/longrunning/operations.proto";
+				service Library {
+					rpc {{.MethodName}}({{.MethodName}}Request) returns (google.longrunning.Operation) {
+					  option (google.longrunning.operation_info) = {
+				        response_type: "{{.ResponseType}}"
+						metadata_type: "OperationMetadata"
+					  };
+					};
+				}
+				message {{.MethodName}}Request {}
+				message {{.ResponseType}} {}
+				message OperationMetadata {}
+			`, test)
+
+			method := file.GetServices()[0].GetMethods()[0]
+			problems := deleteRevisionResponseMessageName.Lint(file)
+			if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}

rules/aip0162/rollback_response_message_name.go

@@ -16,7 +16,6 @@ package aip0162
 
 import (
 	"fmt"
-	"strings"
 
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/locations"
@@ -31,15 +30,17 @@ var rollbackResponseMessageName = &lint.MethodRule{
 		// Rule check: Establish that for methods such as `RollbackBook`, the response
 		// message is `Book`.
 		want := rollbackMethodRegexp.FindStringSubmatch(m.GetName())[1]
-		got := m.GetOutputType().GetName()
+		response := utils.GetResponseType(m)
+		if response == nil {
+			return nil
+		}
+		got := response.GetName()
 		loc := locations.MethodResponseType(m)
+		suggestion := want
 
-		// If LRO, check the response_type short name.
-		if utils.IsOperation(m.GetOutputType()) {
-			t := utils.GetOperationInfo(m).GetResponseType()
-			ndx := strings.LastIndex(t, ".")
-			got = t[ndx+1:]
+		if utils.GetOperationInfo(m) != nil {
 			loc = locations.MethodOperationInfo(m)
+			suggestion = "" // We cannot offer a precise enough location to make a suggestion.
 		}
 
 		// Return a problem if we did not get the expected return name.
@@ -50,7 +51,7 @@ var rollbackResponseMessageName = &lint.MethodRule{
 					want,
 					got,
 				),
-				Suggestion: want,
+				Suggestion: suggestion,
 				Descriptor: m,
 				Location:   loc,
 			}}

rules/aip0162/rollback_response_message_name_test.go

@@ -33,7 +33,6 @@ func TestRollbackResponseMessageName(t *testing.T) {
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			f := testutils.ParseProto3Tmpl(t, `
-				import "google/longrunning/operations.proto";
 				service Library {
 					rpc {{.Method}}(RollbackBookRequest) returns ({{.ResponseType}});
 				}
@@ -56,7 +55,7 @@ func TestRollbackOperationResponse(t *testing.T) {
 		problems     testutils.Problems
 	}{
 		{"Valid", "RollbackBook", "Book", nil},
-		{"Invalid", "RollbackBook", "RollbackBookResponse", testutils.Problems{{Suggestion: "Book"}}},
+		{"Invalid", "RollbackBook", "RollbackBookResponse", testutils.Problems{{Message: "Book"}}},
 		{"Irrelevant", "AcquireBook", "PurgeBooksResponse", nil},
 	} {
 		t.Run(test.name, func(t *testing.T) {

rules/aip0162/tag_revision_response_message_name.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/locations"
+	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
 )
 
@@ -29,7 +30,18 @@ var tagRevisionResponseMessageName = &lint.MethodRule{
 		// Rule check: Establish that for methods such as `TagBookRevision`, the response
 		// message is `Book`.
 		want := tagRevisionMethodRegexp.FindStringSubmatch(m.GetName())[1]
-		got := m.GetOutputType().GetName()
+		response := utils.GetResponseType(m)
+		if response == nil {
+			return nil
+		}
+		got := response.GetName()
+		loc := locations.MethodResponseType(m)
+		suggestion := want
+
+		if utils.GetOperationInfo(m) != nil {
+			loc = locations.MethodOperationInfo(m)
+			suggestion = "" // We cannot offer a precise enough location to make a suggestion.
+		}
 
 		// Return a problem if we did not get the expected return name.
 		if got != want {
@@ -39,9 +51,9 @@ var tagRevisionResponseMessageName = &lint.MethodRule{
 					want,
 					got,
 				),
-				Suggestion: want,
+				Suggestion: suggestion,
 				Descriptor: m,
-				Location:   locations.MethodResponseType(m),
+				Location:   loc,
 			}}
 		}
 		return nil

rules/aip0162/tag_revision_response_message_name_test.go

@@ -33,7 +33,6 @@ func TestTagRevisionResponseMessageName(t *testing.T) {
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			f := testutils.ParseProto3Tmpl(t, `
-				import "google/longrunning/operations.proto";
 				service Library {
 					rpc {{.Method}}(TagBookRevisionRequest) returns ({{.ResponseType}});
 				}
@@ -47,3 +46,37 @@ func TestTagRevisionResponseMessageName(t *testing.T) {
 		})
 	}
 }
+
+func TestTagRevisionResponseMessageNameLRO(t *testing.T) {
+	for _, test := range []struct {
+		name         string
+		Method       string
+		ResponseType string
+		problems     testutils.Problems
+	}{
+		{"Valid", "TagBookRevision", "Book", nil},
+		{"Invalid", "TagBookRevision", "TagBookRevisionResponse", testutils.Problems{{Message: "Book"}}},
+		{"Irrelevant", "AcquireBook", "PurgeBooksResponse", nil},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				import "google/longrunning/operations.proto";
+				service Library {
+					rpc {{.Method}}(TagBookRevisionRequest) returns (google.longrunning.Operation) {
+					  option (google.longrunning.operation_info) = {
+					    response_type: "{{.ResponseType}}"
+						metadata_type: "OperationMetadata"
+					  };
+					};
+				}
+				message TagBookRevisionRequest {}
+				message {{.ResponseType}} {}
+				message OperationMetadata {}
+			`, test)
+			m := f.GetServices()[0].GetMethods()[0]
+			if diff := test.problems.SetDescriptor(m).Diff(tagRevisionResponseMessageName.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}