Skip to content

Commit 0fb5eb7

Browse files
fix(rules): evaluate method synonyms on word boundaries (#1643)
* fix(rules): evaluate method synonyms on word boundaries The synonyms rules in AIP-131, AIP-133, and AIP-134 were incorrectly flagging method names like "ReadyPackage", "ReadingRoom", "PosterBoy", or "MakerSpace" because they only checked if the name started with the synonym prefix ("Read", "Post", "Make"). This change introduces a shared helper utils.HasWordBoundaryPrefix in rules/internal/utils/casing.go and uses it across AIP-131, AIP-133, and AIP-134, detecting word boundaries by checking if the character immediately following the prefix is uppercase (indicating a new word in UpperCamelCase) or if the identifier equals the prefix. Fixes b/555805873 * Apply suggestion from @gemini-code-assist[bot] Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 435ae69 commit 0fb5eb7

7 files changed

Lines changed: 143 additions & 19 deletions

File tree

rules/aip0131/synonyms.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"github.com/googleapis/api-linter/v2/lint"
2222
"github.com/googleapis/api-linter/v2/locations"
23+
"github.com/googleapis/api-linter/v2/rules/internal/utils"
2324
"google.golang.org/protobuf/reflect/protoreflect"
2425
)
2526

@@ -29,7 +30,7 @@ var synonyms = &lint.MethodRule{
2930
LintMethod: func(m protoreflect.MethodDescriptor) []lint.Problem {
3031
name := string(m.Name())
3132
for _, syn := range []string{"Acquire", "Fetch", "Lookup", "Read", "Retrieve"} {
32-
if strings.HasPrefix(name, syn) {
33+
if utils.HasWordBoundaryPrefix(name, syn) {
3334
return []lint.Problem{{
3435
Message: fmt.Sprintf(
3536
`%q can be a synonym for "Get". Should this be a Get method?`,

rules/aip0131/synonyms_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ func TestSynonyms(t *testing.T) {
3131
{"LookupBook", testutils.Problems{{Suggestion: "GetBook"}}},
3232
{"ReadBook", testutils.Problems{{Suggestion: "GetBook"}}},
3333
{"RetrieveBook", testutils.Problems{{Suggestion: "GetBook"}}},
34+
{"ReadyPackage", testutils.Problems{}},
35+
{"ReadingRoom", testutils.Problems{}},
36+
{"LookupsReport", testutils.Problems{}},
37+
{"AcquirerAccount", testutils.Problems{}},
38+
{"FetchingDog", testutils.Problems{}},
39+
{"RetrieverConfig", testutils.Problems{}},
3440
}
3541
for _, test := range tests {
3642
file := testutils.ParseProto3Tmpl(t, `

rules/aip0133/synonyms.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"github.com/googleapis/api-linter/v2/lint"
2222
"github.com/googleapis/api-linter/v2/locations"
23+
"github.com/googleapis/api-linter/v2/rules/internal/utils"
2324
"google.golang.org/protobuf/reflect/protoreflect"
2425
)
2526

@@ -29,7 +30,7 @@ var synonyms = &lint.MethodRule{
2930
LintMethod: func(m protoreflect.MethodDescriptor) []lint.Problem {
3031
name := string(m.Name())
3132
for _, syn := range []string{"Insert", "Make", "Post"} {
32-
if strings.HasPrefix(name, syn) {
33+
if utils.HasWordBoundaryPrefix(name, syn) {
3334
return []lint.Problem{{
3435
Message: fmt.Sprintf(
3536
`%q can be a synonym for "Create". Should this be a Create method?`,

rules/aip0133/synonyms_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ func TestSynonyms(t *testing.T) {
2929
{"InsertBook", testutils.Problems{{Suggestion: "CreateBook"}}},
3030
{"MakeBook", testutils.Problems{{Suggestion: "CreateBook"}}},
3131
{"PostBook", testutils.Problems{{Suggestion: "CreateBook"}}},
32+
{"PosterBoy", testutils.Problems{}},
33+
{"MakerSpace", testutils.Problems{}},
34+
{"InsertionSort", testutils.Problems{}},
3235
}
3336
for _, test := range tests {
3437
t.Run(test.MethodName, func(t *testing.T) {

rules/aip0134/synonyms.go

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ package aip0134
1717
import (
1818
"fmt"
1919
"strings"
20-
"unicode"
2120

2221
"github.com/googleapis/api-linter/v2/lint"
2322
"github.com/googleapis/api-linter/v2/locations"
23+
"github.com/googleapis/api-linter/v2/rules/internal/utils"
2424
"google.golang.org/protobuf/reflect/protoreflect"
2525
)
2626

@@ -33,21 +33,16 @@ var synonyms = &lint.MethodRule{
3333
LintMethod: func(m protoreflect.MethodDescriptor) []lint.Problem {
3434
name := string(m.Name())
3535
for _, syn := range []string{"Patch", "Put", "Set"} {
36-
if strings.HasPrefix(name, syn) {
37-
synLen := len(syn)
38-
nameLen := len(name)
39-
// Check for word boundary: either exact match or next char is uppercase
40-
if nameLen == synLen || unicode.IsUpper(rune(name[synLen])) {
41-
return []lint.Problem{{
42-
Message: fmt.Sprintf(
43-
`%q can be a synonym for "Update". Should this be a Update method?`,
44-
syn,
45-
),
46-
Descriptor: m,
47-
Location: locations.DescriptorName(m),
48-
Suggestion: strings.Replace(name, syn, "Update", 1),
49-
}}
50-
}
36+
if utils.HasWordBoundaryPrefix(name, syn) {
37+
return []lint.Problem{{
38+
Message: fmt.Sprintf(
39+
`%q can be a synonym for "Update". Should this be an Update method?`,
40+
syn,
41+
),
42+
Descriptor: m,
43+
Location: locations.DescriptorName(m),
44+
Suggestion: strings.Replace(name, syn, "Update", 1),
45+
}}
5146
}
5247
}
5348
return nil

rules/internal/utils/casing.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,29 @@
1414

1515
package utils
1616

17+
import (
18+
"strings"
19+
"unicode"
20+
"unicode/utf8"
21+
)
22+
23+
// HasWordBoundaryPrefix reports whether the UpperCamelCase identifier s begins with
24+
// prefix on a word boundary (i.e. s equals prefix or the character immediately
25+
// following prefix is uppercase).
26+
//
27+
// Both s and prefix are case-sensitive. If prefix is empty or s does not start with
28+
// prefix, it returns false.
29+
func HasWordBoundaryPrefix(s, prefix string) bool {
30+
if prefix == "" || !strings.HasPrefix(s, prefix) {
31+
return false
32+
}
33+
if len(s) == len(prefix) {
34+
return true
35+
}
36+
r, _ := utf8.DecodeRuneInString(s[len(prefix):])
37+
return unicode.IsUpper(r)
38+
}
39+
1740
// ToUpperCamelCase returns the UpperCamelCase of a string, including removing
1841
// delimiters (_,-,., ) and using them to denote a new word.
1942
func ToUpperCamelCase(s string) string {

rules/internal/utils/casing_test.go

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,102 @@ func TestToUpperCamelCase(t *testing.T) {
132132
t.Run(test.name, func(t *testing.T) {
133133
got := ToUpperCamelCase(test.input)
134134
if got != test.want {
135-
t.Errorf("ToLowerCamelCase(%q) = %q, got %q", test.input, test.want, got)
135+
t.Errorf("ToUpperCamelCase(%q) = %q, got %q", test.input, test.want, got)
136+
}
137+
})
138+
}
139+
}
140+
141+
func TestHasWordBoundaryPrefix(t *testing.T) {
142+
for _, test := range []struct {
143+
name string
144+
s string
145+
prefix string
146+
want bool
147+
}{
148+
{
149+
name: "ExactMatch",
150+
s: "Read",
151+
prefix: "Read",
152+
want: true,
153+
},
154+
{
155+
name: "WordBoundaryUppercase",
156+
s: "ReadBook",
157+
prefix: "Read",
158+
want: true,
159+
},
160+
{
161+
name: "WordBoundaryAcronym",
162+
s: "ReadHTTPStream",
163+
prefix: "Read",
164+
want: true,
165+
},
166+
{
167+
name: "NonBoundaryLowercaseSuffix",
168+
s: "ReadyPackage",
169+
prefix: "Read",
170+
want: false,
171+
},
172+
{
173+
name: "NonBoundaryIngSuffix",
174+
s: "ReadingRoom",
175+
prefix: "Read",
176+
want: false,
177+
},
178+
{
179+
name: "DifferentPrefix",
180+
s: "GetBook",
181+
prefix: "Read",
182+
want: false,
183+
},
184+
{
185+
name: "ShorterThanPrefix",
186+
s: "Re",
187+
prefix: "Read",
188+
want: false,
189+
},
190+
{
191+
name: "EmptyString",
192+
s: "",
193+
prefix: "Read",
194+
want: false,
195+
},
196+
{
197+
name: "EmptyPrefix",
198+
s: "Read",
199+
prefix: "",
200+
want: false,
201+
},
202+
{
203+
name: "BothEmpty",
204+
s: "",
205+
prefix: "",
206+
want: false,
207+
},
208+
{
209+
name: "DigitSuffix",
210+
s: "Read2Book",
211+
prefix: "Read",
212+
want: false,
213+
},
214+
{
215+
name: "MultibyteUpper",
216+
s: "ReadÜber",
217+
prefix: "Read",
218+
want: true,
219+
},
220+
{
221+
name: "MultibyteLower",
222+
s: "Readüber",
223+
prefix: "Read",
224+
want: false,
225+
},
226+
} {
227+
t.Run(test.name, func(t *testing.T) {
228+
got := HasWordBoundaryPrefix(test.s, test.prefix)
229+
if got != test.want {
230+
t.Errorf("HasWordBoundaryPrefix(%q, %q) = %v, want %v", test.s, test.prefix, got, test.want)
136231
}
137232
})
138233
}

0 commit comments

Comments
 (0)