Add rows.Err weather checked linter (#849)

Add rowserrcheck

Co-authored-by: Isaev Denis <idenx@yandex.com>

.golangci.yml

@@ -55,6 +55,7 @@ linters:
   # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
   disable-all: true
   enable:
+    # - rowserrcheck
     - bodyclose
     - deadcode
     - depguard

README.md

@@ -228,6 +228,7 @@ maligned: Tool to detect Go structs that would take less memory if their fields
 misspell: Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
 nakedret: Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false]
 prealloc: Finds slice declarations that could potentially be preallocated [fast: true, auto-fix: false]
+rowserrcheck: checks whether Err of rows is checked successfully [fast: true, auto-fix: false]
 scopelint: Scopelint checks for unpinned variables in go programs [fast: true, auto-fix: false]
 stylecheck: Stylecheck is a replacement for golint [fast: true, auto-fix: false]
 unconvert: Remove unnecessary type conversions [fast: true, auto-fix: false]
@@ -456,6 +457,7 @@ golangci-lint help linters
 
 - [bodyclose](https://github.com/timakin/bodyclose) - checks whether HTTP response body is closed successfully
 - [golint](https://github.com/golang/lint) - Golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes
+- [rowserrcheck](https://github.com/jingyugao/rowserr) - checks whether Err of rows is checked successfully
 - [stylecheck](https://github.com/dominikh/go-tools/tree/master/stylecheck) - Stylecheck is a replacement for golint
 - [gosec](https://github.com/securego/gosec) - Inspects source code for security problems
 - [interfacer](https://github.com/mvdan/interfacer) - Linter that suggests narrower interface types
@@ -983,6 +985,7 @@ linters:
   # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
   disable-all: true
   enable:
+    # - rowserrcheck
     - bodyclose
     - deadcode
     - depguard
@@ -1204,6 +1207,7 @@ Thanks to developers and authors of used linters:
 - [timakin](https://github.com/timakin)
 - [kisielk](https://github.com/kisielk)
 - [golang](https://github.com/golang)
+- [jingyugao](https://github.com/jingyugao)
 - [dominikh](https://github.com/dominikh)
 - [securego](https://github.com/securego)
 - [opennota](https://github.com/opennota)

go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21
 	github.com/golangci/revgrep v0.0.0-20180526074752-d9c87f5ffaf0
 	github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4
+	github.com/jingyugao/rowserrcheck v0.0.0-20191204022205-72ab7603b68a
 	github.com/matoous/godox v0.0.0-20190911065817-5d6d842e92eb // v1.0
 	github.com/mattn/go-colorable v0.1.4
 	github.com/mitchellh/go-homedir v1.1.0

go.sum

@@ -42,6 +42,8 @@ github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9
 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
 github.com/go-ole/go-ole v1.2.1 h1:2lOsA72HgjxAuMlKpFiCbHTvu44PIVkZ5hqm3RSdI/E=
 github.com/go-ole/go-ole v1.2.1/go.mod h1:7FAglXiTm7HKlQRDeOQ6ZNUHidzCWXuZWq/1dTyBNF8=
+github.com/go-sql-driver/mysql v1.4.0 h1:7LxgVwFb2hIQtMm87NdgAVfXjnt4OePseqT1tKx+opk=
+github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
 github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
 github.com/go-toolsmith/astcast v1.0.0 h1:JojxlmI6STnFVG9yOImLeGREv8W2ocNUM+iOhR6jE7g=
 github.com/go-toolsmith/astcast v1.0.0/go.mod h1:mt2OdQTeAQcY4DQgPSArJjHCcOwlX+Wl/kwN+LbLGQ4=
@@ -128,6 +130,10 @@ github.com/hpcloud/tail v1.0.0 h1:nfCOvKYfkgYP8hkirhJocXT2+zOD8yUNjXaWfTlyFKI=
 github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
 github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
+github.com/jingyugao/rowserrcheck v0.0.0-20191204022205-72ab7603b68a h1:GmsqmapfzSJkm28dhRoHz2tLRbJmqhU86IPgBtN3mmk=
+github.com/jingyugao/rowserrcheck v0.0.0-20191204022205-72ab7603b68a/go.mod h1:xRskid8CManxVta/ALEhJha/pweKBaVG6fWgc0yH25s=
+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=
 github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=
 github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
@@ -148,6 +154,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw=
 github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
+github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
+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/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=
@@ -159,6 +167,8 @@ github.com/mattn/go-colorable v0.1.4 h1:snbPLB8fVfU9iwbbo30TPtbLRzwWu6aJS6Xh4eaa
 github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=
 github.com/mattn/go-isatty v0.0.8 h1:HLtExJ+uU2HOZ+wI0Tt5DtUDrx8yhUqDcp7fYERX4CE=
 github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
+github.com/mattn/go-sqlite3 v1.9.0 h1:pDRiWfl+++eC2FEFRy6jXmQlvp4Yh3z1MJKg4UeYM/4=
+github.com/mattn/go-sqlite3 v1.9.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
 github.com/mattn/goveralls v0.0.2 h1:7eJB6EqsPhRVxvwEXGnqdO2sJI0PTsrWoTMXEk9/OQc=
 github.com/mattn/goveralls v0.0.2/go.mod h1:8d1ZMHsd7fW6IRPKQh46F2WRpyib5/X4FOpevwGNQEw=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
@@ -311,6 +321,7 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 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=
+google.golang.org/appengine v1.1.0 h1:igQkv0AAhEIvTEpD5LIpAfav2eeVO9HBTjvKHVJPRSs=
 google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM=
 google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoALGmiZfu7CAH4rXhgtRTLTxftemlI0sWmxmc=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=

pkg/config/config.go

@@ -179,6 +179,9 @@ type LintersSettings struct {
 		MultiIf   bool `mapstructure:"multi-if"`
 		MultiFunc bool `mapstructure:"multi-func"`
 	}
+	RowsErrCheck struct {
+		Packages []string
+	}
 
 	WSL      WSLSettings
 	Lll      LllSettings

pkg/golinters/rowerrcheck.go

@@ -0,0 +1,23 @@
+package golinters
+
+import (
+	"github.com/jingyugao/rowserrcheck/passes/rowserr"
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
+	"github.com/golangci/golangci-lint/pkg/lint/linter"
+)
+
+func NewRowsErrCheck() *goanalysis.Linter {
+	analyzer := rowserr.NewAnalyzer()
+	return goanalysis.NewLinter(
+		"rowserrcheck",
+		"checks whether Err of rows is checked successfully",
+		[]*analysis.Analyzer{analyzer},
+		nil,
+	).WithLoadMode(goanalysis.LoadModeTypesInfo).
+		WithContextSetter(func(lintCtx *linter.Context) {
+			pkgs := lintCtx.Settings().RowsErrCheck.Packages
+			analyzer.Run = rowserr.NewRun(pkgs...)
+		})
+}

pkg/lint/lintersdb/manager.go

@@ -108,6 +108,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
 			WithLoadForGoAnalysis().
 			WithPresets(linter.PresetStyle).
 			WithURL("https://github.com/golang/lint"),
+		linter.NewConfig(golinters.NewRowsErrCheck()).
+			WithLoadForGoAnalysis().
+			WithPresets(linter.PresetPerformance, linter.PresetBugs).
+			WithURL("https://github.com/jingyugao/rowserr"),
 
 		linter.NewConfig(golinters.NewStaticcheck()).
 			WithLoadForGoAnalysis().

test/testdata/rowserrcheck.go

@@ -0,0 +1,13 @@
+//args: -Erowserrcheck
+package testdata
+
+import (
+	"database/sql"
+)
+
+func RowsErrNotChecked(db *sql.DB) {
+	rows, _ := db.Query("select id from tb") // ERROR "rows.Err must be checked"
+	for rows.Next() {
+
+	}
+}

vendor/github.com/jingyugao/rowserrcheck/LICENSE

@@ -0,0 +1,21 @@
+MIT License
+
+Copyright (c) 2019 Seiji Takahashi
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

vendor/github.com/jingyugao/rowserrcheck/passes/rowserr/rowserr.go

@@ -0,0 +1,319 @@
+package rowserr
+
+import (
+	"fmt"
+	"go/ast"
+	"go/types"
+	"strconv"
+
+	"github.com/gostaticanalysis/analysisutil"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+	"golang.org/x/tools/go/ssa"
+)
+
+func NewAnalyzer(sqlPkgs ...string) *analysis.Analyzer {
+	return &analysis.Analyzer{
+		Name: "rowserrcheck",
+		Doc:  Doc,
+		Run:  NewRun(sqlPkgs...),
+		Requires: []*analysis.Analyzer{
+			buildssa.Analyzer,
+		},
+	}
+}
+
+const (
+	Doc       = "rowserrcheck checks whether Rows.Err is checked"
+	errMethod = "Err"
+	rowsName  = "Rows"
+)
+
+type runner struct {
+	pass     *analysis.Pass
+	rowsTyp  *types.Pointer
+	rowsObj  types.Object
+	skipFile map[*ast.File]bool
+	sqlPkgs  []string
+}
+
+func NewRun(pkgs ...string) func(pass *analysis.Pass) (interface{}, error) {
+	return func(pass *analysis.Pass) (interface{}, error) {
+		pkgs = append(pkgs, "database/sql")
+		for _, pkg := range pkgs {
+			r := new(runner)
+			r.sqlPkgs = pkgs
+			r.run(pass, pkg)
+		}
+		return nil, nil
+	}
+}
+
+// run executes an analysis for the pass. The receiver is passed
+// by value because this func is called in parallel for different passes.
+func (r runner) run(pass *analysis.Pass, pkgPath string) (interface{}, error) {
+	r.pass = pass
+	pssa := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+	funcs := pssa.SrcFuncs
+
+	pkg := pssa.Pkg.Prog.ImportedPackage(pkgPath)
+	if pkg == nil {
+		// skip
+		return nil, nil
+	}
+
+	r.rowsObj = pkg.Type(rowsName).Object()
+	if r.rowsObj == nil {
+		// skip checking
+		return nil, nil
+	}
+
+	resNamed, ok := r.rowsObj.Type().(*types.Named)
+	if !ok {
+		return nil, nil
+	}
+
+	r.rowsTyp = types.NewPointer(resNamed)
+	r.skipFile = map[*ast.File]bool{}
+
+	for _, f := range funcs {
+		if r.noImportedDBSQL(f) {
+			// skip this
+			continue
+		}
+
+		// skip if the function is just referenced
+		var isreffunc bool
+
+		for i := 0; i < f.Signature.Results().Len(); i++ {
+			if types.Identical(f.Signature.Results().At(i).Type(), r.rowsTyp) {
+				isreffunc = true
+			}
+		}
+
+		if isreffunc {
+			continue
+		}
+
+		for _, b := range f.Blocks {
+			for i := range b.Instrs {
+				if r.notCheck(b, i) {
+					pass.Reportf(b.Instrs[i].Pos(), fmt.Sprintf("rows.Err must be checked"))
+				}
+			}
+		}
+	}
+
+	return nil, nil
+}
+
+func (r *runner) notCheck(b *ssa.BasicBlock, i int) bool {
+	call, ok := r.getReqCall(b.Instrs[i])
+	if !ok {
+		return false
+	}
+
+	for _, cRef := range *call.Referrers() {
+		val, ok := r.getResVal(cRef)
+		if !ok {
+			continue
+		}
+
+		if len(*val.Referrers()) == 0 {
+			return true
+		}
+
+		resRefs := *val.Referrers()
+		for _, resRef := range resRefs {
+			switch resRef := resRef.(type) {
+			case *ssa.Store: // Call in Closure function
+				if len(*resRef.Addr.Referrers()) == 0 {
+					return true
+				}
+
+				for _, aref := range *resRef.Addr.Referrers() {
+					if c, ok := aref.(*ssa.MakeClosure); ok {
+						f := c.Fn.(*ssa.Function)
+						if r.noImportedDBSQL(f) {
+							// skip this
+							return false
+						}
+						called := r.isClosureCalled(c)
+
+						return r.calledInFunc(f, called)
+					}
+
+				}
+			case *ssa.Call: // Indirect function call
+				if r.isCloseCall(resRef) {
+					return false
+				}
+				if f, ok := resRef.Call.Value.(*ssa.Function); ok {
+					for _, b := range f.Blocks {
+						for i := range b.Instrs {
+							return r.notCheck(b, i)
+						}
+					}
+				}
+			case *ssa.FieldAddr:
+				for _, bRef := range *resRef.Referrers() {
+					bOp, ok := r.getBodyOp(bRef)
+					if !ok {
+						continue
+					}
+
+					for _, ccall := range *bOp.Referrers() {
+						if r.isCloseCall(ccall) {
+							return false
+						}
+					}
+				}
+			}
+		}
+	}
+
+	return true
+}
+
+func (r *runner) getReqCall(instr ssa.Instruction) (*ssa.Call, bool) {
+	call, ok := instr.(*ssa.Call)
+	if !ok {
+		return nil, false
+	}
+
+	res := call.Call.Signature().Results()
+	flag := false
+
+	for i := 0; i < res.Len(); i++ {
+		flag = flag || types.Identical(res.At(i).Type(), r.rowsTyp)
+	}
+
+	if !flag {
+		return nil, false
+	}
+
+	return call, true
+}
+
+func (r *runner) getResVal(instr ssa.Instruction) (ssa.Value, bool) {
+	switch instr := instr.(type) {
+	case *ssa.Call:
+		if len(instr.Call.Args) == 1 && types.Identical(instr.Call.Args[0].Type(), r.rowsTyp) {
+			return instr.Call.Args[0], true
+		}
+	case ssa.Value:
+		if types.Identical(instr.Type(), r.rowsTyp) {
+			return instr, true
+		}
+	}
+
+	return nil, false
+}
+
+func (r *runner) getBodyOp(instr ssa.Instruction) (*ssa.UnOp, bool) {
+	op, ok := instr.(*ssa.UnOp)
+	if !ok {
+		return nil, false
+	}
+	// fix: try to check type
+	// if op.Type() != r.rowsObj.Type() {
+	// 	return nil, false
+	// }
+	return op, true
+}
+
+func (r *runner) isCloseCall(ccall ssa.Instruction) bool {
+	switch ccall := ccall.(type) {
+	case *ssa.Defer:
+		if ccall.Call.Value != nil && ccall.Call.Value.Name() == errMethod {
+			return true
+		}
+	case *ssa.Call:
+		if ccall.Call.Value != nil && ccall.Call.Value.Name() == errMethod {
+			return true
+		}
+	}
+
+	return false
+}
+
+func (r *runner) isClosureCalled(c *ssa.MakeClosure) bool {
+	for _, ref := range *c.Referrers() {
+		switch ref.(type) {
+		case *ssa.Call, *ssa.Defer:
+			return true
+		}
+	}
+
+	return false
+}
+
+func (r *runner) noImportedDBSQL(f *ssa.Function) (ret bool) {
+	obj := f.Object()
+	if obj == nil {
+		return false
+	}
+
+	file := analysisutil.File(r.pass, obj.Pos())
+	if file == nil {
+		return false
+	}
+
+	if skip, has := r.skipFile[file]; has {
+		return skip
+	}
+	defer func() {
+		r.skipFile[file] = ret
+	}()
+
+	for _, impt := range file.Imports {
+		path, err := strconv.Unquote(impt.Path.Value)
+		if err != nil {
+			continue
+		}
+		path = analysisutil.RemoveVendor(path)
+		for _, pkg := range r.sqlPkgs {
+			if pkg == path {
+				return false
+			}
+		}
+	}
+
+	return true
+}
+
+func (r *runner) calledInFunc(f *ssa.Function, called bool) bool {
+	for _, b := range f.Blocks {
+		for i, instr := range b.Instrs {
+			switch instr := instr.(type) {
+			case *ssa.UnOp:
+				for _, ref := range *instr.Referrers() {
+					if v, ok := ref.(ssa.Value); ok {
+						if vCall, ok := v.(*ssa.Call); ok {
+							if vCall.Call.Value != nil && vCall.Call.Value.Name() == errMethod {
+								if called {
+									return false
+								}
+							}
+						}
+					}
+				}
+			default:
+				if r.notCheck(b, i) || !called {
+					return true
+				}
+			}
+		}
+	}
+	return true
+}
+
+// isNamedType reports whether t is the named type path.name.
+func isNamedType(t types.Type, path, name string) bool {
+	n, ok := t.(*types.Named)
+	if !ok {
+		return false
+	}
+	obj := n.Obj()
+	return obj.Name() == name && obj.Pkg() != nil && obj.Pkg().Path() == path
+}

vendor/modules.txt

@@ -99,6 +99,8 @@ github.com/hashicorp/hcl/json/scanner
 github.com/hashicorp/hcl/json/token
 # github.com/inconshreveable/mousetrap v1.0.0
 github.com/inconshreveable/mousetrap
+# github.com/jingyugao/rowserrcheck v0.0.0-20191204022205-72ab7603b68a
+github.com/jingyugao/rowserrcheck/passes/rowserr
 # github.com/kisielk/gotool v1.0.0
 github.com/kisielk/gotool
 github.com/kisielk/gotool/internal/load