diff --git a/rules/aip0131/http_uri_name.go b/rules/aip0131/http_uri_name.go index f6f3a74b2..a96a2cdfa 100644 --- a/rules/aip0131/http_uri_name.go +++ b/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, } diff --git a/rules/aip0131/http_uri_name_test.go b/rules/aip0131/http_uri_name_test.go index f3942bda1..ae83b9682 100644 --- a/rules/aip0131/http_uri_name_test.go +++ b/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) } }) } diff --git a/rules/aip0134/http_uri_name.go b/rules/aip0134/http_uri_name.go index 703edf81e..e4144258b 100644 --- a/rules/aip0134/http_uri_name.go +++ b/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) }, } diff --git a/rules/aip0134/http_uri_name_test.go b/rules/aip0134/http_uri_name_test.go index bbcd83c31..cdee8468f 100644 --- a/rules/aip0134/http_uri_name_test.go +++ b/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) } }) } diff --git a/rules/aip0135/http_uri_name.go b/rules/aip0135/http_uri_name.go index 3393c8958..5bd61122e 100644 --- a/rules/aip0135/http_uri_name.go +++ b/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, } diff --git a/rules/aip0135/http_uri_name_test.go b/rules/aip0135/http_uri_name_test.go index 4edc14d8f..1d6c3ea9a 100644 --- a/rules/aip0135/http_uri_name_test.go +++ b/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) } }) } diff --git a/rules/aip0165/http_parent_variable.go b/rules/aip0165/http_parent_variable.go index cc6d2730c..02d3e430f 100644 --- a/rules/aip0165/http_parent_variable.go +++ b/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, } diff --git a/rules/aip0165/http_parent_variable_test.go b/rules/aip0165/http_parent_variable_test.go index e3cb596ac..cdab3eb46 100644 --- a/rules/aip0165/http_parent_variable_test.go +++ b/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}, } diff --git a/rules/internal/utils/common_lints.go b/rules/internal/utils/common_lints.go index 5c92d6b3c..3510f6264 100644 --- a/rules/internal/utils/common_lints.go +++ b/rules/internal/utils/common_lints.go @@ -169,10 +169,16 @@ 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), }} @@ -180,3 +186,9 @@ func LintHTTPURIHasParentVariable(m *desc.MethodDescriptor) []lint.Problem { } 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") +}