feat(AIP-123): enforce singular as ID segment (#1403)

* feat(AIP-123): enforce singular as ID segment

* address typos

* change helper name top level to root level

* fix typo

Co-authored-by: Sam Levenick <slevenick@google.com>

* add plural in example and remove extraneous comma

* handle top level singleton in root level check

docs/rules/0123/resource-pattern-singular.md

@@ -0,0 +1,88 @@
+---
+rule:
+  aip: 123
+  name: [core, '0123', resource-pattern-singular]
+  summary: Resource patterns must use the singular as the resource ID segment
+permalink: /123/resource-pattern-singular
+redirect_from:
+  - /0123/resource-pattern-singular
+---
+
+# Resource `pattern` use of `singular`
+
+This rule enforces that messages that have a `google.api.resource` annotation
+use the `singular` form as the resource ID segment, as described in [AIP-123][].
+
+## Details
+
+This rule scans messages with a `google.api.resource` annotation, and validates
+the `singular` form of the resource type name is used as the resource ID segment
+in every pattern.
+
+**Note:** Special consideration is given to type names of child collections that
+stutter next to their parent collection, as described in
+[AIP-122 Nested Collections][nested]. See AIP-122 for more details.
+
+**Important:** Do not accept the suggestion if it would produce a backwards
+incompatible change.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message Book {
+  option (google.api.resource) = {
+    type: "library.googleapis.com/BookShelf"
+    // resource ID segment doesn't match the singular.
+    pattern: "publishers/{publisher}/bookShelves/{shelf}"
+    singular: "bookShelf"
+    plural: "bookShelves"
+  };
+
+  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"
+    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-pattern-singular=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+message Book {
+  option (google.api.resource) = {
+    type: "library.googleapis.com/BookShelf"
+    pattern: "publishers/{publisher}/bookShelves/{shelf}"
+    singular: "bookShelf"
+    plural: "bookShelves"
+  };
+
+  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
+[nested]: https://aip.dev/122#nested-collections

docs/rules/0123/resource-singular.md

@@ -8,7 +8,7 @@ redirect_from:
   - /0123/resource-singular
 ---
 
-# Resource type name
+# Resource `singular`
 
 This rule enforces that messages that have a `google.api.resource` annotation,
 have a properly formatted `singular`, as described in [AIP-123][].
@@ -60,7 +60,7 @@ If you need to violate this rule, use a leading comment above the message.
 //     aip.dev/not-precedent: We need to do this because reasons. --)
 message Book {
   option (google.api.resource) = {
-    type: "library.googleapis.com/Genre/Mystery/Book"
+    type: "library.googleapis.com/Book"
     pattern: "publishers/{publisher}/books/{book}"
     singular: "shelf",
   };

rules/aip0123/aip0123.go

@@ -24,6 +24,7 @@ import (
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
 	"github.com/stoewer/go-strcase"
+	apb "google.golang.org/genproto/googleapis/api/annotations"
 )
 
 // AddRules accepts a register function and registers each of
@@ -44,6 +45,7 @@ func AddRules(r lint.RuleRegistry) error {
 		resourceDefinitionVariables,
 		resourceDefinitionPatterns,
 		resourceDefinitionTypeName,
+		resourcePatternSingular,
 		nameNeverOptional,
 	)
 }
@@ -76,6 +78,91 @@ func getVariables(pattern string) []string {
 	return answer
 }
 
+// isRootLevelResourcePattern determines if the given pattern is that of a
+// root-level resource by checking how many segments it has - root-level
+// resource patterns have only two segments, thus one delimeter.
+func isRootLevelResourcePattern(pattern string) bool {
+	return strings.Count(pattern, "/") <= 1
+}
+
+// getParentIDVariable is a helper that returns the parent resource ID segment
+// for a given pattern. Returns empty string if the pattern has no variables or
+// is a top-level resource.
+func getParentIDVariable(pattern string) string {
+	variables := getVariables(pattern)
+	// no variables shouldn't happen but should be safe
+	if len(variables) == 0 || isRootLevelResourcePattern(pattern) {
+		return ""
+	}
+
+	// TODO: handle if singleton is a *parent*.
+	if utils.IsSingletonResourcePattern(pattern) {
+		// Last variable is the parent's for a singleton child.
+		return variables[len(variables)-1]
+	}
+
+	return variables[len(variables)-2]
+}
+
+// nestedSingular returns the would be reduced singular form of a nested
+// resource. Use isNestedName to check eligibility before using nestedSingular.
+// This will return empty if the resource is not eligible for nested name
+// reduction.
+func nestedSingular(resource *apb.ResourceDescriptor) string {
+	if !isNestedName(resource) {
+		return ""
+	}
+	parentIDVar := getParentIDVariable(resource.GetPattern()[0])
+
+	singular := utils.GetResourceSingular(resource)
+	singularSnake := strcase.SnakeCase(singular)
+
+	return strings.TrimPrefix(singularSnake, parentIDVar+"_")
+}
+
+// isNestedName determines if the resource naming could be reduced as a
+// repetitive, nested collection as per AIP-122 Nested Collections. It does this
+// by analyzing the first resource pattern defined, comparing the resource
+// singular to the resource ID segment of the parent portion of the pattern. If
+// that is a prefix of resource singular (in snake_case form as well), then the
+// resource name could be reduced. For example, given `singular: "userEvent"`
+// and `pattern: "users/{user}/userEvents/{user_event}"`, isNestedName would
+// return `true`, because the `pattern` could be reduced to
+// `"users/{user}/events/{event}"`.
+func isNestedName(resource *apb.ResourceDescriptor) bool {
+	if len(resource.GetPattern()) == 0 {
+		return false
+	}
+	// only evaluate the first pattern b.c patterns cannot be reordered
+	// and nested names must be used consistently, thus from the beginning
+	pattern := resource.GetPattern()[0]
+
+	// Can't be a nested collection if it is a top level resource.
+	if isRootLevelResourcePattern(pattern) {
+		return false
+	}
+
+	singular := utils.GetResourceSingular(resource)
+
+	// If the resource type's singular is not camelCase then it is not a
+	// multi-word type and we do not need to check for nestedness in naming.
+	singularSnake := strcase.SnakeCase(singular)
+	if strings.Count(singularSnake, "_") < 1 {
+		return false
+	}
+
+	// Second to last resource ID variable will be the parent's to compare the
+	// child resource's singular form against. This prevents us from needing to
+	// deal with pluralizations due to the child resource typically being
+	// pluralized on its own noun, not the parent noun.
+	parentIDVar := getParentIDVariable(pattern)
+
+	// For example:
+	// publisher_credit starts with publisher --> nested
+	// book_shelf does not start with library --> not nested re:naming
+	return strings.HasPrefix(singularSnake, parentIDVar)
+}
+
 // getPlainPattern returns the pattern with all variables replaced with "*".
 //
 // For example, a pattern of "publishers/{publisher}/books/{book}" would