Skip to content

Commit 62d28d1

Browse files
committed
respect variable shadowing and type aliasing
1 parent bc2e315 commit 62d28d1

File tree

2 files changed

+69
-12
lines changed

2 files changed

+69
-12
lines changed

analyzer/analyzer.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
7171
}
7272

7373
if allowErrorInDefer {
74-
if ident, ok := p.Type.(*ast.Ident); ok {
75-
if ident.Name == "error" && findDeferWithErrorAssignment(funcBody, n.Name) {
74+
if pass.TypesInfo.TypeOf(p.Type).String() == "error" { // with package prefix, so only built-it error fits
75+
if findDeferWithVariableAssignment(funcBody, pass.TypesInfo, pass.TypesInfo.ObjectOf(n)) {
7676
continue
7777
}
7878
}
@@ -86,7 +86,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
8686
return nil, nil
8787
}
8888

89-
func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool {
89+
func findDeferWithVariableAssignment(body *ast.BlockStmt, info *types.Info, variable types.Object) bool {
9090
found := false
9191

9292
ast.Inspect(body, func(node ast.Node) bool {
@@ -96,7 +96,7 @@ func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool {
9696

9797
if d, ok := node.(*ast.DeferStmt); ok {
9898
if fn, ok2 := d.Call.Fun.(*ast.FuncLit); ok2 {
99-
if findErrorAssignment(fn.Body, name) {
99+
if findVariableAssignment(fn.Body, info, variable) {
100100
found = true
101101
return false
102102
}
@@ -109,7 +109,7 @@ func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool {
109109
return found
110110
}
111111

112-
func findErrorAssignment(body *ast.BlockStmt, name string) bool {
112+
func findVariableAssignment(body *ast.BlockStmt, info *types.Info, variable types.Object) bool {
113113
found := false
114114

115115
ast.Inspect(body, func(node ast.Node) bool {
@@ -120,7 +120,7 @@ func findErrorAssignment(body *ast.BlockStmt, name string) bool {
120120
if a, ok := node.(*ast.AssignStmt); ok {
121121
for _, lh := range a.Lhs {
122122
if i, ok2 := lh.(*ast.Ident); ok2 {
123-
if i.Name == name {
123+
if info.ObjectOf(i) == variable {
124124
found = true
125125
return false
126126
}

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

+63-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package main
22

3+
import "errors"
4+
35
func simple() (err error) {
46
defer func() {
57
err = nil
@@ -43,32 +45,87 @@ func errorIsNoAssigned() (err error) { // want `named return "err" with type "er
4345
return
4446
}
4547

46-
func shadowVariable() (err error) {
48+
func shadowVariable() (err error) { // want `named return "err" with type "error" found`
4749
defer func() {
48-
err := 123 // linter doesn't understand that this is different variable (even if different type) (yet?)
50+
err := errors.New("xxx")
4951
_ = err
5052
}()
5153
return
5254
}
5355

54-
func shadowVariable2() (err error) {
56+
func shadowVariableButAssign() (err error) {
57+
defer func() {
58+
{
59+
err := errors.New("xxx")
60+
_ = err
61+
}
62+
err = nil
63+
}()
64+
return
65+
}
66+
67+
func shadowVariable2() (err error) { // want `named return "err" with type "error" found`
5568
defer func() {
56-
a, err := doSomething() // linter doesn't understand that this is different variable (yet?)
69+
a, err := doSomething()
5770
_ = a
5871
_ = err
5972
}()
6073
return
6174
}
6275

63-
type myError = error // linter doesn't understand that this is the same type (yet?)
76+
type errorAlias = error
77+
78+
func errorAliasIsTheSame() (err errorAlias) {
79+
defer func() {
80+
err = nil
81+
}()
82+
return
83+
}
84+
85+
type myError error // linter doesn't check underlying type (yet?)
86+
87+
func customTypeWithErrorUnderline() (err myError) { // want `named return "err" with type "myError" found`
88+
defer func() {
89+
err = nil
90+
}()
91+
return
92+
}
93+
94+
type myError2 interface{ error } // linter doesn't check interfaces
6495

65-
func customType() (err myError) { // want `named return "err" with type "myError" found`
96+
func customTypeWithTheSameInterface() (err myError2) { // want `named return "err" with type "myError2" found`
6697
defer func() {
6798
err = nil
6899
}()
69100
return
70101
}
71102

103+
var _ error = myError3{}
104+
105+
type myError3 struct{} // linter doesn't check interfaces
106+
107+
func (m myError3) Error() string { return "" }
108+
109+
func customTypeImplementingErrorInterface() (err myError3) { // want `named return "err" with type "myError3" found`
110+
defer func() {
111+
err = struct{}{}
112+
}()
113+
return
114+
}
115+
116+
func shadowErrorType() {
117+
type error interface { // linter understands that this is not built-in error, even if it has the same name
118+
Error() string
119+
}
120+
do := func() (err error) { // want `named return "err" with type "error" found`
121+
defer func() {
122+
err = nil
123+
}()
124+
return
125+
}
126+
do()
127+
}
128+
72129
func notTheLast() (err error, _ int) {
73130
defer func() {
74131
err = nil

0 commit comments

Comments
 (0)