Repository: googleapis/api-linter
Pull request #859 (2021-07-12 14:56:06)

feat: Validate that parent is only variable for AIP-133
Part of #804 


Patch:
diff --git a/docs/rules/0133/http-uri-parent.md b/docs/rules/0133/http-uri-parent.md
index 108d33a..3288028 100644
--- a/docs/rules/0133/http-uri-parent.md
+++ b/docs/rules/0133/http-uri-parent.md
@@ -16,8 +16,8 @@ URI, as mandated in [AIP-133][].
 ## Details
 
 This rule looks at any message beginning with `Create`, and complains
-if the `parent` variable is not included in the URI. It _does_ check additional
-bindings if they are present.
+if `parent` is not the only variable in the URI path. It _does_ check
+additional bindings if they are present.
 
 ## Examples
 
@@ -33,6 +33,19 @@ rpc CreateBook(CreateBookRequest) returns (Book) {
 }
 ```
 
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+rpc CreateBook(CreateBookRequest) returns (Book) {
+  option (google.api.http) = {
+    // The only variable should be `parent`.
+    post: "/v1/{parent=publishers/*}/{book=books/*}"
+    body: "*"
+  };
+}
+```
+
 **Correct** code for this rule:
 
 ```proto
diff --git a/rules/aip0133/http_uri_parent.go b/rules/aip0133/http_uri_parent.go
index 6327798..67aa4da 100644
--- a/rules/aip0133/http_uri_parent.go
+++ b/rules/aip0133/http_uri_parent.go
@@ -21,10 +21,19 @@ import (
 )
 
 // Create methods should have a parent variable if the resource isn't top-level.
+// This should be the only variable in the URI path.
 var httpURIParent = &lint.MethodRule{
 	Name: lint.NewRuleName(133, "http-uri-parent"),
 	OnlyIf: func(m *desc.MethodDescriptor) bool {
 		return isCreateMethod(m) && !hasNoParent(m.GetOutputType())
 	},
-	LintMethod: utils.LintHTTPURIHasParentVariable,
+	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
+		if problems := utils.LintHTTPURIHasParentVariable(m); problems != nil {
+			return problems
+		}
+		if problems := utils.LintHTTPURIVariableCount(m, 1); problems != nil {
+			return problems
+		}
+		return nil
+	},
 }
diff --git a/rules/internal/utils/common_lints.go b/rules/internal/utils/common_lints.go
index 3510f62..fac22b2 100644
--- a/rules/internal/utils/common_lints.go
+++ b/rules/internal/utils/common_lints.go
@@ -187,6 +187,28 @@ func LintHTTPURIHasVariable(m *desc.MethodDescriptor, v string) []lint.Problem {
 	return nil
 }
 
+// LintHTTPURIVariableCount returns a problem if the given method's HTTP rules
+// do not contain the given number of variables in the URI.
+func LintHTTPURIVariableCount(m *desc.MethodDescriptor, n int) []lint.Problem {
+	varsText := "variables"
+	if n == 1 {
+		varsText = "variable"
+	}
+
+	varsCount := 0
+	for _, httpRule := range GetHTTPRules(m) {
+		varsCount += len(httpRule.GetVariables())
+	}
+	if varsCount != n {
+		return []lint.Problem{{
+			Message:    fmt.Sprintf("HTTP URI should contain %d %s.", n, varsText),
+			Descriptor: m,
+			Location:   locations.MethodHTTPRule(m),
+		}}
+	}
+	return nil
+}
+
 // LintHTTPURIHasNameVariable returns a problem if any of the given method's HTTP rules do not
 // have a name variable in the URI.
 func LintHTTPURIHasNameVariable(m *desc.MethodDescriptor) []lint.Problem {


Test patch:
diff --git a/rules/aip0133/http_uri_parent_test.go b/rules/aip0133/http_uri_parent_test.go
index 47986c0..315ee4a 100644
--- a/rules/aip0133/http_uri_parent_test.go
+++ b/rules/aip0133/http_uri_parent_test.go
@@ -32,6 +32,7 @@ func TestHTTPURIParent(t *testing.T) {
 		{"InvalidVarParent", "/v1/{book=publishers/*/books/*}", "CreateBook", "publishers/{publisher}/books/{book}", testutils.Problems{{Message: "`parent` variable"}}},
 		{"NoVarParent", "/v1/publishers/*/books/*", "CreateBook", "publishers/{publisher}/books/{book}", testutils.Problems{{Message: "`parent` variable"}}},
 		{"NoParent", "/v1/books/*", "CreateBook", "books/{book}", nil},
+		{"MultipleVars", "/v1/{parent=publishers/*}/{book=books/*}", "CreateBook", "publishers/{publisher}/books/{book}", testutils.Problems{{Message: "1 variable"}}},
 		{"Irrelevant", "/v1/{book=publishers/*/books/*}", "BuildBook", "publishers/{publisher}/books/{book}", nil},
 	}