diff --git a/rules/aep0004/aep0004.go b/rules/aep0004/aep0004.go index ab75ecfd..32cc57e3 100644 --- a/rules/aep0004/aep0004.go +++ b/rules/aep0004/aep0004.go @@ -91,7 +91,7 @@ func getDesiredPattern(pattern string) string { for _, token := range strings.Split(pattern, "/") { if strings.HasPrefix(token, "{") && strings.HasSuffix(token, "}") { varname := token[1 : len(token)-1] - want = append(want, fmt.Sprintf("{%s}", strings.TrimSuffix(strcase.SnakeCase(varname), "_id"))) + want = append(want, fmt.Sprintf("{%s}", strcase.SnakeCase(varname))) } else { want = append(want, strcase.LowerCamelCase(token)) } diff --git a/rules/aep0004/resource_definition_variables_test.go b/rules/aep0004/resource_definition_variables_test.go index 5e76680f..6b12ffb0 100644 --- a/rules/aep0004/resource_definition_variables_test.go +++ b/rules/aep0004/resource_definition_variables_test.go @@ -27,12 +27,10 @@ func TestResourceDefinitionVariables(t *testing.T) { problems testutils.Problems }{ {"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}", }}}, - {"ID", "publishers/{publisher}/electronicBooks/{electronic_book_id}", testutils.Problems{{ - Message: "publishers/{publisher}/electronicBooks/{electronic_book}", - }}}, } { t.Run(test.name, func(t *testing.T) { f := testutils.ParseProto3Tmpl(t, ` diff --git a/rules/aep0004/resource_name_components_alternate.go b/rules/aep0004/resource_name_components_alternate.go index 8dfb1228..7e205ff8 100644 --- a/rules/aep0004/resource_name_components_alternate.go +++ b/rules/aep0004/resource_name_components_alternate.go @@ -25,7 +25,7 @@ import ( "github.com/jhump/protoreflect/desc" ) -var identifierRegexp = regexp.MustCompile("^{[a-z][-a-z0-9]*[a-z0-9]}$") +var identifierRegexp = regexp.MustCompile("^{[a-z][_a-z0-9-]*[a-z0-9]}$") var resourceNameComponentsAlternate = &lint.MessageRule{ Name: lint.NewRuleName(4, "resource-name-components-alternate"), diff --git a/rules/aep0004/resource_name_components_alternate_test.go b/rules/aep0004/resource_name_components_alternate_test.go index e2c3e83f..55866311 100644 --- a/rules/aep0004/resource_name_components_alternate_test.go +++ b/rules/aep0004/resource_name_components_alternate_test.go @@ -29,6 +29,7 @@ func TestResourceNameComponentsAlternate(t *testing.T) { {"Valid", "author/{author}/books/{book}", testutils.Problems{}}, {"Valid", "publishers/{publisher}/books/{book}/editions/{book-edition}", testutils.Problems{}}, {"ValidSingleton", "user/{user}/config", testutils.Problems{}}, + {"ValidWithIdSuffix", "stores/{store_id}/items/{item_id}", testutils.Problems{}}, {"InvalidDoubleCollection", "author/books/{book}", testutils.Problems{{Message: "must alternate"}}}, {"InvalidDoubleIdentifier", "books/{author}/{book}", testutils.Problems{{Message: "must alternate"}}}, } { diff --git a/rules/aep0004/resource_variables.go b/rules/aep0004/resource_variables.go index bc64537e..2af7f244 100644 --- a/rules/aep0004/resource_variables.go +++ b/rules/aep0004/resource_variables.go @@ -54,16 +54,6 @@ func lintResourceVariables(resource *annotations.ResourceDescriptor, desc desc.D Location: loc, }} } - if strings.HasSuffix(variable, "_id") { - return []lint.Problem{{ - Message: fmt.Sprintf( - "Variable names should omit the `_id` suffix, such as %q.", - getDesiredPattern(pattern), - ), - Descriptor: desc, - Location: loc, - }} - } } } return nil diff --git a/rules/aep0004/resource_variables_test.go b/rules/aep0004/resource_variables_test.go index 383d88c8..87f0e83a 100644 --- a/rules/aep0004/resource_variables_test.go +++ b/rules/aep0004/resource_variables_test.go @@ -27,12 +27,10 @@ func TestResourceVariables(t *testing.T) { problems testutils.Problems }{ {"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}", }}}, - {"ID", "publishers/{publisher}/electronicBooks/{electronic_book_id}", testutils.Problems{{ - Message: "publishers/{publisher}/electronicBooks/{electronic_book}", - }}}, } { t.Run(test.name, func(t *testing.T) { f := testutils.ParseProto3Tmpl(t, ` diff --git a/rules/aep0122/resource_collection_identifiers.go b/rules/aep0122/resource_collection_identifiers.go index 28134cb2..0e52b429 100644 --- a/rules/aep0122/resource_collection_identifiers.go +++ b/rules/aep0122/resource_collection_identifiers.go @@ -45,6 +45,18 @@ var resourceCollectionIdentifiers = &lint.MessageRule{ segs := strings.Split(p, "/") for _, seg := range segs { + if strings.HasPrefix(seg, "{") && strings.HasSuffix(seg, "}") { + // Variable segments can contain underscores, but must be lowercase + varName := seg[1 : len(seg)-1] // Remove { and } + if HasUpper(varName) { + problems = append(problems, lint.Problem{ + Message: "Resource pattern variables must be lowercase.", + Descriptor: m, + Location: locations.MessageResource(m), + }) + } + continue + } if HasUpper(seg) || strings.Contains(seg, "_") { problems = append(problems, lint.Problem{ Message: "Resource patterns must use kebab-case for collection identifiers.", diff --git a/rules/aep0122/resource_collection_identifiers_test.go b/rules/aep0122/resource_collection_identifiers_test.go index ab24f8f0..dfb70af9 100644 --- a/rules/aep0122/resource_collection_identifiers_test.go +++ b/rules/aep0122/resource_collection_identifiers_test.go @@ -27,6 +27,8 @@ func TestResourceCollectionIdentifiers(t *testing.T) { problems testutils.Problems }{ {"Valid", "author/{author}/books/{book}", testutils.Problems{}}, + {"ValidWithIdSuffix", "stores/{store_id}/items/{item_id}", testutils.Problems{}}, + {"InvalidCapitalIdSuffix", "stores/{Store_id}/items/{item_id}", testutils.Problems{{Message: "lowercase"}}}, {"InvalidUpperCase", "author/{author}/Books/{book}", testutils.Problems{{Message: "kebab-case"}}}, {"InvalidStartsWithSlash", "/author/{author}/Books/{book}", testutils.Problems{{Message: "lowercase letter"}}}, {"InvalidStartsWithCapitalLetter", "Author/{author}/Books/{book}", testutils.Problems{{Message: "lowercase letter"}}},