feat(aip-136): response message name (#1387)

docs/rules/0136/response-message-name.md

@@ -0,0 +1,158 @@
+---
+rule:
+  aip: 136
+  name: [core, '0136', response-message-name]
+  summary: Custom methods must have standardized response message names.
+permalink: /136/response-message-name
+redirect_from:
+  - /0136/response-message-name
+---
+
+# Custom methods: Response message
+
+This rule enforces that custom methods have a response message that is named to
+match the RPC name with a `Response` suffix, or that is the resource being operated
+on, as described in [AIP-136][].
+
+## Details
+
+This rule looks at any method that is not a standard method, and complains if
+the name of the corresponding output message does not match the name of the RPC
+with the suffix `Response` appended, or the resource being operated on.
+
+**Note:** To identify the resource being operated on, the rule checks for
+the request field `name`, and comparing its `(google.api.resource_reference).type`,
+if present, to the response message's `(google.api.resource).type`, if present.
+
+## Examples
+
+### Response Suffix
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+// Should be `TranslateTextResponse`.
+rpc TranslateText(TranslateTextRequest) returns (Text) {
+  option (google.api.http) = {
+    post: "/v1/{project=projects/*}:translateText"
+    body: "*"
+  };
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+rpc TranslateText(TranslateTextRequest) returns (TranslateTextResponse) {
+  option (google.api.http) = {
+    post: "/v1/{project=projects/*}:translateText"
+    body: "*"
+  };
+}
+```
+
+### Resource
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+// Should be `Book`.
+rpc ArchiveBook(ArchiveBookRequest) returns (Author) {
+  option (google.api.http) = {
+    post: "/v1/{name=publishers/*/books/*}:archive"
+    body: "*"
+  };
+}
+
+message ArchiveBookRequest {
+  // The book to archive.
+  // Format: publishers/{publisher}/books/{book}
+  string name = 1 [(google.api.resource_reference).type = "library.googleapis.com/Book"];
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+rpc ArchiveBook(ArchiveBookRequest) returns (Book) {
+  option (google.api.http) = {
+    post: "/v1/{name=publishers/*/books/*}:archive"
+    body: "*"
+  };
+}
+
+message ArchiveBookRequest {
+  // The book to archive.
+  // Format: publishers/{publisher}/books/{book}
+  string name = 1 [(google.api.resource_reference).type = "library.googleapis.com/Book"];
+}
+```
+
+### Long Running Operation
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+// Should be `Book` from Operation.
+rpc ArchiveBook(ArchiveBookRequest) returns (google.longrunning.Operation) {
+  option (google.api.http) = {
+    post: "/v1/{name=publishers/*/books/*}:archive"
+    body: "*"
+  };
+  option (google.longrunning.operation_info) = {
+    response_type: "Author"
+  }
+}
+
+message ArchiveBookRequest {
+  // The book to archive.
+  // Format: publishers/{publisher}/books/{book}
+  string name = 1 [(google.api.resource_reference).type = "library.googleapis.com/Book"];
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+rpc ArchiveBook(ArchiveBookRequest) returns (google.longrunning.Operation) {
+  option (google.api.http) = {
+    post: "/v1/{name=publishers/*/books/*}:archive"
+    body: "*"
+  };
+  option (google.longrunning.operation_info) = {
+    response_type: "Book"
+  }
+}
+
+message ArchiveBookRequest {
+  // The book to archive.
+  // Format: publishers/{publisher}/books/{book}
+  string name = 1 [(google.api.resource_reference).type = "library.googleapis.com/Book"];
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the method.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0136::response-message-name=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+rpc ArchiveBook(ArchiveBookRequest) returns (ArchiveBookResp) {
+  option (google.api.http) = {
+    post: "/v1/{name=publishers/*/books/*}:archive"
+    body: "*"
+  };
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-136]: https://aip.dev/136
+[aip.dev/not-precedent]: https://aip.dev/not-precedent

rules/aip0136/aip0136.go

@@ -32,6 +32,7 @@ func AddRules(r lint.RuleRegistry) error {
 		httpMethod,
 		noPrepositions,
 		requestMessageName,
+		responseMessageName,
 		standardMethodsOnly,
 		uriSuffix,
 		verbNoun,

rules/aip0136/response_message_name.go

@@ -0,0 +1,65 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0136
+
+import (
+	"fmt"
+
+	"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"
+)
+
+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{
+	Name:   lint.NewRuleName(136, "response-message-name"),
+	OnlyIf: isCustomMethod,
+	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
+		// A response is considered valid if
+		// - The response name matches the RPC name with a `Response` suffix
+		// - The response is the resource being operated on
+		// To identify the resource being operated on, we inspect the Resource
+		// Reference of the input type's `name` field. This guidance is documented
+		// in https://google.aip.dev/136#resource-based-custom-methods
+
+		// Short-circuit: Output type has `Response` suffix
+		suffixFindings := utils.LintMethodHasMatchingResponseName(m)
+		if len(suffixFindings) == 0 {
+			return nil
+		}
+
+		response := utils.GetResponseType(m)
+		requestResourceType := utils.GetResourceReference(m.GetInputType().FindFieldByName("name")).GetType()
+		responseResourceType := utils.GetResource(response).GetType()
+
+		// Short-circuit: Output type is the resource being operated on
+		if utils.IsResource(response) && responseResourceType == requestResourceType {
+			return nil
+		}
+
+		return []lint.Problem{{
+			Message:    fmt.Sprintf(responseMessageNameErrorMessage, response.GetName()),
+			Descriptor: m,
+			Location:   locations.MethodResponseType(m),
+		}}
+
+	},
+}

rules/aip0136/response_message_name_test.go

@@ -0,0 +1,123 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0136
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestResponseMessageName(t *testing.T) {
+	t.Run("Response Suffix", func(t *testing.T) {
+		// Set up the testing permutations.
+		tests := []struct {
+			testName        string
+			MethodName      string
+			RespMessageName string
+			problems        testutils.Problems
+		}{
+			{"Valid", "ArchiveBook", "ArchiveBookResponse", testutils.Problems{}},
+			{"Invalid", "ArchiveBook", "ArchiveBookResp", testutils.Problems{{Message: "not \"ArchiveBookResp\"."}}},
+		}
+
+		for _, test := range tests {
+			t.Run(test.testName, func(t *testing.T) {
+				file := testutils.ParseProto3Tmpl(t, `
+				package test;
+				import "google/api/resource.proto";
+				service Library {
+					rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.RespMessageName}});
+				}
+				message {{.MethodName}}Request {}
+				message {{.RespMessageName}} {}
+				`, test)
+				method := file.GetServices()[0].GetMethods()[0]
+				problems := responseMessageName.Lint(file)
+				if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
+					t.Error(diff)
+				}
+			})
+		}
+	})
+
+	t.Run("Resource", func(t *testing.T) {
+		// Set up the testing permutations.
+		tests := []struct {
+			testName        string
+			MethodName      string
+			RespMessageName string
+			LRO             bool
+			problems        testutils.Problems
+		}{
+			{"Valid", "ArchiveBook", "Book", false, testutils.Problems{}},
+			{"Valid LRO", "ArchiveBook", "Book", true, testutils.Problems{}},
+			{"Invalid", "ArchiveBook", "Author", false, testutils.Problems{{Message: "not \"Author\"."}}},
+			{"Invalid LRO", "ArchiveBook", "Author", true, testutils.Problems{{Message: "not \"Author\"."}}},
+		}
+
+		for _, test := range tests {
+			t.Run(test.testName, func(t *testing.T) {
+				file := testutils.ParseProto3Tmpl(t, `
+				package test;
+
+				import "google/api/annotations.proto";
+				import "google/api/resource.proto";
+				import "google/longrunning/operations.proto";
+
+				service Library {
+					rpc {{.MethodName}}({{.MethodName}}Request) returns ({{ if .LRO }}google.longrunning.Operation{{ else }}{{.RespMessageName}}{{ end }}) {
+						option (google.api.http) = {
+							post: "/v1/{name=publishers/*/books/*}:foo"
+							body: "*"
+						};
+						{{ if .LRO }}
+						option (google.longrunning.operation_info) = {
+							response_type: "{{ .RespMessageName }}"
+							metadata_type: "{{ .RespMessageName }}Metadata"
+						};
+						{{ end }}
+					};
+				}
+
+				message Book {
+					option (google.api.resource) = {
+						type: "library.googleapis.com/Book"
+						pattern: "publishers/{publisher}/books/{book}"
+					};
+				}
+
+				message Author {
+					option (google.api.resource) = {
+						type: "library.googleapis.com/Author"
+						pattern: "authors/{author}"
+					};
+				}
+
+				message {{.MethodName}}Request {
+					// The book to operate on.
+					// Format: publishers/{publisher}/books/{book}
+					string name = 1 [(google.api.resource_reference).type = "library.googleapis.com/Book"];
+				}
+				`, test)
+				method := file.GetServices()[0].GetMethods()[0]
+				problems := responseMessageName.Lint(file)
+				if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" {
+					t.Error(diff)
+				}
+			})
+		}
+	})
+}