diff --git a/README.md b/README.md index 2e4b65e..319e41b 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`line-length-limit`](./RULES_DESCRIPTIONS.md#line-length-limit) | int | Specifies the maximum number of characters in a line | no | no | | [`call-to-gc`](./RULES_DESCRIPTIONS.md#call-to-gc) | n/a | Warns on explicit call to the garbage collector | no | no | | [`duplicated-imports`](./RULES_DESCRIPTIONS#duplicated-imports) | n/a | Looks for packages that are imported two or more times | no | no | +| [`import-shadowing`](./RULES_DESCRIPTIONS.md#import-shadowing) | n/a | Spots identifiers that shadow an import | no | no | ## Configurable rules @@ -433,13 +434,13 @@ Currently, type checking is enabled by default. If you want to run the linter wi :---: |:---: |:---: |:---: |:---: |:---: | [mgechev](https://github.com/mgechev) |[chavacava](https://github.com/chavacava) |[xuri](https://github.com/xuri) |[gsamokovarov](https://github.com/gsamokovarov) |[morphy2k](https://github.com/morphy2k) |[tamird](https://github.com/tamird) | -[AragurDEV](https://github.com/AragurDEV) |[jamesmaidment](https://github.com/jamesmaidment) |[mapreal19](https://github.com/mapreal19) |[paul-at-start](https://github.com/paul-at-start) |[psapezhko](https://github.com/psapezhko) |[ridvansumset](https://github.com/ridvansumset) | +[AragurDEV](https://github.com/AragurDEV) |[yangdiangzb](https://github.com/yangdiangzb) |[jamesmaidment](https://github.com/jamesmaidment) |[mapreal19](https://github.com/mapreal19) |[paul-at-start](https://github.com/paul-at-start) |[psapezhko](https://github.com/psapezhko) | :---: |:---: |:---: |:---: |:---: |:---: | -[AragurDEV](https://github.com/AragurDEV) |[jamesmaidment](https://github.com/jamesmaidment) |[mapreal19](https://github.com/mapreal19) |[paul-at-start](https://github.com/paul-at-start) |[psapezhko](https://github.com/psapezhko) |[ridvansumset](https://github.com/ridvansumset) | +[AragurDEV](https://github.com/AragurDEV) |[yangdiangzb](https://github.com/yangdiangzb) |[jamesmaidment](https://github.com/jamesmaidment) |[mapreal19](https://github.com/mapreal19) |[paul-at-start](https://github.com/paul-at-start) |[psapezhko](https://github.com/psapezhko) | -[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) |[haya14busa](https://github.com/haya14busa) | -:---: |:---: |:---: | -[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) |[haya14busa](https://github.com/haya14busa) | +[ridvansumset](https://github.com/ridvansumset) |[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) |[haya14busa](https://github.com/haya14busa) | +:---: |:---: |:---: |:---: | +[ridvansumset](https://github.com/ridvansumset) |[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) |[haya14busa](https://github.com/haya14busa) | ## License diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 46842c7..0e9a586 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -34,6 +34,7 @@ List of all available rules. - [increment-decrement](#increment-decrement) - [indent-error-flow](#indent-error-flow) - [imports-blacklist](#imports-blacklist) + - [import-shadowing](#import-shadowing) - [line-length-limit](#line-length-limit) - [max-public-structs](#max-public-structs) - [modifies-parameter](#modifies-parameter) @@ -295,6 +296,13 @@ Example: [imports-blacklist] arguments =["crypto/md5", "crypto/sha1"] ``` +### import-shadowing + +_Description_: In GO it is possible to declare identifiers (packages, structs, +interfaces, parameters, receivers, variables, constants...) that conflict with the +name of an imported package. This rule spots identifiers that shadow an import. + +_Configuration_: N/A ## line-length-limit diff --git a/config.go b/config.go index c77a372..b1f5855 100644 --- a/config.go +++ b/config.go @@ -74,6 +74,7 @@ var allRules = append([]lint.Rule{ &rule.LineLengthLimitRule{}, &rule.CallToGCRule{}, &rule.DuplicatedImportsRule{}, + &rule.ImportShadowingRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/fixtures/import-shadowing.go b/fixtures/import-shadowing.go new file mode 100644 index 0000000..06f91f6 --- /dev/null +++ b/fixtures/import-shadowing.go @@ -0,0 +1,36 @@ +package fixtures + +import ( + ast "go/ast" + "bytes" + "crypto/md5" + "fmt" + _ "net/http" + "strings" + str "strings" +) + +const str = "" // MATCH /The name 'str' shadows an import name/ + +type myAst struct { + ast *ast.GenDecl +} + +type bytes struct {} // MATCH /The name 'bytes' shadows an import name/ + +type fmt interface {} // MATCH /The name 'fmt' shadows an import name/ + +func (ast myAst) foo() {} // MATCH /The name 'ast' shadows an import name/ + +func md5() {} // MATCH /The name 'md5' shadows an import name/ + +func bar(_ string) {} + +func toto() { + strings := map[string]string{} // MATCH /The name 'strings' shadows an import name/ +} + +func titi() { + v := md5+bytes + return ast +} diff --git a/rule/import-shadowing.go b/rule/import-shadowing.go new file mode 100644 index 0000000..b78234c --- /dev/null +++ b/rule/import-shadowing.go @@ -0,0 +1,102 @@ +package rule + +import ( + "fmt" + "go/ast" + "go/token" + "strings" + + "github.com/mgechev/revive/lint" +) + +// ImportShadowingRule lints given else constructs. +type ImportShadowingRule struct{} + +// Apply applies the rule to given file. +func (r *ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + var failures []lint.Failure + + importNames := map[string]struct{}{} + for _, imp := range file.AST.Imports { + importNames[getName(imp)] = struct{}{} + } + + fileAst := file.AST + walker := importShadowing{ + importNames: importNames, + onFailure: func(failure lint.Failure) { + failures = append(failures, failure) + }, + alreadySeen: map[*ast.Object]struct{}{}, + } + + ast.Walk(walker, fileAst) + + return failures +} + +// Name returns the rule name. +func (r *ImportShadowingRule) Name() string { + return "import-shadowing" +} + +func getName(imp *ast.ImportSpec) string { + const pathSep = "/" + const strDelim = `"` + if imp.Name != nil { + return imp.Name.Name + } + + path := imp.Path.Value + i := strings.LastIndex(path, pathSep) + if i == -1 { + return strings.Trim(path, strDelim) + } + + return strings.Trim(path[i+1:], strDelim) +} + +type importShadowing struct { + importNames map[string]struct{} + onFailure func(lint.Failure) + alreadySeen map[*ast.Object]struct{} +} + +// Visit visits AST nodes and checks if id nodes (ast.Ident) shadow an import name +func (w importShadowing) Visit(n ast.Node) ast.Visitor { + switch n := n.(type) { + case *ast.AssignStmt: + if n.Tok == token.DEFINE { + return w // analyze variable declarations of the form id := expr + } + + return nil // skip assigns of the form id = expr (not an id declaration) + case *ast.CallExpr, // skip call expressions (not an id declaration) + *ast.ImportSpec, // skip import section subtree because we already have the list of imports + *ast.KeyValueExpr, // skip analysis of key-val expressions ({key:value}): ids of such expressions, even the same of an import name, do not shadow the import name + *ast.ReturnStmt, // skip skipping analysis of returns, ids in expression were already analyzed + *ast.SelectorExpr, // skip analysis of selector expressions (anId.otherId): because if anId shadows an import name, it was already detected, and otherId does not shadows the import name + *ast.StructType: // skip analysis of struct type because struct fields can not shadow an import name + return nil + case *ast.Ident: + id := n.Name + if id == "_" { + return w // skip _ id + } + + _, isImportName := w.importNames[id] + _, alreadySeen := w.alreadySeen[n.Obj] + if isImportName && !alreadySeen { + w.onFailure(lint.Failure{ + Confidence: 1, + Node: n, + Category: "namming", + Failure: fmt.Sprintf("The name '%s' shadows an import name", id), + }) + + w.alreadySeen[n.Obj] = struct{}{} + } + } + + return w +} diff --git a/test/import-shadowing_test.go b/test/import-shadowing_test.go new file mode 100644 index 0000000..12e64f2 --- /dev/null +++ b/test/import-shadowing_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +func TestImportShadowing(t *testing.T) { + testRule(t, "import-shadowing", &rule.ImportShadowingRule{}) +}