Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
3 tasks done
stevendanna opened this issue Feb 24, 2021 · 4 comments
Closed
3 tasks done
Labels
dependencies Relates to an upstream dependency false positive An error is reported when one does not exist

Comments

@stevendanna
Copy link

stevendanna commented Feb 24, 2021

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 
@stevendanna stevendanna added the bug Something isn't working label Feb 24, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 24, 2021

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@ldez
Copy link
Member

ldez commented Feb 24, 2021

Can you reproduce this issue with just rowserrcheck? If so, it's best to raise an issue to the upstream repository, and then we can bump the version when it's fixed.

@ldez ldez added the dependencies Relates to an upstream dependency label Feb 24, 2021
@stevendanna
Copy link
Author

Done: jingyugao/rowserrcheck#16

@ldez ldez added false positive An error is reported when one does not exist and removed bug Something isn't working labels Feb 27, 2021
@ldez
Copy link
Member

ldez commented Mar 20, 2021

Fixed by #1843

@ldez ldez closed this as completed Mar 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Relates to an upstream dependency false positive An error is reported when one does not exist
Projects
None yet
Development

No branches or pull requests

2 participants