gordonklaus / ineffassign

Detect ineffectual assignments in Go code.
MIT License
394 stars 22 forks source link

Getting warning on err not being used by err is being returned from the function #26

Closed alejoloaiza closed 6 years ago

alejoloaiza commented 6 years ago

Hi, im getting a warning of unused variable: Line 331: warning: err assigned and not used (ineffassign)

Bur error is actually being used, is the return of the function.

here is the code:

            resp := friendResp{}
            err = json.Unmarshal(body, &resp)
            user.Friendship = resp.Friendship
        }
    }
    return err
}

Im using goreportcard.com to run the test. thanks.

gordonklaus commented 6 years ago

Hi @alejoloaiza . I suspect the err you are assigning to is not the same as the one being returned. Please post the whole function so I can determine if this is the case.

alejoloaiza commented 6 years ago

hi @gordonklaus this is the whole function:

func (user *User) Unblock() error {
    insta := user.inst
    data, err := insta.prepareData(
        map[string]interface{}{
            "user_id": user.ID,
        },
    )
    if err == nil {
        body, err := insta.sendRequest(
            &reqOptions{
                Endpoint: fmt.Sprintf(urlUserUnblock, user.ID),
                Query:    generateSignature(data),
                IsPost:   true,
            },
        )
        if err == nil {
            resp := friendResp{}
            err = json.Unmarshal(body, &resp)
            user.Friendship = resp.Friendship
        }
    }
    return err
}
alejoloaiza commented 6 years ago

and this is the reportcard link https://goreportcard.com/report/github.com/ahmdrz/goinsta#ineffassign

gordonklaus commented 6 years ago

@alejoloaiza It is as I suspected. The err declared in body, err := insta.sendRequest is different from the one being returned. Remember that short variable declarations (i.e., using :=) will make new declarations, even if a variable of the same name already exists in an outer scope (the outer err is shadowed).

I would recommend rewriting this function using early returns, as is idiomatic in Go:

func (user *User) Unblock() error {
    insta := user.inst
    data, err := insta.prepareData(
        map[string]interface{}{
            "user_id": user.ID,
        },
    )
    if err != nil {
        return err
    }

    body, err := insta.sendRequest(
        &reqOptions{
            Endpoint: fmt.Sprintf(urlUserUnblock, user.ID),
            Query:    generateSignature(data),
            IsPost:   true,
        },
    )
    if err != nil {
        return err
    }

    resp := friendResp{}
    if err := json.Unmarshal(body, &resp); err != nil {
        return err
    }

    user.Friendship = resp.Friendship
    return nil
}
alejoloaiza commented 6 years ago

thanks a lot @gordonklaus