ineffassign incorrectly claims that an assignment is not used

[kinghajj]

func (f *Foo) Bar() (bzs []Baz, err error) {
    if bs, err := ioutil.ReadFile(os.Getenv(bazesPathKey)); err == nil {
        // ineffassign claims that this is not used, but it's a return value
        err = json.Unmarshal(bs, &bzs)
    }
    return
}

[mholt]

No it's not, you declared err in the line above it; so that assignment refers to the local err, not the return value.

[kinghajj]

I don't believe that's correct. From the Go Programming Language specification:

"Unlike regular variable declarations, a short variable declaration may redeclare variables provided they were originally declared earlier in the same block (or the parameter lists if the block is the function body) with the same type, and at least one of the non-blank variables is new. As a consequence, redeclaration can only appear in a multi-variable short declaration. Redeclaration does not introduce a new variable; it just assigns a new value to the original."

If I understand correctly, the err in the return parameter list and the err in the if statement's declaration section refer to the same variable.

Edit: In fact, I believe the main reason this exception was added was exactly to make error handling more ergonomic.

[gordonklaus]

@kinghajj Give it a try: https://play.golang.org/p/Qi3cTWHg0Z

The key phrase from the spec is "originally declared earlier in the same block (or the parameter lists if the block is the function body)". The short variable declaration is not in the same block (the function body in this case) as the original declaration, so there is no redeclaration.

[kinghajj]

Ah, forgot that those declarations are part of separate blocks, thanks. I guess the best way around this is to move the declaration to the line above and have the if statement just test err. Sorry for causing a fuss over nothing.

[mholt]

Sorry for causing a fuss over nothing.

I'm actually glad you did. When I saw your reply citing the Go spec, I was afraid I had made a mistake by forgetting the rules for multiple assignment. So I learned something new today.

[gordonklaus]

@kinghajj No worries (although you should test your assertions before making them :wink: ).

FWIW, I'd recommend writing your function as:

func (f *Foo) Bar() ([]Baz, error) {
    bs, err := ioutil.ReadFile(os.Getenv(bazesPathKey))
    if err != nil {
        return nil, err
    }
    var bzs []Baz
    if err := json.Unmarshal(bs, &bzs); err != nil {
        return nil, err
    }
    return bzs, nil
}

I think the general guidance is to use named results only for documentation (e.g., when returning two ints – which is which?).