diff --git a/checkers/deferInLoop_checker.go b/checkers/deferInLoop_checker.go new file mode 100644 index 000000000..a252d8e00 --- /dev/null +++ b/checkers/deferInLoop_checker.go @@ -0,0 +1,87 @@ +package checkers + +import ( + "go/ast" + + "golang.org/x/tools/go/ast/astutil" + + "github.com/go-critic/go-critic/checkers/internal/astwalk" + "github.com/go-critic/go-critic/framework/linter" +) + +func init() { + var info linter.CheckerInfo + info.Name = "deferInLoop" + info.Tags = []string{"diagnostic", "experimental"} + info.Summary = "Detects loops inside functions that use defer" + info.Before = ` +for _, filename := range []string{"foo", "bar"} { + f, err := os.Open(filename) + + defer f.Close() +} +` + info.After = ` +func process(filename string) { + f, err := os.Open(filename) + + defer f.Close() +} +/* ... */ +for _, filename := range []string{"foo", "bar"} { + process(filename) +}` + + collection.AddChecker(&info, func(ctx *linter.CheckerContext) (linter.FileWalker, error) { + return astwalk.WalkerForFuncDecl(&deferInLoopChecker{ctx: ctx}), nil + }) +} + +type deferInLoopChecker struct { + astwalk.WalkHandler + ctx *linter.CheckerContext +} + +func (c *deferInLoopChecker) VisitFuncDecl(fn *ast.FuncDecl) { + // TODO: add func args check + // example: t.Run(123, func() { for { defer println(); break } }) + var blockParser func(*ast.BlockStmt, bool) + blockParser = func(block *ast.BlockStmt, inFor bool) { + for _, cur := range block.List { + switch n := cur.(type) { + case *ast.DeferStmt: + if inFor { + c.warn(n) + } + case *ast.RangeStmt: + blockParser(n.Body, true) + case *ast.ForStmt: + blockParser(n.Body, true) + case *ast.GoStmt: + if f, ok := n.Call.Fun.(*ast.FuncLit); ok { + blockParser(f.Body, false) + } + case *ast.ExprStmt: + if f, ok := n.X.(*ast.CallExpr); ok { + if anon, ok := f.Fun.(*ast.FuncLit); ok { + blockParser(anon.Body, false) + } + } + case *ast.BlockStmt: + blockParser(n, inFor) + } + } + } + pre := func(cur *astutil.Cursor) bool { + if n, ok := cur.Node().(*ast.BlockStmt); ok { + blockParser(n, false) + } + return false + } + + astutil.Apply(fn.Body, pre, nil) +} + +func (c *deferInLoopChecker) warn(cause *ast.DeferStmt) { + c.ctx.Warn(cause, "Possible resource leak, 'defer' is called in the 'for' loop") +} diff --git a/checkers/testdata/deferInLoop/negative_tests.go b/checkers/testdata/deferInLoop/negative_tests.go new file mode 100644 index 000000000..fc98bb257 --- /dev/null +++ b/checkers/testdata/deferInLoop/negative_tests.go @@ -0,0 +1,112 @@ +package checker_test + +func testForStmt() { + defer println(111) + + for range []int{1, 2, 3, 4} { + println(222) + break + } + + defer println(222) +} + +func testRangeStmt() { + defer println(222) + + for i := 0; i < 10; i++ { + println(111) + } + + defer println(222) +} + +func testClosure() { + func() { + for { + break + } + defer println(1) + + for { + for { + break + } + break + } + + defer println(1) + }() + + func() { + defer println(123) + + for range []int{1, 2, 3, 4} { + println(222) + } + + defer println(123) + + for range []int{1, 2, 3, 4} { + for range []int{1, 2, 3, 4} { + println(222) + } + } + + defer println(123) + }() + + for { + func() { + defer println(123) + }() + break + } + + for { + go func() { + defer println() + }() + + break + } +} + +func testBlock() { + { + for { + func() { + defer println() + }() + break + } + } + { + for { + go func() { + defer println() + }() + break + } + } + { + for range []int{1, 2, 3, 4} { + go func() { + defer println() + }() + break + } + } + { + for range []int{1, 2, 3, 4} { + { + func() { + { + defer println() + } + }() + } + break + } + } +} diff --git a/checkers/testdata/deferInLoop/positive_tests.go b/checkers/testdata/deferInLoop/positive_tests.go new file mode 100644 index 000000000..bc3475af1 --- /dev/null +++ b/checkers/testdata/deferInLoop/positive_tests.go @@ -0,0 +1,180 @@ +package checker_test + +import "fmt" + +func deferWithCall() { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer fmt.Println("test") + break + } + + for range []int{1, 2, 3, 4} { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer fmt.Println("test") + } +} + +func deferWithClosure() { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + + break + } + + for range []int{1, 2, 3, 4} { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + } +} + +func innerLoops() { + for { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + + break + } + break + } + + for { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer fmt.Println(123) + + break + } + break + } + + for range []int{1, 2, 3, 4} { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + + for range []int{1, 2, 3, 4} { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + } + } +} + +func anonFunc() { + func() { + for range []int{1, 2, 3, 4} { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + + for range []int{1, 2, 3, 4} { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + } + } + }() + + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + + func() { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + + for range []int{1, 2, 3, 4} { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + } + + break + } + + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer func() {}() + break + } + }() + + go func() { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + }() +} + +func contextBlock() { + { + go func() { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + }() + } + + { + func() { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + }() + } + + { + { + func() { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + }() + } + } + + { + for { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + } + + for { + { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + } + + for range []int{1, 2, 3} { + { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + } + + go func() { + { + for { + { + /*! Possible resource leak, 'defer' is called in the 'for' loop */ + defer println(123) + break + } + } + } + }() +}