feat: Require show_deleted List request field to be a singular bool (#…

…831)
googleapis · Apr 7, 2021 · 2c6fcae · 2c6fcae
1 parent 49bfa7e
commit 2c6fcae
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 9 deletions.
diff --git a/docs/rules/0132/request-field-types.md b/docs/rules/0132/request-field-types.md
@@ -21,6 +21,7 @@ type:
 
 - `string filter`
 - `string order_by`
+- `bool show_deleted`
 
 ## Examples
 

diff --git a/docs/rules/0132/request-unknown-fields.md b/docs/rules/0132/request-unknown-fields.md
@@ -23,7 +23,7 @@ comes across any fields other than:
 - `string page_token` ([AIP-158][])
 - `string filter` ([AIP-132][])
 - `string order_by` ([AIP-132][])
-- `bool show_deleted` ([AIP-135][])
+- `bool show_deleted` ([AIP-132][])
 - `google.protobuf.FieldMask read_mask` ([AIP-157][])
 - `View view` ([AIP-157][])
 

diff --git a/rules/aip0132/request_field_types.go b/rules/aip0132/request_field_types.go
@@ -15,19 +15,24 @@
 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"
 )
 
-var knownFields = stringset.New("filter", "order_by")
+var knownFields = map[string]func(*desc.FieldDescriptor) []lint.Problem{
+	"filter":       utils.LintSingularStringField,
+	"order_by":     utils.LintSingularStringField,
+	"show_deleted": utils.LintSingularBoolField,
+}
 
-// List request filter and order_by fields should be singular strings.
+// List fields should have the correct type.
 var requestFieldTypes = &lint.FieldRule{
 	Name: lint.NewRuleName(132, "request-field-types"),
 	OnlyIf: func(f *desc.FieldDescriptor) bool {
-		return isListRequestMessage(f.GetOwner()) && knownFields.Contains(f.GetName())
+		return isListRequestMessage(f.GetOwner()) && knownFields[f.GetName()] != nil
+	},
+	LintField: func(f *desc.FieldDescriptor) []lint.Problem {
+		return knownFields[f.GetName()](f)
 	},
-	LintField: utils.LintSingularStringField,
 }
diff --git a/rules/aip0132/request_field_types_test.go b/rules/aip0132/request_field_types_test.go
@@ -34,6 +34,9 @@ func TestRequestFieldTypes(t *testing.T) {
 		{"OrderBy", "ListBooksRequest", "string order_by", nil},
 		{"OrderByInvalid", "ListBooksRequest", "bytes order_by", testutils.Problems{{Message: "singular string", Suggestion: "string"}}},
 		{"OrderByInvalidRepeated", "ListBooksRequest", "repeated string order_by", testutils.Problems{{Message: "singular string", Suggestion: "string"}}},
+		{"ShowDeleted", "ListBooksRequest", "bool show_deleted", nil},
+		{"ShowDeletedInvalid", "ListBooksRequest", "int32 show_deleted", testutils.Problems{{Message: "singular bool", Suggestion: "bool"}}},
+		{"ShowDeletedInvalidRepeated", "ListBooksRequest", "repeated bool show_deleted", testutils.Problems{{Message: "singular bool", Suggestion: "bool"}}},
 		{"IrrelevantMessage", "Book", "bytes order_by", nil},
 		{"IrrelevantField", "ListBooksRequest", "bytes foo", nil},
 	}

diff --git a/rules/internal/utils/common_lints.go b/rules/internal/utils/common_lints.go
@@ -37,17 +37,26 @@ func LintFieldPresent(m *desc.MessageDescriptor, field string) (*desc.FieldDescr
 
 // LintSingularStringField returns a problem if the field is not a singular string.
 func LintSingularStringField(f *desc.FieldDescriptor) []lint.Problem {
-	if f.GetType() != builder.FieldTypeString().GetType() || f.IsRepeated() {
+	return lintSingularField(f, builder.FieldTypeString(), "string")
+}
+
+func lintSingularField(f *desc.FieldDescriptor, t *builder.FieldType, want string) []lint.Problem {
+	if f.GetType() != t.GetType() || f.IsRepeated() {
 		return []lint.Problem{{
-			Message:    fmt.Sprintf("The `%s` field must be a singular string.", f.GetName()),
-			Suggestion: "string",
+			Message:    fmt.Sprintf("The `%s` field must be a singular %s.", f.GetName(), want),
+			Suggestion: want,
 			Descriptor: f,
 			Location:   locations.FieldType(f),
 		}}
 	}
 	return nil
 }
 
+// LintSingularBoolField returns a problem if the field is not a singular bool.
+func LintSingularBoolField(f *desc.FieldDescriptor) []lint.Problem {
+	return lintSingularField(f, builder.FieldTypeBool(), "bool")
+}
+
 // LintFieldPresentAndSingularString returns a problem if a message does not have the given singular-string field.
 func LintFieldPresentAndSingularString(field string) func(*desc.MessageDescriptor) []lint.Problem {
 	return func(m *desc.MessageDescriptor) []lint.Problem {