Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add denyRegex support for import-alias-naming rule #927

Merged
merged 1 commit into from
Oct 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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("")
}
Loading