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

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.

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.

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.

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

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.

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).

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

@cobexer I think you’ve meant “unused results” instead of “unused variable” since the line you provided wastes CPU\Memory but doesn’t do anything to the app logic, because result of strings.Join is ignored.

@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.

A “this value should not be discarded” would also be useful for slog.Logger.With() to prevent users from writing

slog.With("error", err)

and subsequently logging nothing.

We run into similar issues with logrus.WithError(err) today.

Considering Go refuses to compile programs with unused variables (https://golang.org/doc/faq#unused_variables_and_imports) I was surprised today when I found the code below. Since strings.Split() and strings.Join() are pure functions (https://en.wikipedia.org/wiki/Pure_function) I expect the Go compiler to refuse compiling the code below.

func normalizeChartName(chart string) string {
	strings.Join(strings.Split(chart, "/"), "_") // This line :)
	_, nc := path.Split(chart)
	// We do not want to return the empty string or something else related to filesystem access
	// Instead, return original string
	if nc == "" || nc == "." || nc == ".." {
		return chart
	}
	return nc
}

(from https://github.com/argoproj/argo-cd/blob/24526ded21049496fea5c61bf187ddc3ca2881e3/util/helm/client.go#L321)

@DeedleFake and @DmitriyMV: I am of course talking about unused function results, is my edited comment better understandable for you?

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 . . .

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.

Just how many programs would this proposal break?

Most of them. It is labeled “Go2” for good reason. 😃

Hi from the Java world – I wanted to add some context, but I don’t claim to know what things are like in your universe.

I believe Swift has the opposite default: you mark only the methods whose results are safely ignorable, and Kotlin plans to attempt switching its default in-flight. We recently pulled off this in-flight switch for Google’s entire internal Java codebase (asterisk) with very good results (many bugs found, few user complaints).

That might not be feasible for you (dunno) but it has some advantages:

• You probably wouldn’t need as many add-on features like “read-only implies must-use-result” or the ability to mark a type as in the previous comment.
• Conceptually, I’d claim that having an ignorable result is the real affirmative contractual behavior (i.e. when overriding we can “add” it even if the supertype method doesn’t, but “removing” it would break substitutability). “Must use result” is only the lack of this property.
• For us, anyway, ignorable-result methods the rare special case, and require a bit more thought.

Maybe this is all unhelpful, but anyway we’d be glad to talk further about our Java/Kotlin experiences if it would help.

I think, adding syntax to the function declaration for denoting pure functions could be used without breaking backwards compatibility. In this example, it is denoted by the exclamation mark following the function name.

func returnTrue() bool {
    return true
}

func returnFalse!() bool {
    return false
}

func main() {
    returnTrue() // no err
    returnFalse!() // compile err
    _ = returnFalse!() // no err
}

Proposal tweak: the family of fmt.Println functions should return nothing. Folks that want to check the fmt.Println error should use fmt.PrintlnV or a more appropriate name. That way you get the nice hello world, and error checking isn’t delegated to a linter that often isn’t there. Upgrading existing code is as easy as a string replace. We can leave this painful language wart to the history books.

Why would it fail to compile? Every variable is used.

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 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.

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.