Skip to content

Commit

Permalink
fix: refactor Standard Method message helpers into utils (#1212)
Browse files Browse the repository at this point in the history
  • Loading branch information
noahdietz committed Jul 31, 2023
1 parent b8a0992 commit c6b5d10
Show file tree
Hide file tree
Showing 55 changed files with 210 additions and 130 deletions.
12 changes: 0 additions & 12 deletions rules/aip0131/aip0131.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@
package aip0131

import (
"regexp"

"github.com/googleapis/api-linter/lint"
"github.com/jhump/protoreflect/desc"
)

// AddRules accepts a register function and registers each of
Expand All @@ -43,12 +40,3 @@ func AddRules(r lint.RuleRegistry) error {
unknownFields,
)
}

var (
getReqMessageRegexp = regexp.MustCompile("^Get[A-Za-z0-9]*Request$")
)

// Returns true if this is an AIP-131 Get request message, false otherwise.
func isGetRequestMessage(m *desc.MessageDescriptor) bool {
return getReqMessageRegexp.MatchString(m.GetName())
}
2 changes: 1 addition & 1 deletion rules/aip0131/request_name_behavior.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var requestNameBehavior = &lint.FieldRule{
Name: lint.NewRuleName(131, "request-name-behavior"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isGetRequestMessage(f.GetOwner()) && f.GetName() == "name"
return utils.IsGetRequestMessage(f.GetOwner()) && f.GetName() == "name"
},
LintField: utils.LintRequiredField,
}
2 changes: 1 addition & 1 deletion rules/aip0131/request_name_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
var requestNameField = &lint.FieldRule{
Name: lint.NewRuleName(131, "request-name-field"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isGetRequestMessage(f.GetOwner()) && f.GetName() == "name"
return utils.IsGetRequestMessage(f.GetOwner()) && f.GetName() == "name"
},
LintField: utils.LintSingularStringField,
}
2 changes: 1 addition & 1 deletion rules/aip0131/request_name_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var requestNameReference = &lint.FieldRule{
Name: lint.NewRuleName(131, "request-name-reference"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isGetRequestMessage(f.GetOwner()) && f.GetName() == "name"
return utils.IsGetRequestMessage(f.GetOwner()) && f.GetName() == "name"
},
LintField: utils.LintFieldResourceReference,
}
2 changes: 1 addition & 1 deletion rules/aip0131/request_name_reference_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
var requestNameReferenceType = &lint.FieldRule{
Name: lint.NewRuleName(131, "request-name-reference-type"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isGetRequestMessage(f.GetOwner()) && f.GetName() == "name" && utils.GetResourceReference(f) != nil
return utils.IsGetRequestMessage(f.GetOwner()) && f.GetName() == "name" && utils.GetResourceReference(f) != nil
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
if ref := utils.GetResourceReference(f); ref.GetType() == "" {
Expand Down
3 changes: 2 additions & 1 deletion rules/aip0131/request_name_required.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ import (
"fmt"

"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
)

// The Get standard method should have some required fields.
var requestNameRequired = &lint.MessageRule{
Name: lint.NewRuleName(131, "request-name-required"),
OnlyIf: isGetRequestMessage,
OnlyIf: utils.IsGetRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
if m.FindFieldByName("name") == nil {
return []lint.Problem{{
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0131/request_required_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
// The get request message should not have unrecognized fields.
var requestRequiredFields = &lint.MessageRule{
Name: lint.NewRuleName(131, "request-required-fields"),
OnlyIf: isGetRequestMessage,
OnlyIf: utils.IsGetRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) (problems []lint.Problem) {
// Rule check: Establish that there are no unexpected fields.
allowedRequiredFields := stringset.New("name")
Expand Down
3 changes: 2 additions & 1 deletion rules/aip0131/request_unknown_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ import (
"bitbucket.org/creachadair/stringset"

"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
)

// Get methods should not have unrecognized fields.
var unknownFields = &lint.FieldRule{
Name: lint.NewRuleName(131, "request-unknown-fields"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isGetRequestMessage(f.GetOwner())
return utils.IsGetRequestMessage(f.GetOwner())
},
LintField: func(field *desc.FieldDescriptor) []lint.Problem {
allowedFields := stringset.New("name", "read_mask", "view")
Expand Down
19 changes: 0 additions & 19 deletions rules/aip0132/aip0132.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,7 @@
package aip0132

import (
"regexp"

"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/aip0162"
"github.com/jhump/protoreflect/desc"
)

// AddRules adds all of the AIP-132 rules to the provided registry.
Expand All @@ -46,18 +42,3 @@ func AddRules(r lint.RuleRegistry) error {
unknownFields,
)
}

var (
listReqMessageRegexp = regexp.MustCompile("^List[A-Za-z0-9]*Request$")
listRespMessageRegexp = regexp.MustCompile("^List([A-Za-z0-9]*)Response$")
)

// Return true if this is an AIP-132 List request message, false otherwise.
func isListRequestMessage(m *desc.MessageDescriptor) bool {
return listReqMessageRegexp.MatchString(m.GetName()) && !aip0162.IsListRevisionsRequestMessage(m)
}

// Return true if this is an AIP-132 List response message, false otherwise.
func isListResponseMessage(m *desc.MessageDescriptor) bool {
return listRespMessageRegexp.MatchString(m.GetName()) && !aip0162.IsListRevisionsResponseMessage(m)
}
2 changes: 1 addition & 1 deletion rules/aip0132/request_field_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var knownFields = map[string]func(*desc.FieldDescriptor) []lint.Problem{
var requestFieldTypes = &lint.FieldRule{
Name: lint.NewRuleName(132, "request-field-types"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isListRequestMessage(f.GetOwner()) && knownFields[f.GetName()] != nil
return utils.IsListRequestMessage(f.GetOwner()) && knownFields[f.GetName()] != nil
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
return knownFields[f.GetName()](f)
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0132/request_parent_behavior.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var requestParentBehavior = &lint.FieldRule{
Name: lint.NewRuleName(132, "request-parent-behavior"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isListRequestMessage(f.GetOwner()) && f.GetName() == "parent"
return utils.IsListRequestMessage(f.GetOwner()) && f.GetName() == "parent"
},
LintField: utils.LintRequiredField,
}
2 changes: 1 addition & 1 deletion rules/aip0132/request_parent_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
var requestParentField = &lint.FieldRule{
Name: lint.NewRuleName(132, "request-parent-field"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isListRequestMessage(f.GetOwner()) && f.GetName() == "parent"
return utils.IsListRequestMessage(f.GetOwner()) && f.GetName() == "parent"
},
LintField: utils.LintSingularStringField,
}
2 changes: 1 addition & 1 deletion rules/aip0132/request_parent_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var requestParentReference = &lint.FieldRule{
Name: lint.NewRuleName(132, "request-parent-reference"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isListRequestMessage(f.GetOwner()) && f.GetName() == "parent"
return utils.IsListRequestMessage(f.GetOwner()) && f.GetName() == "parent"
},
LintField: utils.LintFieldResourceReference,
}
2 changes: 1 addition & 1 deletion rules/aip0132/request_parent_required.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
// The List standard method should contain a parent field.
var requestParentRequired = &lint.MessageRule{
Name: lint.NewRuleName(132, "request-parent-required"),
OnlyIf: isListRequestMessage,
OnlyIf: utils.IsListRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
// Rule check: Establish that a `parent` field is present.
if m.FindFieldByName("parent") == nil {
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0132/request_parent_valid_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var requestParentValidReference = &lint.FieldRule{
Name: lint.NewRuleName(132, "request-parent-valid-reference"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
ref := utils.GetResourceReference(f)
return isListRequestMessage(f.GetOwner()) && f.GetName() == "parent" && ref != nil && ref.GetType() != ""
return utils.IsListRequestMessage(f.GetOwner()) && f.GetName() == "parent" && ref != nil && ref.GetType() != ""
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
p := f.GetParent()
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0132/request_required_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
// The list request message should not have unrecognized fields.
var requestRequiredFields = &lint.MessageRule{
Name: lint.NewRuleName(132, "request-required-fields"),
OnlyIf: isListRequestMessage,
OnlyIf: utils.IsListRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) (problems []lint.Problem) {
// Rule check: Establish that there are no unexpected fields.
allowedRequiredFields := stringset.New("parent")
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0132/request_show_deleted_required.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
var requestShowDeletedRequired = &lint.MessageRule{
Name: lint.NewRuleName(132, "request-show-deleted-required"),
OnlyIf: func(m *desc.MessageDescriptor) bool {
if !isListRequestMessage(m) {
if !utils.IsListRequestMessage(m) {
return false
}
// Check for soft-delete support by getting the resource name
Expand Down
3 changes: 2 additions & 1 deletion rules/aip0132/request_unknown_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package aip0132
import (
"bitbucket.org/creachadair/stringset"
"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
)

Expand All @@ -36,7 +37,7 @@ var allowedFields = stringset.New(
var unknownFields = &lint.FieldRule{
Name: lint.NewRuleName(132, "request-unknown-fields"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isListRequestMessage(f.GetOwner())
return utils.IsListRequestMessage(f.GetOwner())
},
LintField: func(field *desc.FieldDescriptor) []lint.Problem {
if !allowedFields.Contains(field.GetName()) {
Expand Down
6 changes: 2 additions & 4 deletions rules/aip0132/response_unknown_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
"github.com/stoewer/go-strcase"
)

// The resource itself is not included here, but also permitted.
Expand All @@ -36,12 +35,11 @@ var respAllowedFields = stringset.New(
var responseUnknownFields = &lint.FieldRule{
Name: lint.NewRuleName(132, "response-unknown-fields"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isListResponseMessage(f.GetOwner())
return utils.IsListResponseMessage(f.GetOwner())
},
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
// A repeated variant of the resource should be permitted.
msgName := f.GetOwner().GetName()
resource := strcase.SnakeCase(listRespMessageRegexp.FindStringSubmatch(msgName)[1])
resource := utils.ListResponseResourceName(f.GetOwner())
if strings.HasSuffix(resource, "_revisions") {
// This is an AIP-162 ListFooRevisions response, which is subtly
// different from an AIP-132 List response. We need to modify the RPC
Expand Down
10 changes: 0 additions & 10 deletions rules/aip0133/aip0133.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package aip0133

import (
"regexp"
"strings"

"github.com/googleapis/api-linter/lint"
Expand Down Expand Up @@ -50,15 +49,6 @@ func AddRules(r lint.RuleRegistry) error {
)
}

var (
createReqMessageRegexp = regexp.MustCompile("^Create[A-Za-z0-9]*Request$")
)

// Returns true if this is an AIP-133 Get request message, false otherwise.
func isCreateRequestMessage(m *desc.MessageDescriptor) bool {
return createReqMessageRegexp.MatchString(m.GetName())
}

// get resource message type name from method
func getResourceMsgName(m *desc.MethodDescriptor) string {
// Usually the response message will be the resource message, and its name will
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0133/request_id_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

var requestIDField = &lint.MessageRule{
Name: lint.NewRuleName(133, "request-id-field"),
OnlyIf: isCreateRequestMessage,
OnlyIf: utils.IsCreateRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
idField := strcase.SnakeCase(strings.TrimPrefix(strings.TrimSuffix(m.GetName(), "Request"), "Create")) + "_id"
if field := m.FindFieldByName(idField); field == nil || utils.GetTypeName(field) != "string" || field.IsRepeated() {
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0133/request_parent_behavior.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var requestParentBehavior = &lint.FieldRule{
Name: lint.NewRuleName(133, "request-parent-behavior"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isCreateRequestMessage(f.GetOwner()) && f.GetName() == "parent"
return utils.IsCreateRequestMessage(f.GetOwner()) && f.GetName() == "parent"
},
LintField: utils.LintRequiredField,
}
2 changes: 1 addition & 1 deletion rules/aip0133/request_parent_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
var requestParentField = &lint.FieldRule{
Name: lint.NewRuleName(133, "request-parent-field"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isCreateRequestMessage(f.GetOwner()) && f.GetName() == "parent"
return utils.IsCreateRequestMessage(f.GetOwner()) && f.GetName() == "parent"
},
LintField: utils.LintSingularStringField,
}
2 changes: 1 addition & 1 deletion rules/aip0133/request_parent_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var requestParentReference = &lint.FieldRule{
Name: lint.NewRuleName(133, "request-parent-reference"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return isCreateRequestMessage(f.GetOwner()) && f.GetName() == "parent"
return utils.IsCreateRequestMessage(f.GetOwner()) && f.GetName() == "parent"
},
LintField: utils.LintFieldResourceReference,
}
2 changes: 1 addition & 1 deletion rules/aip0133/request_parent_required.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

var requestParentRequired = &lint.MessageRule{
Name: lint.NewRuleName(133, "request-parent-required"),
OnlyIf: isCreateRequestMessage,
OnlyIf: utils.IsCreateRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
if m.FindFieldByName("parent") == nil {
// Sanity check: If the resource has a pattern, and that pattern
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0133/request_resource_behavior.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var requestResourceBehavior = &lint.FieldRule{
Name: lint.NewRuleName(133, "request-resource-behavior"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
message := f.GetOwner()
if !isCreateRequestMessage(message) {
if !utils.IsCreateRequestMessage(message) {
return false
}
resourceMsgName := getResourceMsgNameFromReq(message)
Expand Down
3 changes: 2 additions & 1 deletion rules/aip0133/request_resource_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ 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"
"github.com/stoewer/go-strcase"
)

// The create request message should have resource field.
var resourceField = &lint.MessageRule{
Name: lint.NewRuleName(133, "request-resource-field"),
OnlyIf: isCreateRequestMessage,
OnlyIf: utils.IsCreateRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
resourceMsgName := getResourceMsgNameFromReq(m)

Expand Down
3 changes: 2 additions & 1 deletion rules/aip0133/request_unknown_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strings"

"github.com/googleapis/api-linter/lint"
"github.com/googleapis/api-linter/rules/internal/utils"
"github.com/jhump/protoreflect/desc"
"github.com/jhump/protoreflect/desc/builder"
"github.com/stoewer/go-strcase"
Expand All @@ -27,7 +28,7 @@ import (
// The create request message should not have unrecognized fields.
var unknownFields = &lint.MessageRule{
Name: lint.NewRuleName(133, "request-unknown-fields"),
OnlyIf: isCreateRequestMessage,
OnlyIf: utils.IsCreateRequestMessage,
LintMessage: func(m *desc.MessageDescriptor) (problems []lint.Problem) {
resourceMsgName := getResourceMsgNameFromReq(m)

Expand Down
11 changes: 0 additions & 11 deletions rules/aip0134/aip0134.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@
package aip0134

import (
"regexp"
"strings"

"github.com/googleapis/api-linter/lint"
"github.com/jhump/protoreflect/desc"
"github.com/stoewer/go-strcase"
)

Expand All @@ -47,15 +45,6 @@ func AddRules(r lint.RuleRegistry) error {
)
}

var (
updateReqMessageRegexp = regexp.MustCompile("^Update[A-Za-z0-9]*Request$")
)

// Returns true if this is an AIP-134 Update request message, false otherwise.
func isUpdateRequestMessage(m *desc.MessageDescriptor) bool {
return updateReqMessageRegexp.MatchString(m.GetName())
}

func extractResource(reqName string) string {
// Strips "Update" from the beginning and "Request" from the end.
return reqName[6 : len(reqName)-7]
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0134/request_allow_missing_field.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var allowMissing = &lint.MessageRule{
Name: lint.NewRuleName(134, "request-allow-missing-field"),
OnlyIf: func(m *desc.MessageDescriptor) bool {
if !isUpdateRequestMessage(m) {
if !utils.IsUpdateRequestMessage(m) {
return false
}
r := utils.DeclarativeFriendlyResource(m)
Expand Down
Loading

0 comments on commit c6b5d10

Please sign in to comment.