Skip to content

Commit f182a25

Browse files
authored
feat(AIP-160): Validate filter field type (#1524)
Validates that the field for filtering, called "filter," is of type string.
1 parent 73e4f83 commit f182a25

File tree

4 files changed

+203
-0
lines changed

4 files changed

+203
-0
lines changed
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
---
2+
rule:
3+
aip: 160
4+
name: [core, '0160', filter-field-type]
5+
summary: |
6+
The filtering field on List and custom method request messages,
7+
"filter" must be a string.
8+
permalink: /160/filter-field-type
9+
redirect_from:
10+
- /0160/filter-field-type
11+
---
12+
13+
# Filtering: filter field type
14+
15+
This rule enforces that the field used for filtering, called `filter`,
16+
is of type `string`, as mandated by [AIP-160][].
17+
18+
## Details
19+
20+
This rule looks at the request message for List methods and custom
21+
methods. Then, it verifes that any field called `filter` is of type
22+
`string`.
23+
24+
## Examples
25+
26+
**Incorrect** code for this rule:
27+
28+
```proto
29+
// Incorrect.
30+
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
31+
bytes filter = 1;
32+
}
33+
```
34+
35+
```proto
36+
// Incorrect.
37+
rpc CheckoutBook(CheckoutBookRequest) returns (CheckoutBookResponse) {
38+
BookFilters filter = 1;
39+
40+
message BookFilters {
41+
string publisher = 1;
42+
repeated string authors = 2;
43+
}
44+
}
45+
```
46+
47+
**Correct** code for this rule:
48+
49+
```proto
50+
// Correct.
51+
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
52+
string filter = 1;
53+
}
54+
```
55+
56+
```proto
57+
// Correct.
58+
rpc CheckoutBook(CheckoutBookRequest) returns (CheckoutBookResponse) {
59+
string filter = 1;
60+
}
61+
```
62+
63+
## Disabling
64+
65+
If you need to violate this rule, use a leading comment above the method.
66+
Remember to also include an [aip.dev/not-precedent][] comment explaining why.
67+
68+
```proto
69+
// (-- api-linter: core::0160::filter-field-type=disabled
70+
// aip.dev/not-precedent: We need to do this because reasons. --)
71+
rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {
72+
bytes filter = 1;
73+
}
74+
```
75+
76+
If you need to violate this rule for an entire file, place the comment at the
77+
top of the file.
78+
79+
[aip-160]: https://aip.dev/160
80+
[aip.dev/not-precedent]: https://aip.dev/not-precedent
81+

rules/aip0160/aip0160.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,6 @@ func AddRules(r lint.RuleRegistry) error {
2424
return r.Register(
2525
160,
2626
filterFieldName,
27+
filterFieldType,
2728
)
2829
}

rules/aip0160/filter_field_type.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package aip0160
16+
17+
import (
18+
"fmt"
19+
20+
"github.com/googleapis/api-linter/lint"
21+
"github.com/googleapis/api-linter/locations"
22+
"github.com/googleapis/api-linter/rules/internal/utils"
23+
"github.com/jhump/protoreflect/desc"
24+
)
25+
26+
const (
27+
typeMessageFmt = `Guidance: use the field, "string filter", not "%s filter"`
28+
typeSuggestion = "string"
29+
)
30+
31+
var filterFieldType = &lint.MethodRule{
32+
Name: lint.NewRuleName(160, "filter-field-type"),
33+
OnlyIf: func(m *desc.MethodDescriptor) bool {
34+
return utils.IsListMethod(m) || utils.IsCustomMethod(m)
35+
},
36+
LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
37+
var problems []lint.Problem
38+
for _, f := range m.GetInputType().GetFields() {
39+
if f.GetName() == "filter" && utils.GetTypeName(f) != "string" {
40+
problems = append(problems, lint.Problem{
41+
Message: fmt.Sprintf(typeMessageFmt, utils.GetTypeName(f)),
42+
Descriptor: f,
43+
Location: locations.FieldType(f),
44+
Suggestion: typeSuggestion,
45+
})
46+
}
47+
}
48+
return problems
49+
},
50+
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package aip0160
16+
17+
import (
18+
"fmt"
19+
"testing"
20+
21+
"github.com/googleapis/api-linter/lint"
22+
"github.com/googleapis/api-linter/rules/internal/testutils"
23+
)
24+
25+
func TestFiltersFieldType(t *testing.T) {
26+
tests := []struct {
27+
testName string
28+
FieldType string
29+
FieldName string
30+
MethodName string
31+
wantErr bool
32+
}{
33+
{"Valid", "string", "filter", "ListBooks", false},
34+
{"Skip not filters", "string", "filters", "ListBooks", false},
35+
{"Invalid different native type List method", "bytes", "filter", "ListBooks", true},
36+
{"Invalid different message type List method", "BookFilters", "filter", "ListBooks", true},
37+
{"Invalid different native type custom method", "bytes", "filter", "SearchBooks", true},
38+
{"Invalid different message type custom method", "BookFilters", "filter", "SearchBooks", true},
39+
}
40+
for _, test := range tests {
41+
t.Run(test.testName, func(t *testing.T) {
42+
file := testutils.ParseProto3Tmpl(t, `
43+
service Library {
44+
rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.MethodName}}Response) {}
45+
rpc GetBook(GetBookRequest) returns (Book) {}
46+
}
47+
message {{.MethodName}}Request {
48+
{{.FieldType}} {{.FieldName}} = 1;
49+
}
50+
message {{.MethodName}}Response {}
51+
52+
message GetBookRequest {}
53+
message Book { string filters = 1; }
54+
55+
message BookFilters {}
56+
`, test)
57+
field := file.GetMessageTypes()[0].GetFields()[0]
58+
wantProblems := testutils.Problems{}
59+
if test.wantErr {
60+
wantProblems = append(wantProblems, lint.Problem{
61+
Message: fmt.Sprintf(`"string filter", not "%s %s`, test.FieldType, test.FieldName),
62+
Suggestion: "string",
63+
Descriptor: field,
64+
})
65+
}
66+
if diff := wantProblems.Diff(filterFieldType.Lint(file)); diff != "" {
67+
t.Error(diff)
68+
}
69+
})
70+
}
71+
}

0 commit comments

Comments
 (0)