Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
acamadeo committed Feb 7, 2024
1 parent 6ad29fb commit 016aa21
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 9 deletions.
2 changes: 1 addition & 1 deletion docs/rules/0213/common-types-fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ with a specific name that doesn't have the corresponding common type.

Some example pairings of common types and field names that are checked are:

* `google.protobuf.Duration`: "duration"
* `google.protobuf.Duration`: "delay", "duration", "timeout"
* `google.type.Color`: "color", "colour"
* `google.type.PhoneNumber`: "mobile_number", "phone", "phone_number"

Expand Down
58 changes: 50 additions & 8 deletions rules/aip0213/common_types_fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,26 @@ package aip0213

import (
"fmt"
"strings"

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

var fieldNameToCommonType = map[string]string{
"delay": "google.protobuf.Duration",
"duration": "google.protobuf.Duration",
"timeout": "google.protobuf.Duration",

"color": "google.type.Color",
"colour": "google.type.Color",

"dollars": "google.type.Money",
"euros": "google.type.Money",
"money": "google.type.Money",
"pounds": "google.type.Money",
"yen": "google.type.Money",
"yuan": "google.type.Money",
"balance": "google.type.Money",
"cost": "google.type.Money",
"fare": "google.type.Money",
"fee": "google.type.Money",
"price": "google.type.Money",
"spend": "google.type.Money",

"mobile_number": "google.type.PhoneNumber",
"phone": "google.type.PhoneNumber",
Expand All @@ -48,9 +51,10 @@ var fieldNameToCommonType = map[string]string{
var commonTypesFields = &lint.FieldRule{
Name: lint.NewRuleName(213, "common-types-fields"),
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
// Flag this field if it has a name in `fieldNameToCommonType` but
// Flag this field if it ends with a name in `fieldNameToCommonType` but
// doesn't have its corresponding type.
if messageType, ok := fieldNameToCommonType[f.GetName()]; ok {
commonFieldName := commonFieldNameSuffix(f.GetName())
if messageType, ok := fieldNameToCommonType[commonFieldName]; ok {
if f.GetMessageType() == nil || f.GetMessageType().GetFullyQualifiedName() != messageType {
return []lint.Problem{{
Message: fmt.Sprintf("Consider using the common type %q.", messageType),
Expand All @@ -61,3 +65,41 @@ var commonTypesFields = &lint.FieldRule{
return nil
},
}

// Returns the common field name if `fieldName` ends in one.
func commonFieldNameSuffix(fieldName string) string {
fieldParts := strings.Split(fieldName, "_")
for name := range fieldNameToCommonType {
nameParts := strings.Split(name, "_")
// Check if `fieldName` ends with the same underscore-separated terms as
// `name`.
if strListContains(reverseStrList(fieldParts), reverseStrList(nameParts)) {
return name
}
}
return ""
}

// Returns true if list `s` contains list `subList` in order.
func strListContains(s []string, subList []string) bool {
if len(subList) > len(s) {
return false
}
for i := 0; i < len(subList); i++ {
if s[i] != subList[i] {
return false
}
}
return true
}

// Returns list `s` in reverse order.
func reverseStrList(s []string) []string {
out := make([]string, len(s))
copy(out, s)

for i, j := 0, len(s)-1; i < j; i, j = i+1, j-1 {
out[i], out[j] = out[j], out[i]
}
return out
}
5 changes: 5 additions & 0 deletions rules/aip0213/common_types_fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,18 @@ func TestCommonTypesFields(t *testing.T) {
problems testutils.Problems
}{
{"ValidCommonType", "google.protobuf.Duration duration", nil},
{"ValidFieldEndingInCommonTerm", "google.protobuf.Duration max_duration", nil},
{"ValidFieldEndingInMultipleCommonTerms", "google.type.TimeOfDay end_time_of_day", nil},
{"ValidOtherType", "string bar", nil},
{"FieldDoesNotUseMessageType", "map<int32, int32> duration", testutils.Problems{{Message: "common type"}}},
{"FieldEndingInCommonTermDoesNotUseMessageType", "map<int32, int32> max_duration", testutils.Problems{{Message: "common type"}}},
{"FieldEndingInMultipleCommonTermsDoesNotUseMessageType", "map<int32, int32> end_time_of_day", testutils.Problems{{Message: "common type"}}},
{"FieldDoesNotUseCommonType", "int32 duration", testutils.Problems{{Message: "common type"}}},
} {
t.Run(test.name, func(t *testing.T) {
f := testutils.ParseProto3Tmpl(t, `
import "google/protobuf/duration.proto";
import "google/type/timeofday.proto";
message Foo {
{{.Field}} = 1;
Expand Down

0 comments on commit 016aa21

Please sign in to comment.