fix(AIP-123): allow name in nested messages (#1325)

Author: noahdietz

docs/rules/0123/resource-annotation.md

@@ -10,15 +10,15 @@ redirect_from:
 
 # Resource annotation presence
 
-This rule enforces that messages that appear to represent resources have a
-`google.api.resource` annotation, as described in [AIP-123][].
+This rule enforces that top-level messages that appear to represent resources
+have a `google.api.resource` annotation, as described in [AIP-123][].
 
 ## Details
 
-This rule scans all messages, and assumes that messages with a `string name`
-field are resources unless the message name ends with `Request`. For messages
-that are resources, it complains if the `google.api.resource` annotation is
-missing.
+This rule scans all top-level messages, and assumes that messages with a
+ `string name` field are resources unless the message name ends with `Request`.
+For messages that are resources, it complains if the `google.api.resource`
+annotation is missing.
 
 ## Examples
 

rules/aip0123/aip0123.go

@@ -49,8 +49,11 @@ func AddRules(r lint.RuleRegistry) error {
 }
 
 func isResourceMessage(m *desc.MessageDescriptor) bool {
+	// If the parent of this message is a message, it is nested and shoudn't
+	// be considered a resource, even if it has a name field.
+	_, nested := m.GetParent().(*desc.MessageDescriptor)
 	return m.FindFieldByName("name") != nil && !strings.HasSuffix(m.GetName(), "Request") &&
-		!strings.HasSuffix(m.GetName(), "Response")
+		!strings.HasSuffix(m.GetName(), "Response") && !nested
 }
 
 func hasResourceAnnotation(m *desc.MessageDescriptor) bool {

rules/aip0123/resource_annotation_test.go

@@ -38,6 +38,20 @@ func TestResourceAnnotation(t *testing.T) {
 		}
 	})
 
+	t.Run("SkipNested", func(t *testing.T) {
+		f := testutils.ParseProto3String(t, `
+			message Foo {
+				message Bar {
+					string name = 1;
+				}
+				Bar bar = 1;
+			}
+		`)
+		if diff := (testutils.Problems{}).Diff(resourceAnnotation.Lint(f)); diff != "" {
+			t.Errorf(diff)
+		}
+	})
+
 	// The rule should fail if the option is absent on a resource message,
 	// but pass on messages that are not resource messages.
 	for _, test := range []struct {