Merge pull request #25 from hallabro-consulting-ab/pgx-support

Implement support for github.com/jackc/pgx

.gitignore

@@ -1,3 +1,3 @@
 sqlclosecheck
 bin/
-sqlx_examples_results.txt
+sqlx_examples_results.txt

Makefile

@@ -19,6 +19,10 @@ test: build
 	-go vet -vettool=$(BIN)/sqlclosecheck ./testdata/sqlx_examples 2> sqlx_examples_results.txt
 	diff -a sqlx_examples_results.txt ./testdata/sqlx_examples/expected_results.txt
 
+	-go vet -vettool=$(BIN)/sqlclosecheck ./testdata/pgx_examples
+	-go vet -vettool=$(BIN)/sqlclosecheck ./testdata/pgx_examples 2> pgx_examples_results.txt
+	diff -a pgx_examples_results.txt ./testdata/pgx_examples/expected_results.txt
+
 lint:
 	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.54.1
 	./bin/golangci-lint run

README.md

@@ -21,12 +21,3 @@ go vet -vettool=$(which sqlclosecheck) ./...
 go install github.com/ryanrolds/sqlclosecheck@latest
 go vet -vettool=${GOPATH}/bin/sqlclosecheck ./...
 ```
-
-## Roadmap
-
-* ~~Get linter working~~
-* ~~Added some basic test cases~~
-* ~~Require that Close be deferred~~
-* ~~Add sqlx checking~~
-* ~~Test across a bunch of projects~~
-* ~~Introduce linter to [golangci-lint](https://github.com/golangci/golangci-lint).~~

go.mod

@@ -1,14 +1,21 @@
 module github.com/ryanrolds/sqlclosecheck
 
-go 1.19
+go 1.21
 
 require (
-	github.com/go-sql-driver/mysql v1.7.0
+	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.5.0
+	golang.org/x/tools v0.12.0
 )
 
 require (
-	golang.org/x/mod v0.7.0 // indirect
-	golang.org/x/sys v0.4.0 // indirect
+	github.com/jackc/pgpassfile v1.0.0 // indirect
+	github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // 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
 )

go.sum

@@ -1,16 +1,43 @@
+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.0 h1:ueSltNNllEqE3qcWBTD0iQd3IpL/6U+mJxLkazJ7YPc=
-github.com/go-sql-driver/mysql v1.7.0/go.mod h1:OXbVy3sEdcQ2Doequ6Z5BW6fXNQTmx+9S1MCJN5yJMI=
+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/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/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=
-golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=
-golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
-golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
-golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18=
-golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/tools v0.5.0 h1:+bSpV5HIeWkuvgaMfI3UmKRThoTA5ODJTUd8T17NO+4=
-golang.org/x/tools v0.5.0/go.mod h1:N+Kgy78s5I24c24dU8OfWNEotWjutIs8SnJvn5IDq+k=
+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=
+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=

pgx_examples_results.txt

@@ -0,0 +1,4 @@
+# github.com/ryanrolds/sqlclosecheck/testdata/pgx_examples
+testdata/pgx_examples/missing_close.go:8:26: Rows/Stmt was not closed
+testdata/pgx_examples/missing_close.go:17:28: Rows/Stmt was not closed
+testdata/pgx_examples/missing_close.go:26:28: Rows/Stmt was not closed

pkg/analyzer/analyzer.go

@@ -2,7 +2,6 @@ package analyzer
 
 import (
 	"go/types"
-
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/buildssa"
 	"golang.org/x/tools/go/ssa"
@@ -31,13 +30,15 @@ var (
 	sqlPackages = []string{
 		"database/sql",
 		"github.com/jmoiron/sqlx",
+		"github.com/jackc/pgx/v5",
+		"github.com/jackc/pgx/v5/pgxpool",
 	}
 )
 
 func NewAnalyzer() *analysis.Analyzer {
 	return &analysis.Analyzer{
 		Name: "sqlclosecheck",
-		Doc:  "Checks that sql.Rows and sql.Stmt are closed.",
+		Doc:  "Checks that sql.Rows, sql.Stmt, pgx.Query are closed.",
 		Run:  run,
 		Requires: []*analysis.Analyzer{
 			buildssa.Analyzer,
@@ -63,14 +64,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	for _, f := range funcs {
 		for _, b := range f.Blocks {
 			for i := range b.Instrs {
-				// Check if instruction is call that returns a target type
+				// Check if instruction is call that returns a target pointer type
 				targetValues := getTargetTypesValues(b, i, targetTypes)
 				if len(targetValues) == 0 {
 					continue
 				}
 
-				// log.Printf("%s", f.Name())
-
 				// For each found target check if they are closed and deferred
 				for _, targetValue := range targetValues {
 					refs := (*targetValue.value).Referrers()
@@ -88,8 +87,8 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	return nil, nil
 }
 
-func getTargetTypes(pssa *buildssa.SSA, targetPackages []string) []*types.Pointer {
-	targets := []*types.Pointer{}
+func getTargetTypes(pssa *buildssa.SSA, targetPackages []string) []any {
+	targets := []any{}
 
 	for _, sqlPkg := range targetPackages {
 		pkg := pssa.Pkg.Prog.ImportedPackage(sqlPkg)
@@ -98,7 +97,12 @@ func getTargetTypes(pssa *buildssa.SSA, targetPackages []string) []*types.Pointe
 			continue
 		}
 
-		rowsType := getTypePointerFromName(pkg, rowsName)
+		rowsPtrType := getTypePointerFromName(pkg, rowsName)
+		if rowsPtrType != nil {
+			targets = append(targets, rowsPtrType)
+		}
+
+		rowsType := getTypeFromName(pkg, rowsName)
 		if rowsType != nil {
 			targets = append(targets, rowsType)
 		}
@@ -128,12 +132,28 @@ func getTypePointerFromName(pkg *ssa.Package, name string) *types.Pointer {
 	return types.NewPointer(named)
 }
 
+func getTypeFromName(pkg *ssa.Package, name string) *types.Named {
+	pkgType := pkg.Type(name)
+	if pkgType == nil {
+		// this package does not use Rows/Stmt
+		return nil
+	}
+
+	obj := pkgType.Object()
+	named, ok := obj.Type().(*types.Named)
+	if !ok {
+		return nil
+	}
+
+	return named
+}
+
 type targetValue struct {
 	value *ssa.Value
 	instr ssa.Instruction
 }
 
-func getTargetTypesValues(b *ssa.BasicBlock, i int, targetTypes []*types.Pointer) []targetValue {
+func getTargetTypesValues(b *ssa.BasicBlock, i int, targetTypes []any) []targetValue {
 	targetValues := []targetValue{}
 
 	instr := b.Instrs[i]
@@ -149,21 +169,32 @@ func getTargetTypesValues(b *ssa.BasicBlock, i int, targetTypes []*types.Pointer
 		varType := v.Type()
 
 		for _, targetType := range targetTypes {
-			if !types.Identical(varType, targetType) {
+			var tt types.Type
+
+			switch targetType.(type) {
+			case *types.Pointer:
+				tt = targetType.(*types.Pointer)
+			case *types.Named:
+				tt = targetType.(*types.Named)
+			default:
+				continue
+			}
+
+			if !types.Identical(varType, tt) {
 				continue
 			}
 
 			for _, cRef := range *call.Referrers() {
 				switch instr := cRef.(type) {
 				case *ssa.Call:
-					if len(instr.Call.Args) >= 1 && types.Identical(instr.Call.Args[0].Type(), targetType) {
+					if len(instr.Call.Args) >= 1 && types.Identical(instr.Call.Args[0].Type(), tt) {
 						targetValues = append(targetValues, targetValue{
 							value: &instr.Call.Args[0],
 							instr: call,
 						})
 					}
 				case ssa.Value:
-					if types.Identical(instr.Type(), targetType) {
+					if types.Identical(instr.Type(), tt) {
 						targetValues = append(targetValues, targetValue{
 							value: &instr,
 							instr: call,
@@ -177,43 +208,42 @@ func getTargetTypesValues(b *ssa.BasicBlock, i int, targetTypes []*types.Pointer
 	return targetValues
 }
 
-func checkClosed(refs *[]ssa.Instruction, targetTypes []*types.Pointer) bool {
+func checkClosed(refs *[]ssa.Instruction, targetTypes []any) bool {
 	numInstrs := len(*refs)
 	for idx, ref := range *refs {
-		// log.Printf("%T - %s", ref, ref)
-
 		action := getAction(ref, targetTypes)
 		switch action {
-		case actionClosed:
+		case actionClosed, actionReturned, actionHandled:
 			return true
 		case actionPassed:
 			// Passed and not used after
 			if numInstrs == idx+1 {
 				return true
 			}
-		case actionReturned:
-			return true
-		case actionHandled:
-			return true
-		default:
-			// log.Printf(action)
 		}
 	}
 
 	return false
 }
 
-func getAction(instr ssa.Instruction, targetTypes []*types.Pointer) action {
+func getAction(instr ssa.Instruction, targetTypes []any) action {
 	switch instr := instr.(type) {
 	case *ssa.Defer:
-		if instr.Call.Value == nil {
-			return actionUnvaluedDefer
+		if instr.Call.Value != nil {
+			name := instr.Call.Value.Name()
+			if name == closeMethod {
+				return actionClosed
+			}
 		}
 
-		name := instr.Call.Value.Name()
-		if name == closeMethod {
-			return actionClosed
+		if instr.Call.Method != nil {
+			name := instr.Call.Method.Name()
+			if name == closeMethod {
+				return actionClosed
+			}
 		}
+
+		return actionUnvaluedDefer
 	case *ssa.Call:
 		if instr.Call.Value == nil {
 			return actionUnvaluedCall
@@ -265,7 +295,18 @@ func getAction(instr ssa.Instruction, targetTypes []*types.Pointer) action {
 	case *ssa.UnOp:
 		instrType := instr.Type()
 		for _, targetType := range targetTypes {
-			if types.Identical(instrType, targetType) {
+			var tt types.Type
+
+			switch targetType.(type) {
+			case *types.Pointer:
+				tt = targetType.(*types.Pointer)
+			case *types.Named:
+				tt = targetType.(*types.Named)
+			default:
+				continue
+			}
+
+			if types.Identical(instrType, tt) {
 				if checkClosed(instr.Referrers(), targetTypes) {
 					return actionHandled
 				}
@@ -277,20 +318,22 @@ func getAction(instr ssa.Instruction, targetTypes []*types.Pointer) action {
 		}
 	case *ssa.Return:
 		return actionReturned
-	default:
-		// log.Printf("%s", instr)
 	}
 
 	return actionUnhandled
 }
 
-func checkDeferred(pass *analysis.Pass, instrs *[]ssa.Instruction, targetTypes []*types.Pointer, inDefer bool) {
+func checkDeferred(pass *analysis.Pass, instrs *[]ssa.Instruction, targetTypes []any, inDefer bool) {
 	for _, instr := range *instrs {
 		switch instr := instr.(type) {
 		case *ssa.Defer:
 			if instr.Call.Value != nil && instr.Call.Value.Name() == closeMethod {
 				return
 			}
+
+			if instr.Call.Method != nil && instr.Call.Method.Name() == closeMethod {
+				return
+			}
 		case *ssa.Call:
 			if instr.Call.Value != nil && instr.Call.Value.Name() == closeMethod {
 				if !inDefer {
@@ -316,7 +359,18 @@ func checkDeferred(pass *analysis.Pass, instrs *[]ssa.Instruction, targetTypes [
 		case *ssa.UnOp:
 			instrType := instr.Type()
 			for _, targetType := range targetTypes {
-				if types.Identical(instrType, targetType) {
+				var tt types.Type
+
+				switch targetType.(type) {
+				case *types.Pointer:
+					tt = targetType.(*types.Pointer)
+				case *types.Named:
+					tt = targetType.(*types.Named)
+				default:
+					continue
+				}
+
+				if types.Identical(instrType, tt) {
 					checkDeferred(pass, instr.Referrers(), targetTypes, inDefer)
 				}
 			}
@@ -326,10 +380,17 @@ func checkDeferred(pass *analysis.Pass, instrs *[]ssa.Instruction, targetTypes [
 	}
 }
 
-func isTargetType(t types.Type, targetTypes []*types.Pointer) bool {
+func isTargetType(t types.Type, targetTypes []any) bool {
 	for _, targetType := range targetTypes {
-		if types.Identical(t, targetType) {
-			return true
+		switch tt := targetType.(type) {
+		case *types.Pointer:
+			if types.Identical(t, tt) {
+				return true
+			}
+		case *types.Named:
+			if types.Identical(t, tt) {
+				return true
+			}
 		}
 	}