Repository: googleapis/api-linter
Pull request #1055 (2022-12-15 17:10:32)

fix(AIP-127): account for api_version template
Fixes http://b/262723883 by removing the `{$api_version}` template variable from the HTTP patterns before validating them. This is the simplest way to not trip up on such templates. If the pattern uses the wrong version template, then it won't match/replace and the rule will likely warn that the pattern is wrong.

Also fix an redirect loop in the docs for http-template-syntax and http-template-pattern rule pages. The `redirect_from` should be a four digit value, lead by 0 if the AIP number is 3 digits e.g. `0127`. That is too easy to mess up, I wonder if we can verify this somehow...fixes http://b/262448725


Patch:
diff --git a/docs/rules/0127/http-template-pattern.md b/docs/rules/0127/http-template-pattern.md
index 68a48d4..ce1a458 100644
--- a/docs/rules/0127/http-template-pattern.md
+++ b/docs/rules/0127/http-template-pattern.md
@@ -6,7 +6,7 @@ rule:
     HTTP template variable patterns should match the patterns defined by their resources.
 permalink: /127/http-template-pattern
 redirect_from:
-  - /127/http-template-pattern
+  - /0127/http-template-pattern
 ---
 
 # HTTP Pattern Variables
diff --git a/docs/rules/0127/http-template-syntax.md b/docs/rules/0127/http-template-syntax.md
index 05d305d..d10f70c 100644
--- a/docs/rules/0127/http-template-syntax.md
+++ b/docs/rules/0127/http-template-syntax.md
@@ -6,7 +6,7 @@ rule:
     HTTP patterns should follow the HTTP path template syntax.
 permalink: /127/http-template-syntax
 redirect_from:
-  - /127/http-template-syntax
+  - /0127/http-template-syntax
 ---
 
 # HTTP Pattern Variables
diff --git a/rules/aip0127/http_template_syntax.go b/rules/aip0127/http_template_syntax.go
index c3bac63..4553720 100644
--- a/rules/aip0127/http_template_syntax.go
+++ b/rules/aip0127/http_template_syntax.go
@@ -46,7 +46,10 @@ var httpTemplateSyntax = &lint.MethodRule{
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
 		problems := []lint.Problem{}
 		for _, httpRule := range utils.GetHTTPRules(m) {
-			if !templateRegex.MatchString(httpRule.URI) {
+			// Replace the API Versioning template if it matches exactly so as
+			// to not emit false positives.
+			uri := utils.VersionedSegment.ReplaceAllString(httpRule.URI, "v")
+			if !templateRegex.MatchString(uri) {
 				message := fmt.Sprintf("The HTTP pattern %q does not follow proper HTTP path template syntax", httpRule.URI)
 				problems = append(problems, lint.Problem{
 					Message:    message,
diff --git a/rules/internal/utils/http.go b/rules/internal/utils/http.go
index f4b0386..c9af075 100644
--- a/rules/internal/utils/http.go
+++ b/rules/internal/utils/http.go
@@ -109,7 +109,7 @@ func (h *HTTPRule) GetVariables() map[string]string {
 	vars := map[string]string{}
 
 	// Replace the version template variable with "v".
-	uri := templateSegment.ReplaceAllString(h.URI, "v")
+	uri := VersionedSegment.ReplaceAllString(h.URI, "v")
 	for _, match := range plainVar.FindAllStringSubmatch(uri, -1) {
 		vars[match[1]] = "*"
 	}
@@ -123,13 +123,13 @@ func (h *HTTPRule) GetVariables() map[string]string {
 func (h *HTTPRule) GetPlainURI() string {
 	return plainVar.ReplaceAllString(
 		varSegment.ReplaceAllString(
-			templateSegment.ReplaceAllString(h.URI, "v"),
+			VersionedSegment.ReplaceAllString(h.URI, "v"),
 			"$2"),
 		"*")
 }
 
 var (
-	plainVar        = regexp.MustCompile(`\{([^}=]+)\}`)
-	varSegment      = regexp.MustCompile(`\{([^}=]+)=([^}]+)\}`)
-	templateSegment = regexp.MustCompile(`\{\$api_version\}`)
+	plainVar         = regexp.MustCompile(`\{([^}=]+)\}`)
+	varSegment       = regexp.MustCompile(`\{([^}=]+)=([^}]+)\}`)
+	VersionedSegment = regexp.MustCompile(`\{\$api_version\}`)
 )


Test patch:
diff --git a/rules/aip0127/http_template_syntax_test.go b/rules/aip0127/http_template_syntax_test.go
index 7dbdaa9..bbb0f58 100644
--- a/rules/aip0127/http_template_syntax_test.go
+++ b/rules/aip0127/http_template_syntax_test.go
@@ -32,6 +32,7 @@ func TestHttpTemplateSyntax(t *testing.T) {
 
 		// Valid cases
 		{"SingleLiteral", "/v1", true},
+		{"VersionedTemplate", "/{$api_version}/books", true},
 		{"TwoLiterals", "/v1/books", true},
 		{"ThreeLiterals", "/v1/books/shelves", true},
 		{"SingleLiteralWithVerb", "/v1:verb", true},