Skip to content

Commit

Permalink
fix(AIP-142): only lint name of timestamp fields (#1189)
Browse files Browse the repository at this point in the history
[Internal feedback](http://b/290222132) reported this finding as not useful on a field like `string updated_value`. I agree, it does not make sense for this rule to be linting the name of non-timestamp fields. The `time-field-type` rule already catches fields-like-timestamps that aren't typed as such, which when fixed would result in this rule being fired.
  • Loading branch information
noahdietz authored Jul 9, 2023
1 parent c0bef9d commit 08fc388
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
8 changes: 8 additions & 0 deletions rules/aip0142/aip0142.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package aip0142

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

// AddRules adds all of the AIP-142 rules to the provided registry.
Expand All @@ -27,3 +29,9 @@ func AddRules(r lint.RuleRegistry) error {
fieldType,
)
}

// isTimestamp simply returns if the field is of type google.protobuf.Timestamp
// or not.
func isTimestamp(f *desc.FieldDescriptor) bool {
return utils.GetTypeName(f) == "google.protobuf.Timestamp"
}
10 changes: 3 additions & 7 deletions rules/aip0142/time_field_names.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,14 @@ package aip0142
import (
"strings"

"bitbucket.org/creachadair/stringset"
"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 fieldNames = &lint.FieldRule{
Name: lint.NewRuleName(142, "time-field-names"),
OnlyIf: func(f *desc.FieldDescriptor) bool {
return stringset.New("google.protobuf.Timestamp", "int32", "int64", "string").Contains(utils.GetTypeName(f))
},
Name: lint.NewRuleName(142, "time-field-names"),
OnlyIf: isTimestamp,
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
// Look for common non-imperative terms.
mistakes := map[string]string{
Expand All @@ -49,7 +45,7 @@ var fieldNames = &lint.FieldRule{
}

// Look for timestamps that do not end in `_time` or, if repeated, `_times`.
if utils.GetTypeName(f) == "google.protobuf.Timestamp" && !strings.HasSuffix(f.GetName(), "_time") {
if !strings.HasSuffix(f.GetName(), "_time") {
if !f.IsRepeated() {
return []lint.Problem{{
Message: "Timestamp fields should end in `_time`.",
Expand Down
2 changes: 1 addition & 1 deletion rules/aip0142/time_field_names_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func TestFieldName(t *testing.T) {
{"ValidSuffixRepeatedPlural", "repeated google.protobuf.Timestamp", "sample_times", testutils.Problems{}},
{"ValidSuffixRepeated", "repeated google.protobuf.Timestamp", "sample_time", testutils.Problems{}},
{"InvalidNoSuffixRepeated", "repeated google.protobuf.Timestamp", "create", testutils.Problems{{Message: "should end"}}},
{"InvalidIsTypeMistake", "int32", "created", testutils.Problems{{Suggestion: "create_time"}}},
{"SkipNonTimestamp", "int32", "created", testutils.Problems{}},
{"IrrelevantWeirdType", "bytes", "created", nil},
}
for _, test := range tests {
Expand Down

0 comments on commit 08fc388

Please sign in to comment.