Go: proposal: spec: require return values to be explicitly used or ignored (Go 2)

[bcmills]

Today, if a Go function returns both a value and an error, the user must either assign the error to a variable

v, err := computeTheThing()

or explicitly ignore it

v, _ := computeTheThing()

However, errors that are the only return value (as from io.Closer.Close or proto.Unmarshal) or paired with an often-unneeded return value (as from io.Writer.Write) are easy to accidentally forget, and not obvious in the code when forgotten.

tx.Commit()  // Store the transaction in the database!

The same problem can occur even when errors are not involved, as in functional-style APIs:

t := time.Now()
t.Add(10 * time.Second)  // Should be t = t.Add(…)

```go
strconv.AppendQuote(dst, "suffix") // Should be dst = strconv.AppendQuote(…)

For the few cases where the user really does intend to ignore the return-values, it's easy to make that explicit in the code:
```go
_, _ = fmt.Fprintln(&buf, v)  // Writes to a bytes.Buffer cannot fail.

go func() { _, _ = fmt.Fprintln(os.Stderr, findTheBug()) }()

And that transformation should be straightforward to apply to an existing code base (e.g. in a Go 1-to-2 conversion).

On the other hand, the consequences of a forgotten error-check or a dropped assignment can be quite severe (e.g., corrupted entries stored to a production database, silent failure to commit user data to long-term storage, crashes due to unvalidated user inputs).

---

Other modern languages with explicit error propagation avoid this problem by requiring (or allowing API authors to require) return-values to be used.

• Swift warns about unused return values, but allows the warning to be suppressed if the @discardableResult attribute is set.
  
  
  
  • Prior to a change in Swift 3, return-values could be ignored by default (but a warning could be added with @warn_unused_result)
  
  
• Rust has the #[must_use] attribute.
• C++17 has the [[nodiscard]] attribute, which standardizes the longstanding __attribute__((warn_unused_result)) GNU extension.
• The ghc Haskell compiler provides warning flags for unused results (-fwarn-unused-do-bind and -fwarn-wrong-do-bind).
• OCaml warns about unused return values by default. It provides an ignore function in the standard library.

I believe that the OCaml approach in particular would mesh well with the existing design of the Go language.

I propose that Go should reject unused return-values by default.

If we do so, we may also want to consider an ignore built-in or keyword to ignore any number of values:

go func() { ignore(fmt.Fprintln(os.Stderr, findTheBug())) }()

or

go func() { ignore fmt.Fprintln(os.Stderr, findTheBug()) }()

or

go func() { _ fmt.Fprintln(os.Stderr, findTheBug()) }()

If we use a built-in function, we would probably want a corresponding vet check to avoid subtle eager-evaluation bugs:

go ignore(fmt.Fprintln(os.Stderr, findTheBug()))  // BUG: evaluated immediately

---

Related proposals
Extending vet checks for dropped errors: #19727, #20148
Making the language more strict about unused variables in general: #20802
Changing error-propagation: #19991

[ianlancetaylor]

A point of clarification: when you wrote

_ = fmt.Fprintln(&buf, v)

did you mean to write

_, _ = fmt.Fprintln(&buf, v)

or did you forget that fmt.Fprintln returns two values? That is, is your proposal intended to cover a function like fmt.Fprintln that returns two values? I can't quite tell.

[bcmills]

That was a typo on my part. Fixed, and added a suggestion for an ignore builtin.

[ianlancetaylor]

While I certainly sympathize with the concerns here, I just want to state that 1) I think that wrapping expressions in ignore makes code harder to read by burying the lede; 2) in a language that values simplicity having the canonical hello, world example start with _, _ = seems to me to be unfortunate.

That is, I'm on board with the problem, but not with the proposed solutions.

[bcmills]

ignore as a keyword rather than a builtin might address the lede-burying problem somewhat. IMO,

    ignore fmt.Println("Hello, world")

is a bit clearer than

    ignore(fmt.Println("Hello, world"))

But I agree that those are both valid concerns, and I'd be thrilled if we could find some way to address the underlying problems (missed values in general, and missed errors in particular) that resolves those concerns.

[bcmills]

I should note a point that I forgot to make in the original post (now edited to add): this isn't just a problem for errors, but also for functional-style APIs such as time.Time.Add or strconv.Append*. The missed return value is sometimes the only clue the reader has as to whether an API is functional or mutative.

[pborman]

I know this has been discussed before, multiple times. The concern usually is: OMG, they did defer fd.Close() and there might have been an error. If fd is read only then the close error provides no useful information. Either you read the data you wanted or you didn't. Any meaningful errors are captured during read.

go ignore fd.Close()
ignore go fd.Close()

What about maps? Is it okay to ignore the bool, or would I have to always add the , _ to the left hand side. If that is the case then you can't write if myBoolMap[x] { anymore, it becomes if ok, _ := myBoolMap[x]; ok {.

Just how many programs would this proposal break? This seems like a Go 2.0 feature as it breaks the compatibility guarantee. I guess you could test that by writing a go vet function to do this and then run it first over the standard library, and then over random libraries from github or over the Go code internal at your company.

[bcmills]

What about maps?

Map lookups would still support both the one-value and two-value forms, but ignoring the result of a map lookup entirely would (continue to) be an error.

These are ok.

if m[x] {

```go
if _, ok := m[x]; ok {

```go
if y := m[x]; y {

```go
if y, _ := m[x]; y { // Verbose, but ok.

This one is not ok, because the result is ignored entirely. The compiler already rejects it today (https://play.golang.org/p/0WMEDVzY44).
```go
m[x]

[bcmills]

Just how many programs would this proposal break?

Most of them. It is labeled "Go2" for good reason. :)

[robpike]

Having lived through the

(void)printf("hello, world\n");

era, I firmly believe some heuristics in vet are a better way to approach this problem. That's what https://github.com/golang/go/issues/20148 is about.

Like Ian, I appreciate the concern but dislike the proposed remedy.

[nerdatmath]

ISTM we don't really want to ignore errors; I _think_ in most of these cases we really want to panic in case of error, because the programmer believes an error is not possible, or that a meaningful response to an error doesn't exist. So it seems like we want a generic Must. This might seem weird, but what if the exclamation point (!) could be used as a postfix operator to conditionally panic. If the value is a bool, panic if it's not true, and if it's an error, panic if it's not nil. It could also be used with multiple return values, only looking at the last value. Since it's postfix it avoids burying the lede, but indicates the programmer understands they're ignoring an error value. (!) should always consume one or more values but never return anything, and should fail to compile if the type of the last value passed in is not bool or error.

func main() {
    fmt.Println("Hello, world!")!
}

I guess we would also want go f(x)! and defer f(x)! to be equivalent to go func(z T) { f(z)! }(x) and defer func(z T) { f(z)! }(x).

[bcmills]

So, what are the cases where it really is appropriate to ignore _all_ of the return values of a function?

These are always appropriate to ignore:

• Write methods on a bytes.Buffer or hash.Hash (which always return len(p), nil).
• (ioutil.NopCloser).Close (but in that case why use the NopCloser at all?).
• (expvar.Map).Init (which returns the receiver).

These are conditionally appropriate:

• Close on an io.ReadCloser implementation (if it is not also writable).
• (io.Closer).Close, (exec.Cmd).Wait, and builtin.recover (if the current function is already returning some other error).
• Transitive writes to a bytes.Buffer (if the bytes.Buffer detail is obvious enough to avoid bugs if the code is refactored).
• Transitive writes to os.Stdout or os.Stderr, including the fmt.Print functions (if the program is small enough and has few enough dependencies that SIGPIPE is obviously not ignored, and/or the program will obviously exit soon).
• atomic.Add functions (if overflow is benign or impossible).
• math/big operator methods (if the receiver is used elsewhere).
• Container insertions (if the caller is storing an element for later or doesn't care whether it is stored).
  
  
  
  • (*ring.Rink).Link
  
  
  • (*list.List).{InsertBefore,InsertAfter}
  
  
  • (*sync.Map).LoadOrStore
  
  
  • (reflect.Value).TrySend
  
  
• Container deletions (if the caller already has the element or is no longer interested in it)
  
  
  
  • heap.Remove
  
  
  • (*ring.Ring).Unlink
  
  
  • (*list.List).Remove
  
  
• time.AfterFunc (if the caller will never cancel the operation).
• runtime.GOMAXPROCS and runtime.SetMutexProfileFraction (if the argument is nonzero and the previous value will not be restored).
• Slice operations:
  
  
  
  • copy and reflect.Copy (if the number of bytes to be copied is already known)
  
  
  • append (if the argument is known to fit in the slice)
  
  
• (reflect.Value).Call and (reflect.Value).CallSlice (if the function is known to have no return-values, or known to be one of the above cases)

Unclear to me:

• (sql.Tx).Rollback

[rodcorsi]

As you say, we have few cases where it is appropriate to ignore the return value, and we can do this clearly with an attribute.

func Fprintln(w io.Writer, a ...interface{}) (n int, err error) optional

[AlekSi]

Personally, I think that linters like https://github.com/kisielk/errcheck solve the problem quite nicely.

[romainmenke]

IMHO this is not a problem with the language spec and can be remedied by linters.

One of the reasons I stopped writing in Swift is because they kept introducing breaking changes with things like @discardableResult. It didn't fix a problem with the language but broke a lot of things. First I gave up on using third party libs (because most don't have cross version support) then I gave up on Swift entirely.

Please consider the awesome go community before changing something as fundamental as function returns.

[bcmills]

@nerdatmath

I _think_ in most of these cases we really want to panic in case of error, because the programmer believes an error is not possible

A generic "must" operator would not help for forgotten return-values from calls that do not return errors (such as (time.Time).Add). Furthermore, in many cases we really do intend to _ignore_ the error, not not panic if it occurs: for example, if we are already returning a non-nil error from writing a file, we typically do not care whether its Close method returns an error.

[bcmills]

Heuristic tools are for the _author_ of the code, not the _reader_. I want to address both of those users, not just the author.

If there is a consistent rule in the language itself, then the reader can easily see whether a return-value has been discarded, and can know that the author intentionally chose that behavior.

With a heuristic tool, the reader can't distinguish between several possibilities:

1. The call in question does not return a value.
2. The call returns a value, but the tool heuristically decided it was probably not relevant.
   a. …and the heuristic was correct.
   b. …and the heuristic was incorrect, and the author didn't notice.
3. The author of the code ran the tool, but intentionally ignored its warning.
4. The author of the code didn't run the tool.

Because the tool is not part of the language, its use discards important information which can only be recovered by ad-hoc documentation, and that documentation is (in my opinion) even more unpleasant than a leading _, _ = or ignore:

fmt.Fprintln(w, stuff) // Ignore errors: w always wraps a bytes.Buffer.

[bcmills]

The other problem with heuristic tools is that they are unreliable for authors.

If I run lint or vet on my code as I write it, I can't distinguish between:

1. The call in question does not return a relevant value.
2. The call returns a relevant value, but the tool's heuristics indicated that it was not relevant (and my tests didn't catch it).

We could bias toward over-reporting in order to reduce the occurrence of case (2), but that would make it much more likely that authors would simply turn off the check, further decreasing the confidence of _readers_ of the code. We could provide a mechanism to suppress false-positives, but if we're going to do that, why not use it uniformly in the first place?

Given that most of the ignorable return values I could find (in https://github.com/golang/go/issues/20803#issuecomment-312318808) are only _conditionally_ ignorable, I'm skeptical that we can find a set of heuristics that are actually reliable in practice.

[jba]

for example, if we are already returning a non-nil error from writing a file, we typically do not care whether its Close method returns an error.

I think that's a bad example: you probably want to check both, in case of delayed writes. (But I agree with your main point.)

[jimmyfrasche]

This is a really gross idea, but another option:

With type aliases, the author of a package could add

type ignorableError = error //or irrelevantError, unusedError, etc.

and return it when it is always safe to ignore the return error like:

func (T) Close() ignorableError {  //just need to satisfy the interface
  return nil
}

Since it's a type alias it only changes the spelling not the meaning.

Linters could be aware of the idiom and whitelist anything that returned an alias spelled ignorableError to cut down on the noise.

Readers aware of the idiom would know it's safe to ignore from the signature.

It could be promoted from idiom to standard by making it a predeclared identifier.

Of course if you change something so that the error is no longer ignorable and forget to update the signature . . .

[bcmills]

@jimmyfrasche I admire your creativity, but that wouldn't address non-error return types, and as far as I can tell wouldn't help the reader much for conditionally-ignorable return values.

[jimmyfrasche]

@bcmills I was thinking about this again today and I realized that a less gross variant of my last post would be a convention to use the blank identifier to name always-ignorable return params, like:

func (b *BytesBuffer) Write(p []byte) (n int, _ error) {
  //code omitted
  return n, nil
}

or

func (Something) Close() (_ error) {
  return nil
}

It's

• perfectly legal today
• easy to lint †
• easy to update existing code
• mirrors the convention of explicitly ignoring a return _ = f() recognized by existing linters
• covers all return parameters

† it's also easy to lint the case of you used to have an ignorable error but then you made a change and forgot to rename the return param.

It wouldn't help a reader unfamiliar with the convention, but it's easy enough to pick up, especially if it's followed by the stdlib.

I'm not sure there's a way to help with conditionally-ignorable errors. Really if you can only ignore an error in some conditions you:

• need to be absolutely sure you meet those conditions and use something like _ = f() to express your intent at the call site (comment appreciated, of course)
• need a linter that can abstractly interpret the code and prove that error is always non-nil
• hedge your bets and deal with the error at the call site even if you're pretty sure that the path is never hit

[bcmills]

@jimmyfrasche

I'm not sure there's a way to help with conditionally-ignorable errors.

Given the list in https://github.com/golang/go/issues/20803#issuecomment-312318808, I think those are the only ones that really matter. Even the canonical example of an intentionally-ignored error, fmt.Printf, is only safe to ignore if the program is small and self-contained. (The special cases for bytes.Buffer and hash.Hash are easy enough to encode in a linting tool either way.)

[jimmyfrasche]

@bcmills Special cases for bytes.Buffer in the standard library are easy enough to special case in a linting tool but that doesn't help with the problem of making it clear to someone looking at the docs that it can always be ignored or the problem of discovering such funcs outside the standard library. A convention here would be useful to people and linters.

Let's say there was a way to document that you can sometimes ignore a result, for example an error, however. What does that tell you? That you can sometimes ignore the error. That's not very helpful unless you know when you can ignore the error. Short of a machine-checkable proof that a very sophisticated linter can use to analyze your program and verify that you can indeed safely ignore the error, the only recourse I see is to document the function with the cases when it is safe to ignore the error (and hope that if they change the docs get updated). A less sophisticated linter could see an annotation that a result is optional and that you didn't use it and assume you know what you're doing and not say anything, but that doesn't seem especially helpful.

If there were a way to annotate that you must use a result, it's easy to detect when you didn't. But that's most things so it could infect the majority of func sigs. (Though coupled with the _ convention for always ignorables it would give you the set of optionally ignorables as a by-product).

For the sake of argument say that 90% of results must be checked, 9% must be checked in certain scenarios, and 1% never need to be checked. Let's say we go with annotating the optionally and always ignorables because there are fewer of them (10% vs 90%). Then we can assume that anything that is not annotated must be checked. Then

• it's always safe to ignore a result that's been marked always ignorable
• it's never safe to ignore a result that is not annotated
• it's still not clear whether it's safe to ignore a result marked optionally ignorable so a linter is going to have to say something about it.

The only gain I see is annotating the always ignorables since it at least can safely remove some noise from linter output. The _ = f() notation let's you be explicit at the call site as a signal to readers and linters alike that you're ignoring a result.

[gladkikhartem]

@bcmills

I think there should be no ignorable errors, because they are really confusing from a standpoint of a plain logic.
If you could ignore it - why does the function is returning it in a first place!? If It's something to conform to an interface - you should not ignore an error, because you never know what could be behind that interface in a future. Functions should adapt to strict error handling, rather than error handling should adapt to 1% of functions that return "ignorable" errors.

I think simple handling ALL return parameters is enough to eliminate typos. It's a balance between cognitive load and being error-prone. And if person explicitly ignores error - it's a mental problem, rather than the problem of exception handling.

Not even mentioning the fact that if you introduce new stuff to Go - script-kiddies would use it everywhere.

[jba]

If It's something to conform to an interface - you should not ignore an error, because you never know what could be behind that interface in a future.

If you want to talk logic, there is a logical flaw in that argument. You are conflating implementing a method in order to conform to an interface, with invoking that method through an interface type. For example, I will never need to check the error from Write in

var buf bytes.Buffer
buf.Write(...)

[bcmills]

CC: @dgryski

[networkimprov]

Because mistakenly ignored errors are oft-cited as a drawback of Go, the great majority of the Go community likely expects a solution to arrive with Go 2 Error Handling.

The initial Go 2 draft design doesn't address this, but it could be realized via named handlers and a predefined _ignore_ handler. In a debug or test mode, the ignore handler can log its inputs. The return value handled is not limited to type error. (Note: the proposed check keyword is not really suitable for named handlers.)

At last count, 17 posts on the feedback wiki suggest named handlers. I'll post links if anyone's interested.

This and related matters are outlined in Requirements to Consider for Go 2 Error Handling.

// op is a symbol, e.g. ?#@ or unambiguous pair

fn op hname (a)       // invoke handler hname on non-zero return value
v, op hname := fn(a)  // syntax variant for assignment

fn op _ (a)           // invokes predefined handler to ignore/log result
fn op   (a)           // alternative for brevity
fn op panic (a)       // invokes predefined handler to panic
fn op !     (a)       // alternative for brevity

// choosing # as the symbol

func giveError(int) (int, error) { ... }

work(giveError#(a))        // ignore
work(giveError#!(a))       // panic
work(giveError(a))         // compile error for argument count

giveError#(a)              // ignore
giveError#!(a)             // panic
giveError(a)               // vet flag for ignored error

v, #_     := giveError(a)  // ignore
v, #panic := giveError(a)  // panic
v, _      := giveError(a)  // vet flag for ignored error
v, err    := giveError(a)  // ok

// the vet flags should be compile errors, but that breaks Go 1 code
// more examples in "Requirements to Consider..." linked above

[DeedleFake]

The initial Go 2 draft design doesn't address this, but it could be realized via named handlers and a predefined ignore handler.

I'm not so sure, actually. I would guess that, as the proposal postulates, the vast majority of accidentally ignored errors are from functions that return _only_ an error or from functions where the other return values aren't usually needed, such as for fmt.Println(). check and the various named handlers proposals are primarily designed to reduce repetition for error handling by working around some aesthetic issues with multiple return values by providing some cleaner ways of dealing with a common special case. They don't fix accidentally ignored errors because it's completely outside the scope of what they're trying to deal with. Both could be relatively easily extended to handle accidentally ignored errors though by simply requiring some form of handling for an error. In other words,

// Compile error.
fmt.Println()

// Fine.
check fmt.Println()

// Also fine. Assumes the no-unused-variables errors will handle it
// after this.
_, err := fmt.Println()

// Also fine.
_, _ = fmt.Println()

// Still fine. There's only so much you can do to stop the user from
// shooting themselves in the foot before it gets annoying.
_, err = fmt.Println()

[networkimprov]

@DeedleFake see #20148 & https://github.com/golang/go/issues/20803#issuecomment-312103971

and note work(giveError#(a)) & giveError#(a)