Repository: googleapis/api-linter
Pull request #193 (2019-09-27 08:26:30)

[fix] Be stricter about rule names.
We should enforce a stricter standard on rule names.

- They should always be lower-case (no upper-case).
- They should always start with a letter or number.
- They should always use `-` as a word separator, not `_` (kebab-case).
- They should not begin or end with a word separator.

This PR does all of that with the exception of enforcing no
trailing hyphens (I am willing to assume that given the enforcement
of the remaining rules, nobody will try that).


Patch:
diff --git a/lint/rule_name.go b/lint/rule_name.go
index 11241a7..4335bfa 100644
--- a/lint/rule_name.go
+++ b/lint/rule_name.go
@@ -19,13 +19,15 @@ import (
 	"strings"
 )
 
-// RuleName is an identifier for a rule. Allowed characters include a-z, A-Z, 0-9, -, _. The
-// namespace separator :: is allowed between RuleName segments (for example, my_namespace::my_rule).
+// RuleName is an identifier for a rule. Allowed characters include a-z, 0-9, -.
+//
+// The namespace separator :: is allowed between RuleName segments
+// (for example, my-namespace::my-rule).
 type RuleName string
 
 const nameSeparator string = "::"
 
-var ruleNameValidator = regexp.MustCompile("^([a-zA-Z0-9-_]+(::[a-zA-Z0-9-_]+)?)+$")
+var ruleNameValidator = regexp.MustCompile("^([a-z0-9][a-z0-9-]*(::[a-z0-9][a-z0-9-]*)?)+$")
 
 // NewRuleName creates a RuleName from segments.
 // It will join the segments with the "::" separator.


Test patch:
diff --git a/lint/rule_name_test.go b/lint/rule_name_test.go
index c3a2103..513856d 100644
--- a/lint/rule_name_test.go
+++ b/lint/rule_name_test.go
@@ -16,26 +16,53 @@ package lint
 
 import "testing"
 
-func TestRuleName_IsValid(t *testing.T) {
+func TestRuleNameValid(t *testing.T) {
 	tests := []struct {
-		name  RuleName
-		valid bool
+		testName string
+		ruleName RuleName
 	}{
-		{"", false},
-		{"a:::b", false},
-		{"a::::b", false},
-		{"a", true},
-		{"::my_rule", false},
-		{"my_namespace::", false},
-		{"my_namespace:my_rule", false},
-		{"my_namespace::my_rule", true},
-		{"my_rule", true},
+		{"Lower", "aip"},
+		{"LowerNumber", "aip0121"},
+		{"LowerNumberKebab", "aip-0121"},
+		{"Namespaced", "aip::0121"},
+		{"NamespacedHyphen", "core::aip-0121"},
 	}
 
 	for _, test := range tests {
-		if test.name.IsValid() != test.valid {
-			t.Errorf("%q.IsValid()=%t; want %t", test.name, test.name.IsValid(), test.valid)
-		}
+		t.Run(test.testName, func(t *testing.T) {
+			if !test.ruleName.IsValid() {
+				t.Errorf("Rule name %q is invalid; want valid.", test.ruleName)
+			}
+		})
+	}
+}
+
+func TestRuleNameInvalid(t *testing.T) {
+	tests := []struct {
+		testName string
+		ruleName RuleName
+	}{
+		{"EmptyString", ""},
+		{"TripleColon", "a:::b"},
+		{"QuadrupleColon", "a::::b"},
+		{"CapitalLetter", "A"},
+		{"LeadingDoubleColon", "::my-rule"},
+		{"TrailingDoubleColon", "my-namespace::"},
+		{"LeadingHyphen", "-core::aip-0131"},
+		{"LeadingSegmentHyphen", "core::-aip-0131"},
+		{"OnlyHyphen", "-"},
+		{"SingleColon", "core:aip-0131"},
+		{"Underscore", "core::aip_0131"},
+		{"CamelCase", "myRule"},
+		{"PascalCase", "MyRule"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			if test.ruleName.IsValid() {
+				t.Errorf("Rule name %q is valid; want invalid.", test.ruleName)
+			}
+		})
 	}
 }
 
@@ -46,7 +73,7 @@ func TestNewRuleName(t *testing.T) {
 	}{
 		{[]string{}, ""},
 		{[]string{""}, ""},
-		{[]string{"my_namespace", "my_rule"}, "my_namespace::my_rule"},
+		{[]string{"my-namespace", "my-rule"}, "my-namespace::my-rule"},
 		{[]string{"my", "name", "space", "foo"}, "my::name::space::foo"},
 	}