From 16316a5405bd967e926a1482f3bd1e85e1c45eed Mon Sep 17 00:00:00 2001
From: Noah Dietz <noahdietz@users.noreply.github.com>
Date: Thu, 25 Jan 2024 08:19:05 -0800
Subject: [PATCH] fix(AIP-123): allow name in nested messages (#1325)

---
 docs/rules/0123/resource-annotation.md    | 12 ++++++------
 rules/aip0123/aip0123.go                  |  5 ++++-
 rules/aip0123/resource_annotation_test.go | 14 ++++++++++++++
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/docs/rules/0123/resource-annotation.md b/docs/rules/0123/resource-annotation.md
index 1096c7262..9abe5223c 100644
--- a/docs/rules/0123/resource-annotation.md
+++ b/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
 
diff --git a/rules/aip0123/aip0123.go b/rules/aip0123/aip0123.go
index bd1ef6f86..f586e90e7 100644
--- a/rules/aip0123/aip0123.go
+++ b/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 {
diff --git a/rules/aip0123/resource_annotation_test.go b/rules/aip0123/resource_annotation_test.go
index b89dc2684..09b9a420a 100644
--- a/rules/aip0123/resource_annotation_test.go
+++ b/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 {