checker: add defer at the end of function checker (#901)

Detects uneeded defer at the end of function.

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)
+}

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)
 }

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()
+}

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
+}