From 03826dd7f362357c2c95bd58fd0601206049daff Mon Sep 17 00:00:00 2001 From: Marcin Federowicz Date: Thu, 21 Dec 2023 04:28:50 +0100 Subject: [PATCH] add param ignoreUsed to ignore redundant aliases but used in code (useful for sdk packages) --- RULES_DESCRIPTIONS.md | 13 +++- rule/redundant-import-alias.go | 90 ++++++++++++++++++---- test/redundant-import-alias_test.go | 26 ++++++- testdata/redundant-import-alias-ignored.go | 22 ++++++ testdata/redundant-import-alias.go | 26 +++++-- 5 files changed, 153 insertions(+), 24 deletions(-) create mode 100644 testdata/redundant-import-alias-ignored.go diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 49d8f45c8..d21b59fb2 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -672,9 +672,18 @@ _Configuration_: N/A ## redundant-import-alias -_Description_: This rule warns on redundant import aliases. This happens when the alias used on the import statement matches the imported package name. +_Description_: This rule warns on redundant import aliases. This happens when the alias used on the import statement matches the imported package name. -_Configuration_: N/A +_Configuration_: + +* `ignoreUsed` : (bool) ignore aliases used in code (useful when using popular sdk packages). + +Example: + +```toml +[rule.redundant-import-alias] + arguments = [{ ignoreUsed = true}] +``` ## string-format diff --git a/rule/redundant-import-alias.go b/rule/redundant-import-alias.go index fa5281f24..ce1fbcaa1 100644 --- a/rule/redundant-import-alias.go +++ b/rule/redundant-import-alias.go @@ -3,24 +3,35 @@ package rule import ( "fmt" "go/ast" - "strings" + "sync" "github.com/mgechev/revive/lint" ) +const ( + defaultIgnoreUsed = false +) + // RedundantImportAlias lints given else constructs. -type RedundantImportAlias struct{} +type RedundantImportAlias struct { + ignoreUsed bool + sync.Mutex +} // Apply applies the rule to given file. -func (*RedundantImportAlias) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (r *RedundantImportAlias) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) + var failures []lint.Failure + redundants := r.checkRedundantAliases(file.AST) + for _, imp := range file.AST.Imports { if imp.Name == nil { continue } - if getImportPackageName(imp) == imp.Name.Name { + if _, exists := redundants[imp.Name.Name]; exists { failures = append(failures, lint.Failure{ Confidence: 1, Failure: fmt.Sprintf("Import alias \"%s\" is redundant", imp.Name.Name), @@ -29,7 +40,6 @@ func (*RedundantImportAlias) Apply(file *lint.File, _ lint.Arguments) []lint.Fai }) } } - return failures } @@ -38,15 +48,69 @@ func (*RedundantImportAlias) Name() string { return "redundant-import-alias" } -func getImportPackageName(imp *ast.ImportSpec) string { - const pathSep = "/" - const strDelim = `"` +func (r *RedundantImportAlias) checkRedundantAliases(node ast.Node) map[string]string { + + var aliasedPackages = make(map[string]string) + + // First pass: Identify all aliases and their usage - by default alias is redundant + ast.Inspect(node, func(n ast.Node) bool { + imp, ok := n.(*ast.ImportSpec) + if !ok { + return true + } + + if imp.Name != nil && imp.Path != nil { + aliasedPackages[imp.Name.Name] = "redundant" + } + return true + }) + + if r.ignoreUsed { + // Second pass: remove one time used aliases + ast.Inspect(node, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return true + } + + x, ok := sel.X.(*ast.Ident) + if !ok { + return true + } + + // This alias is being used; it's not redundant + if _, exists := aliasedPackages[x.Name]; exists { + delete(aliasedPackages, x.Name) + } - path := imp.Path.Value - i := strings.LastIndex(path, pathSep) - if i == -1 { - return strings.Trim(path, strDelim) + return true + }) } - return strings.Trim(path[i+1:], strDelim) + return aliasedPackages +} + +func (r *RedundantImportAlias) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() + + r.ignoreUsed = defaultIgnoreUsed + if len(arguments) > 0 { + + args, ok := arguments[0].(map[string]any) + if !ok { + panic(fmt.Sprintf("Invalid argument to the redundant-import-alias rule. Expecting a k,v map, got %T", arguments[0])) + } + + for k, v := range args { + switch k { + case "ignoreUsed": + value, ok := v.(bool) + if !ok { + panic(fmt.Sprintf("Invalid argument to the redundant-import-alias rule, expecting string representation of an bool. Got '%v' (%T)", v, v)) + } + r.ignoreUsed = value + } + } + } } diff --git a/test/redundant-import-alias_test.go b/test/redundant-import-alias_test.go index e7bff5faf..e0b7838a9 100644 --- a/test/redundant-import-alias_test.go +++ b/test/redundant-import-alias_test.go @@ -1,11 +1,33 @@ package test import ( - "github.com/mgechev/revive/rule" "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" ) // TestRedundantImportAlias rule. +func TestRedundantImportIgnoredAliases(t *testing.T) { + + args := []any{map[string]any{ + "ignoreUsed": true, + }} + + testRule(t, "redundant-import-alias-ignored", &rule.RedundantImportAlias{}, &lint.RuleConfig{ + Arguments: args, + }) + +} + func TestRedundantImportAlias(t *testing.T) { - testRule(t, "redundant-import-alias", &rule.RedundantImportAlias{}) + + args := []any{map[string]any{ + "ignoreUsed": false, + }} + + testRule(t, "redundant-import-alias", &rule.RedundantImportAlias{}, &lint.RuleConfig{ + Arguments: args, + }) + } diff --git a/testdata/redundant-import-alias-ignored.go b/testdata/redundant-import-alias-ignored.go new file mode 100644 index 000000000..0754b8ca1 --- /dev/null +++ b/testdata/redundant-import-alias-ignored.go @@ -0,0 +1,22 @@ +package fixtures + +import ( + runpb "cloud.google.com/go/run/apiv2/runpb" + md5 "crypto/md5" + strings "strings" // MATCH /Import alias "strings" is redundant/ + + "crypto/md5" + _ "crypto/md5" // MATCH /Import alias "_" is redundant/ + + "strings" + str "strings" // MATCH /Import alias "str" is redundant/ + +) + +func UseRunpb() { + runpb.RegisterTasksServer() +} + +func UseMd5() { + fmt.PrintLn(md5.Size) +} diff --git a/testdata/redundant-import-alias.go b/testdata/redundant-import-alias.go index 555bca261..546327e70 100644 --- a/testdata/redundant-import-alias.go +++ b/testdata/redundant-import-alias.go @@ -1,12 +1,24 @@ package fixtures -import( +import ( + runpb "cloud.google.com/go/run/apiv2/runpb" // MATCH /Import alias "runpb" is redundant/ + + md5 "crypto/md5" // MATCH /Import alias "md5" is redundant/ + + strings "strings" // MATCH /Import alias "strings" is redundant/ + "crypto/md5" - "strings" - _ "crypto/md5" - str "strings" - strings "strings" // MATCH /Import alias "strings" is redundant/ - crypto "crypto/md5" - md5 "crypto/md5" // MATCH /Import alias "md5" is redundant/ + _ "crypto/md5" // MATCH /Import alias "_" is redundant/ + + "strings" + str "strings" // MATCH /Import alias "str" is redundant/ + ) +func UseRunpb() { + runpb.RegisterTasksServer() +} + +func UseMd5() { + fmt.PrintLn(md5.Size) +}