Merge branch 'main' into aip-136-ignore-revisions

rules/aip0162/commit_response_message_name.go

@@ -33,7 +33,18 @@ var commitResponseMessageName = &lint.MethodRule{
 		if !ok {
 			return nil
 		}
-		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 {
@@ -43,9 +54,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

@@ -28,17 +28,30 @@ var deleteRevisionResponseMessageName = &lint.MethodRule{
 	Name:   lint.NewRuleName(162, "delete-revision-response-message-name"),
 	OnlyIf: utils.IsDeleteRevisionMethod,
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		got := m.GetOutputType().GetName()
 		want, ok := utils.ExtractRevisionResource(m)
 		if !ok {
 			return nil
 		}
+		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.
+		}
+
 		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"
@@ -34,15 +33,17 @@ var rollbackResponseMessageName = &lint.MethodRule{
 		if !ok {
 			return nil
 		}
-		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.
@@ -53,7 +54,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

@@ -33,7 +33,18 @@ var tagRevisionResponseMessageName = &lint.MethodRule{
 		if !ok {
 			return nil
 		}
-		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 {
@@ -43,9 +54,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)
+			}
+		})
+	}
+}