refactor: Use common lint for HTTP variable rules more widely (#834)

rules/aip0131/http_uri_name.go

@@ -16,27 +16,12 @@ package aip0131
 
 import (
 	"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"
 )
 
 // Get methods should have a proper HTTP pattern.
 var httpNameField = &lint.MethodRule{
-	Name:   lint.NewRuleName(131, "http-uri-name"),
-	OnlyIf: isGetMethod,
-	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		// Establish that the RPC has no HTTP body.
-		for _, httpRule := range utils.GetHTTPRules(m) {
-			if _, ok := httpRule.GetVariables()["name"]; !ok {
-				return []lint.Problem{{
-					Message:    "Get methods should include the `name` field in the URI.",
-					Descriptor: m,
-					Location:   locations.MethodHTTPRule(m),
-				}}
-			}
-		}
-
-		return nil
-	},
+	Name:       lint.NewRuleName(131, "http-uri-name"),
+	OnlyIf:     isGetMethod,
+	LintMethod: utils.LintHTTPURIHasNameVariable,
 }

rules/aip0131/http_uri_name_test.go

@@ -28,8 +28,8 @@ func TestHttpNameField(t *testing.T) {
 		problems   testutils.Problems
 	}{
 		{"Valid", "/v1/{name=publishers/*/books/*}", "GetBook", testutils.Problems{}},
-		{"InvalidVarName", "/v1/{book=publishers/*/books/*}", "GetBook", testutils.Problems{{Message: "`name` field"}}},
-		{"NoVarName", "/v1/publishers/*/books/*", "GetBook", testutils.Problems{{Message: "`name` field"}}},
+		{"InvalidVarName", "/v1/{book=publishers/*/books/*}", "GetBook", testutils.Problems{{Message: "`name`"}}},
+		{"NoVarName", "/v1/publishers/*/books/*", "GetBook", testutils.Problems{{Message: "`name`"}}},
 		{"Irrelevant", "/v1/{book=publishers/*/books/*}", "AcquireBook", testutils.Problems{}},
 	}
 
@@ -49,7 +49,7 @@ func TestHttpNameField(t *testing.T) {
 			`, test)
 			method := f.GetServices()[0].GetMethods()[0]
 			if diff := test.problems.SetDescriptor(method).Diff(httpNameField.Lint(f)); diff != "" {
-				t.Errorf(diff)
+				t.Error(diff)
 			}
 		})
 	}

rules/aip0134/http_uri_name.go

@@ -18,7 +18,6 @@ import (
 	"fmt"
 
 	"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"
 	"github.com/stoewer/go-strcase"
@@ -31,18 +30,6 @@ var httpNameField = &lint.MethodRule{
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
 		fieldName := strcase.SnakeCase(m.GetName()[6:])
 		want := fmt.Sprintf("%s.name", fieldName)
-
-		// Establish that the RPC has expected HTTP pattern.
-		for _, httpRule := range utils.GetHTTPRules(m) {
-			if _, ok := httpRule.GetVariables()[want]; !ok {
-				return []lint.Problem{{
-					Message:    fmt.Sprintf("Update methods should include the `%s` field in the URI.", want),
-					Descriptor: m,
-					Location:   locations.MethodHTTPRule(m),
-				}}
-			}
-		}
-
-		return nil
+		return utils.LintHTTPURIHasVariable(m, want)
 	},
 }

rules/aip0134/http_uri_name_test.go

@@ -30,15 +30,15 @@ func TestHttpNameField(t *testing.T) {
 		{"Valid", "/v1/{big_book.name=publishers/*/books/*}", "UpdateBigBook", nil},
 		{"ValidWithNumber", "/v1/{dv360.name=publishers/*/dv360s/*}", "UpdateDv360", nil},
 		{"InvalidNoUnderscore", "/v1/{bigbook.name=publishers/*/books/*}",
-			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name` field"}}},
+			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name`"}}},
 		{"InvalidVarNameBook", "/v1/{big_book=publishers/*/books/*}",
-			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name` field"}}},
+			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name`"}}},
 		{"InvalidVarNameName", "/v1/{name=publishers/*/books/*}",
-			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name` field"}}},
+			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name`"}}},
 		{"InvalidVarNameReversed", "/v1/{name.big_book=publishers/*/books/*}",
-			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name` field"}}},
+			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name`"}}},
 		{"NoVarName", "/v1/publishers/*/books/*",
-			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name` field"}}},
+			"UpdateBigBook", testutils.Problems{{Message: "`big_book.name`"}}},
 		{"Irrelevant", "/v1/{book=publishers/*/books/*}",
 			"AcquireBigBook", nil},
 	}
@@ -59,7 +59,7 @@ func TestHttpNameField(t *testing.T) {
 			`, test)
 			method := f.GetServices()[0].GetMethods()[0]
 			if diff := test.problems.SetDescriptor(method).Diff(httpNameField.Lint(f)); diff != "" {
-				t.Errorf(diff)
+				t.Error(diff)
 			}
 		})
 	}

rules/aip0135/http_uri_name.go

@@ -16,27 +16,12 @@ package aip0135
 
 import (
 	"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"
 )
 
 // Delete methods should have a proper HTTP pattern.
 var httpNameField = &lint.MethodRule{
-	Name:   lint.NewRuleName(135, "http-uri-name"),
-	OnlyIf: isDeleteMethod,
-	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		// Establish that the RPC has no HTTP body.
-		for _, httpRule := range utils.GetHTTPRules(m) {
-			if _, ok := httpRule.GetVariables()["name"]; !ok {
-				return []lint.Problem{{
-					Message:    "Delete methods should include the `name` field in the URI.",
-					Descriptor: m,
-					Location:   locations.MethodHTTPRule(m),
-				}}
-			}
-		}
-
-		return nil
-	},
+	Name:       lint.NewRuleName(135, "http-uri-name"),
+	OnlyIf:     isDeleteMethod,
+	LintMethod: utils.LintHTTPURIHasNameVariable,
 }

rules/aip0135/http_uri_name_test.go

@@ -29,8 +29,8 @@ func TestHttpNameField(t *testing.T) {
 	}{
 		{"Valid", "/v1/{name=publishers/*/books/*}", "DeleteBook", nil},
 		{"ValidRevision", "/v1/{name=publishers/*/books/*}:deleteRevision", "DeleteBookRevision", nil},
-		{"InvalidVarName", "/v1/{book=publishers/*/books/*}", "DeleteBook", testutils.Problems{{Message: "`name` field"}}},
-		{"NoVarName", "/v1/publishers/*/books/*", "DeleteBook", testutils.Problems{{Message: "`name` field"}}},
+		{"InvalidVarName", "/v1/{book=publishers/*/books/*}", "DeleteBook", testutils.Problems{{Message: "`name`"}}},
+		{"NoVarName", "/v1/publishers/*/books/*", "DeleteBook", testutils.Problems{{Message: "`name`"}}},
 		{"Irrelevant", "/v1/{book=publishers/*/books/*}", "AcquireBook", nil},
 	}
 
@@ -50,7 +50,7 @@ func TestHttpNameField(t *testing.T) {
 			`, test)
 			method := f.GetServices()[0].GetMethods()[0]
 			if diff := test.problems.SetDescriptor(method).Diff(httpNameField.Lint(f)); diff != "" {
-				t.Errorf(diff)
+				t.Error(diff)
 			}
 		})
 	}

rules/aip0165/http_parent_variable.go

@@ -16,7 +16,6 @@ package aip0165
 
 import (
 	"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"
 )
@@ -27,17 +26,5 @@ var httpParentVariable = &lint.MethodRule{
 	OnlyIf: func(m *desc.MethodDescriptor) bool {
 		return isPurgeMethod(m) && m.GetInputType().FindFieldByName("parent") != nil
 	},
-	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		for _, httpRule := range utils.GetHTTPRules(m) {
-			if _, ok := httpRule.GetVariables()["parent"]; !ok {
-				return []lint.Problem{{
-					Message:    "Purge methods should include the `parent` field in the URI.",
-					Descriptor: m,
-					Location:   locations.MethodHTTPRule(m),
-				}}
-			}
-		}
-
-		return nil
-	},
+	LintMethod: utils.LintHTTPURIHasParentVariable,
 }

rules/aip0165/http_parent_variable_test.go

@@ -29,8 +29,8 @@ func TestHTTPParentVariable(t *testing.T) {
 		problems   testutils.Problems
 	}{
 		{"Valid", "/v1/{parent=publishers/*/books/*}", "PurgeBooks", "string parent = 1;", nil},
-		{"InvalidVarParent", "/v1/{book=publishers/*/books/*}", "PurgeBooks", "string parent = 1;", testutils.Problems{{Message: "`parent` field"}}},
-		{"NoVarParent", "/v1/publishers/*/books/*", "PurgeBooks", "string parent = 1;", testutils.Problems{{Message: "`parent` field"}}},
+		{"InvalidVarParent", "/v1/{book=publishers/*/books/*}", "PurgeBooks", "string parent = 1;", testutils.Problems{{Message: "`parent`"}}},
+		{"NoVarParent", "/v1/publishers/*/books/*", "PurgeBooks", "string parent = 1;", testutils.Problems{{Message: "`parent`"}}},
 		{"NoParent", "/v1/books/*", "PurgeBooks", "", nil},
 		{"Irrelevant", "/v1/{book=publishers/*/books/*}", "BuildBook", "string parent = 1;", nil},
 	}

rules/internal/utils/common_lints.go

@@ -169,14 +169,26 @@ func LintMethodHasMatchingResponseName(m *desc.MethodDescriptor) []lint.Problem
 // LintHTTPURIHasParentVariable returns a problem if any of the given method's HTTP rules do not
 // have a parent variable in the URI.
 func LintHTTPURIHasParentVariable(m *desc.MethodDescriptor) []lint.Problem {
+	return LintHTTPURIHasVariable(m, "parent")
+}
+
+// LintHTTPURIHasVariable returns a problem if any of the given method's HTTP rules do not
+// have the given variable in the URI.
+func LintHTTPURIHasVariable(m *desc.MethodDescriptor, v string) []lint.Problem {
 	for _, httpRule := range GetHTTPRules(m) {
-		if _, ok := httpRule.GetVariables()["parent"]; !ok {
+		if _, ok := httpRule.GetVariables()[v]; !ok {
 			return []lint.Problem{{
-				Message:    "HTTP URI should include a `parent` variable.",
+				Message:    fmt.Sprintf("HTTP URI should include a `%s` variable.", v),
 				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 {
+	return LintHTTPURIHasVariable(m, "name")
+}