From f099cf7743679a9985f85c7adb07c5d2a18fff43 Mon Sep 17 00:00:00 2001
From: Tomasz Janiszewski <janiszt@gmail.com>
Date: Mon, 20 Jan 2020 16:49:13 +0100
Subject: [PATCH] checker: add defer at the end of function checker

Detects uneeded defer at the end of function.
---
 checkers/deferAtTheEnd_checker.go             | 96 +++++++++++++++++++
 .../testdata/deferAtTheEnd/negative_tests.go  | 37 +++++++
 .../testdata/deferAtTheEnd/positive_tests.go  | 49 ++++++++++
 3 files changed, 182 insertions(+)
 create mode 100644 checkers/deferAtTheEnd_checker.go
 create mode 100644 checkers/testdata/deferAtTheEnd/negative_tests.go
 create mode 100644 checkers/testdata/deferAtTheEnd/positive_tests.go

diff --git a/checkers/deferAtTheEnd_checker.go b/checkers/deferAtTheEnd_checker.go
new file mode 100644
index 000000000..576ebd2ee
--- /dev/null
+++ b/checkers/deferAtTheEnd_checker.go
@@ -0,0 +1,96 @@
+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)
+	fnlit, ok := deferStmt.Call.Fun.(*ast.FuncLit)
+	if ok {
+		// To avoid long and multi-line warning messages,
+		// collapse the function literals.
+		s = "defer " + astfmt.Sprint(fnlit.Type)
+		if fnlit.Body.List == nil {
+			s += "{}"
+		} else {
+			s += "{…}"
+		}
+		if fnlit.Type.Params.List == nil {
+			s += "()"
+		} else {
+			s += "(…)"
+		}
+	}
+	c.ctx.Warn(deferStmt, "%s is placed just before return", 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..d6b597321
--- /dev/null
+++ b/checkers/testdata/deferAtTheEnd/positive_tests.go
@@ -0,0 +1,49 @@
+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
+}