Repository: googleapis/api-linter
Pull request #455 (2019-12-20 18:33:23)

[Refactor] Allow AIP grouping function injection.

mingzhi: > This is fine with me, although it is not entirely obvious how it solves the problem internally. I assume that we add new files to the `lint` package that are internal only?

Exactly.

Patch:
diff --git a/lint/rule_groups.go b/lint/rule_groups.go
new file mode 100644
index 0000000..8b057f7
--- /dev/null
+++ b/lint/rule_groups.go
@@ -0,0 +1,42 @@
+package lint
+
+import "fmt"
+
+// A list of functions, each of which returns the group name for the given AIP
+// number and if no group is found, returns an empty string.
+// NOTE: the list will be evaluated in the FILO order.
+//
+// At Google, we inject additional group naming functions into this list.
+// Example: google_aip_groups.go
+// package lint
+// func init() {
+//   aipGroups = append(aipGroups, aipInternalGroup)
+// }
+//
+// func aipInternalGroup(aip int) string {
+//   if aip > 9000 {
+//	   return "internal"
+//   }
+//   return ""
+// }
+var aipGroups = []func(int) string{
+	aipCoreGroup,
+}
+
+func aipCoreGroup(aip int) string {
+	if aip > 0 && aip < 1000 {
+		return "core"
+	}
+	return ""
+}
+
+// getRuleGroup takes an AIP number and returns the appropriate group.
+// It panics if no group is found.
+func getRuleGroup(aip int, groups []func(int) string) string {
+	for i := len(groups) - 1; i >= 0; i-- {
+		if group := groups[i](aip); group != "" {
+			return group
+		}
+	}
+	panic(fmt.Sprintf("Invalid AIP number %d: no available group.", aip))
+}
diff --git a/lint/rule_name.go b/lint/rule_name.go
index 29942ed..f07258d 100644
--- a/lint/rule_name.go
+++ b/lint/rule_name.go
@@ -30,20 +30,11 @@ const nameSeparator string = "::"
 
 var ruleNameValidator = regexp.MustCompile("^([a-z0-9][a-z0-9-]*(::[a-z0-9][a-z0-9-]*)?)+$")
 
-// getRuleGroup takes an AIP number and returns the appropriate group.
-// It panics if no group is found.
-func getRuleGroup(aip int) string {
-	if aip > 0 && aip < 1000 {
-		return "core"
-	}
-	panic(fmt.Sprintf("Invalid AIP number %d: no available group.", aip))
-}
-
 // NewRuleName creates a RuleName from an AIP number and a unique name within
 // that AIP.
 func NewRuleName(aip int, name string) RuleName {
 	return RuleName(strings.Join([]string{
-		getRuleGroup(aip),
+		getRuleGroup(aip, aipGroups),
 		fmt.Sprintf("%04d", aip),
 		name,
 	}, nameSeparator))
diff --git a/lint/rule_registry.go b/lint/rule_registry.go
index a660855..0de239f 100644
--- a/lint/rule_registry.go
+++ b/lint/rule_registry.go
@@ -38,7 +38,7 @@ var errDuplicatedRuleName = errors.New("duplicate rule name")
 // Register registers the list of rules of the same AIP.
 // Return an error if any of the rules is found duplicate in the registry.
 func (r RuleRegistry) Register(aip int, rules ...ProtoRule) error {
-	rulePrefix := getRuleGroup(aip) + nameSeparator + fmt.Sprintf("%04d", aip)
+	rulePrefix := getRuleGroup(aip, aipGroups) + nameSeparator + fmt.Sprintf("%04d", aip)
 	for _, rl := range rules {
 		if !rl.GetName().IsValid() {
 			return errInvalidRuleName


Test patch:
diff --git a/lint/rule_groups_test.go b/lint/rule_groups_test.go
new file mode 100644
index 0000000..0796c20
--- /dev/null
+++ b/lint/rule_groups_test.go
@@ -0,0 +1,70 @@
+package lint
+
+import (
+	"testing"
+)
+
+func TestAIPCoreGroup(t *testing.T) {
+	tests := []struct {
+		name  string
+		aip   int
+		group string
+	}{
+		{"InCoreGroup", 1, "core"},
+		{"NotInCoreGroup_AIP<=0", 0, ""},
+		{"NotInCoreGroup_AIP>=1000", 1000, ""},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if got := aipCoreGroup(test.aip); got != test.group {
+				t.Errorf("aipCoreGroup(%d) got %s, but want %s", test.aip, got, test.group)
+			}
+		})
+	}
+}
+
+func TestGetRuleGroupPanic(t *testing.T) {
+	var groups []func(int) string
+	defer func() {
+		if r := recover(); r == nil {
+			t.Errorf("getRuleGroup did not panic")
+		}
+	}()
+	getRuleGroup(0, groups)
+}
+
+func TestGetRuleGroup(t *testing.T) {
+	var groupOne = func(aip int) string {
+		if aip == 1 {
+			return "ONE"
+		}
+		return ""
+	}
+	var groupTwo = func(aip int) string {
+		if aip == 2 {
+			return "TWO"
+		}
+		return ""
+	}
+	var groups = []func(int) string{
+		groupOne,
+		groupTwo,
+	}
+
+	tests := []struct {
+		name  string
+		aip   int
+		group string
+	}{
+		{"GroupOne", 1, "ONE"},
+		{"GroupTwo", 2, "TWO"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if got := getRuleGroup(test.aip, groups); got != test.group {
+				t.Errorf("getRuleGroup(%d) got %s, but want %s", test.aip, got, test.group)
+			}
+		})
+	}
+}