[go-nuts] Could "go vet" catch ignored errors?

The various cases are:

1. f := os.Open("file")
2. f, _ := os.Open("file")
3. os.Open("file")

Case 1) is a compiler error (only one lval, 2 expected). Case 2) is
explicitly ignored, so neither the compiler nor go vet should complain.
Case 3) causes no compiler error, and is the one case that would get caught
by your proposal in go vet, right? So the warning would always be in the
3rd case, regardless of how many values are returned by the function, as
long as one of them is an error.

I can see this being annoying in some cases (File.Close() comes to mind, as
it is often in a defer call, without the lval). But then, isn't it go vet's
role to be the verbose, annoying little brother? If I make a parallel with
javascript, jshint (not Douglas Crockford's stricter jslint) is very
customizable and can be configured to complain about the things you want.
I'd like go vet to be that flexible, though honestly I haven't used it
enough to comment on it.

Le vendredi 25 janvier 2013 14:05:18 UTC-5, Greg Ward a écrit :

This is actually something I was thinking about... what *is* the correct
way to call file.close in a defer? Seems like defer doesn't give you any
way to handle the error (not that there's a lot you can do about the file
not closing... but you could still return the error to the caller, if you
weren't using defer).

It's just a little more verbose. Assuming named returns:

defer func() { err = x.Close() }()

Ahh, named returns, that's how you can make it work. Cool.

Sorry to derail the thread.

For the record, I think having go vet complain about errors that are not
assigned to a variable is a good idea. Yes, this would catch defer
x.Close(), which is probably a good thing.

This code will ignore error if it was present. It probably should be:

defer func() {
if err != nil {
err = x.Close()
}
}()


But… How often do you check error returned by fmt.Print() for example?

--

But that version doesn't Close() x if there is already a possibly unrelated
error.
If you need to check an error on a Close() or similar you probably
shouldn't defer it.

My bad, thanks. It should be even more verbose:

defer func() {
e := x.Close()
if err == nil {
err = e
}
}()

Close() may return error. Typically, previous error is more interesting.


It's useful when function has several returns/panics.


My question is: should we care about every possible error? Should we check
result of fmt.Print()? Should we check result of os.File.Close() Should we
write code like above, or just type "defer x.Close()"?

--

If you are not abusing panic it is probably ok to not close anything when
you have a panic.

I should get more sleep. Yes - this is what I usually do in this situation (where I care about defered returned errors/behaviours).

On 26/01/2013, at 8:38 AM, "Alexey Palazhchenko" wrote:

But that version doesn't Close() x if there is already a possibly unrelated error.

My bad, thanks. It should be even more verbose:

defer func() {
e := x.Close()
if err == nil {
err = e
}
}()

Close() may return error. Typically, previous error is more interesting.

This works: http://play.golang.org/p/TdTvw5X2cO

--

This is why:

http://golang.org/ref/spec#Defer_statements

Good point. What you've written is how my actual code looks where I do this. The laziness of non-executed code.

On 26/01/2013, at 8:10 AM, "Alexey Palazhchenko" wrote:

It's just a little more verbose. Assuming named returns:

defer func() { err = x.Close() }()

This code will ignore error if it was present. It probably should be:

defer func() {
if err != nil {
err = x.Close()
}
}()


But… How often do you check error returned by fmt.Print() for example?

--

The "correct way" varies based entirely on whether you care. For *os.File,
usually all you could do for a close error is log the error (it's not like
you'd continually try closing the file until it returned without an error).
There are cases where Close() has a side-effect, such as archive/zip and
encoding/base64 (a base64 Close going to fail, though), where it is
important to check for the presence of the error.

This has been discussed numerous times on the mailing list, do a search in
the Google group.

In short: It should be possible to implement in the vet tool once go/types
is available in Go 1.1. It would probably require a user-configurable
whitelist for certain functions you usually don't care about (eg:
fmt.Printf and friends).