Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 11 additions & 12 deletions docs/rules/0132/resource-reference-type.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
rule:
aep: 132
name: [core, '0132', resource-reference-type]
summary: List should use a `child_type` reference to the paginated resource.
summary: List should use `resource_reference_child_type` to reference the paginated resource.
permalink: /132/resource-reference-type
redirect_from:
- /0132/resource-reference-type
Expand All @@ -11,15 +11,15 @@ redirect_from:
# List methods: Parent field resource reference

This rule enforces that all `List` standard methods with a `string parent`
field use a proper `(aep.api.field_info).resource_reference`, that being either a
`child_type` referring to the pagianted resource or a `type` referring directly
to the parent resource, as mandated in [AEP-132][].
field use a proper `(aep.api.field_info).resource_reference_child_type` to refer to the
paginated resource, as mandated in [AEP-132][].

## Details

This rule looks at any message matching `List*Request` and complains if the
`(aep.api.field_info).resource_reference` on the `parent` field refers to the wrong
resource.
This rule looks at any message matching `List*Request` and complains if the
`(aep.api.field_info).resource_reference_child_type` or `(aep.api.field_info).resource_reference`
on the `parent` field refers to the wrong resource. The preferred approach is to use
`resource_reference_child_type` to reference the child resource being paginated.

## Examples

Expand All @@ -28,9 +28,8 @@ resource.
```proto
// Incorrect.
message ListBooksRequest {
// `child_type` should be used instead of `type` when referring to the
// paginated resource on a parent field.
string parent = 1 [(aep.api.field_info).resource_reference.type = "library.googleapis.com/Book"];
// Should reference the correct child resource type.
string parent = 1 [(aep.api.field_info).resource_reference_child_type = "library.googleapis.com/Shelf"];
int32 page_size = 2;
string page_token = 3;
}
Expand All @@ -41,7 +40,7 @@ message ListBooksRequest {
```proto
// Correct.
message ListBooksRequest {
string parent = 1 [(aep.api.field_info).resource_reference.child_type = "library.googleapis.com/Book"];
string parent = 1 [(aep.api.field_info).resource_reference_child_type = "library.googleapis.com/Book"];
int32 page_size = 2;
string page_token = 3;
}
Expand All @@ -56,7 +55,7 @@ Remember to also include an [aep.dev/not-precedent][] comment explaining why.
message ListBooksRequest {
// (-- api-linter: core::0132::resource-reference-type=disabled
// aep.dev/not-precedent: We need to do this because reasons. --)
string parent = 1 [(aep.api.field_info).resource_reference.type = "library.googleapis.com/Book"];
string parent = 1 [(aep.api.field_info).resource_reference_child_type = "library.googleapis.com/Shelf"];
int32 page_size = 2;
string page_token = 3;
}
Expand Down
27 changes: 15 additions & 12 deletions docs/rules/0133/resource-reference-type.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
rule:
aep: 133
name: [core, '0133', resource-reference-type]
summary: Create should use a `child_type` reference to the created resource.
summary:
Create should use `resource_reference_child_type` to reference the created
resource.
permalink: /133/resource-reference-type
redirect_from:
- /0133/resource-reference-type
Expand All @@ -11,15 +13,17 @@ redirect_from:
# Create methods: Parent field resource reference

This rule enforces that all `Create` standard methods with a `string parent`
field use a proper `(aep.api.field_info).resource_reference`, that being either a
`child_type` referring to the created resource or a `type` referring directly
to the parent resource, as mandated in [AEP-133][].
field use a proper `(aep.api.field_info).resource_reference_child_type` or
`(aep.api.field_info.).resource_reference` to refer to the created resource, as
mandated in [AEP-133][].

## Details

This rule looks at any message matching `Create*Request` and complains if the
`(aep.api.field_info).resource_reference` on the `parent` field refers to the wrong
resource.
This rule looks at any message matching `Create*Request` and complains if the
`(aep.api.field_info).resource_reference_child_type` or
`(aep.api.field_info).resource_reference` on the `parent` field refers to the
wrong resource. The preferred approach is to use
`resource_reference_child_type` to reference the child resource being created.

## Examples

Expand All @@ -28,9 +32,8 @@ resource.
```proto
// Incorrect.
message CreateBooksRequest {
// `child_type` should be used instead of `type` when referring to the
// created resource on a parent field.
string parent = 1 [(aep.api.field_info).resource_reference.type = "library.googleapis.com/Book"];
// Should reference the correct child resource type.
string parent = 1 [(aep.api.field_info).resource_reference_child_type = "library.googleapis.com/Shelf"];
Book book = 2;
}
```
Expand All @@ -40,7 +43,7 @@ message CreateBooksRequest {
```proto
// Correct.
message CreateBooksRequest {
string parent = 1 [(aep.api.field_info).resource_reference.child_type = "library.googleapis.com/Book"];
string parent = 1 [(aep.api.field_info).resource_reference_child_type = "library.googleapis.com/Book"];
Book book = 2;
}
```
Expand All @@ -54,7 +57,7 @@ Remember to also include an [aep.dev/not-precedent][] comment explaining why.
message CreateBooksRequest {
// (-- api-linter: core::0133::resource-reference-type=disabled
// aep.dev/not-precedent: We need to do this because reasons. --)
string parent = 1 [(aep.api.field_info).resource_reference.type = "library.googleapis.com/Book"];
string parent = 1 [(aep.api.field_info).resource_reference_child_type = "library.googleapis.com/Shelf"];
Book book = 2;
}
```
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.24.0

require (
bitbucket.org/creachadair/stringset v0.0.14
buf.build/gen/go/aep/api/protocolbuffers/go v1.36.10-20251016045117-f9844266f27f.1
buf.build/gen/go/aep/api/protocolbuffers/go v1.36.10-20251102152130-5f3e69139afa.1
buf.build/go/bufplugin v0.9.0
cloud.google.com/go/longrunning v0.7.0
github.com/bmatcuk/doublestar/v4 v4.9.1
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ bitbucket.org/creachadair/stringset v0.0.14 h1:t1ejQyf8utS4GZV/4fM+1gvYucggZkfhb
bitbucket.org/creachadair/stringset v0.0.14/go.mod h1:Ej8fsr6rQvmeMDf6CCWMWGb14H9mz8kmDgPPTdiVT0w=
buf.build/gen/go/aep/api/protocolbuffers/go v1.36.10-20251016045117-f9844266f27f.1 h1:jK64CGldQTGX2ESi+v41uERp+z28OFqGyAn/T/Qs8Q0=
buf.build/gen/go/aep/api/protocolbuffers/go v1.36.10-20251016045117-f9844266f27f.1/go.mod h1:JOZpZ+zS3G2OKO02hKlEazAGR5X9uVpFyeCamXBXg5c=
buf.build/gen/go/aep/api/protocolbuffers/go v1.36.10-20251102152130-5f3e69139afa.1 h1:SEKgDODwWdsVL9nnOPkFBlZ9wAuO7kRQNobSZb5JR7I=
buf.build/gen/go/aep/api/protocolbuffers/go v1.36.10-20251102152130-5f3e69139afa.1/go.mod h1:JOZpZ+zS3G2OKO02hKlEazAGR5X9uVpFyeCamXBXg5c=
buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.10-20250718181942-e35f9b667443.1 h1:FzJGrb8r7vir+P3zJ5Ebey8p54LYTYtQsrM/U35YO9Q=
buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go v1.36.10-20250718181942-e35f9b667443.1/go.mod h1:E6HwqUm4Ag7bXtg/tX7jHWO7CgpknbmeACgDax0icV0=
buf.build/gen/go/bufbuild/protovalidate/protocolbuffers/go v1.36.10-20250912141014-52f32327d4b0.1 h1:31on4W/yPcV4nZHL4+UCiCvLPsMqe/vJcNg8Rci0scc=
Expand Down
13 changes: 9 additions & 4 deletions rules/aep0121/no_mutable_cycles.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,23 @@ func findCycles(start string, node *desc.MessageDescriptor, seen stringset.Set,
}
ref := utils.GetResourceReference(f)
// Skip indirect references for now.
if ref.GetChildType() != "" {
if len(ref.GetChildType()) > 0 {
continue
}
if ref.GetType() == start {
types := ref.GetType()
if len(types) == 0 {
continue
}
refType := types[0]
if refType == start {
cycle := strings.Join(append(chain, start), " > ")
problems = append(problems, lint.Problem{
Message: "mutable resource reference introduces a reference cycle:\n" + cycle,
Descriptor: f,
Location: locations.FieldResourceReference(f),
})
} else if !seen.Contains(ref.GetType()) {
next := utils.FindResourceMessage(ref.GetType(), node.GetFile())
} else if !seen.Contains(refType) {
next := utils.FindResourceMessage(refType, node.GetFile())
// Skip unresolvable references.
if next == nil {
continue
Expand Down
9 changes: 7 additions & 2 deletions rules/aep0127/http_template_pattern.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,21 @@ func httpResourceReferences(httpRule *utils.HTTPRule, msg *desc.MessageDescripto

// Extract the name of the resource referenced by this field.
ref := utils.GetResourceReference(field)
if ref == nil || ref.GetChildType() != "" {
if ref == nil || len(ref.GetChildType()) > 0 {
// TODO(#1047): Support the case where a resource has
// multiple parent resources.
continue
}

types := ref.GetType()
if len(types) == 0 {
continue
}

resourceRefs = append(resourceRefs, resourceReference{
fieldPath: fieldPath,
pathTemplate: template,
resourceRefName: ref.GetType(),
resourceRefName: types[0],
})
}
return resourceRefs
Expand Down
2 changes: 1 addition & 1 deletion rules/aep0131/request_path_reference_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var requestPathReferenceType = &lint.FieldRule{
return utils.IsGetRequestMessage(f.GetOwner()) && f.GetName() == "path" && utils.GetResourceReference(f) != nil
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
if ref := utils.GetResourceReference(f); ref.GetType() == "" {
if ref := utils.GetResourceReference(f); len(ref.GetType()) == 0 || ref.GetType()[0] == "" {
return []lint.Problem{{
Message: fmt.Sprintf("The `%s` field `(aep.api.field_info).resource_reference` annotation should be a direct `type` reference.", f.GetName()),
Descriptor: f,
Expand Down
6 changes: 4 additions & 2 deletions rules/aep0132/request_parent_valid_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ var requestParentValidReference = &lint.FieldRule{
RuleType: lint.NewRuleType(lint.MustRule),
OnlyIf: func(f *desc.FieldDescriptor) bool {
ref := utils.GetResourceReference(f)
return utils.IsListRequestMessage(f.GetOwner()) && f.GetName() == "parent" && ref != nil && ref.GetType() != ""
types := ref.GetType()
return utils.IsListRequestMessage(f.GetOwner()) && f.GetName() == "parent" && ref != nil && len(types) > 0
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
p := f.GetParent()
msg, _ := p.(*desc.MessageDescriptor)
res := utils.GetResourceReference(f).GetType()
types := utils.GetResourceReference(f).GetType()
res := types[0]

response := utils.FindMessage(f.GetFile(), strings.Replace(msg.GetName(), "Request", "Response", 1))
if response == nil {
Expand Down
28 changes: 14 additions & 14 deletions rules/aep0132/resource_reference_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,26 +45,26 @@ var resourceReferenceType = &lint.MethodRule{
parent := m.GetInputType().FindFieldByName("parent")
ref := utils.GetResourceReference(parent)

// In AEP format, resource_reference is just a string. When used in List methods,
// it should match the child resource type. The old Google API format distinguishes
// between `type` and `child_type`, but AEP format just uses the string value.
// If child_type is set (Google API format), check it. Otherwise, check the type field
// and treat it as an implicit child_type reference.
if ref.GetChildType() != "" {
// Google API format with explicit child_type
if resource.GetType() != ref.GetChildType() {
// Check resource reference matches the child resource type.
// In AEP format, use resource_reference_child_type to reference the child resource.
// For backwards compatibility, resource_reference (type) is also supported.
childTypes := ref.GetChildType()
types := ref.GetType()

if len(childTypes) > 0 {
// AEP format with resource_reference_child_type
if resource.GetType() != childTypes[0] {
return []lint.Problem{{
Message: "List should use a `child_type` reference to the paginated resource.",
Message: "List should use `resource_reference_child_type` to reference the paginated resource.",
Descriptor: parent,
Location: locations.FieldResourceReference(parent),
}}
}
} else if ref.GetType() != "" {
// AEP format or Google API format with only type set
// In AEP format, this should match the child resource type
if resource.GetType() != ref.GetType() {
} else if len(types) > 0 {
// AEP format with resource_reference only (backwards compatibility)
if resource.GetType() != types[0] {
return []lint.Problem{{
Message: "List should use a `child_type` reference to the paginated resource.",
Message: "List should use `resource_reference_child_type` to reference the paginated resource.",
Descriptor: parent,
Location: locations.FieldResourceReference(parent),
}}
Expand Down
14 changes: 8 additions & 6 deletions rules/aep0132/resource_reference_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,16 @@ option (google.api.resource) = {

// Set up testing permutations.
tests := []struct {
testName string
TypeName string
testName string
Annotation string
ResourceAnnotation string
problems testutils.Problems
}{
{"ValidMatch", "library.googleapis.com/Book", bookResource, nil},
{"InvalidMismatch", "library.googleapis.com/Shelf", bookResource, testutils.Problems{{Message: "`child_type`"}}},
{"SkipNoResource", "library.googleapis.com/Book", "", nil},
{"ValidMatch_resource_reference", `resource_reference = "library.googleapis.com/Book"`, bookResource, nil},
{"InvalidMismatch_resource_reference", `resource_reference = "library.googleapis.com/Shelf"`, bookResource, testutils.Problems{{Message: "`resource_reference_child_type`"}}},
{"ValidMatch_resource_reference_child_type", `resource_reference_child_type = "library.googleapis.com/Book"`, bookResource, nil},
{"InvalidMismatch_resource_reference_child_type", `resource_reference_child_type = "library.googleapis.com/Shelf"`, bookResource, testutils.Problems{{Message: "`resource_reference_child_type`"}}},
{"SkipNoResource", `resource_reference = "library.googleapis.com/Book"`, "", nil},
}

// Run each test.
Expand All @@ -50,7 +52,7 @@ option (google.api.resource) = {
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {}
}
message ListBooksRequest {
string parent = 1 [(aep.api.field_info).resource_reference = "{{ .TypeName }}"];
string parent = 1 [(aep.api.field_info).{{ .Annotation }}];
}
message ListBooksResponse {
repeated string unreachable = 2;
Expand Down
28 changes: 14 additions & 14 deletions rules/aep0133/resource_reference_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,26 @@ var resourceReferenceType = &lint.MethodRule{
parent := m.GetInputType().FindFieldByName("parent")
ref := utils.GetResourceReference(parent)

// In AEP format, resource_reference is just a string. When used in Create methods,
// it should match the created resource type. The old Google API format distinguishes
// between `type` and `child_type`, but AEP format just uses the string value.
// If child_type is set (Google API format), check it. Otherwise, check the type field
// and treat it as an implicit child_type reference.
if ref.GetChildType() != "" {
// Google API format with explicit child_type
if resource.GetType() != ref.GetChildType() {
// Check resource reference matches the created resource type.
// In AEP format, use resource_reference_child_type to reference the created resource.
// For backwards compatibility, resource_reference (type) is also supported.
childTypes := ref.GetChildType()
types := ref.GetType()

if len(childTypes) > 0 {
// AEP format with resource_reference_child_type
if resource.GetType() != childTypes[0] {
return []lint.Problem{{
Message: "Create should use a `child_type` reference to the created resource.",
Message: "Create should use `resource_reference_child_type` to reference the created resource.",
Descriptor: parent,
Location: locations.FieldResourceReference(parent),
}}
}
} else if ref.GetType() != "" {
// AEP format or Google API format with only type set
// In AEP format, this should match the created resource type
if resource.GetType() != ref.GetType() {
} else if len(types) > 0 {
// AEP format with resource_reference only (backwards compatibility)
if resource.GetType() != types[0] {
return []lint.Problem{{
Message: "Create should use a `child_type` reference to the created resource.",
Message: "Create should use `resource_reference_child_type` to reference the created resource.",
Descriptor: parent,
Location: locations.FieldResourceReference(parent),
}}
Expand Down
26 changes: 15 additions & 11 deletions rules/aep0133/resource_reference_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ import (
func TestResourceReferenceType(t *testing.T) {
// Set up testing permutations.
tests := []struct {
testName string
TypeName string
problems testutils.Problems
testName string
Annotation string
problems testutils.Problems
}{
{"ValidMatch", "library.googleapis.com/Book", nil},
{"InvalidMismatch", "library.googleapis.com/Shelf", testutils.Problems{{Message: "`child_type`"}}},
{"ValidMatch_resource_reference", `resource_reference = "library.googleapis.com/Book"`, nil},
{"InvalidMismatch_resource_reference", `resource_reference = "library.googleapis.com/Shelf"`, testutils.Problems{{Message: "`resource_reference_child_type`"}}},
{"ValidMatch_resource_reference_child_type", `resource_reference_child_type = "library.googleapis.com/Book"`, nil},
{"InvalidMismatch_resource_reference_child_type", `resource_reference_child_type = "library.googleapis.com/Shelf"`, testutils.Problems{{Message: "`resource_reference_child_type`"}}},
}

// Run each test.
Expand All @@ -42,7 +44,7 @@ func TestResourceReferenceType(t *testing.T) {
rpc CreateBook(CreateBookRequest) returns (Book) {}
}
message CreateBookRequest {
string parent = 1 [(aep.api.field_info).resource_reference = "{{ .TypeName }}"];
string parent = 1 [(aep.api.field_info).{{ .Annotation }}];
}
message Book {
option (google.api.resource) = {
Expand All @@ -65,13 +67,15 @@ func TestResourceReferenceTypeLRO(t *testing.T) {
// Set up testing permutations.
tests := []struct {
testName string
TypeName string
Annotation string
ResponseType string
problems testutils.Problems
}{
{"ValidMatch", "library.googleapis.com/Book", "Book", nil},
{"InvalidMismatch", "library.googleapis.com/Shelf", "Book", testutils.Problems{{Message: "`child_type`"}}},
{"SkipUnresolvableResponse", "library.googleapis.com/Shelf", "Foo", nil},
{"ValidMatch_resource_reference", `resource_reference = "library.googleapis.com/Book"`, "Book", nil},
{"InvalidMismatch_resource_reference", `resource_reference = "library.googleapis.com/Shelf"`, "Book", testutils.Problems{{Message: "`resource_reference_child_type`"}}},
{"ValidMatch_resource_reference_child_type", `resource_reference_child_type = "library.googleapis.com/Book"`, "Book", nil},
{"InvalidMismatch_resource_reference_child_type", `resource_reference_child_type = "library.googleapis.com/Shelf"`, "Book", testutils.Problems{{Message: "`resource_reference_child_type`"}}},
{"SkipUnresolvableResponse", `resource_reference = "library.googleapis.com/Shelf"`, "Foo", nil},
}

// Run each test.
Expand All @@ -90,7 +94,7 @@ func TestResourceReferenceTypeLRO(t *testing.T) {
}
}
message CreateBookRequest {
string parent = 1 [(aep.api.field_info).resource_reference = "{{ .TypeName }}"];
string parent = 1 [(aep.api.field_info).{{ .Annotation }}];
}
message Book {
option (google.api.resource) = {
Expand Down
Loading