Merge remote-tracking branch 'upstream/master'

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) |
 
-[<img alt="AragurDEV" src="https://avatars0.githubusercontent.com/u/11004008?v=4&s=117" width="117">](https://github.com/AragurDEV) |[<img alt="jamesmaidment" src="https://avatars3.githubusercontent.com/u/2050324?v=4&s=117" width="117">](https://github.com/jamesmaidment) |[<img alt="mapreal19" src="https://avatars2.githubusercontent.com/u/3055997?v=4&s=117" width="117">](https://github.com/mapreal19) |[<img alt="paul-at-start" src="https://avatars2.githubusercontent.com/u/5486775?v=4&s=117" width="117">](https://github.com/paul-at-start) |[<img alt="psapezhko" src="https://avatars3.githubusercontent.com/u/10865586?v=4&s=117" width="117">](https://github.com/psapezhko) |[<img alt="ridvansumset" src="https://avatars2.githubusercontent.com/u/26631560?v=4&s=117" width="117">](https://github.com/ridvansumset) |
+[<img alt="AragurDEV" src="https://avatars0.githubusercontent.com/u/11004008?v=4&s=117" width="117">](https://github.com/AragurDEV) |[<img alt="yangdiangzb" src="https://avatars3.githubusercontent.com/u/16643665?v=4&s=117" width="117">](https://github.com/yangdiangzb) |[<img alt="jamesmaidment" src="https://avatars3.githubusercontent.com/u/2050324?v=4&s=117" width="117">](https://github.com/jamesmaidment) |[<img alt="mapreal19" src="https://avatars2.githubusercontent.com/u/3055997?v=4&s=117" width="117">](https://github.com/mapreal19) |[<img alt="paul-at-start" src="https://avatars2.githubusercontent.com/u/5486775?v=4&s=117" width="117">](https://github.com/paul-at-start) |[<img alt="psapezhko" src="https://avatars3.githubusercontent.com/u/10865586?v=4&s=117" width="117">](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) |
 
-[<img alt="Jarema" src="https://avatars0.githubusercontent.com/u/7369771?v=4&s=117" width="117">](https://github.com/Jarema) |[<img alt="vkrol" src="https://avatars3.githubusercontent.com/u/153412?v=4&s=117" width="117">](https://github.com/vkrol) |[<img alt="haya14busa" src="https://avatars0.githubusercontent.com/u/3797062?v=4&s=117" width="117">](https://github.com/haya14busa) |
+[<img alt="ridvansumset" src="https://avatars2.githubusercontent.com/u/26631560?v=4&s=117" width="117">](https://github.com/ridvansumset) |[<img alt="Jarema" src="https://avatars0.githubusercontent.com/u/7369771?v=4&s=117" width="117">](https://github.com/Jarema) |[<img alt="vkrol" src="https://avatars3.githubusercontent.com/u/153412?v=4&s=117" width="117">](https://github.com/vkrol) |[<img alt="haya14busa" src="https://avatars0.githubusercontent.com/u/3797062?v=4&s=117" width="117">](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) |
 
 ## License
 

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
 

config.go

@@ -74,6 +74,7 @@ var allRules = append([]lint.Rule{
 	&rule.LineLengthLimitRule{},
 	&rule.CallToGCRule{},
 	&rule.DuplicatedImportsRule{},
+	&rule.ImportShadowingRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

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
+}

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
+}

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{})
+}