Skip to content

Commit

Permalink
redundant-import-alias: add param ignoreUsed to ignore redundant alia…
Browse files Browse the repository at this point in the history
…ses but used in code (useful for sdk packages)
  • Loading branch information
mfederowicz committed Dec 22, 2023
1 parent bf0ff67 commit 8825aa7
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 46 deletions.
13 changes: 11 additions & 2 deletions RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
98 changes: 58 additions & 40 deletions rule/redundant-import-alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,38 @@ 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 := checkRedundantAliases(file.AST)
redundants := r.checkRedundantAliases(file.AST)

for _, imp := range file.AST.Imports {
if imp.Name == nil {
continue
}

if _, exists := redundants[imp.Name.Name]; exists {
var aliastype = redundants[imp.Name.Name]
failures = append(failures, lint.Failure{
Confidence: 1,
Failure: fmt.Sprintf("Import alias \"%s\" is %s", imp.Name.Name, aliastype),
Failure: fmt.Sprintf("Import alias \"%s\" is redundant", imp.Name.Name),
Node: imp,
Category: "imports",
})
Expand All @@ -40,22 +48,9 @@ func (*RedundantImportAlias) Name() string {
return "redundant-import-alias"
}

func getImportPackageName(imp *ast.ImportSpec) string {
const pathSep = "/"
const strDelim = `"`

path := imp.Path.Value
i := strings.LastIndex(path, pathSep)
if i == -1 {
return strings.Trim(path, strDelim)
}

return strings.Trim(path[i+1:], strDelim)
}
func (r *RedundantImportAlias) checkRedundantAliases(node ast.Node) map[string]string {

func checkRedundantAliases(node ast.Node) map[string]string {
var redundants = make(map[string]string)
var aliasedPackages = make(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 {
Expand All @@ -70,29 +65,52 @@ func checkRedundantAliases(node ast.Node) map[string]string {
return true
})

// Second pass: Check if any of the aliases are redundant
ast.Inspect(node, func(n ast.Node) bool {
sel, ok := n.(*ast.SelectorExpr)
if !ok {
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)
}

return true
}
})
}

return aliasedPackages
}

func (r *RedundantImportAlias) configure(arguments lint.Arguments) {
r.Lock()
defer r.Unlock()

x, ok := sel.X.(*ast.Ident)
r.ignoreUsed = defaultIgnoreUsed
if len(arguments) > 0 {

args, ok := arguments[0].(map[string]any)
if !ok {
return true
panic(fmt.Sprintf("Invalid argument to the redundant-import-alias rule. Expecting a k,v map, got %T", arguments[0]))
}

// This alias is being used; it's not redundant
if _, exists := aliasedPackages[x.Name]; exists {
delete(aliasedPackages, x.Name)
}

return true
})

for name, value := range aliasedPackages {
redundants[name] = value
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
}
}
}

return redundants
}
26 changes: 24 additions & 2 deletions test/redundant-import-alias_test.go
Original file line number Diff line number Diff line change
@@ -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,
})

}
22 changes: 22 additions & 0 deletions testdata/redundant-import-alias-ignored.go
Original file line number Diff line number Diff line change
@@ -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)
}
6 changes: 4 additions & 2 deletions testdata/redundant-import-alias.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package fixtures

import (
runpb "cloud.google.com/go/run/apiv2/runpb"
md5 "crypto/md5"
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"
Expand Down

0 comments on commit 8825aa7

Please sign in to comment.