Repository: googleapis/api-linter
Pull request #1189 (2023-07-07 20:58:15)

fix(AIP-142): only lint name of timestamp fields
[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.


Patch:
diff --git a/rules/aip0142/aip0142.go b/rules/aip0142/aip0142.go
index 92fa95d..3948d01 100644
--- a/rules/aip0142/aip0142.go
+++ b/rules/aip0142/aip0142.go
@@ -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.
@@ -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"
+}
diff --git a/rules/aip0142/time_field_names.go b/rules/aip0142/time_field_names.go
index 218fe13..52c937a 100644
--- a/rules/aip0142/time_field_names.go
+++ b/rules/aip0142/time_field_names.go
@@ -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{
@@ -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`.",


Test patch:
diff --git a/rules/aip0142/time_field_names_test.go b/rules/aip0142/time_field_names_test.go
index d73658d..4898033 100644
--- a/rules/aip0142/time_field_names_test.go
+++ b/rules/aip0142/time_field_names_test.go
@@ -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 {