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: check for unused return value from testing/quick.Check #12834
Comments
CL https://golang.org/cl/15381 mentions this issue. |
Just to note prior art, the functionality you're proposing sounds similar to github.com/kisielk/errcheck. |
I'm aware of errcheck, but in general missed err checking can't be
introduced to cmd/vet due to false positives as it is not possible to
know if the missing err is intentional or not (unless we force the users
to always use _ = someFuncReturningError(), which is ugly.)
But there are certain kinds of error that shouldn't ever be ignored,
testing/quick.Check is certainly one. cmd/vet can add a test for those.
|
One complication is that not checking the return value
from quick.Check is ok if the func value actually calls
t.Error/t.Fatal on failure. I guess the value of this proposal
is greatly reduced by this.
|
Couldn't this proposal take that into account? On 5 October 2015 at 10:58, Minux Ma notifications@github.com wrote:
|
Update #12834. Change-Id: If7bbcc249517f2f2d8a7dcbba6411ede92331abe Reviewed-on: https://go-review.googlesource.com/15381 Reviewed-by: Damian Gryski <dgryski@gmail.com> Reviewed-by: David Crawshaw <crawshaw@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Is there still interest in this? Seems low impact to me. |
No thanks. One of the criteria for vet is that the check being added must be for something that happens often, so that the benefit pays for the maintenance cost. Approximately no one uses quick.Check (we probably should have killed it before Go 1). There isn't enough benefit here. |
/cc @robpike |
https://golang.org/cl/15263 fixed a place where
quick.Check
isused without its return value being checked for non-nil.
I did a quick search, and found that unfortunately, there are 6 more
similar cases in standard library alone. (see https://golang.org/cl/15381)
I'm wondering if cmd/vet should check for this case as not checking
the result from quick.Check means the automated checks are useless.
And perhaps cmd/vet should also check for other functions whose
returned value must not be ignored.
I imagine that we don't want a source code annotation is similar to
gcc's
__attribute__((warn_unused_result))
?(How about project specific cmd/vet rules? If there are enough
interest, we might be able to start a discussion on this topic too.
I'm thinking about an example based approach, aka eg.)
The text was updated successfully, but these errors were encountered: