Repository: googleapis/api-linter
Pull request #1051 (2022-12-05 19:40:21)

feat(AIP-127): Validate HTTP pattern syntax
Part of #1049 

This constructs a regex containing the syntax rules defined [here](https://github.com/googleapis/googleapis/blob/83c3605afb5a39952bf0a0809875d41cf2a558ca/google/api/http.proto#L224). This approach is trickier to debug than writing a parser. However, it seems likely that with sufficient testing this rule will not require much debugging, especially since it does not seem the syntax rules will change anytime soon.

conventional-commit-lint-gcf[bot]: <!-- probot comment [1356997]-->
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use `automerge` label. Good luck human!

 -- conventional-commit-lint bot
https://conventionalcommits.org/
noahdietz: Try a PR title like `feat(AIP-127): enforce HTTP pattern formatting`. That will make the conventional commit bot happy :)

Patch:
diff --git a/docs/rules/0127/http-template-syntax.md b/docs/rules/0127/http-template-syntax.md
new file mode 100644
index 0000000..05d305d
--- /dev/null
+++ b/docs/rules/0127/http-template-syntax.md
@@ -0,0 +1,93 @@
+---
+rule:
+  aip: 127
+  name: [core, '0127', http-template-syntax]
+  summary: |
+    HTTP patterns should follow the HTTP path template syntax.
+permalink: /127/http-template-syntax
+redirect_from:
+  - /127/http-template-syntax
+---
+
+# HTTP Pattern Variables
+
+This rule enforces that HTTP annotation patterns follow the path template syntax
+rules, as mandated in [AIP-127][].
+
+## Details
+
+This rule ensures that `google.api.http` patterns adhere to the following
+[syntax rules]
+(https://github.com/googleapis/googleapis/blob/83c3605afb5a39952bf0a0809875d41cf2a558ca/google/api/http.proto#L224).
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+rpc GetBook(GetBookRequest) returns (Book) {
+    option (google.api.http) = {
+        // Should start with a leading slash.
+        get: "v1/{name=shelves/*}"
+    };
+}
+```
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+rpc AddAuthor(AddAuthorRequest) returns (AddAuthorResponse) {
+    option (google.api.http) = {
+        // Verb should be marked off with the ':' character.
+        post: "/v1/{book=publishers/*/books/*}-addAuthor"
+        body: "*"
+    };
+}
+```
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+rpc CreateBook(CreateBookRequest) returns (Book) {
+  option (google.api.http) = {
+    // The triple wildcard ('***') is not a part of the syntax.
+    post: "/v1/{parent=publishers/***}"
+    body: "book"
+  };
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+rpc GetBook(GetBookRequest) returns (Book) {
+    option (google.api.http) = {
+        get: "/v1/{name=shelves/*}"
+    };
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the field.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0127::http-template-syntax=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+rpc GetBook(GetBookRequest) returns (Book) {
+    option (google.api.http) = {
+        get: "v1/{name=shelves/*}"
+    };
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-127]: https://aip.dev/127
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
\ No newline at end of file
diff --git a/rules/aip0127/aip0127.go b/rules/aip0127/aip0127.go
index cd4f03e..d3affa4 100644
--- a/rules/aip0127/aip0127.go
+++ b/rules/aip0127/aip0127.go
@@ -25,6 +25,7 @@ func AddRules(r lint.RuleRegistry) error {
 		127,
 		hasAnnotation,
 		httpTemplatePattern,
+		httpTemplateSyntax,
 		leadingSlash,
 		resourceNameExtraction,
 	)
diff --git a/rules/aip0127/http_template_syntax.go b/rules/aip0127/http_template_syntax.go
new file mode 100644
index 0000000..c3bac63
--- /dev/null
+++ b/rules/aip0127/http_template_syntax.go
@@ -0,0 +1,60 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0127
+
+import (
+	"fmt"
+	"regexp"
+
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/locations"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+)
+
+var (
+	literal          = `(\w+)`
+	identifier       = `(\w+)`
+	verb             = fmt.Sprintf("(:%s)", literal)
+	fieldPath        = fmt.Sprintf(`(%s(\.%s)*)`, identifier, identifier)
+	variableSegment  = fmt.Sprintf(`(\*\*|\*|%s)`, literal)
+	variableTemplate = fmt.Sprintf("(%s(/%s)*)", variableSegment, variableSegment)
+	variable         = fmt.Sprintf(`(\{%s(=%s)?\})`, fieldPath, variableTemplate)
+	segment          = fmt.Sprintf(`(\*\*|\*|%s|%s)`, literal, variable)
+	segments         = fmt.Sprintf("(%s(/%s)*)", segment, segment)
+	template         = fmt.Sprintf("^/%s(%s)?$", segments, verb)
+	templateRegex    = regexp.MustCompile(template)
+)
+
+// HTTP URL pattern should follow the syntax rules described here:
+// https://github.com/googleapis/googleapis/blob/16db2fb7fab4668bdfa09966513e03581d8f5e35/google/api/http.proto#L224.
+var httpTemplateSyntax = &lint.MethodRule{
+	Name:   lint.NewRuleName(127, "http-template-syntax"),
+	OnlyIf: utils.HasHTTPRules,
+	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
+		problems := []lint.Problem{}
+		for _, httpRule := range utils.GetHTTPRules(m) {
+			if !templateRegex.MatchString(httpRule.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,
+					Descriptor: m,
+					Location:   locations.MethodHTTPRule(m),
+				})
+			}
+		}
+		return problems
+	},
+}


Test patch:
diff --git a/rules/aip0127/http_template_syntax_test.go b/rules/aip0127/http_template_syntax_test.go
new file mode 100644
index 0000000..7dbdaa9
--- /dev/null
+++ b/rules/aip0127/http_template_syntax_test.go
@@ -0,0 +1,104 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0127
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestHttpTemplateSyntax(t *testing.T) {
+	tests := []struct {
+		testName string
+		URI      string
+		valid    bool
+	}{
+		// NOTE: These examples are contrived to test the enforcment of the
+		// template syntax. Many of these examples either fail or do not make
+		// sense in the context of other AIP rules.
+
+		// Valid cases
+		{"SingleLiteral", "/v1", true},
+		{"TwoLiterals", "/v1/books", true},
+		{"ThreeLiterals", "/v1/books/shelves", true},
+		{"SingleLiteralWithVerb", "/v1:verb", true},
+		{"MultipleLiteralsWithVerb", "/v1/books:verb", true},
+		{"SingleWildcard", "/v1/*", true},
+		{"DoubleWildcard", "/v1/**", true},
+		{"SingleWildcardWithVerb", "/v1/*:verb", true},
+		{"DoubleWildcardWithVerb", "/v1/**:verb", true},
+		{"SingleWildcardFollowedByLiteral", "/v1/*/books", true},
+		{"DoubleWildcardFollowedByLiteral", "/v1/**/books", true},
+		{"LiteralFollowedBySingleWildcard", "/v1/books/*", true},
+		{"LiteralFollowedByDoubleWildcard", "/v1/books/**", true},
+		{"VariableWithFieldpath", "/v1/{field}", true},
+		{"VariableWithNestedFieldpath", "/v1/{field.subfield}", true},
+		{"VariableWithUltraNestedFieldpath", "/v1/{field.subfield.subsubfield}", true},
+		{"VariableWithLiteralTemplate", "/v1/{field=books}", true},
+		{"VariableWithSingleWildcardTemplate", "/v1/{field=*}", true},
+		{"VariableWithDoubleWildcardTemplate", "/v1/{field=**}", true},
+		{"VariableWithSingleWildcardFollowedByLiteral", "/v1/{field=*/books}", true},
+		{"VariableWithDoubleWildcardFollowedByLiteral", "/v1/{field=**/books}", true},
+		{"VariableWithLiteralFollowedBySingleWildcard", "/v1/{field=books/*}", true},
+		{"VariableWithLiteralFollowedByDoubleWildcard", "/v1/{field=books/**}", true},
+		{"VariableFollowedByLiteral", "/v1/{field}/books", true},
+		{"VariableFollowedByVariable", "/v1/{field}/{otherField}", true},
+		{"VariableWithTemplateFollowedByLiteral", "/v1/{field=books/*}/shelves", true},
+		{"VariableFollowedByVariableWithTemplate", "/v1/{field}/{otherField=books/*}", true},
+		{"VariableWithTemplateFollowedByVariableWithTemplate", "/v1/{field=books/*}/{otherField=shelves/*}", true},
+
+		// Invalid cases
+		{"LiteralWithoutLeadingSlash", "v1", false},
+		{"LiteralFollowedBySlash", "/v1/", false},
+		{"WrongVerbDelimiter", "/v1-verb", false},
+		{"MultipleVerbs", "/v1:verb:verb", false},
+		{"VerbFollowedBySlash", "/v1:verb/", false},
+		{"MultipleLiteralsWithWrongDelimiter", "/v1|books", false},
+		{"SingleWildcardFollowedBySlash", "/v1/*/", false},
+		{"DoubleWildcardFollowedBySlash", "/v1/**/", false},
+		{"TripleWildcard", "/v1/***", false},
+		{"WrongVariableMarker", "/v1/[field]", false},
+		{"WrongVariableSubfieldOperator", "/v1/[field->subfield]", false},
+		{"VariableTemplateContainsVariable", "/v1/{field={otherField=*}}", false},
+		{"WrongVariableTemplateAssignmentOperator", "/v1/{field≈books}", false},
+	}
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+				import "google/api/annotations.proto";
+				service Library {
+					rpc FooMethod(FooMethodRequest) returns (FooMethodResponse) {
+						option (google.api.http) = {
+							get: "{{.URI}}"
+						};
+					}
+				}
+				message FooMethodRequest {}
+				message FooMethodResponse {}
+			`, test)
+
+			problems := httpTemplateSyntax.Lint(file)
+
+			if test.valid && len(problems) > 0 {
+				t.Fatalf("Expected valid HTTP path template syntax but got invalid")
+			}
+
+			if !test.valid && len(problems) == 0 {
+				t.Fatalf("Expected invalid HTTP path template syntax but got valid")
+			}
+		})
+	}
+}