Repository: mgechev/revive (go)

Issue:
import-shadowing reacts on method names while it shouldn't
**Describe the bug**
`import-shadowing` rule marks method names as shadowing, but it's not a true shadowing since it's a part of a struct. I'm not sure if it was done intentionally or not, but it seems to be a bug.

**To Reproduce**

```go
package somepkg

import (
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

type Struct struct{}

func (s *Struct) reconcile(md metav1.ObjectMeta) (reconcile.Result, error) {
	return reconcile.Result{}, nil
}
```

Run revive against this file and get this (I use `golangci-lint`, but I guess it's a problem in revive itself):

```
somepkg/t.go:10:18: import-shadowing: The name 'reconcile' shadows an import name (revive)
func (s *Struct) reconcile(md metav1.ObjectMeta) (reconcile.Result, error) {
                 ^
```

**Expected behavior**
It should not consider method names as a shadow for the imported packages. Only package-level functions (i.e. not methods) should be considered.

**Additional context**
I use `golangci-lint` [v1.54.2](https://github.com/golangci/golangci-lint/releases/tag/v1.54.2).


Pull request:
fix: false positive in import-shadowing for method names
Closes #875

Patch:
diff --git a/rule/import-shadowing.go b/rule/import-shadowing.go
index 2bab704..046aeb6 100644
--- a/rule/import-shadowing.go
+++ b/rule/import-shadowing.go
@@ -29,6 +29,7 @@ func (*ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fail
 			failures = append(failures, failure)
 		},
 		alreadySeen: map[*ast.Object]struct{}{},
+		skipIdents:  map[*ast.Ident]struct{}{},
 	}
 
 	ast.Walk(walker, fileAst)
@@ -62,6 +63,7 @@ type importShadowing struct {
 	importNames      map[string]struct{}
 	onFailure        func(lint.Failure)
 	alreadySeen      map[*ast.Object]struct{}
+	skipIdents       map[*ast.Ident]struct{}
 }
 
 // Visit visits AST nodes and checks if id nodes (ast.Ident) shadow an import name
@@ -80,6 +82,10 @@ func (w importShadowing) Visit(n ast.Node) ast.Visitor {
 		*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.FuncDecl:
+		if n.Recv != nil {
+			w.skipIdents[n.Name] = struct{}{}
+		}
 	case *ast.Ident:
 		if n == w.packageNameIdent {
 			return nil // skip the ident corresponding to the package name of this file
@@ -92,11 +98,12 @@ func (w importShadowing) Visit(n ast.Node) ast.Visitor {
 
 		_, isImportName := w.importNames[id]
 		_, alreadySeen := w.alreadySeen[n.Obj]
-		if isImportName && !alreadySeen {
+		_, skipIdent := w.skipIdents[n]
+		if isImportName && !alreadySeen && !skipIdent {
 			w.onFailure(lint.Failure{
 				Confidence: 1,
 				Node:       n,
-				Category:   "namming",
+				Category:   "naming",
 				Failure:    fmt.Sprintf("The name '%s' shadows an import name", id),
 			})
 


Test patch:
diff --git a/testdata/import-shadowing.go b/testdata/import-shadowing.go
index 124f108..230f807 100644
--- a/testdata/import-shadowing.go
+++ b/testdata/import-shadowing.go
@@ -23,6 +23,14 @@ type fmt interface {} // MATCH /The name 'fmt' shadows an import name/
 
 func (ast myAst) foo() {} // MATCH /The name 'ast' shadows an import name/
 
+func (a myAst) fmt() { // this should be skipped (method, not a pkg func)
+	var fmt string // MATCH /The name 'fmt' shadows an import name/
+}
+
+func (a myAst) md5() { // this should be skipped (method, not a pkg func)
+	strings := map[string]string{} // MATCH /The name 'strings' shadows an import name/
+}
+
 func md5() {} // MATCH /The name 'md5' shadows an import name/
 
 func bar(_ string) {}