Hello there,
I found a few cases where rowserrcheck reports false positive.
I'd like to report those issues but the rowserrcheck repository doesn't have enabled the Github issues.
How should I report them? Should I do it in this repository and mention the author?
I hit one false positive too. Following is a reproduction of the issue.
package main
import (
"database/sql"
"fmt"
"math/rand"
_ "github.com/lib/pq"
)
func main() {
db, err := sql.Open("postgres", "postgres://localhost:5432/postgres")
if err != nil {
panic(err)
}
defer db.Close()
var rows *sql.Rows
if rand.Float64() < 0.5 {
rows, err = db.Query("select 1")
} else {
rows, err = db.Query("select 2")
}
if err != nil {
panic(err)
}
defer rows.Close()
for rows.Next() {
fmt.Println("new rows")
}
if err := rows.Err(); err != nil {
panic(err)
}
}
It reports the following errors but actually, it's handled.
test.go:20:23: rows.Err must be checked (rowserrcheck)
rows, err = db.Query("select 1")
^
test.go:22:23: rows.Err must be checked (rowserrcheck)
rows, err = db.Query("select 2")
^
/cc @jingyugao
Running into same issue. I disabled rowserrcheck for now in my .golangci.yml until this is fixed via:
linters:
enable:
- deadcode
- depguard
- errcheck
...
disable:
- rowserrcheck
Good to know all of them.
I haven't posted an example that I built to show what false positive I found and when some of them that follow a similar pattern doesn't appear, because I'd like to know from the people who manage this repository if we should report them here following some pattern (e.g. mentioning the author) or wherever else.
I'm still waiting for that response.
Probably author is not very interested in contributions (and actually it is fine, open source is hard to maintain) and IMO the best solution is to fork, fix and eventually merge fixes to upstream if possible.
We can switch to forked version if we fail to merge with upstream.
Thanks for your feedback. I will look it in weekday.
The wuhan pneumonia make everything chaos.
Oh, please take care of yourself! Sorry if it sounded ungrateful, I didn't mean that :(
Probably author is not very interested in contributions (and actually it is fine, open source is hard to maintain) and IMO the best solution is to fork, fix and eventually merge fixes to upstream if possible.
We can switch to forked version if we fail to merge with upstream.
Sure, then I was going to post my code snippet here, but with @jingyugao response, I'll wait until he gets better and he indicates what's the best way for him to report potential bugs.
In the test case above, the key part is the call to defer rows.Close() before the rows.Err() call.
You can reduce the sample code to this and it will still fail
db, err := sql.Open("postgres", "postgres://localhost:5432/postgres")
if err != nil {
panic(err)
}
defer db.Close()
rows, _ := db.Query("select 1")
defer rows.Close()
if err := rows.Err(); err != nil {
panic(err)
}
Hitting the same false positive