From fd9a130d7ad5236498b467092eb3796829a941bf Mon Sep 17 00:00:00 2001 From: Denis Voytyuk <5462781+denisvmedia@users.noreply.github.com> Date: Sun, 29 Oct 2023 13:19:53 +0100 Subject: [PATCH] Add denyRegex support for import-alias-naming rule (#927) --- README.md | 2 +- RULES_DESCRIPTIONS.md | 20 ++++- rule/import-alias-naming.go | 77 +++++++++++++++---- test/import-alias-naming_test.go | 29 +++++++ ...ming-custom-config-with-multiple-values.go | 17 ++++ ...ias-naming-custom-config-with-only-deny.go | 17 ++++ 6 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 testdata/import-alias-naming-custom-config-with-multiple-values.go create mode 100644 testdata/import-alias-naming-custom-config-with-only-deny.go diff --git a/README.md b/README.md index b6286520c..8769df883 100644 --- a/README.md +++ b/README.md @@ -534,7 +534,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`datarace`](./RULES_DESCRIPTIONS.md#datarace) | n/a | Spots potential dataraces | no | no | | [`comment-spacings`](./RULES_DESCRIPTIONS.md#comment-spacings) | []string | Warns on malformed comments | no | no | | [`redundant-import-alias`](./RULES_DESCRIPTIONS.md#redundant-import-alias) | n/a | Warns on import aliases matching the imported package name | no | no | -| [`import-alias-naming`](./RULES_DESCRIPTIONS.md#import-alias-naming) | string (defaults to ^[a-z][a-z0-9]{0,}$) | Conventions around the naming of import aliases. | no | no | +| [`import-alias-naming`](./RULES_DESCRIPTIONS.md#import-alias-naming) | string or map[string]string (defaults to allow regex pattern ^[a-z][a-z0-9]{0,}$) | Conventions around the naming of import aliases. | no | no | | [`enforce-map-style`](./RULES_DESCRIPTIONS.md#enforce-map-style) | string (defaults to "any") | Enforces consistent usage of `make(map[type]type)` or `map[type]type{}` for map initialization. Does not affect `make(map[type]type, size)` constructions. | no | no | | [`enforce-slice-style`](./RULES_DESCRIPTIONS.md#enforce-slice-style) | string (defaults to "any") | Enforces consistent usage of `make([]type, 0)` or `[]type{}` for slice initialization. Does not affect `make(map[type]type, non_zero_len, or_non_zero_cap)` constructions. | no | no | diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index f4aeb4867..dad372b55 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -489,13 +489,27 @@ _Description_: Aligns with Go's naming conventions, as outlined in the official the principles of good package naming. Users can follow these guidelines by default or define a custom regex rule. Importantly, aliases with underscores ("_") are always allowed. -_Configuration_: (string) regular expression to match the aliases (default: ^[a-z][a-z0-9]{0,}$) +_Configuration_ (1): (string) as plain string accepts allow regexp pattern for aliases (default: ^[a-z][a-z0-9]{0,}$), +_Configuration_ (2): (map[string]string) as a map accepts two values: +* for a key "allowRegex" accepts allow regexp pattern +* for a key "denyRegex deny regexp pattern -Example: +_Note_: If both `allowRegex` and `denyRegex` are provided, the alias must comply with both of them. +If none are given (i.e. an empty map), the default value ^[a-z][a-z0-9]{0,}$ for allowRegex is used. +Unknown keys will result in an error. + +Example (1): + +```toml +[rule.import-alias-naming] + arguments = ["^[a-z][a-z0-9]{0,}$"] +``` + +Example (2): ```toml [rule.import-alias-naming] - arguments =["^[a-z][a-z0-9]{0,}$"] + arguments = [ { allowRegex = "^[a-z][a-z0-9]{0,}$", denyRegex = '^v\d+$' } ] ``` ## import-shadowing diff --git a/rule/import-alias-naming.go b/rule/import-alias-naming.go index 292392b5d..0c6513d25 100644 --- a/rule/import-alias-naming.go +++ b/rule/import-alias-naming.go @@ -10,14 +10,15 @@ import ( // ImportAliasNamingRule lints import alias naming. type ImportAliasNamingRule struct { - configured bool - namingRuleRegexp *regexp.Regexp + configured bool + allowRegexp *regexp.Regexp + denyRegexp *regexp.Regexp sync.Mutex } -const defaultNamingRule = "^[a-z][a-z0-9]{0,}$" +const defaultImportAliasNamingAllowRule = "^[a-z][a-z0-9]{0,}$" -var defaultNamingRuleRegexp = regexp.MustCompile(defaultNamingRule) +var defaultImportAliasNamingAllowRegexp = regexp.MustCompile(defaultImportAliasNamingAllowRule) func (r *ImportAliasNamingRule) configure(arguments lint.Arguments) { r.Lock() @@ -26,20 +27,31 @@ func (r *ImportAliasNamingRule) configure(arguments lint.Arguments) { return } - if len(arguments) < 1 { - r.namingRuleRegexp = defaultNamingRuleRegexp + if len(arguments) == 0 { + r.allowRegexp = defaultImportAliasNamingAllowRegexp return } - namingRule, ok := arguments[0].(string) // Alt. non panicking version - if !ok { - panic(fmt.Sprintf("Invalid argument '%v' for 'import-alias-naming' rule. Expecting string, got %T", arguments[0], arguments[0])) + switch namingRule := arguments[0].(type) { + case string: + r.setAllowRule(namingRule) + case map[string]any: // expecting map[string]string + for k, v := range namingRule { + switch k { + case "allowRegex": + r.setAllowRule(v) + case "denyRegex": + r.setDenyRule(v) + default: + panic(fmt.Sprintf("Invalid map key for 'import-alias-naming' rule. Expecting 'allowRegex' or 'denyRegex', got %v", k)) + } + } + default: + panic(fmt.Sprintf("Invalid argument '%v' for 'import-alias-naming' rule. Expecting string or map[string]string, got %T", arguments[0], arguments[0])) } - var err error - r.namingRuleRegexp, err = regexp.Compile(namingRule) - if err != nil { - panic(fmt.Sprintf("Invalid argument to the import-alias-naming rule. Expecting %q to be a valid regular expression, got: %v", namingRule, err)) + if r.allowRegexp == nil && r.denyRegexp == nil { + r.allowRegexp = defaultImportAliasNamingAllowRegexp } } @@ -60,10 +72,19 @@ func (r *ImportAliasNamingRule) Apply(file *lint.File, arguments lint.Arguments) continue } - if !r.namingRuleRegexp.MatchString(alias.Name) { + if r.allowRegexp != nil && !r.allowRegexp.MatchString(alias.Name) { + failures = append(failures, lint.Failure{ + Confidence: 1, + Failure: fmt.Sprintf("import name (%s) must match the regular expression: %s", alias.Name, r.allowRegexp.String()), + Node: alias, + Category: "imports", + }) + } + + if r.denyRegexp != nil && r.denyRegexp.MatchString(alias.Name) { failures = append(failures, lint.Failure{ Confidence: 1, - Failure: fmt.Sprintf("import name (%s) must match the regular expression: %s", alias.Name, r.namingRuleRegexp.String()), + Failure: fmt.Sprintf("import name (%s) must NOT match the regular expression: %s", alias.Name, r.denyRegexp.String()), Node: alias, Category: "imports", }) @@ -77,3 +98,29 @@ func (r *ImportAliasNamingRule) Apply(file *lint.File, arguments lint.Arguments) func (*ImportAliasNamingRule) Name() string { return "import-alias-naming" } + +func (r *ImportAliasNamingRule) setAllowRule(value any) { + namingRule, ok := value.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument '%v' for import-alias-naming allowRegexp rule. Expecting string, got %T", value, value)) + } + + namingRuleRegexp, err := regexp.Compile(namingRule) + if err != nil { + panic(fmt.Sprintf("Invalid argument to the import-alias-naming allowRegexp rule. Expecting %q to be a valid regular expression, got: %v", namingRule, err)) + } + r.allowRegexp = namingRuleRegexp +} + +func (r *ImportAliasNamingRule) setDenyRule(value any) { + namingRule, ok := value.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument '%v' for import-alias-naming denyRegexp rule. Expecting string, got %T", value, value)) + } + + namingRuleRegexp, err := regexp.Compile(namingRule) + if err != nil { + panic(fmt.Sprintf("Invalid argument to the import-alias-naming denyRegexp rule. Expecting %q to be a valid regular expression, got: %v", namingRule, err)) + } + r.denyRegexp = namingRuleRegexp +} diff --git a/test/import-alias-naming_test.go b/test/import-alias-naming_test.go index e772a091a..a4782c20d 100644 --- a/test/import-alias-naming_test.go +++ b/test/import-alias-naming_test.go @@ -9,7 +9,36 @@ import ( func TestImportAliasNaming(t *testing.T) { testRule(t, "import-alias-naming", &rule.ImportAliasNamingRule{}) + testRule(t, "import-alias-naming", &rule.ImportAliasNamingRule{}, &lint.RuleConfig{ + Arguments: lint.Arguments{ + map[string]any{}, + }, + }) +} + +func TestImportAliasNaming_CustomConfig(t *testing.T) { testRule(t, "import-alias-naming-custom-config", &rule.ImportAliasNamingRule{}, &lint.RuleConfig{ Arguments: []any{`^[a-z]+$`}, }) } + +func TestImportAliasNaming_CustomConfigWithMultipleRules(t *testing.T) { + testRule(t, "import-alias-naming-custom-config-with-multiple-values", &rule.ImportAliasNamingRule{}, &lint.RuleConfig{ + Arguments: []any{ + map[string]any{ + "allowRegex": `^[a-z][a-z0-9]*$`, + "denyRegex": `^((v\d+)|(v\d+alpha\d+))$`, + }, + }, + }) +} + +func TestImportAliasNaming_CustomConfigWithOnlyDeny(t *testing.T) { + testRule(t, "import-alias-naming-custom-config-with-only-deny", &rule.ImportAliasNamingRule{}, &lint.RuleConfig{ + Arguments: []any{ + map[string]any{ + "denyRegex": `^((v\d+)|(v\d+alpha\d+))$`, + }, + }, + }) +} diff --git a/testdata/import-alias-naming-custom-config-with-multiple-values.go b/testdata/import-alias-naming-custom-config-with-multiple-values.go new file mode 100644 index 000000000..cbfb20b8f --- /dev/null +++ b/testdata/import-alias-naming-custom-config-with-multiple-values.go @@ -0,0 +1,17 @@ +package fixtures + +import ( + _ "strings" + bar_foo "strings" // MATCH /import name (bar_foo) must match the regular expression: ^[a-z][a-z0-9]*$/ + fooBAR "strings" // MATCH /import name (fooBAR) must match the regular expression: ^[a-z][a-z0-9]*$/ + v1 "strings" // MATCH /import name (v1) must NOT match the regular expression: ^((v\d+)|(v\d+alpha\d+))$/ + v1alpha1 "strings" // MATCH /import name (v1alpha1) must NOT match the regular expression: ^((v\d+)|(v\d+alpha\d+))$/ + magical "magic/hat" +) + +func somefunc() { + fooBAR.Clone("") + bar_foo.Clone("") + v1.Clone("") + magical.Clone("") +} diff --git a/testdata/import-alias-naming-custom-config-with-only-deny.go b/testdata/import-alias-naming-custom-config-with-only-deny.go new file mode 100644 index 000000000..771ef556c --- /dev/null +++ b/testdata/import-alias-naming-custom-config-with-only-deny.go @@ -0,0 +1,17 @@ +package fixtures + +import ( + _ "strings" + bar_foo "strings" + fooBAR "strings" + v1 "strings" // MATCH /import name (v1) must NOT match the regular expression: ^((v\d+)|(v\d+alpha\d+))$/ + v1alpha1 "strings" // MATCH /import name (v1alpha1) must NOT match the regular expression: ^((v\d+)|(v\d+alpha\d+))$/ + magical "magic/hat" +) + +func somefunc() { + fooBAR.Clone("") + bar_foo.Clone("") + v1.Clone("") + magical.Clone("") +}