Skip to content

Commit d46cbaf

Browse files
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 <ocano@scaleway.com>
1 parent e4050d2 commit d46cbaf

14 files changed

Lines changed: 56 additions & 151 deletions

docs/rules/0004/resource-plural.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ verifies the `plural` field exists.
2727
message Book {
2828
// no plural annotation
2929
option (aep.api.resource) = {
30-
type: "library.googleapis.com/BookShelf"
31-
pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
30+
type: "library.googleapis.com/book-shelf"
31+
pattern: "publishers/{publisher}/book-shelves/{book_shelf}"
3232
};
3333
3434
string path = 1;
@@ -41,9 +41,9 @@ message Book {
4141
// Correct.
4242
message Book {
4343
option (aep.api.resource) = {
44-
type: "library.googleapis.com/BookShelf"
45-
pattern: "publishers/{publisher}/bookShelves/{book_shelf}"
46-
plural: "bookShelves",
44+
type: "library.googleapis.com/book-shelf"
45+
pattern: "publishers/{publisher}/book-shelves/{book_shelf}"
46+
plural: "book-shelves",
4747
};
4848
4949
string path = 1;

internal/cmd/quality-checker/rule_name.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,11 @@ import (
1818
"fmt"
1919
"os"
2020
"regexp"
21-
22-
"github.com/stoewer/go-strcase"
21+
"strings"
2322
)
2423

2524
func checkRuleName(aep int, name string) []error {
26-
path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strcase.SnakeCase(name))
25+
path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strings.ReplaceAll(name, "-", "_"))
2726

2827
// Read in the file.
2928
contentsBytes, err := os.ReadFile(path)

internal/cmd/quality-checker/rule_registered.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@ import (
1919
"os"
2020
"regexp"
2121
"strings"
22-
23-
"github.com/stoewer/go-strcase"
2422
)
2523

2624
func checkRuleRegistered(aep int, name string) []error {
27-
path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strcase.SnakeCase(name))
25+
path := fmt.Sprintf("rules/aep%04d/%s.go", aep, strings.ReplaceAll(name, "-", "_"))
2826

2927
// Read in the file.
3028
contents, err := os.ReadFile(path)

rules/aep0004/aep0004.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func getDesiredPattern(pattern string) string {
8686
varname := token[1 : len(token)-1]
8787
want = append(want, fmt.Sprintf("{%s}", strcase.SnakeCase(varname)))
8888
} else {
89-
want = append(want, strcase.LowerCamelCase(token))
89+
want = append(want, strcase.KebabCase(token))
9090
}
9191
}
9292
return strings.Join(want, "/")

rules/aep0004/resource_pattern.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,24 +50,36 @@ func lintResourcePatternCommon(patterns []string, desc desc.Descriptor, loc *dpb
5050
}}
5151
}
5252

53-
// Ensure that the constant segments of the pattern uses camel case,
53+
// Ensure that the constant segments of the pattern uses kebab case,
5454
// not snake case, and there are no spaces.
5555
for _, pattern := range patterns {
5656
plainPattern := getPlainPattern(pattern)
5757

58+
if strings.Contains(plainPattern, " ") {
59+
return []lint.Problem{{
60+
Message: "Resource patterns should not have spaces",
61+
Descriptor: desc,
62+
Location: loc,
63+
}}
64+
}
65+
5866
if strings.Contains(plainPattern, "_") {
5967
return []lint.Problem{{
6068
Message: fmt.Sprintf(
61-
"Resource patterns should use camel case (apart from the variable names), such as %q.",
69+
"Resource patterns should use kebab case (apart from the variable names), such as %q.",
6270
getDesiredPattern(pattern),
6371
),
6472
Descriptor: desc,
6573
Location: loc,
6674
}}
6775
}
68-
if strings.Contains(plainPattern, " ") {
76+
77+
if getDesiredPattern(pattern) != pattern {
6978
return []lint.Problem{{
70-
Message: "Resource patterns should not have spaces",
79+
Message: fmt.Sprintf(
80+
"Resource patterns should use kebab case (apart from the variable names), such as %q.",
81+
getDesiredPattern(pattern),
82+
),
7183
Descriptor: desc,
7284
Location: loc,
7385
}}

rules/aep0004/resource_pattern_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@ func TestResourcePattern(t *testing.T) {
2727
problems testutils.Problems
2828
}{
2929
{"Valid", `pattern: "publishers/{publisher}/books/{book}"`, testutils.Problems{}},
30-
{"ValidCamel", `pattern: "publishers/{publisher}/electronicBooks/{electronic_book}"`, testutils.Problems{}},
30+
{"InvalidCamel", `pattern: "publishers/{publisher}/electronicBooks/{electronic_book}"`, testutils.Problems{{
31+
Message: "Resource patterns should use kebab case (apart from the variable names), such as \"publishers/{publisher}/electronic-books/{electronic_book}\".",
32+
}}},
3133
{"Missing", "", testutils.Problems{{Message: "declare resource name pattern"}}},
3234
{"SnakeCase", `pattern: "book_publishers/{book_publisher}/books/{book}"`, testutils.Problems{{
33-
Message: "bookPublishers/{book_publisher}/books/{book}",
35+
Message: `Resource patterns should use kebab case (apart from the variable names), such as "book-publishers/{book_publisher}/books/{book}".`,
3436
}}},
3537
{"HasSpaces", `pattern: "publishers/{publisher}/ books /{book}"`, testutils.Problems{{
3638
Message: "Resource patterns should not have spaces",

rules/aep0004/resource_plural.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/aep-dev/api-linter/locations"
2222
"github.com/aep-dev/api-linter/rules/internal/utils"
2323
"github.com/jhump/protoreflect/desc"
24+
"github.com/stoewer/go-strcase"
2425
)
2526

2627
var resourcePlural = &lint.MessageRule{
@@ -31,18 +32,18 @@ var resourcePlural = &lint.MessageRule{
3132
r := utils.GetResource(m)
3233
l := locations.MessageResource(m)
3334
p := r.GetPlural()
34-
pLower := utils.ToKebabCase(p)
35+
pKebab := strcase.KebabCase(p)
3536
if p == "" {
3637
return []lint.Problem{{
3738
Message: "Resources should declare plural.",
3839
Descriptor: m,
3940
Location: l,
4041
}}
4142
}
42-
if pLower != p {
43+
if pKebab != p {
4344
return []lint.Problem{{
4445
Message: fmt.Sprintf(
45-
"Resource plural should be lowerCamelCase: %q", pLower,
46+
"Resource plural should be kebab-case: %q", pKebab,
4647
),
4748
Descriptor: m,
4849
Location: l,

rules/aep0004/resource_plural_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ func TestResourcePlural(t *testing.T) {
4141
"InvalidUpperCamel",
4242
`plural: "BookShelves"`,
4343
testutils.Problems{{
44-
Message: "Resource plural should be lowerCamelCase",
44+
Message: `Resource plural should be kebab-case: "book-shelves"`,
4545
}},
4646
},
4747
{
4848
"InvalidDash",
4949
`plural: "Book-Shelves"`,
5050
testutils.Problems{{
51-
Message: "Resource plural should be lowerCamelCase",
51+
Message: `Resource plural should be kebab-case: "book-shelves"`,
5252
}},
5353
},
5454
} {

rules/aep0004/resource_singular.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/aep-dev/api-linter/locations"
2222
"github.com/aep-dev/api-linter/rules/internal/utils"
2323
"github.com/jhump/protoreflect/desc"
24+
"github.com/stoewer/go-strcase"
2425
)
2526

2627
var resourceSingular = &lint.MessageRule{
@@ -32,7 +33,7 @@ var resourceSingular = &lint.MessageRule{
3233
l := locations.MessageResource(m)
3334
s := r.GetSingular()
3435
_, typeName, ok := utils.SplitResourceTypeName(r.GetType())
35-
lowerTypeName := utils.ToKebabCase(typeName)
36+
lowerTypeName := strcase.KebabCase(typeName)
3637
if s == "" {
3738
return []lint.Problem{{
3839
Message: fmt.Sprintf("Resources should declare singular: %q", lowerTypeName),

rules/aep0004/resource_type_name.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@ package aep0004
1616

1717
import (
1818
"fmt"
19+
"regexp"
1920

2021
"github.com/aep-dev/api-linter/lint"
2122
"github.com/aep-dev/api-linter/locations"
2223
"github.com/aep-dev/api-linter/rules/internal/utils"
2324
"github.com/jhump/protoreflect/desc"
25+
"github.com/stoewer/go-strcase"
2426
)
2527

2628
var resourceTypeName = &lint.MessageRule{
@@ -32,21 +34,29 @@ var resourceTypeName = &lint.MessageRule{
3234
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
3335
resource := utils.GetResource(m)
3436
_, typeName, ok := utils.SplitResourceTypeName(resource.GetType())
35-
kebabCase := utils.ToKebabCase(typeName)
37+
3638
if !ok {
3739
return []lint.Problem{{
3840
Message: "Resource type names must be of the form {Service Name}/{Type}.",
3941
Descriptor: m,
4042
Location: locations.MessageResource(m),
4143
}}
4244
}
45+
46+
kebabCase := removeNonAlphanumeric(strcase.KebabCase(typeName))
4347
if kebabCase != typeName {
4448
return []lint.Problem{{
45-
Message: fmt.Sprintf("Type must be kebob-case with alphanumeric characters: %q", kebabCase),
49+
Message: fmt.Sprintf("Type must be kebab-case with alphanumeric characters: %q", kebabCase),
4650
Descriptor: m,
4751
Location: locations.MessageResource(m),
4852
}}
4953
}
5054
return nil
5155
},
5256
}
57+
58+
var notAlphaNumericReg = regexp.MustCompile(`[^a-zA-Z0-9-]+`)
59+
60+
func removeNonAlphanumeric(s string) string {
61+
return notAlphaNumericReg.ReplaceAllString(s, "")
62+
}

0 commit comments

Comments
 (0)