Skip to content

Commit 73e4f83

Browse files
shwoodardshwoodard
authored
feat(AIP-160): Validate filter field name. (#1523)
Validates that the field for filtering is called "filter" and not "filters" for List and custom methods' request messages.
1 parent 2ed9a5d commit 73e4f83

File tree

6 files changed

+255
-0
lines changed

6 files changed

+255
-0
lines changed

‎docs/rules/0160/filter-field-name.md‎

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

‎rules/aip0160/aip0160.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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 contains rules defined in https://aip.dev/160.
16+
package aip0160
17+
18+
import (
19+
"github.com/googleapis/api-linter/lint"
20+
)
21+
22+
// AddRules adds all of the AIP-160 rules to the provided registry.
23+
func AddRules(r lint.RuleRegistry) error {
24+
return r.Register(
25+
160,
26+
filterFieldName,
27+
)
28+
}

‎rules/aip0160/aip0160_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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+
"testing"
19+
20+
"github.com/googleapis/api-linter/lint"
21+
)
22+
23+
func TestAddRules(t *testing.T) {
24+
if err := AddRules(lint.NewRuleRegistry()); err != nil {
25+
t.Errorf("AddRules got an error: %v", err)
26+
}
27+
}

‎rules/aip0160/filter_field_name.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+
nameMessageFmt = `Guidance: use the field, "string filter", not "string %s"`
28+
nameSuggestion = "filter"
29+
)
30+
31+
var filterFieldName = &lint.MethodRule{
32+
Name: lint.NewRuleName(160, "filter-field-name"),
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() == "filters" {
40+
problems = append(problems, lint.Problem{
41+
Message: fmt.Sprintf(nameMessageFmt, f.GetName()),
42+
Descriptor: f,
43+
Location: locations.DescriptorName(f),
44+
Suggestion: nameSuggestion,
45+
})
46+
}
47+
}
48+
return problems
49+
},
50+
}

‎rules/aip0160/filter_field_name_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
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 TestFiltersFieldName(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+
{"Valid different native type List method", "bytes", "filter", "ListBooks", false},
35+
{"Valid different message type List method", "BookFilters", "filter", "ListBooks", false},
36+
{"Valid different native type custom method", "bytes", "filter", "SearchBooks", false},
37+
{"Valid different message type custom method", "BookFilters", "filter", "SearchBooks", false},
38+
{"Invalid list method", "string", "filters", "ListBooks", true},
39+
{"Invalid custom method", "string", "filters", "SearchBooks", true},
40+
}
41+
for _, test := range tests {
42+
t.Run(test.testName, func(t *testing.T) {
43+
file := testutils.ParseProto3Tmpl(t, `
44+
service Library {
45+
rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.MethodName}}Response) {}
46+
rpc GetBook(GetBookRequest) returns (Book) {}
47+
}
48+
message {{.MethodName}}Request {
49+
{{.FieldType}} {{.FieldName}} = 1;
50+
}
51+
message {{.MethodName}}Response {}
52+
53+
message GetBookRequest {}
54+
message Book { string filters = 1; }
55+
56+
message BookFilters {}
57+
`, test)
58+
field := file.GetMessageTypes()[0].GetFields()[0]
59+
wantProblems := testutils.Problems{}
60+
if test.wantErr {
61+
wantProblems = append(wantProblems, lint.Problem{
62+
Message: fmt.Sprintf(`"string filter", not "%s %s`, test.FieldType, test.FieldName),
63+
Suggestion: "filter",
64+
Descriptor: field,
65+
})
66+
}
67+
if diff := wantProblems.Diff(filterFieldName.Lint(file)); diff != "" {
68+
t.Error(diff)
69+
}
70+
})
71+
}
72+
}

‎rules/rules.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ import (
7979
"github.com/googleapis/api-linter/rules/aip0157"
8080
"github.com/googleapis/api-linter/rules/aip0158"
8181
"github.com/googleapis/api-linter/rules/aip0159"
82+
"github.com/googleapis/api-linter/rules/aip0160"
8283
"github.com/googleapis/api-linter/rules/aip0162"
8384
"github.com/googleapis/api-linter/rules/aip0163"
8485
"github.com/googleapis/api-linter/rules/aip0164"
@@ -129,6 +130,7 @@ var aipAddRulesFuncs = []addRulesFuncType{
129130
aip0157.AddRules,
130131
aip0158.AddRules,
131132
aip0159.AddRules,
133+
aip0160.AddRules,
132134
aip0162.AddRules,
133135
aip0163.AddRules,
134136
aip0164.AddRules,

0 commit comments

Comments
 (0)