Golangci-lint: How can I report some issues about rowserrcheck linter?

Created on 27 Jan 2020  路  9Comments  路  Source: golangci/golangci-lint

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?

dependencies

All 9 comments

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

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bacongobbler picture bacongobbler  路  4Comments

ferhatelmas picture ferhatelmas  路  4Comments

DarthHater picture DarthHater  路  4Comments

simonpasquier picture simonpasquier  路  4Comments

jsm picture jsm  路  5Comments