add rule unslosedResource

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## [Unreleased]
 ### Added:
+- new rule unclosedResource
 - new rule syncPoolNonPtr
 
 ### Changed:

Makefile

@@ -4,11 +4,9 @@ test:
 	go test --count=1 -race .
 
 lint:
-	@echo "Running golangci-lint..."
-	@golangci-lint run --skip-dirs testdata --config=.golangci.yml
+	ruleguard -rules=rules.go ./...
 
 ci-lint: install-linter lint
 
 install-linter:
-	@curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH_DIR)/bin v1.44.0
-	@$(GOPATH_DIR)/bin/golangci-lint run -v
+	@go install  github.com/quasilyte/go-ruleguard/cmd/ruleguard@cb19258d2ade88dbf466420bb4585dc747bcec57

go.mod

@@ -3,15 +3,16 @@ module github.com/delivery-club/delivery-club-rules
 go 1.17
 
 require (
-	github.com/quasilyte/go-ruleguard v0.3.16-0.20220120183406-c57998eb544d
-	github.com/quasilyte/go-ruleguard/dsl v0.3.15
+	github.com/quasilyte/go-ruleguard v0.3.16-0.20220202142729-cb19258d2ade
+	github.com/quasilyte/go-ruleguard/dsl v0.3.16
 	golang.org/x/tools v0.1.9-0.20211228192929-ee1ca4ffc4da
 )
 
 require (
 	github.com/go-toolsmith/astcopy v1.0.0 // indirect
 	github.com/go-toolsmith/astequal v1.0.1 // indirect
 	github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5 // indirect
+	github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567 // indirect
 	golang.org/x/mod v0.5.1 // indirect
 	golang.org/x/sys v0.0.0-20211019181941-9d821ace8654 // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect

go.sum

@@ -9,15 +9,17 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/quasilyte/go-ruleguard v0.3.1-0.20210203134552-1b5a410e1cc8/go.mod h1:KsAh3x0e7Fkpgs+Q9pNLS5XpFSvYCEVl5gP9Pp1xp30=
-github.com/quasilyte/go-ruleguard v0.3.16-0.20220120183406-c57998eb544d h1:S22nV+Hs92FQsvM7fL9SoGt6tsmeBUjRY5OaKMA5mzE=
-github.com/quasilyte/go-ruleguard v0.3.16-0.20220120183406-c57998eb544d/go.mod h1:LeEyOmPzEUSWMOUnTmyMVQe9SOxaG0CeiletX8qcleY=
+github.com/quasilyte/go-ruleguard v0.3.16-0.20220202142729-cb19258d2ade h1:G3m+kdJMAKQX7Xg30c52uuL3SF/bDKPB3vRVmEugsgg=
+github.com/quasilyte/go-ruleguard v0.3.16-0.20220202142729-cb19258d2ade/go.mod h1:VMX+OnnSw4LicdiEGtRSD/1X8kW7GuEscjYNr4cOIT4=
 github.com/quasilyte/go-ruleguard/dsl v0.3.0/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
-github.com/quasilyte/go-ruleguard/dsl v0.3.15 h1:rClYn6lk8wUV5kXnQG4JVsRQjZhSetaNtwml5wkFp5g=
-github.com/quasilyte/go-ruleguard/dsl v0.3.15/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
+github.com/quasilyte/go-ruleguard/dsl v0.3.16 h1:yJtIpd4oyNS+/c/gKqxNwoGO9+lPOsy1A4BzKjJRcrI=
+github.com/quasilyte/go-ruleguard/dsl v0.3.16/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
 github.com/quasilyte/go-ruleguard/rules v0.0.0-20201231183845-9e62ed36efe1/go.mod h1:7JTjp89EGyU1d6XfBiXihJNG37wB2VRkd125Q1u7Plc=
 github.com/quasilyte/go-ruleguard/rules v0.0.0-20211022131956-028d6511ab71/go.mod h1:4cgAphtvu7Ftv7vOT2ZOYhC6CvBxZixcasr8qIOTA50=
 github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5 h1:PDWGei+Rf2bBiuZIbZmM20J2ftEy9IeUCHA8HbQqed8=
 github.com/quasilyte/gogrep v0.0.0-20220120141003-628d8b3623b5/go.mod h1:wSEyW6O61xRV6zb6My3HxrQ5/8ke7NE2OayqCHa3xRM=
+github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567 h1:M8mH9eK4OUR4lu7Gd+PU1fV2/qnDNfzT635KRSObncs=
+github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567/go.mod h1:DWNGW8A4Y+GyBgPuaQJuWiy0XYftx4Xm/y5Jqk9I6VQ=
 github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=

rules.go

@@ -310,6 +310,29 @@ func regexpCompileInLoop(m dsl.Matcher) {
 		Report(`don't compile regex in the loop, move it outside of the loop`)
 }
 
+func unclosedResource(m dsl.Matcher) {
+	varEscapeFunction := func(x dsl.Var) bool {
+		return x.Contains(`$_($*_, $res, $*_)`) || x.Contains(`$_{$*_, $res, $*_}`) ||
+			x.Contains(`$_{$*_, $_: $res, $*_}`) || x.Contains(`$_ <- $res`) ||
+			x.Contains(`return $*_, $res, $*_`) || x.Contains(`$_[$_] = $res`) ||
+			x.Contains(`$_[$res] = $_`)
+	}
+
+	m.Match(`$res, $err := $open($*_); $*body`,
+		`$res, $err = $open($*_); $*body`,
+		`var $res, $err = $open($*_); $*body`,
+	).
+		Where(
+			m["res"].Type.Implements(`io.Closer`) &&
+				!m["res"].Object.IsGlobal() &&
+				m["err"].Type.Implements(`error`) &&
+				!m["body"].Contains(`$res.Close()`) &&
+				!varEscapeFunction(m["body"]),
+		).
+		Report(`$res.Close() should be deferred right after the $open error check`).
+		At(m["res"])
+}
+
 func simplifyErrorCheck(m dsl.Matcher) {
 	m.Match(`$err := $f($*args); if $err != nil { $*body }`).
 		Where(m["err"].Type.Implements("error") &&
@@ -336,8 +359,8 @@ func simplifyErrorCheck(m dsl.Matcher) {
 func syncPoolNonPtr(m dsl.Matcher) {
 	isPointer := func(x dsl.Var) bool {
 		return x.Type.Underlying().Is("*$_") || x.Type.Underlying().Is("chan $_") ||
-				x.Type.Underlying().Is("map[$_]$_") || x.Type.Underlying().Is("interface{$*_}") ||
-				x.Type.Underlying().Is(`func($*_) $*_`) || x.Type.Underlying().Is(`unsafe.Pointer`)
+			x.Type.Underlying().Is("map[$_]$_") || x.Type.Underlying().Is("interface{$*_}") ||
+			x.Type.Underlying().Is(`func($*_) $*_`) || x.Type.Underlying().Is(`unsafe.Pointer`)
 	}
 
 	m.Match(`$x.Put($y)`).

testdata/src/queryWithoutContext/sqlstd.go

@@ -15,6 +15,8 @@ type decoratorWithParams struct {
 
 func warnings() {
 	db, _ := sql.Open("PostgreSQL", "test")
+	defer db.Close()
+
 	db.Query(`SELECT 1`)    // want `don't send query to external storage without context`
 	db.QueryRow(`SELECT 1`) // want `don't send query to external storage without context`
 	db.Exec(`SELECT 1`)     // want `don't send query to external storage without context`
@@ -48,9 +50,11 @@ func warnings() {
 	tx.Stmt(nil)         // want `don't send query to external storage without context`
 
 	stmt, _ := db.Prepare(query) // want `don't send query to external storage without context`
-	stmt.Query(query)            // want `don't send query to external storage without context`
-	stmt.Exec(query)             // want `don't send query to external storage without context`
-	stmt.QueryRow(query)         // want `don't send query to external storage without context`
+	defer stmt.Close()
+
+	stmt.Query(query)    // want `don't send query to external storage without context`
+	stmt.Exec(query)     // want `don't send query to external storage without context`
+	stmt.QueryRow(query) // want `don't send query to external storage without context`
 
 	db.ExecContext(context.Background(), query)
 	tx.StmtContext(context.Background(), nil)

testdata/src/unclosedResource/negative.go

@@ -0,0 +1,127 @@
+package unclosedResource
+
+import (
+	"database/sql"
+	"io/ioutil"
+	"os"
+)
+
+func negative1() {
+	ff, err := ioutil.TempFile("/fo", "bo")
+	if err != nil {
+		print(err)
+	}
+	defer ff.Close()
+
+	ff, err = ioutil.TempFile("/fo", "bo")
+	if err != nil {
+		print(err)
+	}
+	print(123)
+	ff.Close()
+}
+
+var globalVar *os.File
+
+func negative2() {
+	globalVar, _ = ioutil.TempFile("", "") // false negative because "_" doesn't have error type
+	kk := globalVar.Name()
+
+	print(kk)
+}
+
+func negativeGlobalVar() {
+	var err error
+	globalVar, err = ioutil.TempFile("", "") // global var
+	kk := globalVar.Name()
+
+	print(kk, err)
+}
+
+func negative3() *os.File {
+	file, _ := ioutil.TempFile("", "") // var escape the function
+
+	return file
+}
+
+func negative4() {
+	var files []*os.File
+	file, _ := ioutil.TempFile("", "") // var escape the function in another var
+
+	files = append(files, file)
+}
+
+func negative5() {
+	var filesMap map[string]*os.File
+	file, _ := ioutil.TempFile("", "") // var escape the function in another var
+
+	filesMap[file.Name()] = file
+}
+
+func negative6() {
+	type st struct {
+		*os.File
+	}
+	var (
+		fileDecorator1 st
+		fileDecorator2 st
+	)
+	file, _ := ioutil.TempFile("", "") // var escape the function in another var
+
+	fileDecorator1 = st{file}
+	fileDecorator2 = st{
+		File: file,
+	}
+
+	kk, kkk := fileDecorator1.Name(), fileDecorator2.Name()
+
+	print(kk, kkk)
+}
+
+func negative7() {
+	var ch chan *os.File
+	file, _ := ioutil.TempFile("", "") // var escape the function in another var
+
+	ch <- file
+}
+
+func negative8() []int {
+	db, _ := sql.Open("", "")
+	defer db.Close()
+
+	rows, _ := db.QueryContext(nil, "")
+
+	var (
+		i      int
+		result []int
+	)
+	defer rows.Close()
+
+	for rows.Next() {
+		rows.Scan(&i)
+		result = append(result, i)
+	}
+
+	return result
+}
+
+func negative9() {
+	closure := func() (*os.File, error) {
+		return nil, nil
+	}
+
+	f, _ := closure()
+	f.Name()
+	defer f.Close()
+}
+
+// test for: https://github.com/quasilyte/go-ruleguard/issues/366
+func dataRace() {
+	f, err := os.Open("bar")
+	print(f.Name())
+
+	f, err = os.Open("bar")
+	if err == nil {
+		defer f.Close()
+	}
+}

testdata/src/unclosedResource/positive.go

@@ -0,0 +1,83 @@
+package unclosedResource
+
+import (
+	"database/sql"
+	"io/ioutil"
+	"os"
+)
+
+func warning1() {
+	f, err := os.Open("bar") //want `\Qf.Close() should be deferred right after the os.Open error check`
+	if err == nil {
+		print(f.Name())
+	}
+
+	f, err = os.Open("bar") //want `\Qf.Close() should be deferred right after the os.Open error check`
+	print(f.Name())
+}
+
+func warning2() {
+	f, err := os.Open("bar")
+	if err == nil {
+		defer f.Close()
+	}
+
+	f, err = os.Open("bar") //want `\Qf.Close() should be deferred right after the os.Open error check`
+	print(f.Name())
+}
+
+func warning3() {
+	var ff, err = os.Open("foo.txt") //want `\Qff.Close() should be deferred right after the os.Open error check`
+	if err != nil {
+		print(ff.Fd())
+	}
+
+	ff, err = ioutil.TempFile("/kek", "foo") //want `\Qff.Close() should be deferred right after the ioutil.TempFile error check`
+	print(ff.Name())
+}
+
+func warning4() {
+	ff, err := os.Open("foo.txt") //want `\Qff.Close() should be deferred right after the os.Open error check`
+	if err != nil {
+		print(ff.Fd())
+	}
+
+	ff, err = ioutil.TempFile("/kek", "foo") //want `\Qff.Close() should be deferred right after the ioutil.TempFile error check`
+	print(ff.Name())
+}
+
+func warning5() {
+	f, err := os.Open("bar") //want `\Qf.Close() should be deferred right after the os.Open error check`
+	print(f.Name())
+
+	ff, err := os.Open("bar")
+	if err == nil {
+		defer ff.Close()
+	}
+}
+
+func warning6() []int {
+	db, _ := sql.Open("", "")              //want `\Qdb.Close() should be deferred right after the sql.Open error check`
+	var rows, _ = db.QueryContext(nil, "") //want `\Qrows.Close() should be deferred right after the db.QueryContext error check`
+
+	var (
+		i      int
+		result []int
+	)
+
+	for rows.Next() {
+		_ = rows.Scan(&i)
+		result = append(result, i)
+	}
+
+	return result
+}
+
+func warning7() {
+	closure := func() (*os.File, error) {
+		return nil, nil
+	}
+
+	f, _ := closure() // want `\Qf.Close() should be deferred right after the closure error check`
+	f.Name()
+}

todo

@@ -1,2 +1 @@
 1. timer.Stop check
-2. not closed resources check