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

Ignore errors whose return value is named _ #130

Closed
jimmyfrasche opened this issue Sep 19, 2017 · 8 comments
Closed

Ignore errors whose return value is named _ #130

jimmyfrasche opened this issue Sep 19, 2017 · 8 comments

Comments

@jimmyfrasche
Copy link

We can use _ = f() to explicitly ignore a returned error in code, but there is no means to specify in code that f never returns an error.

However, this is legal:

func f() (_ error) {
  return nil
}

Explicitly naming the error return to discard mirrors the convention of assigning to discard. As a bonus, godoc lists named returns so code following this pattern is easily recognizable from its signature. (It could of course be lying and actually return an error, but that seems like something to be detected by a different linter).

From discussion on golang/go#20803 related to #114

I'd be happy to make a PR, though looking at the code it's not immediately obvious where to integrate this.

@dmitshur
Copy link
Contributor

I'm not sure this is viable. Some Go code ends up giving the error return value blank identifier name for completely unrelated reasons. E.g., imagine you start with:

type Bar struct { ... }

func Foo() (Bar, error) {
    ...
}

Then you need to add another return value of type string:

func Foo() (Foo, string, error) {
    ...
}

However, that reads poorly in terms of documentation, it's not clear what the meaning of the string is. So you give it a descriptive name:

func Foo() (Foo, description string, error) {
    ...
}

That doesn't compile because named and unnamed return values are mixed, so to avoid changing the rest of the code, you give blank identifier names to the other variables:

func Foo() (_ Foo, description string, _ error) {
    ...
}

There is existing Go code that uses blank identifier names in return variables. It's relatively rare, but it exists, including in the Go standard library.

That's why I don't think it's viable to try to give it special meaning. People who wrote code like that in the past may not have expected it to mean something.

@dmitshur
Copy link
Contributor

but there is no means to specify in code that f never returns an error.

There are two ways:

  1. Don't include the error return value. If it's always nil, it's not needed. (Presumably, this isn't always possible, such as when implementing existing interfaces.)

  2. Document that the error can can never be non-nil. An example of this in Go standard library is https://godoc.org/bytes#Buffer.Write, which says "err is always nil".

@jimmyfrasche
Copy link
Author

@shurcooL yes, the purpose is to set an enforceable convention for when you must return an error to satisfy a type signature, like with interfaces, but will never return an error because you do not need to.

A comment is great for people, but unless that's in a standardized format it would be hard for tools to extract that information so you end up with white lists that exist outside the codebase.

It would be nice if it could just be in the code.

The meaning, or rather meaninglessness, of _ is established, though not enforced by the compiler here. It seems a logical and suggestive way to signal to tools and readers that the return will always be nil.

If someone writes code that uses _ as the name of any return that matters, then that should fail any code review, regardless of whether a check like this makes it into errcheck or similar tools. It's confusing unless the return is meant to be discarded. I don't think I've ever seen any code that used it as a return name (other than some I have written where it meant the return could be ignored).

@dmitshur
Copy link
Contributor

dmitshur commented Sep 21, 2017

If someone writes code that uses _ as the name of any return that matters, then that should fail any code review, regardless of whether a check like this makes it into errcheck or similar tools. It's confusing unless the return is meant to be discarded. I don't think I've ever seen any code that used it as a return name (other than some I have written where it meant the return could be ignored).

I'm not in agreement with that. I think using _ instead of another return variable name is a valid thing to do in some contexts, and I don't take it to mean that the value should be discarded. It's a way of not giving a return variable a name, which can be useful to avoid unwanted shadowing.

@jimmyfrasche
Copy link
Author

That's awful

@dmitshur
Copy link
Contributor

dmitshur commented Sep 21, 2017

Here's an example of it being used (in good taste, IMO) in the standard library:

https://github.com/golang/go/blob/eca45997dfd6cd14a59fbdea2385f6648a0dc786/src/mime/multipart/multipart.go#L120-L126

The int return value is given the name n for documentation purposes, and so that n, r.err = assignment works without having to do var n int first. But _ error is used instead of err error because there's no need for giving it a name.

That code certainly does return non-nil errors, despite having _ error.

This is just an example, and the above is just my opinion.

@jimmyfrasche
Copy link
Author

I'd send a patch to Go to fix that if it weren't such a hassle.

@jimmyfrasche
Copy link
Author

If I were reading code out loud, I'd pronounce _ = 1 + x as "discard one plus x" and _ error as "discard error".

This is the only context where you can name something _ without sending it down a blackhole. Exploiting that fact is clever in the pejorative sense, imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants