Repository: googleapis/api-linter
Pull request #576 (2020-06-26 20:03:53)

Allow plural `_times` suffix or singular `_time` suffix for repeated Timestamps.

klmilam: I haven't proposed a change to the AIP. Should I do so?

I originally proposed this change in cl/318410399 and Luke requested that I make the change here.
noahdietz: > I haven't proposed a change to the AIP. Should I do so?

Yeah, I think it's the right thing to do, especially for API reviewers that might not be aware that this is now allowed, they will see it when reviewing the AIP. 

https://github.com/googleapis/aip

klmilam: Opened PR https://github.com/googleapis/aip/pull/539

Patch:
diff --git a/docs/rules/0142/time-field-names.md b/docs/rules/0142/time-field-names.md
index c16f5a9..aff03d3 100644
--- a/docs/rules/0142/time-field-names.md
+++ b/docs/rules/0142/time-field-names.md
@@ -16,7 +16,8 @@ a `_time` suffix, as mandated in [AIP-142][].
 ## Details
 
 This rule looks at each `google.protobuf.Timestamp` field and ensures that it
-has a `_time` suffix.
+has a `_time` suffix. If it is a repeated field, the `_times` suffix is also
+allowed.
 
 It also looks for common field names, regardless of type, and complains if they
 are used. These are:
@@ -35,6 +36,7 @@ are used. These are:
 message Book {
   string name = 1;
   google.protobuf.Timestamp published = 2;  // Should be `publish_time`.
+  repeated google.protobuf.Timestamp updated = 3; // Should be `update_time` or `update_times`.
 }
 ```
 
@@ -45,6 +47,7 @@ message Book {
 message Book {
   string name = 1;
   google.protobuf.Timestamp publish_time = 2;
+  repeated google.protobuf.Timestamp update_times = 3;
 }
 ```
 
diff --git a/rules/aip0142/time_field_names.go b/rules/aip0142/time_field_names.go
index c7eb16e..5a87ed0 100644
--- a/rules/aip0142/time_field_names.go
+++ b/rules/aip0142/time_field_names.go
@@ -44,13 +44,21 @@ var fieldNames = &lint.FieldRule{
 			}
 		}
 
-		// Look for timestamps that do not end in `_time`.
+		// 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") {
-			return []lint.Problem{{
-				Message:    "Timestamp fields should end in `_time`.",
-				Descriptor: f,
-				Location:   locations.DescriptorName(f),
-			}}
+			if !f.IsRepeated() {
+				return []lint.Problem{{
+					Message:    "Timestamp fields should end in `_time`.",
+					Descriptor: f,
+					Location:   locations.DescriptorName(f),
+				}}
+			} else if !strings.HasSuffix(f.GetName(), "_times") {
+				return []lint.Problem{{
+					Message:    "Repeated Timestamp fields should end in `_time` or `_times`.",
+					Descriptor: f,
+					Location:   locations.DescriptorName(f),
+				}}
+			}
 		}
 
 		return nil


Test patch:
diff --git a/rules/aip0142/time_field_names_test.go b/rules/aip0142/time_field_names_test.go
index d3b41da..c1cf7d9 100644
--- a/rules/aip0142/time_field_names_test.go
+++ b/rules/aip0142/time_field_names_test.go
@@ -31,6 +31,9 @@ func TestFieldName(t *testing.T) {
 		{"InvalidIsMistake", "google.protobuf.Timestamp", "created", testutils.Problems{{Suggestion: "create_time"}}},
 		{"InvalidContainsMistake", "google.protobuf.Timestamp", "last_modified", testutils.Problems{{Suggestion: "update_time"}}},
 		{"InvalidNoSuffix", "google.protobuf.Timestamp", "create", testutils.Problems{{Message: "should end"}}},
+		{"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"}}},
 	}
 	for _, test := range tests {