fix(AIP-192): enforce deprecated comment on all non-file descriptors (#…

…1233)

docs/rules/0192/deprecated-comment.md

@@ -2,22 +2,22 @@
 rule:
   aip: 192
   name: [core, '0192', deprecated-comment]
-  summary: Deprecated methods must have a corresponding comment.
+  summary: Deprecated elements 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][].
+This rule enforces that every element 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
+This rule looks at each 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

rules/aip0192/aip0192.go

@@ -35,13 +35,21 @@ func AddRules(r lint.RuleRegistry) error {
 	)
 }
 
-// Returns true if this is a deprecated method or service, false otherwise.
+// Returns true if this is a deprecated descriptor, 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()
+	case *desc.FieldDescriptor:
+		return d.GetFieldOptions().GetDeprecated()
+	case *desc.EnumDescriptor:
+		return d.GetEnumOptions().GetDeprecated()
+	case *desc.EnumValueDescriptor:
+		return d.GetEnumValueOptions().GetDeprecated()
+	case *desc.MessageDescriptor:
+		return d.GetMessageOptions().GetDeprecated()
 	default:
 		return false
 	}

rules/aip0192/deprecated_test.go

@@ -104,3 +104,115 @@ func TestDeprecatedService(t *testing.T) {
 		})
 	}
 }
+
+func TestDeprecatedField(t *testing.T) {
+	tests := []struct {
+		testName     string
+		FieldComment string
+		problems     testutils.Problems
+	}{
+		{"ValidFieldDeprecated", "// Deprecated: Don't use this.\n// Field comment.", nil},
+		{"InvalidFieldDeprecated", "// Field comment.", testutils.Problems{{Message: `Use "Deprecated: <reason>"`}}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+        message GetBookRequest {
+			{{.FieldComment}}
+			string name = 1 [deprecated = true];
+		}
+      `, test)
+
+			problems := deprecatedComment.Lint(file)
+			if diff := test.problems.SetDescriptor(file.GetMessageTypes()[0].GetFields()[0]).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}
+
+func TestDeprecatedEnum(t *testing.T) {
+	tests := []struct {
+		testName    string
+		EnumComment string
+		problems    testutils.Problems
+	}{
+		{"ValidEnumDeprecated", "// Deprecated: Don't use this.\n// Enum comment.", nil},
+		{"InvalidEnumDeprecated", "// Enum comment.", testutils.Problems{{Message: `Use "Deprecated: <reason>"`}}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+		{{.EnumComment}}
+		enum State {
+			option deprecated = true;
+			
+			STATE_UNSPECIFIED = 0;
+		}
+      `, test)
+
+			problems := deprecatedComment.Lint(file)
+			if diff := test.problems.SetDescriptor(file.GetEnumTypes()[0]).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}
+
+func TestDeprecatedEnumValue(t *testing.T) {
+	tests := []struct {
+		testName         string
+		EnumValueComment string
+		problems         testutils.Problems
+	}{
+		{"ValidEnumValueDeprecated", "// Deprecated: Don't use this.\n// EnumValue comment.", nil},
+		{"InvalidEnumValueDeprecated", "// EnumValue comment.", testutils.Problems{{Message: `Use "Deprecated: <reason>"`}}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+		enum State {
+			{{.EnumValueComment}}
+			STATE_UNSPECIFIED = 0 [deprecated = true];
+		}
+      `, test)
+
+			problems := deprecatedComment.Lint(file)
+			if diff := test.problems.SetDescriptor(file.GetEnumTypes()[0].GetValues()[0]).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}
+
+func TestDeprecatedMessage(t *testing.T) {
+	tests := []struct {
+		testName       string
+		MessageComment string
+		problems       testutils.Problems
+	}{
+		{"ValidMessageDeprecated", "// Deprecated: Don't use this.\n// Message comment.", nil},
+		{"InvalidMessageDeprecated", "// Message comment.", testutils.Problems{{Message: `Use "Deprecated: <reason>"`}}},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+		{{.MessageComment}}
+        message GetBookRequest {
+			option deprecated = true;
+
+			string name = 1;
+		}
+      `, test)
+
+			problems := deprecatedComment.Lint(file)
+			if diff := test.problems.SetDescriptor(file.GetMessageTypes()[0]).Diff(problems); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}