Skip to content

Commit 15657fd

Browse files
authored
feat: add cloud AIP group (#970)
1 parent fcdecc0 commit 15657fd

File tree

8 files changed

+101
-1
lines changed

8 files changed

+101
-1
lines changed

docs/rules/cloud.md

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
permalink: /rules/cloud/
3+
---
4+
5+
# Cloud rules
6+
7+
Cloud rules are based on [Cloud-specific AIPs][]. They are not enabled by
8+
default, and should only be enabled for Cloud APIs.
9+
10+
{% include linter-group-listing.html start=2500 end=2599 %}
11+
{% include linter-group-listing.html start=25000 end=25999 %}
12+
13+
[Cloud-specific aips]: https://aip.dev/cloud

docs/rules/index.md

+25
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,31 @@ into the "core" group of rules, and remaining blocks are grouped based on the
6161
</div>
6262
</a>
6363
</li>
64+
<li>
65+
<a class="glue-tile glue-tile--border glue-tile--hoverable"
66+
aria-label="Image tile" href="rules/cloud/" tabindex="0">
67+
<div class="glue-tile__header glue-tile__header--icon">
68+
<svg role="img" class="glue-tile__icon">
69+
<use xlink:href="#glue-color-google-logo" x="-76"></use>
70+
</svg>
71+
</div>
72+
<div class="glue-tile__body">
73+
<h4 class="glue-tile__headline">
74+
Cloud rules
75+
</h4>
76+
<p class="glue-tile__description">
77+
Cloud rules are based on Cloud-specific AIPs, and are disabled by default.
78+
</p>
79+
<ul class="glue-tile__links">
80+
<li class="glue-tile__link">
81+
<svg role="img" class="glue-tile__link glue-tile__link--arrow">
82+
<use xlink:href="#mi-arrow-forward-no-bg"></use>
83+
</svg>
84+
</li>
85+
</ul>
86+
</div>
87+
</a>
88+
</li>
6489
</ul>
6590

6691
[aip.dev]: https://aip.dev/

lint/config.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ func ReadConfigsYAML(f io.Reader) (Configs, error) {
9191

9292
// IsRuleEnabled returns true if a rule is enabled by the configs.
9393
func (configs Configs) IsRuleEnabled(rule string, path string) bool {
94-
enabled := true
94+
// Enabled by default if the rule does not belong to one of the default
95+
// disabled groups. Otherwise, needs to be explicitly enabled.
96+
enabled := !matchRule(rule, defaultDisabledRules...)
9597
for _, c := range configs {
9698
if c.matchPath(path) {
9799
if matchRule(rule, c.DisabledRules...) {

lint/config_test.go

+24
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,30 @@ func TestRuleConfigs_IsRuleEnabled(t *testing.T) {
211211
"testrule::a",
212212
disabled,
213213
},
214+
{
215+
"NoConfigMatched_DefaultDisabled",
216+
Configs{
217+
{
218+
IncludedPaths: []string{"a.proto"},
219+
DisabledRules: []string{"testrule"},
220+
},
221+
},
222+
"b.proto",
223+
"cloud::25164::generic-fields",
224+
disabled,
225+
},
226+
{
227+
"ConfigMatched_DefaultDisabled_Enabled",
228+
Configs{
229+
{
230+
IncludedPaths: []string{"a.proto"},
231+
EnabledRules: []string{"cloud"},
232+
},
233+
},
234+
"a.proto",
235+
"cloud::25164::generic-fields",
236+
enabled,
237+
},
214238
}
215239
for _, test := range tests {
216240
t.Run(test.name, func(t *testing.T) {

lint/rule_enabled.go

+4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ import (
2121
"github.com/jhump/protoreflect/desc"
2222
)
2323

24+
// defaultDisabledRules is the list of rules or groups that are by default
25+
// disabled, because they are scoped to a very specific set of AIPs.
26+
var defaultDisabledRules = []string{"cloud"}
27+
2428
// Disable all rules for deprecated descriptors.
2529
func disableDeprecated(d desc.Descriptor) bool {
2630
switch v := d.(type) {

lint/rule_groups.go

+8
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import "fmt"
2222
var aipGroups = []func(int) string{
2323
aipCoreGroup,
2424
aipClientLibrariesGroup,
25+
aipCloudGroup,
2526
}
2627

2728
func aipCoreGroup(aip int) string {
@@ -38,6 +39,13 @@ func aipClientLibrariesGroup(aip int) string {
3839
return ""
3940
}
4041

42+
func aipCloudGroup(aip int) string {
43+
if (aip >= 2500 && aip <= 2599) || (aip >= 25000 && aip <= 25999) {
44+
return "cloud"
45+
}
46+
return ""
47+
}
48+
4149
// getRuleGroup takes an AIP number and returns the appropriate group.
4250
// It panics if no group is found.
4351
func getRuleGroup(aip int, groups []func(int) string) string {

lint/rule_urls.go

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import "strings"
1818
var ruleURLMappings = []func(string) string{
1919
coreRuleURL,
2020
clientLibrariesRuleUrl,
21+
cloudRuleUrl,
2122
}
2223

2324
func coreRuleURL(ruleName string) string {
@@ -28,6 +29,10 @@ func clientLibrariesRuleUrl(ruleName string) string {
2829
return groupUrl(ruleName, "client-libraries")
2930
}
3031

32+
func cloudRuleUrl(ruleName string) string {
33+
return groupUrl(ruleName, "cloud")
34+
}
35+
3136
func groupUrl(ruleName, groupName string) string {
3237
base := "https://linter.aip.dev/"
3338
nameParts := strings.Split(ruleName, "::") // e.g., client-libraries::0122::camel-case-uri -> ["client-libraries", "0122", "camel-case-uri"]

lint/rule_urls_test.go

+19
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,25 @@ func TestClientLibrariesRuleURL(t *testing.T) {
4242
}
4343
}
4444

45+
func TestCloudRuleURL(t *testing.T) {
46+
tests := []struct {
47+
name string
48+
rule string
49+
url string
50+
}{
51+
{"CloudRule", "cloud::2500::generic-fields", "https://linter.aip.dev/2500/generic-fields"},
52+
{"NotCloudRule", "test::0122::camel-case-uri", ""},
53+
}
54+
55+
for _, test := range tests {
56+
t.Run(test.name, func(t *testing.T) {
57+
if got := cloudRuleUrl(test.rule); got != test.url {
58+
t.Errorf("cloudRuleUrl(%s) got %s, but want %s", test.name, got, test.url)
59+
}
60+
})
61+
}
62+
}
63+
4564
func TestGetRuleURL(t *testing.T) {
4665
var mapping1 = func(name string) string {
4766
if name == "one" {

0 commit comments

Comments
 (0)