Skip to content

Commit 7dfafc4

Browse files
fix(AIP-140): restrict uri naming suggestions to fields with URI in comments (#1541)
* fix(AIP-140): restrict `uri` naming suggestions to fields with URI in comments AIP-140 was recently updated to clarify that field names may use `url` when the field can only represent a URL. The current URI lint rule for AIP-140 will unconditionally discourage using `url` in field names. This change updates the URI lint rule to be more conservative by only suggesting renaming the field to use `uri` when the field's comments use the term URI. Related update to AIP-140: aip-dev/google.aip.dev#1551 * Add a test case for a `uri` field with a comment mentioning "URI". * Add a test case for a `url` field where comment has intra-word "uri" substring --------- Co-authored-by: Santiago Quiroga <22756465+quirogas@users.noreply.github.com>
1 parent 8448403 commit 7dfafc4

File tree

2 files changed

+33
-16
lines changed

2 files changed

+33
-16
lines changed

rules/aip0140/uri.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package aip0140
1616

1717
import (
18+
"regexp"
1819
"strings"
1920

2021
"bitbucket.org/creachadair/stringset"
@@ -23,18 +24,26 @@ import (
2324
"github.com/jhump/protoreflect/desc"
2425
)
2526

27+
var uriInCommentRegexp = regexp.MustCompile(`\b(uri|URI)\b`)
28+
2629
var uri = &lint.FieldRule{
2730
Name: lint.NewRuleName(140, "uri"),
2831
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
2932
nameSegments := stringset.New(strings.Split(f.GetName(), "_")...)
30-
if nameSegments.Contains("url") {
31-
return []lint.Problem{{
32-
Message: "Use `uri` instead of `url` in field names.",
33-
Descriptor: f,
34-
Location: locations.DescriptorName(f),
35-
Suggestion: strings.ReplaceAll(f.GetName(), "url", "uri"),
36-
}}
33+
if !nameSegments.Contains("url") {
34+
return nil
35+
}
36+
37+
comment := f.GetSourceInfo().GetLeadingComments()
38+
if !uriInCommentRegexp.MatchString(comment) {
39+
return nil
3740
}
38-
return nil
41+
42+
return []lint.Problem{{
43+
Message: "Field uses `url` in field name but comment refers to URIs. Use `uri` instead of `url` in field name if field represents a URI.",
44+
Descriptor: f,
45+
Location: locations.DescriptorName(f),
46+
Suggestion: strings.ReplaceAll(f.GetName(), "url", "uri"),
47+
}}
3948
},
4049
}

rules/aip0140/uri_test.go

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,29 @@ import (
2323
func TestURI(t *testing.T) {
2424
for _, test := range []struct {
2525
name string
26+
Comment string
2627
Field string
2728
problems testutils.Problems
2829
}{
29-
{"Valid", "uri", nil},
30-
{"ValidPrefix", "uri_foo", nil},
31-
{"ValidSuffix", "foo_uri", nil},
32-
{"ValidIntermixed", "foo_uri_bar", nil},
33-
{"Invalid", "url", testutils.Problems{{Message: "uri", Suggestion: "uri"}}},
34-
{"InvalidPrefix", "url_foo", testutils.Problems{{Message: "uri", Suggestion: "uri_foo"}}},
35-
{"InvalidSuffix", "foo_url", testutils.Problems{{Message: "uri", Suggestion: "foo_uri"}}},
36-
{"InvalidIntermixed", "foo_url_bar", testutils.Problems{{Message: "uri", Suggestion: "foo_uri_bar"}}},
30+
{"Valid", "", "uri", nil},
31+
{"ValidPrefix", "", "uri_foo", nil},
32+
{"ValidSuffix", "", "foo_uri", nil},
33+
{"ValidIntermixed", "", "foo_uri_bar", nil},
34+
{"ValidWithURIComment", "// A URI.", "uri", nil},
35+
{"ValidURL", "", "url", nil},
36+
{"ValidURLPrefix", "", "url_foo", nil},
37+
{"ValidURLSuffix", "", "foo_url", nil},
38+
{"ValidURLIntermixed", "", "foo_url_bar", nil},
39+
{"ValidIntraWordURIComment", "// A URL to a curio.", "url", nil},
40+
{"Invalid", "// A URI.", "url", testutils.Problems{{Message: "uri", Suggestion: "uri"}}},
41+
{"InvalidPrefix", "// A uri.", "url_foo", testutils.Problems{{Message: "uri", Suggestion: "uri_foo"}}},
42+
{"InvalidSuffix", "// A URI.", "foo_url", testutils.Problems{{Message: "uri", Suggestion: "foo_uri"}}},
43+
{"InvalidIntermixed", "// A uri.", "foo_url_bar", testutils.Problems{{Message: "uri", Suggestion: "foo_uri_bar"}}},
3744
} {
3845
t.Run(test.name, func(t *testing.T) {
3946
f := testutils.ParseProto3Tmpl(t, `
4047
message Foo {
48+
{{.Comment}}
4149
string {{.Field}} = 1;
4250
}
4351
`, test)

0 commit comments

Comments
 (0)