diff --git a/go.mod b/go.mod index 208b680b17d8..403ca39d0ef3 100644 --- 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 diff --git a/go.sum b/go.sum index 5b846169e04c..279fc61d2b88 100644 --- 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= diff --git a/pkg/golinters/sqlclosecheck.go b/pkg/golinters/sqlclosecheck.go new file mode 100644 index 000000000000..48ca246e70a2 --- /dev/null +++ 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) +} diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 8329ac273a3c..b310ad7a3c06 100644 --- 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). diff --git a/test/testdata/sqlclosecheck.go b/test/testdata/sqlclosecheck.go new file mode 100644 index 000000000000..c9ed58da649d --- /dev/null +++ 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 +}