Repository: googleapis/api-linter
Pull request #836 (2021-04-09 03:00:20)

feat: Add linter for AIP-192 deprecated service and methods
Corresponds to [this AIP PR](https://github.com/aip-dev/google.aip.dev/pull/736).

Note to future me: This was my first Golang PR.

lukesneeringer: I am good with this once my question in https://github.com/aip-dev/google.aip.dev/pull/736 is decided. (If we decide to do "last paragraph", I expect it will require code updates here.)
lukesneeringer: aip-dev/google.aip.dev#736 is decided in favor of this implementation. This can go in as soon as that PR does.

Patch:
diff --git a/docs/rules/0192/deprecated-comment.md b/docs/rules/0192/deprecated-comment.md
new file mode 100644
index 0000000..87bd38e
--- /dev/null
+++ b/docs/rules/0192/deprecated-comment.md
@@ -0,0 +1,71 @@
+---
+rule:
+  aip: 192
+  name: [core, '0192', deprecated-comment]
+  summary: Deprecated methods must have a corresponding comment.
+permalink: /192/deprecated-comment
+redirect_from:
+  - /0192/deprecated-comment
+---
+
+# Deprecated comments
+
+This rule enforces that every RPC marked with the protobuf `deprecated` option
+has `"Deprecated: <reason>"` as the first line in the public leading comment, as
+mandated in [AIP-192][].
+
+## Details
+
+This rule looks at each method descriptor in each proto file, and complains if
+the protobuf `deprecated` option is set to `true`, but the first line of the public
+comment does not begin with "Deprecated: ".
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// A library service.
+service Library {
+  // Incorrect.
+  // Retrieves a book.
+  rpc GetBook(GetBookRequest) returns (Book) {
+    option deprecated = true;
+  }
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// A library service.
+service Library {
+  // Deprecated: Please borrow a physical book instead.
+  // Correct.
+  // Retrieves a book.
+  rpc GetBook(GetBookRequest) returns (Book) {
+    option deprecated = true;
+  }
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the descriptor.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// A library service.
+service Library {
+  // (-- api-linter: core::0192::deprecated-comment=disabled
+  //     aip.dev/not-precedent: We need to do this because reasons. --)
+  // Incorrect.
+  // Retrieves a book.
+  rpc GetBook(GetBookRequest) returns (Book) {
+    option deprecated = true;
+  }
+}
+```
+
+[aip-192]: https://aip.dev/192
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0192/aip0192.go b/rules/aip0192/aip0192.go
index 82c4c93..d58f888 100644
--- a/rules/aip0192/aip0192.go
+++ b/rules/aip0192/aip0192.go
@@ -19,6 +19,7 @@ import (
 	"strings"
 
 	"github.com/googleapis/api-linter/lint"
+	"github.com/jhump/protoreflect/desc"
 )
 
 // AddRules adds all of the AIP-192 rules to the provided registry.
@@ -26,6 +27,7 @@ func AddRules(r lint.RuleRegistry) error {
 	return r.Register(
 		192,
 		absoluteLinks,
+		deprecatedComment,
 		hasComments,
 		noHTML,
 		noMarkdownHeadings,
@@ -35,6 +37,18 @@ func AddRules(r lint.RuleRegistry) error {
 	)
 }
 
+// Returns true if this is a deprecated method or service, false otherwise.
+func isDeprecated(d desc.Descriptor) bool {
+	switch d := d.(type) {
+	case *desc.MethodDescriptor:
+		return d.GetMethodOptions().GetDeprecated()
+	case *desc.ServiceDescriptor:
+		return d.GetServiceOptions().GetDeprecated()
+	default:
+		return false
+	}
+}
+
 func separateInternalComments(comments ...string) struct {
 	Internal []string
 	External []string
diff --git a/rules/aip0192/deprecated_comment.go b/rules/aip0192/deprecated_comment.go
new file mode 100644
index 0000000..4e1de3f
--- /dev/null
+++ b/rules/aip0192/deprecated_comment.go
@@ -0,0 +1,37 @@
+// Copyright 2021 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 aip0192
+
+import (
+	"strings"
+
+	"github.com/googleapis/api-linter/lint"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var deprecatedComment = &lint.DescriptorRule{
+	Name:   lint.NewRuleName(192, "deprecated-comment"),
+	OnlyIf: isDeprecated,
+	LintDescriptor: func(d desc.Descriptor) []lint.Problem {
+		comment := separateInternalComments(d.GetSourceInfo().GetLeadingComments())
+		if len(comment.External) > 0 && strings.HasPrefix(comment.External[0], "Deprecated:") {
+			return nil
+		}
+		return []lint.Problem{{
+			Message:    `Use "Deprecated: <reason>" as the first line in the comment.`,
+			Descriptor: d,
+		}}
+	},
+}


Test patch:
diff --git a/rules/aip0192/deprecated_test.go b/rules/aip0192/deprecated_test.go
new file mode 100644
index 0000000..a69c178
--- /dev/null
+++ b/rules/aip0192/deprecated_test.go
@@ -0,0 +1,106 @@
+// Copyright 2021 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 aip0192
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+// These are split up since templating doesn't play nicely with inserting protobuf options.
+func TestValidDescriptor(t *testing.T) {
+	file := testutils.ParseProto3String(t, `
+    // A library service.
+    service Library {
+      // Retrieves a book.
+      rpc GetBook(GetBookRequest) returns (Book);
+    }
+    message GetBookRequest {}
+    message Book {}
+  `)
+
+	serviceProblems := testutils.Problems{}
+	if diff := serviceProblems.Diff(deprecatedComment.Lint(file)); diff != "" {
+		t.Error(diff)
+	}
+
+	methodProblems := testutils.Problems{}
+	if diff := methodProblems.Diff(deprecatedComment.Lint(file)); diff != "" {
+		t.Error(diff)
+	}
+}
+
+func TestDeprecatedMethod(t *testing.T) {
+	tests := []struct {
+		testName      string
+		MethodComment string
+		problems      testutils.Problems
+	}{
+		{"ValidMethodDeprecated", "// Deprecated: Don't use this.\n// Method comment.", nil},
+		{"InvalidMethodDeprecated", "// Method comment.", testutils.Problems{{Message: `Use "Deprecated: <reason>"`}}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+        service Library {
+
+          {{.MethodComment}}
+          rpc GetBook(GetBookRequest) returns (Book) {
+            option deprecated = true;
+          }
+        }
+        message GetBookRequest {}
+        message Book {}
+      `, test)
+
+			problems := deprecatedComment.Lint(file)
+			if diff := test.problems.SetDescriptor(file.GetServices()[0].GetMethods()[0]).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}
+
+func TestDeprecatedService(t *testing.T) {
+	tests := []struct {
+		testName       string
+		ServiceComment string
+		problems       testutils.Problems
+	}{
+		{"ValidServiceDeprecated", "// Deprecated: Don't use this.\n// Service comment.", nil},
+		{"InvalidServiceDeprecated", "// Service comment.", testutils.Problems{{Message: `Use "Deprecated: <reason>"`}}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+        {{.ServiceComment}}
+        service Library {
+          option deprecated = true;
+          rpc GetBook(GetBookRequest) returns (Book);
+        }
+        message GetBookRequest {}
+        message Book {}
+      `, test)
+
+			problems := deprecatedComment.Lint(file)
+			if diff := test.problems.SetDescriptor(file.GetServices()[0]).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}