Skip to content

Commit

Permalink
feat(AIP-203): disallow IDENTIFIER on non-name (#1244)
Browse files Browse the repository at this point in the history
  • Loading branch information
noahdietz authored Aug 30, 2023
1 parent a3895eb commit 1022df2
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 0 deletions.
77 changes: 77 additions & 0 deletions docs/rules/0203/resource-identifier-only.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
---
rule:
aip: 203
name: [core, '0203', resource-identifier-only]
summary: Only the resource's name field can have field behavior IDENTIFIER.
permalink: /203/resource-identifier-only
redirect_from:
- /0203/resource-identifier-only
---

# Resource name: IDENTIFIER

This rule enforces that only the field representing the resource's name is
annotated with with `(google.api.field_behavior) = IDENTIFIER`, as mandated
by [AIP-203][].

## Details

This rule looks at every field with `(google.api.field_behavior) = IDENTIFER`
and complains if that field is not the resource's name field.

## Examples

**Incorrect** code for this rule:

```proto
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "books/{book}"
};
string name = 1 [(google.api.field_behavior) = IDENTIFER];
// Incorrect. Must not be assigned IDENTIFIER.
string description = 2 [(google.api.field_behavior) = IDENTIFER];
}
```

**Correct** code for this rule:

```proto
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "books/{book}"
};
string name = 1 [(google.api.field_behavior) = IDENTIFIER];
// Correct.
string description = 2;
}
```

## Disabling

If you need to violate this rule, use a leading comment above the field.
Remember to also include an [aip.dev/not-precedent][] comment explaining why.

```proto
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "books/{book}"
};
string name = 1 [(google.api.field_behavior) = IDENTIFER];
// (-- api-linter: core::0203::resource-identifier-only=disabled
// aip.dev/not-precedent: We need to do this because reasons. --)
string description = 2 [(google.api.field_behavior) = IDENTIFER];
}
```

If you need to violate this rule for an entire file, place the comment at the
top of the file.

[aip-203]: https://aip.dev/203
[aip.dev/not-precedent]: https://aip.dev/not-precedent
1 change: 1 addition & 0 deletions rules/aip0203/aip0203.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ func AddRules(r lint.RuleRegistry) error {
203,
fieldBehaviorRequired,
unorderedListRepeated,
resourceIdentifierOnly,
)
}
39 changes: 39 additions & 0 deletions rules/aip0203/resource_identifier_only.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2020 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package aip0203

import (
"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
)

var resourceIdentifierOnly = &lint.FieldRule{
Name: lint.NewRuleName(203, "resource-identifier-only"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return utils.GetFieldBehavior(f).Contains("IDENTIFIER")
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
m := f.GetParent().(*desc.MessageDescriptor)
if nf := utils.GetResourceNameField(utils.GetResource(m)); utils.IsResource(m) && f.GetName() == nf {
return nil
}

return []lint.Problem{{
Message: "The IDENTIFIER `google.api.field_behavior` annotation must only be applied to a resource's name field.",
Descriptor: f,
}}
},
}
80 changes: 80 additions & 0 deletions rules/aip0203/resource_identifier_only_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package aip0203

import (
"testing"

"github.com/googleapis/api-linter/rules/internal/testutils"
)

func TestResourceIdentifierOnly(t *testing.T) {
testCases := []struct {
name, message string
ResourceField, NameField, NonResourceExtensions string
problems testutils.Problems
}{
{
name: "Valid",
message: "Book",
ResourceField: "string name = 1 [(google.api.field_behavior) = IDENTIFIER];",
problems: nil,
},
{
name: "ValidNameField",
message: "Book",
ResourceField: "string resource_name = 1 [(google.api.field_behavior) = IDENTIFIER];",
NameField: "resource_name",
problems: nil,
},
{
name: "InvalidNonResource",
message: "NonResource",
NonResourceExtensions: "[(google.api.field_behavior) = IDENTIFIER]",
problems: testutils.Problems{{Message: "resource's name field"}},
},
{
name: "InvalidResourceNonIdentifier",
message: "Book",
ResourceField: "string foo = 1 [(google.api.field_behavior) = IDENTIFIER];",
problems: testutils.Problems{{Message: "resource's name field"}},
},
}

for _, test := range testCases {
t.Run(test.name, func(t *testing.T) {
file := testutils.ParseProto3Tmpl(t, `
import "google/api/field_behavior.proto";
import "google/api/resource.proto";
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "books/{book}"
name_field: "{{.NameField}}"
};
{{.ResourceField}}
}
message NonResource {
string foo = 1 {{.NonResourceExtensions}};
}
`, test)
f := file.FindMessage(test.message).GetFields()[0]
problems := resourceIdentifierOnly.Lint(file)
if diff := test.problems.SetDescriptor(f).Diff(problems); diff != "" {
t.Error(diff)
}
})
}
}
13 changes: 13 additions & 0 deletions rules/internal/utils/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,16 @@ func GetResourcePlural(r *apb.ResourceDescriptor) string {

return r.Plural
}

// GetResourceNameField is a convenience method for getting the name of the
// field that represents the resource's name. This is either set by the
// `name_field` attribute, or defaults to "name".
func GetResourceNameField(r *apb.ResourceDescriptor) string {
if r == nil {
return ""
}
if n := r.GetNameField(); n != "" {
return n
}
return "name"
}

0 comments on commit 1022df2

Please sign in to comment.