diff --git a/checkers/deferAtTheEnd_checker.go b/checkers/deferAtTheEnd_checker.go new file mode 100644 index 000000000..a36cb4901 --- /dev/null +++ b/checkers/deferAtTheEnd_checker.go @@ -0,0 +1,85 @@ +package checkers + +import ( + "go/ast" + + "github.com/go-lintpack/lintpack" + "github.com/go-lintpack/lintpack/astwalk" + "github.com/go-toolsmith/astfmt" +) + +func init() { + var info lintpack.CheckerInfo + info.Name = "deferAtTheEnd" + info.Tags = []string{"diagnostic", "experimental"} + info.Summary = "Detects calls to defer at the end of function" + info.Before = ` +func() { + defer os.Remove(filename) +}` + info.After = ` +func() { + os.Remove(filename) +}` + + collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker { + return astwalk.WalkerForFuncDecl(&deferAtTheEnd{ctx: ctx}) + }) +} + +type deferAtTheEnd struct { + astwalk.WalkHandler + ctx *lintpack.CheckerContext +} + +func (c *deferAtTheEnd) VisitFuncDecl(funcDecl *ast.FuncDecl) { + ast.Inspect(funcDecl.Body, func(n ast.Node) bool { + funDecl, ok := n.(*ast.BlockStmt) + if ok { + c.checkDeferBeforeReturn(funDecl) + } + return true + }) +} + +func (c *deferAtTheEnd) checkDeferBeforeReturn(funcDecl *ast.BlockStmt) { + retIndex := len(funcDecl.List) + for i, stmt := range funcDecl.List { + retStmt, ok := stmt.(*ast.ReturnStmt) + if !ok { + continue + } + if containsCallExpr(retStmt) { + continue + } + retIndex = i + break + + } + if retIndex == 0 { + return + } + + if deferStmt, ok := funcDecl.List[retIndex-1].(*ast.DeferStmt); ok { + c.warn(deferStmt) + } +} + +func containsCallExpr(retStmt *ast.ReturnStmt) bool { + for _, expr := range retStmt.Results { + if _, ok := expr.(*ast.CallExpr); ok { + return true + } + } + return false +} + +func (c *deferAtTheEnd) warn(deferStmt *ast.DeferStmt) { + s := astfmt.Sprint(deferStmt) + if fnlit, ok := deferStmt.Call.Fun.(*ast.FuncLit); ok { + // To avoid long and multi-line warning messages, + // collapse the function literals. + s = "defer " + astfmt.Sprint(fnlit.Type) + "{...}(...)" + } + c.ctx.Warn(deferStmt, "%s is placed just before return", s) +} diff --git a/checkers/exitAfterDefer_checker.go b/checkers/exitAfterDefer_checker.go index 05ed6ae9e..2f746fb5e 100644 --- a/checkers/exitAfterDefer_checker.go +++ b/checkers/exitAfterDefer_checker.go @@ -66,13 +66,11 @@ func (c *exitAfterDeferChecker) VisitFuncDecl(fn *ast.FuncDecl) { } func (c *exitAfterDeferChecker) warn(cause *ast.CallExpr, deferStmt *ast.DeferStmt) { - var s string + s := astfmt.Sprint(deferStmt) if fnlit, ok := deferStmt.Call.Fun.(*ast.FuncLit); ok { // To avoid long and multi-line warning messages, // collapse the function literals. s = "defer " + astfmt.Sprint(fnlit.Type) + "{...}(...)" - } else { - s = astfmt.Sprint(deferStmt) } c.ctx.Warn(cause, "%s clutters `%s`", cause.Fun, s) } diff --git a/checkers/testdata/deferAtTheEnd/negative_tests.go b/checkers/testdata/deferAtTheEnd/negative_tests.go new file mode 100644 index 000000000..a8232e5f0 --- /dev/null +++ b/checkers/testdata/deferAtTheEnd/negative_tests.go @@ -0,0 +1,37 @@ +package checker_test + +func foo_1() { + foo_2() + return +} + +func foo_2() int { + func() {}() + return 0 +} + +func foo_3() { + func() {}() +} + +func foo_4() { + func() { + foo_1() + return + }() +} + +func foo_5() { + defer func() { + defer foo_1() + foo_1() + return + }() + foo1() + return +} + +func foo_6() int { + defer func() {}() + return foo_2() +} diff --git a/checkers/testdata/deferAtTheEnd/positive_tests.go b/checkers/testdata/deferAtTheEnd/positive_tests.go new file mode 100644 index 000000000..c3f9e7bd5 --- /dev/null +++ b/checkers/testdata/deferAtTheEnd/positive_tests.go @@ -0,0 +1,58 @@ +package checker_test + +func foo1() { + /*! defer foo2() is placed just before return */ + defer foo2() + return +} + +func foo2() int { + /*! defer func(){...}(...) is placed just before return */ + defer func() {}() + return 0 +} + +func foo3() { + /*! defer func(){...}(...) is placed just before return */ + defer func() {}() +} + +func foo4() { + /*! defer func(){...}(...) is placed just before return */ + defer func() { + /*! defer foo1() is placed just before return */ + defer foo1() + return + }() +} + +func foo5() { + func() { + /*! defer foo1() is placed just before return */ + defer foo1() + return + }() + foo1() + return +} + +func foo6() { + /*! defer func(){...}(...) is placed just before return */ + defer func() { + for { + /*! defer foo1() is placed just before return */ + defer foo1() + return + } + }() + return +} + +func foo7() { + if true { + /*! defer foo1() is placed just before return */ + defer foo1() + return + } + return +}