gordonklaus / ineffassign

Detect ineffectual assignments in Go code.
MIT License
397 stars 23 forks source link

control variable used on defer #80

Closed amery closed 1 year ago

amery commented 1 year ago

on some functions, to make error handling cleaner, I put a sentinel on defer to take care of undoing incomplete "transactions". it looks like this:

func doThings(...) error {
    var ok bool
    defer undoThingsUnless(ok, ...)
    // ...
    if  err != nil {
        return err
    }
    // ...
    if  err != nil {
        return err
    }

    ok = true
    return nil
}

actual code example: https://github.com/darvaza-proxy/darvaza/blob/master/shared/net/bind/listen.go#L114 or https://github.com/darvaza-proxy/darvaza/blob/master/shared/net/bind/bind.go#L165

I don't know the version, but I just noticed it on goreportcard. https://goreportcard.com/report/github.com/darvaza-proxy/darvaza/shared

gordonklaus commented 1 year ago

Hi @amery .

Your code doesn't work the way you think it does. Arguments to deferred function calls are evaluated at the defer statement. You are always passing false to undoThingsUnless. Try passing &ok instead.

When you went to open this issue, did you see an issue template called "False positive"? Did you ignore it, and why? (E.g., are you not familiar with the term "false positive"?) I ask because I created that issue template for the sake of reducing spurious reports like this one — and yet here we are. Can you think of a better name for the issue template? I'd appreciate your feedback :)