Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: cmd/vet: add check for unchecked error return values #40286

Closed
lolbinarycat opened this issue Jul 18, 2020 · 5 comments
Closed

proposal: cmd/vet: add check for unchecked error return values #40286

lolbinarycat opened this issue Jul 18, 2020 · 5 comments
Labels
Projects
Milestone

Comments

@lolbinarycat
Copy link

@lolbinarycat lolbinarycat commented Jul 18, 2020

if you are using a lot of functions that return an error value, one mistake you might make is this:

val1, err := someFunc()
if err != nil {
    return err
}
someStruct.someField, err = someOtherFunc()
someOtherStruct.someField, err = Func3()
if err != nil {
    return err
}

It may look simple in the example, but if you have a bunch of loops and if statements, it could be much harder to see, e.g.

// ...
for someCondition {
    strct.Field, err = GetFoo()
}
if err != nil {
    return err
}

This will catch an error if it is the last call is an error, but will let it past otherwise. It is also hard to track down as you don't know what you're looking for, or where you're looking for it, as the function will likely continue running but produce no output, or segfault due to derefrencing a nil pointer (that was supposed to be set by a function that returned an error).
this is worsened by #337.

@gopherbot gopherbot added this to the Proposal milestone Jul 18, 2020
@gopherbot gopherbot added the Proposal label Jul 18, 2020
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Jul 19, 2020
@bcmills
Copy link
Member

@bcmills bcmills commented Jul 20, 2020

See previously #20803, #20148, #19727.

@lolbinarycat
Copy link
Author

@lolbinarycat lolbinarycat commented Jul 20, 2020

See previously #20803, #20148, #19727.

Thanks, I will note that none of these address the exact pattern I'm talking about.
namely, #20148 is about forgetting to assign to err, while this issue is about assigning to err, but then forgetting to check it.
Although, they are similar enough to perhaps be parts of the same go vet rule if implemented.

@ALTree ALTree changed the title proposal: cmd/vet: add check for unchecked error return values. proposal: cmd/vet: add check for unchecked error return values Jul 20, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 20, 2020

If I'm reading this right, I think this is actually a dup of #23142.

@lolbinarycat
Copy link
Author

@lolbinarycat lolbinarycat commented Jul 20, 2020

If I'm reading this right, I think this is actually a dup of #23142.

They both come from the same underlying cause, but the person who originally opened that issue didn't open it with that title, and it reads more like someone asking for help than proposing a change.

This proposal also is more focused on error values (i.e. variables of type error).

I also would be willing to try to create a prototype for this.

@rsc
Copy link
Contributor

@rsc rsc commented Jul 22, 2020

Turned #23142 into a proposal; closing this one as a duplicate of that.

@rsc rsc closed this Jul 22, 2020
@rsc rsc moved this from Incoming to Declined in Proposals Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.