Skip to content

Commit 853036d

Browse files
firefartfirefart
authored
Merge pull request #8 from maratori/allow-error-in-defer
Add flag `allow-error-in-defer`
2 parents a9830d2 + a16bc58 commit 853036d

File tree

4 files changed

+311
-1
lines changed

4 files changed

+311
-1
lines changed

‎analyzer/analyzer.go

+71
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package analyzer
22

33
import (
4+
"flag"
45
"go/ast"
56
"go/types"
67

@@ -9,14 +10,25 @@ import (
910
"golang.org/x/tools/go/ast/inspector"
1011
)
1112

13+
const FlagAllowErrorInDefer = "allow-error-in-defer"
14+
1215
var Analyzer = &analysis.Analyzer{
1316
Name: "nonamedreturns",
1417
Doc: "Reports all named returns",
18+
Flags: flags(),
1519
Run: run,
1620
Requires: []*analysis.Analyzer{inspect.Analyzer},
1721
}
1822

23+
func flags() flag.FlagSet {
24+
fs := flag.FlagSet{}
25+
fs.Bool(FlagAllowErrorInDefer, false, "do not complain about named error, if it is assigned inside defer")
26+
return fs
27+
}
28+
1929
func run(pass *analysis.Pass) (interface{}, error) {
30+
allowErrorInDefer := pass.Analyzer.Flags.Lookup(FlagAllowErrorInDefer).Value.String() == "true"
31+
2032
inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
2133

2234
// only filter function defintions
@@ -27,12 +39,15 @@ func run(pass *analysis.Pass) (interface{}, error) {
2739

2840
inspector.Preorder(nodeFilter, func(node ast.Node) {
2941
var funcResults *ast.FieldList
42+
var funcBody *ast.BlockStmt
3043

3144
switch n := node.(type) {
3245
case *ast.FuncLit:
3346
funcResults = n.Type.Results
47+
funcBody = n.Body
3448
case *ast.FuncDecl:
3549
funcResults = n.Type.Results
50+
funcBody = n.Body
3651
default:
3752
return
3853
}
@@ -51,10 +66,66 @@ func run(pass *analysis.Pass) (interface{}, error) {
5166
}
5267

5368
for _, n := range p.Names {
69+
if allowErrorInDefer {
70+
if ident, ok := p.Type.(*ast.Ident); ok {
71+
if ident.Name == "error" && findDeferWithErrorAssignment(funcBody, n.Name) {
72+
continue
73+
}
74+
}
75+
}
76+
5477
pass.Reportf(node.Pos(), "named return %q with type %q found", n.Name, types.ExprString(p.Type))
5578
}
5679
}
5780
})
5881

5982
return nil, nil
6083
}
84+
85+
func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool {
86+
found := false
87+
88+
ast.Inspect(body, func(node ast.Node) bool {
89+
if found {
90+
return false // stop inspection
91+
}
92+
93+
if d, ok := node.(*ast.DeferStmt); ok {
94+
if fn, ok2 := d.Call.Fun.(*ast.FuncLit); ok2 {
95+
if findErrorAssignment(fn.Body, name) {
96+
found = true
97+
return false
98+
}
99+
}
100+
}
101+
102+
return true
103+
})
104+
105+
return found
106+
}
107+
108+
func findErrorAssignment(body *ast.BlockStmt, name string) bool {
109+
found := false
110+
111+
ast.Inspect(body, func(node ast.Node) bool {
112+
if found {
113+
return false // stop inspection
114+
}
115+
116+
if a, ok := node.(*ast.AssignStmt); ok {
117+
for _, lh := range a.Lhs {
118+
if i, ok2 := lh.(*ast.Ident); ok2 {
119+
if i.Name == name {
120+
found = true
121+
return false
122+
}
123+
}
124+
}
125+
}
126+
127+
return true
128+
})
129+
130+
return found
131+
}

‎analyzer/analyzer_test.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,11 @@ func TestAll(t *testing.T) {
1515
}
1616

1717
testdata := filepath.Join(filepath.Dir(wd), "testdata")
18-
analysistest.Run(t, testdata, Analyzer, "p")
18+
analysistest.Run(t, testdata, Analyzer, "default-config")
19+
20+
err = Analyzer.Flags.Set(FlagAllowErrorInDefer, "true")
21+
if err != nil {
22+
t.Fatalf("Failed to set flag: %s", err)
23+
}
24+
analysistest.Run(t, testdata, Analyzer, "allow-error-in-defer")
1925
}

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

+226
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
package main
2+
3+
func simple() (err error) {
4+
defer func() {
5+
err = nil
6+
}()
7+
return
8+
}
9+
10+
func twoReturnParams() (i int, err error) { // want `named return "i" with type "int" found`
11+
defer func() {
12+
i = 0
13+
err = nil
14+
}()
15+
return
16+
}
17+
18+
// TODO: enable test after https://github.com/firefart/nonamedreturns/pull/7
19+
//func allUnderscoresExceptError() (_ int, err error) {
20+
// defer func() {
21+
// err = nil
22+
// }()
23+
// return
24+
//}
25+
26+
func customName() (myName error) {
27+
defer func() {
28+
myName = nil
29+
}()
30+
return
31+
}
32+
33+
func errorIsNoAssigned() (err error) { // want `named return "err" with type "error" found`
34+
defer func() {
35+
_ = err
36+
processError(err)
37+
if err == nil {
38+
}
39+
switch err {
40+
case nil:
41+
default:
42+
}
43+
}()
44+
return
45+
}
46+
47+
func shadowVariable() (err error) {
48+
defer func() {
49+
err := 123 // linter doesn't understand that this is different variable (even if different type) (yet?)
50+
_ = err
51+
}()
52+
return
53+
}
54+
55+
func shadowVariable2() (err error) {
56+
defer func() {
57+
a, err := doSomething() // linter doesn't understand that this is different variable (yet?)
58+
_ = a
59+
_ = err
60+
}()
61+
return
62+
}
63+
64+
type myError = error // linter doesn't understand that this is the same type (yet?)
65+
66+
func customType() (err myError) { // want `named return "err" with type "myError" found`
67+
defer func() {
68+
err = nil
69+
}()
70+
return
71+
}
72+
73+
// TODO: replace `i` with `_` after https://github.com/firefart/nonamedreturns/pull/7
74+
func notTheLast() (err error, i int) { // want `named return "i" with type "int" found`
75+
defer func() {
76+
i = 0
77+
err = nil
78+
}()
79+
return
80+
}
81+
82+
func twoErrorsCombined() (err1, err2 error) {
83+
defer func() {
84+
err1 = nil
85+
err2 = nil
86+
}()
87+
return
88+
}
89+
90+
func twoErrorsSeparated() (err1 error, err2 error) {
91+
defer func() {
92+
err1 = nil
93+
err2 = nil
94+
}()
95+
return
96+
}
97+
98+
func errorSlice() (err []error) { // want `named return "err" with type "\[\]error" found`
99+
defer func() {
100+
err = nil
101+
}()
102+
return
103+
}
104+
105+
func deferWithVariable() (err error) { // want `named return "err" with type "error" found`
106+
f := func() {
107+
err = nil
108+
}
109+
defer f() // linter can't catch closure passed via variable (yet?)
110+
return
111+
}
112+
113+
func uberMultierr() (err error) { // want `named return "err" with type "error" found`
114+
defer func() {
115+
multierrAppendInto(&err, nil) // linter doesn't allow it (yet?)
116+
}()
117+
return
118+
}
119+
120+
func deferInDefer() (err error) {
121+
defer func() {
122+
defer func() {
123+
err = nil
124+
}()
125+
}()
126+
return
127+
}
128+
129+
func twoDefers() (err error) {
130+
defer func() {}()
131+
defer func() {
132+
err = nil
133+
}()
134+
return
135+
}
136+
137+
func callFunction() (err error) {
138+
defer func() {
139+
_, err = doSomething()
140+
}()
141+
return
142+
}
143+
144+
func callFunction2() (err error) {
145+
defer func() {
146+
var a int
147+
a, err = doSomething()
148+
_ = a
149+
}()
150+
return
151+
}
152+
153+
func deepInside() (err error) {
154+
if true {
155+
switch true {
156+
case false:
157+
for i := 0; i < 10; i++ {
158+
go func() {
159+
select {
160+
default:
161+
defer func() {
162+
if true {
163+
switch true {
164+
case false:
165+
for j := 0; j < 10; j++ {
166+
go func() {
167+
select {
168+
default:
169+
err = nil
170+
}
171+
}()
172+
}
173+
}
174+
}
175+
}()
176+
}
177+
}()
178+
}
179+
}
180+
}
181+
return
182+
}
183+
184+
var goodFuncLiteral = func() (err error) {
185+
defer func() {
186+
err = nil
187+
}()
188+
return
189+
}
190+
191+
var badFuncLiteral = func() (err error) { // want `named return "err" with type "error" found`
192+
defer func() {
193+
_ = err
194+
}()
195+
return
196+
}
197+
198+
func funcLiteralInsideFunc() error {
199+
do := func() (err error) {
200+
defer func() {
201+
err = nil
202+
}()
203+
return
204+
}
205+
return do()
206+
}
207+
208+
type x struct{}
209+
210+
func (x) goodMethod() (err error) {
211+
defer func() {
212+
err = nil
213+
}()
214+
return
215+
}
216+
217+
func (x) badMethod() (err error) { // want `named return "err" with type "error" found`
218+
defer func() {
219+
_ = err
220+
}()
221+
return
222+
}
223+
224+
func processError(error) {}
225+
func doSomething() (int, error) { return 10, nil }
226+
func multierrAppendInto(*error, error) bool { return false } // https://pkg.go.dev/go.uber.org/multierr#AppendInto

‎testdata/src/p/p.go renamed to ‎testdata/src/default-config/default_config.go

+7
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ var e = func() (err error) { // want `named return "err" with type "error" found
2323
return
2424
}
2525

26+
func deferWithError() (err error) { // want `named return "err" with type "error" found`
27+
defer func() {
28+
err = nil // use flag to allow this
29+
}()
30+
return
31+
}
32+
2633
var (
2734
f = func() {
2835
return

0 commit comments

Comments
 (0)