Skip to content

Commit

Permalink
feat: Require a name field in messages with google.api.resource annot…
Browse files Browse the repository at this point in the history
…ations (#839)
  • Loading branch information
apasel422 authored Apr 21, 2021
1 parent 193516f commit 26a1c82
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 0 deletions.
83 changes: 83 additions & 0 deletions docs/rules/0123/resource-name-field.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
---
rule:
aip: 123
name: [core, '0123', resource-name-field]
summary: Resource messages should have a `string name` field.
permalink: /123/resource-name-field
redirect_from:
- /0123/resource-name-field
---

# Resource `name` field

This rule enforces that messages that appear to represent resources have a
`string name` field, as described in [AIP-123][].

## Details

This rule scans all messages that have a `google.api.resource` annotation, and
complains if the `name` field is missing or if it is any type other than
singular `string`.

## Examples

**Incorrect** code for this rule:

```proto
// Incorrect: missing `string name` field.
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "publishers/{publisher}/books/{book}"
};
}
```

```proto
// Incorrect.
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "publishers/{publisher}/books/{book}"
};
// Should be `string`, not `bytes`.
bytes name = 1;
}
```

**Correct** code for this rule:

```proto
// Correct.
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "publishers/{publisher}/books/{book}"
};
string name = 1;
}
```

## Disabling

If you need to violate this rule, use a leading comment above the message, or
above the field if it is the wrong type.

```proto
// (-- api-linter: core::0123::resource-name-field=disabled
// aip.dev/not-precedent: We need to do this because reasons. --)
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/Book"
pattern: "publishers/{publisher}/books/{book}"
};
}
```

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

[aip-123]: http://aip.dev/123
[aip.dev/not-precedent]: https://aip.dev/not-precedent
1 change: 1 addition & 0 deletions rules/aip0123/aip0123.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func AddRules(r lint.RuleRegistry) error {
123,
duplicateResource,
resourceAnnotation,
resourceNameField,
resourcePattern,
resourceReferenceType,
resourceVariables,
Expand Down
26 changes: 26 additions & 0 deletions rules/aip0123/resource_name_field.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2021 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 aip0123

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

var resourceNameField = &lint.MessageRule{
Name: lint.NewRuleName(123, "resource-name-field"),
OnlyIf: utils.IsResource,
LintMessage: utils.LintFieldPresentAndSingularString("name"),
}
55 changes: 55 additions & 0 deletions rules/aip0123/resource_name_field_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2021 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 aip0123

import (
"strings"
"testing"

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

func TestResourceNameField(t *testing.T) {
for _, test := range []struct {
name string
Options string
Field string
problems testutils.Problems
}{
{"ValidBothPresent", `option (google.api.resource) = { type: "foo" };`, `string name = 1;`, nil},
{"InvalidNoField", `option (google.api.resource) = { type: "foo" };`, ``, testutils.Problems{{Message: "`name`"}}},
{"InvalidTypeNotString", `option (google.api.resource) = { type: "foo" };`, `int32 name = 1;`, testutils.Problems{{Suggestion: "string"}}},
{"InvalidTypeRepeated", `option (google.api.resource) = { type: "foo" };`, `repeated string name = 1;`, testutils.Problems{{Suggestion: "string"}}},
{"IrrelevantNoAnnotation", ``, ``, nil},
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
import "google/api/resource.proto";
message Book {
{{.Options}}
{{.Field}}
}
`, test)
var d desc.Descriptor = f.GetMessageTypes()[0]
if strings.HasPrefix(test.name, "InvalidType") {
d = f.GetMessageTypes()[0].GetFields()[0]
}
if diff := test.problems.SetDescriptor(d).Diff(resourceNameField.Lint(f)); diff != "" {
t.Error(diff)
}
})
}
}

0 comments on commit 26a1c82

Please sign in to comment.