Skip to content

Commit

Permalink
Add denyRegex support for import-alias-naming rule (#927)
Browse files Browse the repository at this point in the history
  • Loading branch information
denisvmedia authored Oct 29, 2023
1 parent cb72bd8 commit fd9a130
Show file tree
Hide file tree
Showing 6 changed files with 143 additions and 19 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |

Expand Down
20 changes: 17 additions & 3 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
77 changes: 62 additions & 15 deletions rule/import-alias-naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
}
}

Expand All @@ -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",
})
Expand All @@ -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
}
29 changes: 29 additions & 0 deletions test/import-alias-naming_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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+))$`,
},
},
})
}
17 changes: 17 additions & 0 deletions testdata/import-alias-naming-custom-config-with-multiple-values.go
Original file line number Diff line number Diff line change
@@ -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("")
}
17 changes: 17 additions & 0 deletions testdata/import-alias-naming-custom-config-with-only-deny.go
Original file line number Diff line number Diff line change
@@ -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("")
}

0 comments on commit fd9a130

Please sign in to comment.