diff --git a/docs/rules/0123/resource-plural.md b/docs/rules/0123/resource-plural.md new file mode 100644 index 000000000..e9f1f38cf --- /dev/null +++ b/docs/rules/0123/resource-plural.md @@ -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 diff --git a/docs/rules/0123/resource-singular.md b/docs/rules/0123/resource-singular.md new file mode 100644 index 000000000..a036fa687 --- /dev/null +++ b/docs/rules/0123/resource-singular.md @@ -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 diff --git a/rules/aip0123/aip0123.go b/rules/aip0123/aip0123.go index b5dd741e3..5588a8bfa 100644 --- a/rules/aip0123/aip0123.go +++ b/rules/aip0123/aip0123.go @@ -35,7 +35,9 @@ func AddRules(r lint.RuleRegistry) error { resourceAnnotation, resourceNameField, resourcePattern, + resourcePlural, resourceReferenceType, + resourceSingular, resourceTypeName, resourceVariables, resourceDefinitionVariables, diff --git a/rules/aip0123/resource_plural.go b/rules/aip0123/resource_plural.go new file mode 100644 index 000000000..5058eacc1 --- /dev/null +++ b/rules/aip0123/resource_plural.go @@ -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 + }, +} diff --git a/rules/aip0123/resource_plural_test.go b/rules/aip0123/resource_plural_test.go new file mode 100644 index 000000000..0f6d9e604 --- /dev/null +++ b/rules/aip0123/resource_plural_test.go @@ -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) + } + }) + } +} diff --git a/rules/aip0123/resource_singular.go b/rules/aip0123/resource_singular.go new file mode 100644 index 000000000..7bbd9db0e --- /dev/null +++ b/rules/aip0123/resource_singular.go @@ -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 + }, +} diff --git a/rules/aip0123/resource_singular_test.go b/rules/aip0123/resource_singular_test.go new file mode 100644 index 000000000..3ab0dfcb7 --- /dev/null +++ b/rules/aip0123/resource_singular_test.go @@ -0,0 +1,66 @@ +// 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 TestResourceSingular(t *testing.T) { + for _, test := range []struct { + name string + Singular string + problems testutils.Problems + }{ + { + "Valid", + `singular: "book"`, + nil, + }, + { + "InvalidDoesntMatchType", + `singular: "shelf"`, + testutils.Problems{{ + Message: "book", + }}, + }, + { + "InvalidMissing", + ``, + testutils.Problems{{ + Message: "Resources should declare singular", + }}, + }, + } { + 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/Book" + pattern: "publishers/{publisher}/books/{book}" + {{.Singular}} + }; + string name = 1; + } + `, test) + m := f.GetMessageTypes()[0] + if diff := test.problems.SetDescriptor(m).Diff(resourceSingular.Lint(f)); diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/rules/aip0123/resource_type_name.go b/rules/aip0123/resource_type_name.go index 36ae6f70d..fd4281ee4 100644 --- a/rules/aip0123/resource_type_name.go +++ b/rules/aip0123/resource_type_name.go @@ -16,8 +16,6 @@ package aip0123 import ( "fmt" - "regexp" - "unicode" "github.com/googleapis/api-linter/lint" "github.com/googleapis/api-linter/locations" @@ -25,10 +23,6 @@ import ( "github.com/jhump/protoreflect/desc" ) -var ( - resourceNameRegex = regexp.MustCompile(`^[\p{L}A-Za-z0-9]+$`) -) - var resourceTypeName = &lint.MessageRule{ Name: lint.NewRuleName(123, "resource-type-name"), OnlyIf: func(m *desc.MessageDescriptor) bool { @@ -37,6 +31,7 @@ var resourceTypeName = &lint.MessageRule{ LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { resource := utils.GetResource(m) _, typeName, ok := utils.SplitResourceTypeName(resource.GetType()) + upperTypeName := utils.ToUpperCamelCase(typeName) if !ok { return []lint.Problem{{ Message: "Resource type names must be of the form {Service Name}/{Type}.", @@ -44,18 +39,9 @@ var resourceTypeName = &lint.MessageRule{ Location: locations.MessageResource(m), }} } - if !unicode.IsUpper(rune(typeName[0])) { - return []lint.Problem{{ - Message: fmt.Sprintf("Type %q must be UpperCamelCase", typeName), - Descriptor: m, - Location: locations.MessageResource(m), - }} - } - if !resourceNameRegex.MatchString(typeName) { + if upperTypeName != typeName { return []lint.Problem{{ - Message: fmt.Sprintf( - "Type %q must only contain alphanumeric characters", typeName, - ), + Message: fmt.Sprintf("Type must be UpperCamelCase with alphanumeric characters: %q", upperTypeName), Descriptor: m, Location: locations.MessageResource(m), }} diff --git a/rules/aip0123/resource_type_name_test.go b/rules/aip0123/resource_type_name_test.go index f6038ba8f..e074f9369 100644 --- a/rules/aip0123/resource_type_name_test.go +++ b/rules/aip0123/resource_type_name_test.go @@ -27,12 +27,14 @@ func TestResourceTypeName(t *testing.T) { problems testutils.Problems }{ {"Valid", "library.googleapis.com/Book", testutils.Problems{}}, - {"ValidWithUnicode", "library.googleapis.com/BoØkLibre", testutils.Problems{}}, {"InvalidTooMany", "library.googleapis.com/shelf/Book", testutils.Problems{{Message: "{Service Name}/{Type}"}}}, {"InvalidNotEnough", "library.googleapis.com~Book", testutils.Problems{{Message: "{Service Name}/{Type}"}}}, - {"InvalidLowerCamelCase", "library.googleapis.com/bookLoan", testutils.Problems{{Message: "Type \"bookLoan\" must be UpperCamelCase"}}}, - {"InvalidTypeNotAlphaNumeric", "library.googleapis.com/Book.:3", testutils.Problems{{Message: "Type \"Book.:3\" must only contain alphanumeric characters"}}}, - {"InvalidTypeContainsEmoji", "library.googleapis.com/Book♥️", testutils.Problems{{Message: "Type \"Book♥️\" must only contain alphanumeric characters"}}}, + {"InvalidWithUnicode", "library.googleapis.com/BoØkLibre", testutils.Problems{{Message: `Type must be UpperCamelCase`}}}, + {"InvalidLowerCamelCase", "library.googleapis.com/bookLoan", testutils.Problems{{Message: `Type must be UpperCamelCase with alphanumeric characters: "BookLoan"`}}}, + {"InvalidTypeNotAlphaNumeric", "library.googleapis.com/Book.:3", testutils.Problems{{Message: `Type must be UpperCamelCase with alphanumeric characters: "Book3"`}}}, + {"InvalidTypeContainsEmoji", "library.googleapis.com/Book♥️", testutils.Problems{{Message: `Type must be UpperCamelCase with alphanumeric characters: "Book"`}}}, + {"InvalidTypeContainsDashes", "library.googleapis.com/Book-Shelf️", testutils.Problems{{Message: `Type must be UpperCamelCase with alphanumeric characters: "BookShelf"`}}}, + {"InvalidTypeContainsUnderscore", "library.googleapis.com/Book_Shelf️", testutils.Problems{{Message: `Type must be UpperCamelCase with alphanumeric characters: "BookShelf"`}}}, } { t.Run(test.name, func(t *testing.T) { f := testutils.ParseProto3Tmpl(t, ` diff --git a/rules/internal/utils/casing.go b/rules/internal/utils/casing.go new file mode 100644 index 000000000..43f25b1f7 --- /dev/null +++ b/rules/internal/utils/casing.go @@ -0,0 +1,68 @@ +// 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 utils + +// ToUpperCamelCase returns the UpperCamelCase of a string, including removing +// delimiters (_,-,., ) and using them to denote a new word. +func ToUpperCamelCase(s string) string { + return toCamelCase(s, true, false) +} + +// ToLowerCamelCase returns the lowerCamelCase of a string, including removing +// delimiters (_,-,., ) and using them to denote a new word. +func ToLowerCamelCase(s string) string { + return toCamelCase(s, false, true) +} + +func toCamelCase(s string, makeNextUpper bool, makeNextLower bool) string { + asLower := make([]rune, 0, len(s)) + for _, r := range s { + if isLower(r) { + if makeNextUpper { + r = r & '_' // make uppercase + } + asLower = append(asLower, r) + } else if isUpper(r) { + if makeNextLower { + r = r | ' ' // make lowercase + makeNextLower = false + } + asLower = append(asLower, r) + } else if isNumber(r) { + asLower = append(asLower, r) + } + makeNextUpper = false + makeNextLower = false + + if r == '-' || r == '_' || r == ' ' || r == '.' { + // handle snake case scenarios, which generally indicates + // a delimited word. + makeNextUpper = true + } + } + return string(asLower) +} + +func isUpper(r rune) bool { + return ('A' <= r && r <= 'Z') +} + +func isNumber(r rune) bool { + return ('0' <= r && r <= '9') +} + +func isLower(r rune) bool { + return ('a' <= r && r <= 'z') +} diff --git a/rules/internal/utils/casing_test.go b/rules/internal/utils/casing_test.go new file mode 100644 index 000000000..1cdbff8b1 --- /dev/null +++ b/rules/internal/utils/casing_test.go @@ -0,0 +1,139 @@ +// 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 utils + +import "testing" + +func TestToLowerCamelCase(t *testing.T) { + for _, test := range []struct { + name string + input string + want string + }{ + { + name: "OneWord", + input: "Foo", + want: "foo", + }, + { + name: "OneWordNoop", + input: "foo", + want: "foo", + }, + { + name: "TwoWords", + input: "bookShelf", + want: "bookShelf", + }, + { + name: "WithDash", + input: "book-shelf", + want: "bookShelf", + }, + { + name: "WithNumbers", + input: "universe42love", + want: "universe42love", + }, + { + name: "WithUnderscore", + input: "book_shelf", + want: "bookShelf", + }, + { + name: "WithUnderscore", + input: "book_shelf", + want: "bookShelf", + }, + { + name: "WithSpaces", + input: "book shelf", + want: "bookShelf", + }, + { + name: "WithPeriods", + input: "book.shelf", + want: "bookShelf", + }, + } { + t.Run(test.name, func(t *testing.T) { + got := ToLowerCamelCase(test.input) + if got != test.want { + t.Errorf("ToLowerCamelCase(%q) = %q, got %q", test.input, test.want, got) + } + }) + } +} + +func TestToUpperCamelCase(t *testing.T) { + for _, test := range []struct { + name string + input string + want string + }{ + { + name: "OneWord", + input: "foo", + want: "Foo", + }, + { + name: "OneWordNoop", + input: "Foo", + want: "Foo", + }, + { + name: "TwoWords", + input: "bookShelf", + want: "BookShelf", + }, + { + name: "WithDash", + input: "book-shelf", + want: "BookShelf", + }, + { + name: "WithNumbers", + input: "universe42love", + want: "Universe42love", + }, + { + name: "WithUnderscore", + input: "Book_shelf", + want: "BookShelf", + }, + { + name: "WithUnderscore", + input: "Book_shelf", + want: "BookShelf", + }, + { + name: "WithSpaces", + input: "Book shelf", + want: "BookShelf", + }, + { + name: "WithPeriods", + input: "book.shelf", + want: "BookShelf", + }, + } { + t.Run(test.name, func(t *testing.T) { + got := ToUpperCamelCase(test.input) + if got != test.want { + t.Errorf("ToLowerCamelCase(%q) = %q, got %q", test.input, test.want, got) + } + }) + } +}