uber-go / nilaway

Static analysis tool to detect potential nil panics in Go code
Apache License 2.0
3.19k stars 66 forks source link

False positive when wrapping errors #276

Open johannaschwarzcheck24 opened 2 months ago

johannaschwarzcheck24 commented 2 months ago

A minimal example:

The wrap function returns nil if the error passed to it is nil. The test function will only call wrap and pass err if err is not nil.

func wrap(msg string, err error) error {
    if err == nil {
        return nil
    }
    return errors.New(msg)
}

func test() (*string, error) {
    var err error
    if err != nil {
        return nil, wrap("failed to read file", err)
    }
    s := "qwe"
    return &s, nil
}

func main() {
    b, err := test()
    if err != nil {
        log.Fatal(err)
    }
    _ = *b
}

nilaway reports a potential nil panic:

error: Potential nil panic detected. Observed nil flow from source to dereference point: 
        - nilaway/main.go:18:10: literal `nil` returned from `test()` in position 0 when the error return in position 1 is not guaranteed to be non-nil through all paths
        - nilaway/main.go:29:7: result 0 of `test()` dereferenced via the assignment(s):
                - `test()` to `b` at nilaway/main.go:25:2
sonalmahajan15 commented 2 months ago

Hmm, we had added context sensitivity via contract support to track such error propagation and not report a false positive. Perhaps some corner case that we missed. @yuxincs, can you take a look?

@johannaschwarzcheck24, thanks for reporting!