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

Jump to bottom

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
FrozenDueToAge Proposal
Projects
Proposals (old)
Milestone
Proposal

Comments

@lolbinarycat
Copy link

lolbinarycat commented Jul 18, 2020 •
edited

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
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jul 19, 2020
@bcmills
Copy link
Contributor

bcmills commented Jul 20, 2020

See previously #20803, #20148, #19727.

@lolbinarycat
Copy link
Author

lolbinarycat commented Jul 20, 2020 •
edited by ALTree

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

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

@lolbinarycat
Copy link
Author

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 rsc mentioned this issue Jul 22, 2020
Open
@rsc
Copy link
Contributor

rsc commented Jul 22, 2020

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

@rsc rsc closed this as completed Jul 22, 2020
@rsc rsc moved this from Incoming to Declined in Proposals (old) Jul 22, 2020
@rsc rsc mentioned this issue Jul 22, 2020
Open
@golang golang locked and limited conversation to collaborators Jul 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
No open projects
Proposals (old)
Declined
Milestone
Proposal
Development

No branches or pull requests
5 participants
@rsc @ianlancetaylor @bcmills @gopherbot @lolbinarycat