Merge pull request #10 from maratori/shadow-var

respect variable shadowing and type aliasing

Author: firefart

analyzer/analyzer.go

@@ -28,6 +28,7 @@ func flags() flag.FlagSet {
 
 func run(pass *analysis.Pass) (interface{}, error) {
 	allowErrorInDefer := pass.Analyzer.Flags.Lookup(FlagAllowErrorInDefer).Value.String() == "true"
+	errorType := types.Universe.Lookup("error").Type()
 
 	inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 
@@ -70,12 +71,10 @@ func run(pass *analysis.Pass) (interface{}, error) {
 					continue
 				}
 
-				if allowErrorInDefer {
-					if ident, ok := p.Type.(*ast.Ident); ok {
-						if ident.Name == "error" && findDeferWithErrorAssignment(funcBody, n.Name) {
-							continue
-						}
-					}
+				if allowErrorInDefer &&
+					types.Identical(pass.TypesInfo.TypeOf(p.Type), errorType) &&
+					findDeferWithVariableAssignment(funcBody, pass.TypesInfo, pass.TypesInfo.ObjectOf(n)) {
+					continue
 				}
 
 				pass.Reportf(node.Pos(), "named return %q with type %q found", n.Name, types.ExprString(p.Type))
@@ -86,7 +85,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
-func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool {
+func findDeferWithVariableAssignment(body *ast.BlockStmt, info *types.Info, variable types.Object) bool {
 	found := false
 
 	ast.Inspect(body, func(node ast.Node) bool {
@@ -96,7 +95,7 @@ func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool {
 
 		if d, ok := node.(*ast.DeferStmt); ok {
 			if fn, ok2 := d.Call.Fun.(*ast.FuncLit); ok2 {
-				if findErrorAssignment(fn.Body, name) {
+				if findVariableAssignment(fn.Body, info, variable) {
 					found = true
 					return false
 				}
@@ -109,7 +108,7 @@ func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool {
 	return found
 }
 
-func findErrorAssignment(body *ast.BlockStmt, name string) bool {
+func findVariableAssignment(body *ast.BlockStmt, info *types.Info, variable types.Object) bool {
 	found := false
 
 	ast.Inspect(body, func(node ast.Node) bool {
@@ -120,7 +119,7 @@ func findErrorAssignment(body *ast.BlockStmt, name string) bool {
 		if a, ok := node.(*ast.AssignStmt); ok {
 			for _, lh := range a.Lhs {
 				if i, ok2 := lh.(*ast.Ident); ok2 {
-					if i.Name == name {
+					if info.ObjectOf(i) == variable {
 						found = true
 						return false
 					}

testdata/src/allow-error-in-defer/allow_error_in_defer.go

@@ -1,5 +1,7 @@
 package main
 
+import "errors"
+
 func simple() (err error) {
 	defer func() {
 		err = nil
@@ -43,32 +45,87 @@ func errorIsNoAssigned() (err error) { // want `named return "err" with type "er
 	return
 }
 
-func shadowVariable() (err error) {
+func shadowVariable() (err error) { // want `named return "err" with type "error" found`
 	defer func() {
-		err := 123 // linter doesn't understand that this is different variable (even if different type) (yet?)
+		err := errors.New("xxx")
 		_ = err
 	}()
 	return
 }
 
-func shadowVariable2() (err error) {
+func shadowVariableButAssign() (err error) {
+	defer func() {
+		{
+			err := errors.New("xxx")
+			_ = err
+		}
+		err = nil
+	}()
+	return
+}
+
+func shadowVariable2() (err error) { // want `named return "err" with type "error" found`
 	defer func() {
-		a, err := doSomething() // linter doesn't understand that this is different variable (yet?)
+		a, err := doSomething()
 		_ = a
 		_ = err
 	}()
 	return
 }
 
-type myError = error // linter doesn't understand that this is the same type (yet?)
+type errorAlias = error
+
+func errorAliasIsTheSame() (err errorAlias) {
+	defer func() {
+		err = nil
+	}()
+	return
+}
+
+type myError error // linter doesn't check underlying type (yet?)
+
+func customTypeWithErrorUnderline() (err myError) { // want `named return "err" with type "myError" found`
+	defer func() {
+		err = nil
+	}()
+	return
+}
+
+type myError2 interface{ error } // linter doesn't check interfaces
 
-func customType() (err myError) { // want `named return "err" with type "myError" found`
+func customTypeWithTheSameInterface() (err myError2) { // want `named return "err" with type "myError2" found`
 	defer func() {
 		err = nil
 	}()
 	return
 }
 
+var _ error = myError3{}
+
+type myError3 struct{} // linter doesn't check interfaces
+
+func (m myError3) Error() string { return "" }
+
+func customTypeImplementingErrorInterface() (err myError3) { // want `named return "err" with type "myError3" found`
+	defer func() {
+		err = struct{}{}
+	}()
+	return
+}
+
+func shadowErrorType() {
+	type error interface { // linter understands that this is not built-in error, even if it has the same name
+		Error() string
+	}
+	do := func() (err error) { // want `named return "err" with type "error" found`
+		defer func() {
+			err = nil
+		}()
+		return
+	}
+	do()
+}
+
 func notTheLast() (err error, _ int) {
 	defer func() {
 		err = nil