Added sqlclosecheck to linters (#1203)

* Added sqlclosecheck to linters * Addressed feedback * Updated sqlclosecheck and tools
2020-07-04 07:30:16 -07:00 · 2020-07-04 07:30:16 -07:00 · b22e3f1874
commit b22e3f1874
parent 954127e56d
5 changed files with 380 additions and 1 deletions
--- a/go.mod
+++ b/go.mod
@ -37,6 +37,7 @@ require (
 	github.com/nishanths/exhaustive v0.0.0-20200525081945-8e46705b6132
 	github.com/pkg/errors v0.9.1
 	github.com/ryancurrah/gomodguard v1.1.0
+	github.com/ryanrolds/sqlclosecheck v0.3.0
 	github.com/securego/gosec/v2 v2.3.0
 	github.com/shirou/gopsutil v0.0.0-20190901111213-e4ec7b275ada // v2.19.8
 	github.com/sirupsen/logrus v1.6.0
@ -54,7 +55,8 @@ require (
 	github.com/ultraware/whitespace v0.0.4
 	github.com/uudashr/gocognit v1.0.1
 	github.com/valyala/quicktemplate v1.5.0
-	golang.org/x/tools v0.0.0-20200519015757-0d0afa43d58a
+	golang.org/x/mod v0.3.0 // indirect
+	golang.org/x/tools v0.0.0-20200702044944-0cc1aa72b347
 	gopkg.in/yaml.v2 v2.3.0
 	honnef.co/go/tools v0.0.1-2020.1.4
 	mvdan.cc/gofumpt v0.0.0-20200513141252-abc0db2c416a
--- a/go.sum
+++ b/go.sum
@ -196,6 +196,7 @@ github.com/jingyugao/rowserrcheck v0.0.0-20191204022205-72ab7603b68a h1:Gmsqmapf
 github.com/jingyugao/rowserrcheck v0.0.0-20191204022205-72ab7603b68a/go.mod h1:xRskid8CManxVta/ALEhJha/pweKBaVG6fWgc0yH25s=
 github.com/jirfag/go-printf-func-name v0.0.0-20191110105641-45db9963cdd3 h1:jNYPNLe3d8smommaoQlK7LOA5ESyUJJ+Wf79ZtA7Vp4=
 github.com/jirfag/go-printf-func-name v0.0.0-20191110105641-45db9963cdd3/go.mod h1:HEWGJkRDzjJY2sqdDwxccsGicWEf9BQOZsq2tV+xzM0=
+github.com/jmoiron/sqlx v1.2.0/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks=
 github.com/jmoiron/sqlx v1.2.1-0.20190826204134-d7d95172beb5 h1:lrdPtrORjGv1HbbEvKWDUAy97mPpFm4B8hp77tcCUJY=
 github.com/jmoiron/sqlx v1.2.1-0.20190826204134-d7d95172beb5/go.mod h1:1FEQNm3xlJgrMD+FBdI9+xvCksHtbpVBBw5dYhBSsks=
 github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
@ -223,6 +224,7 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/kyoh86/exportloopref v0.1.4 h1:t8QP+vBUykOFp6Bks/ZVYm3+Rp3+aj+AKWpGXgK4anA=
 github.com/kyoh86/exportloopref v0.1.4/go.mod h1:h1rDl2Kdj97+Kwh4gdz3ujE7XHmH51Q0lUiZ1z4NLj8=
+github.com/lib/pq v1.0.0 h1:X5PMW56eZitiTeO7tKzZxFCSpbFZJtkMMooicw2us9A=
 github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
 github.com/logrusorgru/aurora v0.0.0-20181002194514-a7b3b318ed4e/go.mod h1:7rIyQOR62GCctdiQpZ/zOJlFyk6y+94wXzv6RNZgaR4=
 github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
@ -314,6 +316,8 @@ github.com/rogpeppe/go-internal v1.5.2/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTE
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/ryancurrah/gomodguard v1.1.0 h1:DWbye9KyMgytn8uYpuHkwf0RHqAYO6Ay/D0TbCpPtVU=
 github.com/ryancurrah/gomodguard v1.1.0/go.mod h1:4O8tr7hBODaGE6VIhfJDHcwzh5GUccKSJBU0UMXJFVM=
+github.com/ryanrolds/sqlclosecheck v0.3.0 h1:AZx+Bixh8zdUBxUA1NxbxVAS78vTPq4rCb8OUZI9xFw=
+github.com/ryanrolds/sqlclosecheck v0.3.0/go.mod h1:1gREqxyTGR3lVtpngyFo3hZAgk0KCtEdgEkHwDbigdA=
 github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
 github.com/securego/gosec/v2 v2.3.0 h1:y/9mCF2WPDbSDpL3QDWZD3HHGrSYw0QSHnCqTfs4JPE=
@ -426,6 +430,8 @@ golang.org/x/mod v0.1.0/go.mod h1:0QHyrYULN0/3qlju5TqG8bIK38QM8yzMo5ekMj3DlcY=
 golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
 golang.org/x/mod v0.2.0 h1:KU7oHjnv3XNWfa5COkzUifxZmxp1TyI7ImMXqFxLwvQ=
 golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
+golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@ -524,6 +530,10 @@ golang.org/x/tools v0.0.0-20200422022333-3d57cf2e726e/go.mod h1:EkVYQZoAsY45+roY
 golang.org/x/tools v0.0.0-20200428185508-e9a00ec82136/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20200519015757-0d0afa43d58a h1:gILuVKC+ZPD6g/tj6zBOdnOH1ZHI0zZ86+KLMogc6/s=
 golang.org/x/tools v0.0.0-20200519015757-0d0afa43d58a/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200625211823-6506e20df31f h1:LZ/huls0xxEEg5l+BxB7UuQZReNVYe8fQAQu/AzPI0U=
+golang.org/x/tools v0.0.0-20200625211823-6506e20df31f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200702044944-0cc1aa72b347 h1:/e4fNMHdLn7SQSxTrRZTma2xjQW6ELdxcnpqMhpo9X4=
+golang.org/x/tools v0.0.0-20200702044944-0cc1aa72b347/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
--- a/pkg/golinters/sqlclosecheck.go
+++ b/pkg/golinters/sqlclosecheck.go
@ -0,0 +1,21 @@
+package golinters
+
+import (
+	"github.com/ryanrolds/sqlclosecheck/pkg/analyzer"
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
+)
+
+func NewSQLCloseCheck() *goanalysis.Linter {
+	analyzers := []*analysis.Analyzer{
+		analyzer.NewAnalyzer(),
+	}
+
+	return goanalysis.NewLinter(
+		"sqlclosecheck",
+		"Checks that sql.Rows and sql.Stmt are closed.",
+		analyzers,
+		nil,
+	).WithLoadMode(goanalysis.LoadModeTypesInfo)
+}
--- a/pkg/lint/lintersdb/manager.go
+++ b/pkg/lint/lintersdb/manager.go
@ -288,6 +288,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithPresets(linter.PresetBugs).
 			WithLoadForGoAnalysis().
 			WithURL("https://github.com/nishanths/exhaustive"),
+		linter.NewConfig(golinters.NewSQLCloseCheck()).
+			WithPresets(linter.PresetBugs).
+			WithLoadForGoAnalysis().
+			WithURL("https://github.com/ryanrolds/sqlclosecheck"),
 		// nolintlint must be last because it looks at the results of all the previous linters for unused nolint directives
 		linter.NewConfig(golinters.NewNoLintLint()).
 			WithPresets(linter.PresetStyle).
--- a/test/testdata/sqlclosecheck.go
+++ b/test/testdata/sqlclosecheck.go
@ -0,0 +1,342 @@
+//args: -Esqlclosecheck
+package testdata
+
+import (
+	"context"
+	"database/sql"
+	"log"
+	"strings"
+
+	_ "github.com/go-sql-driver/mysql"
+	"github.com/jmoiron/sqlx"
+)
+
+var (
+	ctx    context.Context
+	db     *sql.DB
+	dbx    *sqlx.DB
+	age    = 27
+	userID = 43
+)
+
+func rowsCorrectDeferBlock() {
+
+	rows, err := db.QueryContext(ctx, "SELECT name FROM users WHERE age=?", age)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	defer func() {
+		err := rows.Close()
+		if err != nil {
+			log.Print("problem closing rows")
+		}
+	}()
+
+	names := make([]string, 0)
+	for rows.Next() {
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			log.Fatal(err)
+		}
+		names = append(names, name)
+	}
+
+	// Check for errors from iterating over rows.
+	if err := rows.Err(); err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("%s are %d years old", strings.Join(names, ", "), age)
+}
+
+func rowsCorrectDefer() {
+	rows, err := db.QueryContext(ctx, "SELECT name FROM users WHERE age=?", age)
+	if err != nil {
+		log.Fatal(err)
+	}
+	defer rows.Close()
+
+	names := make([]string, 0)
+	for rows.Next() {
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			log.Fatal(err)
+		}
+		names = append(names, name)
+	}
+
+	// Check for errors from iterating over rows.
+	if err := rows.Err(); err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("%s are %d years old", strings.Join(names, ", "), age)
+}
+
+func rowsMissingClose() {
+	rows, err := db.QueryContext(ctx, "SELECT name FROM users WHERE age=?", age) // ERROR "Rows/Stmt was not closed"
+	if err != nil {
+		log.Fatal(err)
+	}
+	// defer rows.Close()
+
+	names := make([]string, 0)
+	for rows.Next() {
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			log.Fatal(err)
+		}
+		names = append(names, name)
+	}
+
+	// Check for errors from iterating over rows.
+	if err := rows.Err(); err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("%s are %d years old", strings.Join(names, ", "), age)
+}
+
+func rowsNonDeferClose() {
+	rows, err := db.QueryContext(ctx, "SELECT name FROM users WHERE age=?", age)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	names := make([]string, 0)
+	for rows.Next() {
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			log.Fatal(err)
+		}
+		names = append(names, name)
+	}
+
+	// Check for errors from iterating over rows.
+	if err := rows.Err(); err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("%s are %d years old", strings.Join(names, ", "), age)
+
+	rows.Close() // ERROR "Close should use defer"
+}
+
+func rowsPassedAndClosed() {
+	rows, err := db.QueryContext(ctx, "SELECT name FROM users")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	rowsClosedPassed(rows)
+}
+
+func rowsClosedPassed(rows *sql.Rows) {
+	rows.Close()
+}
+
+func rowsPassedAndNotClosed(rows *sql.Rows) {
+	rows, err := db.QueryContext(ctx, "SELECT name FROM users")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	rowsDontClosedPassed(rows)
+}
+
+func rowsDontClosedPassed(*sql.Rows) {
+
+}
+
+func rowsReturn() (*sql.Rows, error) {
+	rows, err := db.QueryContext(ctx, "SELECT name FROM users WHERE age=?", age)
+	if err != nil {
+		log.Fatal(err)
+	}
+	return rows, nil
+}
+
+func rowsReturnShort() (*sql.Rows, error) {
+	return db.QueryContext(ctx, "SELECT name FROM users WHERE age=?", age)
+}
+
+func stmtCorrectDeferBlock() {
+	// In normal use, create one Stmt when your process starts.
+	stmt, err := db.PrepareContext(ctx, "SELECT username FROM users WHERE id = ?")
+	if err != nil {
+		log.Fatal(err)
+	}
+	defer func() {
+		err := stmt.Close()
+		if err != nil {
+			log.Print("problem closing stmt")
+		}
+	}()
+
+	// Then reuse it each time you need to issue the query.
+	var username string
+	err = stmt.QueryRowContext(ctx, userID).Scan(&username)
+	switch {
+	case err == sql.ErrNoRows:
+		log.Fatalf("no user with id %d", userID)
+	case err != nil:
+		log.Fatal(err)
+	default:
+		log.Printf("username is %s\n", username)
+	}
+}
+
+func stmtCorrectDefer() {
+	// In normal use, create one Stmt when your process starts.
+	stmt, err := db.PrepareContext(ctx, "SELECT username FROM users WHERE id = ?")
+	if err != nil {
+		log.Fatal(err)
+	}
+	defer stmt.Close()
+
+	// Then reuse it each time you need to issue the query.
+	var username string
+	err = stmt.QueryRowContext(ctx, userID).Scan(&username)
+	switch {
+	case err == sql.ErrNoRows:
+		log.Fatalf("no user with id %d", userID)
+	case err != nil:
+		log.Fatal(err)
+	default:
+		log.Printf("username is %s\n", username)
+	}
+}
+
+func stmtMissingClose() {
+	// In normal use, create one Stmt when your process starts.
+	stmt, err := db.PrepareContext(ctx, "SELECT username FROM users WHERE id = ?") // ERROR "Rows/Stmt was not closed"
+	if err != nil {
+		log.Fatal(err)
+	}
+	// defer stmt.Close()
+
+	// Then reuse it each time you need to issue the query.
+	var username string
+	err = stmt.QueryRowContext(ctx, userID).Scan(&username)
+	switch {
+	case err == sql.ErrNoRows:
+		log.Fatalf("no user with id %d", userID)
+	case err != nil:
+		log.Fatal(err)
+	default:
+		log.Printf("username is %s\n", username)
+	}
+}
+
+func stmtNonDeferClose() {
+	// In normal use, create one Stmt when your process starts.
+	stmt, err := db.PrepareContext(ctx, "SELECT username FROM users WHERE id = ?")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	// Then reuse it each time you need to issue the query.
+	var username string
+	err = stmt.QueryRowContext(ctx, userID).Scan(&username)
+	switch {
+	case err == sql.ErrNoRows:
+		log.Fatalf("no user with id %d", userID)
+	case err != nil:
+		log.Fatal(err)
+	default:
+		log.Printf("username is %s\n", username)
+	}
+
+	stmt.Close() // ERROR "Close should use defer"
+}
+
+func stmtReturn() (*sql.Stmt, error) {
+	stmt, err := db.PrepareContext(ctx, "SELECT username FROM users WHERE id = ?")
+	if err != nil {
+		return nil, err
+	}
+
+	return stmt, nil
+}
+
+func stmtReturnShort() (*sql.Stmt, error) {
+	return db.PrepareContext(ctx, "SELECT username FROM users WHERE id = ?")
+}
+
+func sqlxCorrectDefer() {
+	rows, err := dbx.Queryx("SELECT name FROM users WHERE age=?", age)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	defer rows.Close()
+
+	names := make([]string, 0)
+	for rows.Next() {
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			log.Fatal(err)
+		}
+		names = append(names, name)
+	}
+
+	// Check for errors from iterating over rows.
+	if err := rows.Err(); err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("%s are %d years old", strings.Join(names, ", "), age)
+}
+
+func sqlxNonDeferClose() {
+	rows, err := dbx.Queryx("SELECT name FROM users WHERE age=?", age)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	names := make([]string, 0)
+	for rows.Next() {
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			log.Fatal(err)
+		}
+		names = append(names, name)
+	}
+
+	// Check for errors from iterating over rows.
+	if err := rows.Err(); err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("%s are %d years old", strings.Join(names, ", "), age)
+
+	rows.Close() // ERROR "Close should use defer"
+}
+
+func sqlxMissingClose() {
+	rows, err := dbx.Queryx("SELECT name FROM users WHERE age=?", age) // ERROR "Rows/Stmt was not closed"
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	// defer rows.Close()
+
+	names := make([]string, 0)
+	for rows.Next() {
+		var name string
+		if err := rows.Scan(&name); err != nil {
+			log.Fatal(err)
+		}
+		names = append(names, name)
+	}
+
+	// Check for errors from iterating over rows.
+	if err := rows.Err(); err != nil {
+		log.Fatal(err)
+	}
+	log.Printf("%s are %d years old", strings.Join(names, ", "), age)
+}
+
+func sqlxReturnRows() (*sqlx.Rows, error) {
+	rows, err := dbx.Queryx("SELECT name FROM users WHERE age=?", age)
+	if err != nil {
+		return nil, err
+	}
+
+	return rows, nil
+}