From 34a224edd78d5040388cc188c46d6c83f75af4f5 Mon Sep 17 00:00:00 2001 From: Olivier Cano Date: Fri, 29 May 2026 16:08:01 +0200 Subject: [PATCH] fix(AEP0004): recommend kebab case * Make error messages recommend kebab case for singular, plural, and segments * Fix segment implementation to forbid camelCase * Remove utilsToKebabCase in favor of strcase.KebabCase in order to fix some edge cases (e.g. `Foo-Bar`) Signed-off-by: Olivier Cano --- docs/rules/0004/resource-plural.md | 10 +-- internal/cmd/quality-checker/rule_name.go | 5 +- .../cmd/quality-checker/rule_registered.go | 4 +- rules/aep0004/aep0004.go | 2 +- rules/aep0004/resource_pattern.go | 20 ++++- rules/aep0004/resource_pattern_test.go | 6 +- rules/aep0004/resource_plural.go | 7 +- rules/aep0004/resource_plural_test.go | 4 +- rules/aep0004/resource_singular.go | 3 +- rules/aep0004/resource_type_name.go | 12 ++- rules/aep0004/resource_type_name_test.go | 12 +-- rules/aep0004/resource_variables_test.go | 2 +- rules/internal/utils/casing.go | 40 ---------- rules/internal/utils/casing_test.go | 78 ------------------- 14 files changed, 54 insertions(+), 151 deletions(-) delete mode 100644 rules/internal/utils/casing.go delete mode 100644 rules/internal/utils/casing_test.go diff --git a/docs/rules/0004/resource-plural.md b/docs/rules/0004/resource-plural.md index 2a83f998..4d5f948b 100644 --- a/docs/rules/0004/resource-plural.md +++ b/docs/rules/0004/resource-plural.md @@ -27,8 +27,8 @@ verifies the `plural` field exists. message Book { // no plural annotation option (aep.api.resource) = { - type: "library.googleapis.com/BookShelf" - pattern: "publishers/{publisher}/bookShelves/{book_shelf}" + type: "library.googleapis.com/book-shelf" + pattern: "publishers/{publisher}/book-shelves/{book_shelf}" }; string path = 1; @@ -41,9 +41,9 @@ message Book { // Correct. message Book { option (aep.api.resource) = { - type: "library.googleapis.com/BookShelf" - pattern: "publishers/{publisher}/bookShelves/{book_shelf}" - plural: "bookShelves", + type: "library.googleapis.com/book-shelf" + pattern: "publishers/{publisher}/book-shelves/{book_shelf}" + plural: "book-shelves", }; string path = 1; diff --git a/internal/cmd/quality-checker/rule_name.go b/internal/cmd/quality-checker/rule_name.go index da6d98b1..41bff0d6 100644 --- a/internal/cmd/quality-checker/rule_name.go +++ b/internal/cmd/quality-checker/rule_name.go @@ -18,12 +18,11 @@ import ( "fmt" "os" "regexp" - - "github.com/stoewer/go-strcase" + "strings" ) func checkRuleName(aep int, name string) []error { - path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strcase.SnakeCase(name)) + path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strings.ReplaceAll(name, "-", "_")) // Read in the file. contentsBytes, err := os.ReadFile(path) diff --git a/internal/cmd/quality-checker/rule_registered.go b/internal/cmd/quality-checker/rule_registered.go index 230ee192..0388f4fb 100644 --- a/internal/cmd/quality-checker/rule_registered.go +++ b/internal/cmd/quality-checker/rule_registered.go @@ -19,12 +19,10 @@ import ( "os" "regexp" "strings" - - "github.com/stoewer/go-strcase" ) func checkRuleRegistered(aep int, name string) []error { - path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strcase.SnakeCase(name)) + path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strings.ReplaceAll(name, "-", "_")) // Read in the file. contents, err := os.ReadFile(path) diff --git a/rules/aep0004/aep0004.go b/rules/aep0004/aep0004.go index 1eb205b0..13011f99 100644 --- a/rules/aep0004/aep0004.go +++ b/rules/aep0004/aep0004.go @@ -86,7 +86,7 @@ func getDesiredPattern(pattern string) string { varname := token[1 : len(token)-1] want = append(want, fmt.Sprintf("{%s}", strcase.SnakeCase(varname))) } else { - want = append(want, strcase.LowerCamelCase(token)) + want = append(want, strcase.KebabCase(token)) } } return strings.Join(want, "/") diff --git a/rules/aep0004/resource_pattern.go b/rules/aep0004/resource_pattern.go index e7c1e801..ba11e598 100644 --- a/rules/aep0004/resource_pattern.go +++ b/rules/aep0004/resource_pattern.go @@ -50,24 +50,36 @@ func lintResourcePatternCommon(patterns []string, desc desc.Descriptor, loc *dpb }} } - // Ensure that the constant segments of the pattern uses camel case, + // Ensure that the constant segments of the pattern uses kebab case, // not snake case, and there are no spaces. for _, pattern := range patterns { plainPattern := getPlainPattern(pattern) + if strings.Contains(plainPattern, " ") { + return []lint.Problem{{ + Message: "Resource patterns should not have spaces", + Descriptor: desc, + Location: loc, + }} + } + if strings.Contains(plainPattern, "_") { return []lint.Problem{{ Message: fmt.Sprintf( - "Resource patterns should use camel case (apart from the variable names), such as %q.", + "Resource patterns should use kebab case (apart from the variable names), such as %q.", getDesiredPattern(pattern), ), Descriptor: desc, Location: loc, }} } - if strings.Contains(plainPattern, " ") { + + if getDesiredPattern(pattern) != pattern { return []lint.Problem{{ - Message: "Resource patterns should not have spaces", + Message: fmt.Sprintf( + "Resource patterns should use kebab case (apart from the variable names), such as %q.", + getDesiredPattern(pattern), + ), Descriptor: desc, Location: loc, }} diff --git a/rules/aep0004/resource_pattern_test.go b/rules/aep0004/resource_pattern_test.go index ba502284..ab576ec8 100644 --- a/rules/aep0004/resource_pattern_test.go +++ b/rules/aep0004/resource_pattern_test.go @@ -27,10 +27,12 @@ func TestResourcePattern(t *testing.T) { problems testutils.Problems }{ {"Valid", `pattern: "publishers/{publisher}/books/{book}"`, testutils.Problems{}}, - {"ValidCamel", `pattern: "publishers/{publisher}/electronicBooks/{electronic_book}"`, testutils.Problems{}}, + {"InvalidCamel", `pattern: "publishers/{publisher}/electronicBooks/{electronic_book}"`, testutils.Problems{{ + Message: "Resource patterns should use kebab case (apart from the variable names), such as \"publishers/{publisher}/electronic-books/{electronic_book}\".", + }}}, {"Missing", "", testutils.Problems{{Message: "declare resource name pattern"}}}, {"SnakeCase", `pattern: "book_publishers/{book_publisher}/books/{book}"`, testutils.Problems{{ - Message: "bookPublishers/{book_publisher}/books/{book}", + Message: `Resource patterns should use kebab case (apart from the variable names), such as "book-publishers/{book_publisher}/books/{book}".`, }}}, {"HasSpaces", `pattern: "publishers/{publisher}/ books /{book}"`, testutils.Problems{{ Message: "Resource patterns should not have spaces", diff --git a/rules/aep0004/resource_plural.go b/rules/aep0004/resource_plural.go index 9d701c77..57da88c0 100644 --- a/rules/aep0004/resource_plural.go +++ b/rules/aep0004/resource_plural.go @@ -21,6 +21,7 @@ import ( "github.com/aep-dev/api-linter/locations" "github.com/aep-dev/api-linter/rules/internal/utils" "github.com/jhump/protoreflect/desc" + "github.com/stoewer/go-strcase" ) var resourcePlural = &lint.MessageRule{ @@ -31,7 +32,7 @@ var resourcePlural = &lint.MessageRule{ r := utils.GetResource(m) l := locations.MessageResource(m) p := r.GetPlural() - pLower := utils.ToKebabCase(p) + pKebab := strcase.KebabCase(p) if p == "" { return []lint.Problem{{ Message: "Resources should declare plural.", @@ -39,10 +40,10 @@ var resourcePlural = &lint.MessageRule{ Location: l, }} } - if pLower != p { + if pKebab != p { return []lint.Problem{{ Message: fmt.Sprintf( - "Resource plural should be lowerCamelCase: %q", pLower, + "Resource plural should be kebab-case: %q", pKebab, ), Descriptor: m, Location: l, diff --git a/rules/aep0004/resource_plural_test.go b/rules/aep0004/resource_plural_test.go index 60d22f53..216f6d1f 100644 --- a/rules/aep0004/resource_plural_test.go +++ b/rules/aep0004/resource_plural_test.go @@ -41,14 +41,14 @@ func TestResourcePlural(t *testing.T) { "InvalidUpperCamel", `plural: "BookShelves"`, testutils.Problems{{ - Message: "Resource plural should be lowerCamelCase", + Message: `Resource plural should be kebab-case: "book-shelves"`, }}, }, { "InvalidDash", `plural: "Book-Shelves"`, testutils.Problems{{ - Message: "Resource plural should be lowerCamelCase", + Message: `Resource plural should be kebab-case: "book-shelves"`, }}, }, } { diff --git a/rules/aep0004/resource_singular.go b/rules/aep0004/resource_singular.go index 2290a72b..fc4046fc 100644 --- a/rules/aep0004/resource_singular.go +++ b/rules/aep0004/resource_singular.go @@ -21,6 +21,7 @@ import ( "github.com/aep-dev/api-linter/locations" "github.com/aep-dev/api-linter/rules/internal/utils" "github.com/jhump/protoreflect/desc" + "github.com/stoewer/go-strcase" ) var resourceSingular = &lint.MessageRule{ @@ -32,7 +33,7 @@ var resourceSingular = &lint.MessageRule{ l := locations.MessageResource(m) s := r.GetSingular() _, typeName, ok := utils.SplitResourceTypeName(r.GetType()) - lowerTypeName := utils.ToKebabCase(typeName) + lowerTypeName := strcase.KebabCase(typeName) if s == "" { return []lint.Problem{{ Message: fmt.Sprintf("Resources should declare singular: %q", lowerTypeName), diff --git a/rules/aep0004/resource_type_name.go b/rules/aep0004/resource_type_name.go index d71a29f2..cad7fc01 100644 --- a/rules/aep0004/resource_type_name.go +++ b/rules/aep0004/resource_type_name.go @@ -16,11 +16,13 @@ package aep0004 import ( "fmt" + "regexp" "github.com/aep-dev/api-linter/lint" "github.com/aep-dev/api-linter/locations" "github.com/aep-dev/api-linter/rules/internal/utils" "github.com/jhump/protoreflect/desc" + "github.com/stoewer/go-strcase" ) var resourceTypeName = &lint.MessageRule{ @@ -32,7 +34,6 @@ var resourceTypeName = &lint.MessageRule{ LintMessage: func(m *desc.MessageDescriptor) []lint.Problem { resource := utils.GetResource(m) _, typeName, ok := utils.SplitResourceTypeName(resource.GetType()) - kebabCase := utils.ToKebabCase(typeName) if !ok { return []lint.Problem{{ Message: "Resource type names must be of the form {Service Name}/{Type}.", @@ -40,9 +41,10 @@ var resourceTypeName = &lint.MessageRule{ Location: locations.MessageResource(m), }} } + kebabCase := removeNonAlphanumeric(strcase.KebabCase(typeName)) if kebabCase != typeName { return []lint.Problem{{ - Message: fmt.Sprintf("Type must be kebob-case with alphanumeric characters: %q", kebabCase), + Message: fmt.Sprintf("Type must be kebab-case with alphanumeric characters: %q", kebabCase), Descriptor: m, Location: locations.MessageResource(m), }} @@ -50,3 +52,9 @@ var resourceTypeName = &lint.MessageRule{ return nil }, } + +var notAlphaNumericReg = regexp.MustCompile(`[^a-zA-Z0-9-]+`) + +func removeNonAlphanumeric(s string) string { + return notAlphaNumericReg.ReplaceAllString(s, "") +} diff --git a/rules/aep0004/resource_type_name_test.go b/rules/aep0004/resource_type_name_test.go index 7c341c1b..b85f8d30 100644 --- a/rules/aep0004/resource_type_name_test.go +++ b/rules/aep0004/resource_type_name_test.go @@ -29,13 +29,13 @@ func TestResourceTypeName(t *testing.T) { {"Valid", "library.googleapis.com/book", 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}"}}}, - {"InvalidWithUnicode", "library.googleapis.com/BoØkLibre", testutils.Problems{{Message: `Type must be kebob-case`}}}, - {"InvalidLowerCamelCase", "library.googleapis.com/bookLoan", testutils.Problems{{Message: `Type must be kebob-case with alphanumeric characters: "book-loan"`}}}, + {"InvalidWithUnicode", "library.googleapis.com/BoØkLibre", testutils.Problems{{Message: `Type must be kebab-case`}}}, + {"InvalidLowerCamelCase", "library.googleapis.com/bookLoan", testutils.Problems{{Message: `Type must be kebab-case with alphanumeric characters: "book-loan"`}}}, {"ValidLowerCamelCase", "library.googleapis.com/book-loan", testutils.Problems{}}, - {"InvalidTypeNotAlphaNumeric", "library.googleapis.com/Book.:3", testutils.Problems{{Message: `Type must be kebob-case with alphanumeric characters: "book-:3"`}}}, - {"InvalidTypeContainsEmoji", "library.googleapis.com/Book♥️", testutils.Problems{{Message: `Type must be kebob-case with alphanumeric characters: "book♥️"`}}}, - {"InvalidTypeContainsDashes", "library.googleapis.com/Book-Shelf️", testutils.Problems{{Message: `Type must be kebob-case with alphanumeric characters: "book--she`}}}, - {"InvalidTypeContainsUnderscore", "library.googleapis.com/Book_Shelf️", testutils.Problems{{Message: `Type must be kebob-case with alphanumeric characters: "book--she`}}}, + {"InvalidTypeNotAlphaNumeric", "library.googleapis.com/Book.:3", testutils.Problems{{Message: `Type must be kebab-case with alphanumeric characters: "book3"`}}}, + {"InvalidTypeContainsEmoji", "library.googleapis.com/Book♥️", testutils.Problems{{Message: `Type must be kebab-case with alphanumeric characters: "book"`}}}, + {"InvalidTypeContainsDashes", "library.googleapis.com/Book-Shelf️", testutils.Problems{{Message: `Type must be kebab-case with alphanumeric characters: "book-she`}}}, + {"InvalidTypeContainsUnderscore", "library.googleapis.com/Book_Shelf️", testutils.Problems{{Message: `Type must be kebab-case with alphanumeric characters: "book-she`}}}, } { t.Run(test.name, func(t *testing.T) { f := testutils.ParseProto3Tmpl(t, ` diff --git a/rules/aep0004/resource_variables_test.go b/rules/aep0004/resource_variables_test.go index a0e1488b..9aee22fd 100644 --- a/rules/aep0004/resource_variables_test.go +++ b/rules/aep0004/resource_variables_test.go @@ -29,7 +29,7 @@ func TestResourceVariables(t *testing.T) { {"Valid", "publishers/{publisher}/electronicBooks/{electronic_book}", testutils.Problems{}}, {"ValidWithIdSuffix", "publishers/{publisher_id}/electronicBooks/{electronic_book_id}", testutils.Problems{}}, {"CamelCase", "publishers/{publisher}/electronicBooks/{electronicBook}", testutils.Problems{{ - Message: "publishers/{publisher}/electronicBooks/{electronic_book}", + Message: `Variable names in patterns should use snake case, such as "publishers/{publisher}/electronic-books/{electronic_book}".`, }}}, } { t.Run(test.name, func(t *testing.T) { diff --git a/rules/internal/utils/casing.go b/rules/internal/utils/casing.go deleted file mode 100644 index 6063f2a0..00000000 --- a/rules/internal/utils/casing.go +++ /dev/null @@ -1,40 +0,0 @@ -// 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 - -// ToKebabCase returns the kebob-case of a word (book-edition). -func ToKebabCase(s string) string { - asLower := make([]rune, 0, len(s)) - for i, r := range s { - if isUpper(r) { - r = r | ' ' // make lowercase - - // Only insert hypen after first word. - if i != 0 { - asLower = append(asLower, '-') - } - asLower = append(asLower, r) - } else if r == '-' || r == '_' || r == ' ' || r == '.' { - asLower = append(asLower, '-') - } else { - asLower = append(asLower, r) - } - } - return string(asLower) -} - -func isUpper(r rune) bool { - return ('A' <= r && r <= 'Z') -} diff --git a/rules/internal/utils/casing_test.go b/rules/internal/utils/casing_test.go deleted file mode 100644 index c6003968..00000000 --- a/rules/internal/utils/casing_test.go +++ /dev/null @@ -1,78 +0,0 @@ -// 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 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: "book-shelf", - }, - { - name: "WithDash", - input: "book-shelf", - want: "book-shelf", - }, - { - name: "WithNumbers", - input: "universe42love", - want: "universe42love", - }, - { - name: "WithUnderscore", - input: "Book_shelf", - want: "book-shelf", - }, - { - name: "WithUnderscore", - input: "Book_shelf", - want: "book-shelf", - }, - { - name: "WithSpaces", - input: "Book shelf", - want: "book-shelf", - }, - { - name: "WithPeriods", - input: "book.shelf", - want: "book-shelf", - }, - } { - t.Run(test.name, func(t *testing.T) { - got := ToKebabCase(test.input) - if got != test.want { - t.Errorf("ToKebabCase(%q) = %q, got %q", test.input, test.want, got) - } - }) - } -}