gostaticanalysis / sqlrows Goto Github PK

9407ba5#commitcomment-33037565

https://github.com/gostaticanalysis/sqlrows/blob/master/passes/sqlrows/sqlrows.go#L94
If refs is nil, *refs become a panic.

I believe the program does not function like it should. It only works if I put rows.Close directly under the line of the query (which is not good). If I make that a defer rows.Close() I get:
./main.go:16:8: using rows before checking for errors
which you should expect

        rows, err := readDB.Query("SELECT 1")
        if err != nil {
                panic(err)
        }
        defer rows.Close()

$ go vet -vettool=$(which sqlrows) ./...
#./main.go:15:27: rows.Close must be called

Source code

package main
  
import (
        "database/sql"
        _ "github.com/go-sql-driver/mysql"
)

func main(){
        
        readDB, err := sql.Open("mysql", "root:root@tcp(localhost:3306)/mysql?parseTime=true&charset=utf8mb4")
        if err != nil {
                panic(err.Error()) 
        }       
        
        rows, err := readDB.Query("SELECT 1")
        // rows.Close() // no warnings, which is incorrect
        // defer rows.Close() // gives ./main.go:16:8: using rows before checking for errors
        if err != nil {
                panic(err)
        }
        defer rows.Close() // correct but does not get noticed

}

Hi, thanks for creating this. I would love to use this tool if this supports checking transactions too. Usually, rows.Close() is something that is not missed. But I miss closing transactions often.

I would want to check that transactions are always either committed or rolled back before the function returns.

Something like

tx, err := db.Begin()
if err != nil {
	return err
}
err = doSomething()
if err != nil {
	tx.Rollback()
	return err
}
return nil // error !! We are not doing tx.Commit() before returning.

Will it be possible to add something like this ? Thanks.

sqlrows panics when parsing the following code:

var rows *sql.Rows
if foo {
    rows, err = queryFoo()
} else {
    rows, err = queryBar()
}
if err != nil { return err }
defer rows.Close()

After digging in, it seems this is because var rows *sql.Rows comes in as an *ssa.Phi rather than an *ssa.Extract.

The following code seems to cause sqlrows to panic:

rows, err := db.Query(a, b, c)
if err != nil {
    return err
}
func() {
    defer rows.Close()

    // read from rows...
}()

After digging in some, it seems that the deferred rows.Close() call inside of the closure is dereferencing the *sql.Rows value, because it comes out as an *ssa.UnOp instead of an *ssa.Extract.

pls join revive for a better linter: https://github.com/mgechev/revive