Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
sqlclosecheck
bin/
sqlx_examples_results.txt
pgx_examples_results.txt
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
BIN := bin

PHONY: build install test
PHONY: build install test clean

$(BIN):
mkdir -p $@
Expand All @@ -26,3 +26,7 @@ test: build
lint:
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.54.1
./bin/golangci-lint run

clean:
rm -rf $(BIN)
rm -f pgx_examples_results.txt sqlx_examples_results.txt
20 changes: 10 additions & 10 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ module github.com/ryanrolds/sqlclosecheck
go 1.20

require (
github.com/go-sql-driver/mysql v1.7.1
github.com/jackc/pgx/v5 v5.4.3
github.com/jmoiron/sqlx v1.3.5
golang.org/x/tools v0.12.0
github.com/go-sql-driver/mysql v1.8.1
github.com/jackc/pgx/v5 v5.6.0
github.com/jmoiron/sqlx v1.4.0
golang.org/x/tools v0.22.0
)

require (
filippo.io/edwards25519 v1.1.0 // indirect
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect
github.com/jackc/puddle/v2 v2.2.1 // indirect
golang.org/x/crypto v0.12.0 // indirect
golang.org/x/mod v0.12.0 // indirect
golang.org/x/sync v0.3.0 // indirect
golang.org/x/sys v0.11.0 // indirect
golang.org/x/text v0.12.0 // indirect
golang.org/x/crypto v0.24.0 // indirect
golang.org/x/mod v0.18.0 // indirect
golang.org/x/sync v0.7.0 // indirect
golang.org/x/text v0.16.0 // indirect
)
52 changes: 24 additions & 28 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,43 +1,39 @@
filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA=
filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
github.com/go-sql-driver/mysql v1.7.1 h1:lUIinVbN1DY0xBg0eMOzmmtGoHwWBbvnWubQUrtU8EI=
github.com/go-sql-driver/mysql v1.7.1/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI=
github.com/go-sql-driver/mysql v1.8.1 h1:LedoTUt/eveggdHS9qUFC1EFSa8bU2+1pZjSRpvNJ1Y=
github.com/go-sql-driver/mysql v1.8.1/go.mod h1:wEBSXgmK//2ZFJyE+qWnIsVGmvmEKlqwuVSjsCm7DZg=
github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM=
github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg=
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a h1:bbPeKD0xmW/Y25WS6cokEszi5g+S0QxI/d45PkRi7Nk=
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM=
github.com/jackc/pgx/v5 v5.4.3 h1:cxFyXhxlvAifxnkKKdlxv8XqUf59tDlYjnV5YYfsJJY=
github.com/jackc/pgx/v5 v5.4.3/go.mod h1:Ig06C2Vu0t5qXC60W8sqIthScaEnFvojjj9dSljmHRA=
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo=
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM=
github.com/jackc/pgx/v5 v5.6.0 h1:SWJzexBzPL5jb0GEsrPMLIsi/3jOo7RHlzTjcAeDrPY=
github.com/jackc/pgx/v5 v5.6.0/go.mod h1:DNZ/vlrUnhWCoFGxHAG8U2ljioxukquj7utPDgtQdTw=
github.com/jackc/puddle/v2 v2.2.1 h1:RhxXJtFG022u4ibrCSMSiu5aOq1i77R3OHKNJj77OAk=
github.com/jackc/puddle/v2 v2.2.1/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
github.com/jmoiron/sqlx v1.3.5 h1:vFFPA71p1o5gAeqtEAwLU4dnX2napprKtHr7PYIcN3g=
github.com/jmoiron/sqlx v1.3.5/go.mod h1:nRVWtLre0KfCLJvgxzCsLVMogSvQ1zNJtpYr2Ccp0mQ=
github.com/lib/pq v1.2.0 h1:LXpIM/LZ5xGFhOpXAQUIMM1HdyqzVYM13zNdjCEEcA0=
github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/mattn/go-sqlite3 v1.14.6 h1:dNPt6NO46WmLVt2DLNpwczCmdV5boIZ6g/tlDrlRUbg=
github.com/mattn/go-sqlite3 v1.14.6/go.mod h1:NyWgC/yNuGj7Q9rpYnZvas74GogHl5/Z4A/KQRfk6bU=
github.com/jmoiron/sqlx v1.4.0 h1:1PLqN7S1UYp5t4SrVVnt4nUVNemrDAtxlulVe+Qgm3o=
github.com/jmoiron/sqlx v1.4.0/go.mod h1:ZrZ7UsYB/weZdl2Bxg6jCRO9c3YHl8r3ahlKmRT4JLY=
github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/mattn/go-sqlite3 v1.14.22 h1:2gZY6PC6kBnID23Tichd1K+Z0oS6nE/XwU+Vz/5o4kU=
github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
golang.org/x/crypto v0.12.0 h1:tFM/ta59kqch6LlvYnPa0yx5a83cL2nHflFhYKvv9Yk=
golang.org/x/crypto v0.12.0/go.mod h1:NF0Gs7EO5K4qLn+Ylc+fih8BSTeIjAP05siRnAh98yw=
golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc=
golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E=
golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM=
golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/text v0.12.0 h1:k+n5B8goJNdU7hSvEtMUz3d1Q6D/XW4COJSJR6fN0mc=
golang.org/x/text v0.12.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
golang.org/x/tools v0.12.0 h1:YW6HUoUmYBpwSgyaGaZq1fHjrBjX1rlpZ54T6mu2kss=
golang.org/x/tools v0.12.0/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM=
golang.org/x/crypto v0.24.0 h1:mnl8DM0o513X8fdIkmyFE/5hTYxbwYOjDS/+rK6qpRI=
golang.org/x/crypto v0.24.0/go.mod h1:Z1PMYSOR5nyMcyAVAIQSKCDwalqy85Aqn1x3Ws4L5DM=
golang.org/x/mod v0.18.0 h1:5+9lSbEzPSdWkH32vYPBwEpX8KwDbM52Ud9xBUvNlb0=
golang.org/x/mod v0.18.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI=
golang.org/x/tools v0.22.0 h1:gqSGLZqv+AI9lIQzniJ0nZDRG5GBPsSi+DRNHWNz6yA=
golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
4 changes: 0 additions & 4 deletions pgx_examples_results.txt

This file was deleted.

5 changes: 5 additions & 0 deletions pkg/analyzer/defer_only.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,11 @@ func getAction(instr ssa.Instruction, targetTypes []any) action {
return actionClosed
}

// the defer closure takes in a parameter, check inside the defer block
if _, ok := instr.Call.Value.(*ssa.Parameter); ok && instr.Call.Method.Name() == closeMethod {
return actionClosed
}

if !isTarget {
return actionPassed
}
Expand Down
35 changes: 35 additions & 0 deletions pkg/analyzer/testdata/pgx/correct_defer_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package pgx

import (
"log"

"github.com/jackc/pgx/v5"
)

func correctDeferBlockTx() {
Expand All @@ -15,6 +17,17 @@ func correctDeferBlockTx() {
}()
}

func correctDeferBlockWithParameterTx() {
rows, err := pgxTx.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

defer func(rows pgx.Rows) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you plan to support the t.Cleanup way, I mean, either the detection, or adding them to correct cases ?

I didn't look at all code (past, or added) but I'm wondering.

So I prefer to raise the point, and get a "already supported, RTFM" 😁😅

Of course, it can be addressed in another PR

rows.Close()
}(rows)
}

func correctDeferBlockConn() {
rows, err := pgxConn.Query(ctx, "SELECT username FROM users")
if err != nil {
Expand All @@ -26,6 +39,17 @@ func correctDeferBlockConn() {
}()
}

func correctDeferBlockWithParameterConn() {
rows, err := pgxConn.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

defer func(rows pgx.Rows) {
rows.Close()
}(rows)
}

func correctDeferBlockPgxPool() {
rows, err := pgxPool.Query(ctx, "SELECT username FROM users")
if err != nil {
Expand All @@ -36,3 +60,14 @@ func correctDeferBlockPgxPool() {
rows.Close()
}()
}

func correctDeferBlockWithParameterPgxPool() {
rows, err := pgxPool.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

defer func(rows pgx.Rows) {
rows.Close()
}(rows)
}
31 changes: 31 additions & 0 deletions pkg/analyzer/testdata/rows/correct_defer_block.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rows

import (
"database/sql"
"log"
"strings"
)
Expand Down Expand Up @@ -34,3 +35,33 @@ func correctDeferBlock() {
}
log.Printf("%s are %d years old", strings.Join(names, ", "), age)
}

func correctDeferBlockWithParameter() {
age := 27
rows, err := db.QueryContext(ctx, "SELECT name FROM users WHERE age=?", age)
if err != nil {
log.Fatal(err)
}

defer func(rows *sql.Rows) {
err := rows.Close()
if err != nil {
log.Print("problem closing rows")
}
}(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)
}
27 changes: 27 additions & 0 deletions pkg/analyzer/testdata/stmt/correct_defer_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,30 @@ func correctDeferBlock() {
log.Printf("username is %s\n", username)
}
}

func correctDeferBlockWithPArameter() {
// 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(stmt *sql.Stmt) {
err := stmt.Close()
if err != nil {
log.Print("problem closing stmt")
}
}(stmt)

// Then reuse it each time you need to issue the query.
id := 43
var username string
err = stmt.QueryRowContext(ctx, id).Scan(&username)
switch {
case err == sql.ErrNoRows:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking side remark.

I know you are using the direct sql lib method, but I would have used errors.Is

Suggested change
case err == sql.ErrNoRows:
case errors.Is(err, sql.ErrNoRows):

My suggestion is related to all the codebase

log.Fatalf("no user with id %d", id)
case err != nil:
log.Fatal(err)
default:
log.Printf("username is %s\n", username)
}
}
35 changes: 35 additions & 0 deletions testdata/pgx_examples/correct_defer_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package pgx_examples

import (
"log"

"github.com/jackc/pgx/v5"
)

func correctDeferBlockTx() {
Expand All @@ -15,6 +17,17 @@ func correctDeferBlockTx() {
}()
}

func correctDeferBlockWithParameterTx() {
rows, err := pgxTx.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

defer func(rows pgx.Rows) {
rows.Close()
}(rows)
}

func correctDeferBlockConn() {
rows, err := pgxConn.Query(ctx, "SELECT username FROM users")
if err != nil {
Expand All @@ -26,6 +39,17 @@ func correctDeferBlockConn() {
}()
}

func correctDeferBlockWithParameterConn() {
rows, err := pgxConn.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

defer func(rows pgx.Rows) {
rows.Close()
}(rows)
}

func correctDeferBlockPgxPool() {
rows, err := pgxPool.Query(ctx, "SELECT username FROM users")
if err != nil {
Expand All @@ -36,3 +60,14 @@ func correctDeferBlockPgxPool() {
rows.Close()
}()
}

func correctDeferBlockWithParameterPgxPool() {
rows, err := pgxPool.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

defer func(rows pgx.Rows) {
rows.Close()
}(rows)
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the copy paste you do with small variation, maybe these function could be generated.

But I would understand if you think it overkill

2 changes: 2 additions & 0 deletions testdata/pgx_examples/expected_results.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@
testdata/pgx_examples/missing_close.go:8:26: Rows/Stmt/NamedStmt was not closed
testdata/pgx_examples/missing_close.go:17:28: Rows/Stmt/NamedStmt was not closed
testdata/pgx_examples/missing_close.go:26:28: Rows/Stmt/NamedStmt was not closed
testdata/pgx_examples/missing_close_defer.go:10:26: Rows/Stmt/NamedStmt was not closed
testdata/pgx_examples/missing_close_defer.go:23:26: Rows/Stmt/NamedStmt was not closed

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised to see such a file

Why not using the / want Go idiomatic comment in the code?

33 changes: 33 additions & 0 deletions testdata/pgx_examples/missing_close_defer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package pgx_examples

import (
"log"

"github.com/jackc/pgx/v5"
)

func missingCloseDeferBlock() {
rows, err := pgxTx.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

_ = rows

defer func() {

}()
}

func missingCloseDeferBlockWithParameter() {
rows, err := pgxTx.Query(ctx, "SELECT username FROM users")
if err != nil {
log.Fatal(err)
}

_ = rows

defer func(rows pgx.Rows) {

}(rows)
}
Loading