Skip to content

Commit

Permalink
feat(AIP-123): add singular and plural validation (#1143)
Browse files Browse the repository at this point in the history
Matching AIP-123 in requiring singular and plural annotations,
and for singular matching the lowerCamelCase of type.

Wrote custom ToUpperCamelCase as existing one does not consider
underscores / dashes as invalid characters, which are considered
delimiters in the camel case conversion. Further investigation on replacing
it with #1150.
  • Loading branch information
toumorokoshi authored May 11, 2023
1 parent 61cc67c commit 0d1ff55
Show file tree
Hide file tree
Showing 11 changed files with 613 additions and 21 deletions.
74 changes: 74 additions & 0 deletions docs/rules/0123/resource-plural.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
---
rule:
aip: 123
name: [core, '0123', resource-plural]
summary: Resource plural is required
permalink: /123/resource-plural
redirect_from:
- /0123/resource-plural
---

# Resource type name

This rule enforces that messages that have a `google.api.resource` annotation,
have a properly formatted `plural`, as described in [AIP-123][].

## Details

This rule scans messages with a `google.api.resource` annotation, and
verifies the `plural` field exists.

## Examples

**Incorrect** code for this rule:

```proto
// Incorrect.
message Book {
// no plural annotation
option (google.api.resource) = {
type: "library.googleapis.com/BookShelf"
pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
};
string name = 1;
}
```

**Correct** code for this rule:

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

## Disabling

If you need to violate this rule, use a leading comment above the message.

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

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
76 changes: 76 additions & 0 deletions docs/rules/0123/resource-singular.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
rule:
aip: 123
name: [core, '0123', resource-singular]
summary: Resource singular is required and must be lowerCamelCase of type
permalink: /123/resource-singular
redirect_from:
- /0123/resource-singular
---

# Resource type name

This rule enforces that messages that have a `google.api.resource` annotation,
have a properly formatted `singular`, as described in [AIP-123][].

## Details

This rule scans messages with a `google.api.resource` annotation, and validates
the format of the `singular` field is the lower camel case of type.

## Examples

**Incorrect** code for this rule:

```proto
// Incorrect.
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/BookShelf"
pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
// does not match type.
singular: "shelf",
};
string name = 1;
}
```

**Correct** code for this rule:

```proto
// Correct.
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/BookShelf"
pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
singular: "bookShelf",
};
string name = 1;
}
```

## Disabling

If you need to violate this rule, use a leading comment above the message.

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

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
2 changes: 2 additions & 0 deletions rules/aip0123/aip0123.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ func AddRules(r lint.RuleRegistry) error {
resourceAnnotation,
resourceNameField,
resourcePattern,
resourcePlural,
resourceReferenceType,
resourceSingular,
resourceTypeName,
resourceVariables,
resourceDefinitionVariables,
Expand Down
52 changes: 52 additions & 0 deletions rules/aip0123/resource_plural.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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 aip0123

import (
"fmt"

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

var resourcePlural = &lint.MessageRule{
Name: lint.NewRuleName(123, "resource-plural"),
OnlyIf: hasResourceAnnotation,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
r := utils.GetResource(m)
l := locations.MessageResource(m)
p := r.GetPlural()
pLower := utils.ToLowerCamelCase(p)
if p == "" {
return []lint.Problem{{
Message: "Resources should declare plural.",
Descriptor: m,
Location: l,
}}
}
if pLower != p {
return []lint.Problem{{
Message: fmt.Sprintf(
"Resource plural should be lowerCamelCase: %q", pLower,
),
Descriptor: m,
Location: l,
}}
}
return nil
},
}
73 changes: 73 additions & 0 deletions rules/aip0123/resource_plural_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// 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 aip0123

import (
"testing"

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

func TestResourcePlural(t *testing.T) {
for _, test := range []struct {
name string
Plural string
problems testutils.Problems
}{
{
"Valid",
`plural: "bookShelves"`,
nil,
},
{
"InvalidMissing",
``,
testutils.Problems{{
Message: "Resources should declare plural",
}},
},
{
"InvalidUpperCamel",
`plural: "BookShelves"`,
testutils.Problems{{
Message: "Resource plural should be lowerCamelCase",
}},
},
{
"InvalidDash",
`plural: "Book-Shelves"`,
testutils.Problems{{
Message: "Resource plural should be lowerCamelCase",
}},
},
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
import "google/api/resource.proto";
message Book {
option (google.api.resource) = {
type: "library.googleapis.com/BookShelf"
pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
{{.Plural}}
};
string name = 1;
}
`, test)
m := f.GetMessageTypes()[0]
if diff := test.problems.SetDescriptor(m).Diff(resourcePlural.Lint(f)); diff != "" {
t.Error(diff)
}
})
}
}
54 changes: 54 additions & 0 deletions rules/aip0123/resource_singular.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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 aip0123

import (
"fmt"

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

var resourceSingular = &lint.MessageRule{
Name: lint.NewRuleName(123, "resource-singular"),
OnlyIf: hasResourceAnnotation,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
r := utils.GetResource(m)
l := locations.MessageResource(m)
s := r.GetSingular()
_, typeName, ok := utils.SplitResourceTypeName(r.GetType())
lowerTypeName := utils.ToLowerCamelCase(typeName)
if s == "" {
return []lint.Problem{{
Message: fmt.Sprintf("Resources should declare singular: %q", lowerTypeName),
Descriptor: m,
Location: l,
}}
}
if !ok || lowerTypeName != s {
return []lint.Problem{{
Message: fmt.Sprintf(
"Resource singular should be lower camel case of type: %q",
lowerTypeName,
),
Descriptor: m,
Location: l,
}}
}
return nil
},
}
Loading

0 comments on commit 0d1ff55

Please sign in to comment.