Skip to content

rowserrcheck false positive when deferred Close() is wrapped in a func #1774

Closed
@stevendanna

Description

@stevendanna

rowserrcheck appears to produce a false positive when a deferred rows.Close() is wrapped in a func. This is perhaps similar to #1670, but occurs even on versions that should include that fix.

The false positive can be reproduced by the following example:

func Bug(db *sql.DB) {
        rows, err := db.Query("select 1")
        if err != nil {
                panic(err)
        }

        defer func() { _ = rows.Close() }()
        for rows.Next() {
                fmt.Println("new rows")
        }

        if err := rows.Err(); err != nil {
                panic(err)
        }
}

This example will report the following error, despite rows.Err() clearly being checked.

repro.go:9:23: rows.Err must be checked (rowserrcheck)
        rows, err := db.Query("select 1")

Interestingly, if defer func() { _ = rows.Close() }() is changed to defer rows.Close(), the false positive goes away.


  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Please include the following information:

Version of golangci-lint
$ golangci-lint --version
golangci-lint has version 1.37.1 built from b39dbcd on 2021-02-22T04:33:13Z
Config file
$ cat .golangci.yml
Go environment
$ go version && go env
go version go1.15.8 darwin/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/ssd/Library/Caches/go-build"
GOENV="/Users/ssd/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/ssd/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/ssd/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15.8/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15.8/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/b4/2b8v9mcx5x9d64k7q3m0fz_40000gp/T/go-build620955549=/tmp/go-build -gno-record-gcc-switches -fno-common"```

</details>

<details><summary>Verbose output of running</summary>

```console
$ golangci-lint cache clean
$ golangci-lint run --disable-all --enable rowserrcheck -v repro.go
INFO [config_reader] Config search paths: [./ /Users/ssd/tmp /Users/ssd /Users /] 
INFO [lintersdb] Active 1 linters: [rowserrcheck] 
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|exports_file|files|imports|name|types_sizes) took 246.449759ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 194.581µs 
INFO [linters context/goanalysis] analyzers took 4.398224ms with top 10 stages: buildssa: 4.378385ms, rowserrcheck: 19.839µs 
INFO [runner] Processors filtering stat (out/in): exclude-rules: 1/1, max_same_issues: 1/1, severity-rules: 1/1, path_prettifier: 1/1, filename_unadjuster: 1/1, identifier_marker: 1/1, max_per_file_from_linter: 1/1, cgo: 1/1, nolint: 1/1, path_prefixer: 1/1, sort_results: 1/1, skip_files: 1/1, autogenerated_exclude: 1/1, exclude: 1/1, uniq_by_line: 1/1, diff: 1/1, max_from_linter: 1/1, source_code: 1/1, path_shortener: 1/1, skip_dirs: 1/1 
INFO [runner] processing took 236.307µs with stages: nolint: 66.688µs, path_prettifier: 41.128µs, autogenerated_exclude: 40.819µs, source_code: 35.318µs, identifier_marker: 24.321µs, exclude-rules: 10.103µs, skip_dirs: 9.009µs, cgo: 2.244µs, max_same_issues: 1.341µs, uniq_by_line: 1.197µs, path_shortener: 1.063µs, max_from_linter: 715ns, filename_unadjuster: 699ns, diff: 357ns, max_per_file_from_linter: 333ns, exclude: 241ns, severity-rules: 219ns, skip_files: 210ns, sort_results: 173ns, path_prefixer: 129ns 
INFO [runner] linters took 62.858379ms with stages: rowserrcheck: 62.55521ms 
repro.go:9:23: rows.Err must be checked (rowserrcheck)
        rows, err := db.Query("select 1")
                             ^
INFO File cache stats: 1 entries of total size 280B 
INFO Memory: 5 samples, avg is 72.6MB, max is 73.3MB 
INFO Execution took 322.928163ms 

Metadata

Metadata

Assignees

No one assigned

    Labels

    dependenciesRelates to an upstream dependencyfalse positiveAn error is reported when one does not exist

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions