Skip to content

Commit

Permalink
fix(AIP-123): allow name in nested messages (#1325)
Browse files Browse the repository at this point in the history
  • Loading branch information
noahdietz committed Jan 25, 2024
1 parent 7adfaba commit 16316a5
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 7 deletions.
12 changes: 6 additions & 6 deletions docs/rules/0123/resource-annotation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion rules/aip0123/aip0123.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions rules/aip0123/resource_annotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 16316a5

Please sign in to comment.