Go: Proposal: selector-specific control flow & gofmt rules to improve error-checking readability

Created on 3 Jun 2020  ·  9Comments  ·  Source: golang/go

In order to reduce basic error-checking bloat in Go software and improve readability, I propose shifting eligible error-checking to the right by the addition of control-flow that operates on the error type and a related gofmt rule.

Errors passed to the caller should not interrupt logical-flow

Go has an error-checking issue. Whenever a function is called that returns a value and an error. At least 3 additional lines of code are required to check it.

This isn't an issue when the code is actually handling the error (as the error handling becomes part of the logical-flow of the code) but when the error is simply passed to the caller, the error-checking obstructs the logical-flow of the code and makes it more difficult to understand what the code is trying to do.

In a perfect world, there wouldn't be any errors and IO functions could be written as such.
The behaviour of this function is easy to reason about.

func OpenImage(name string) image.Image {
    return image.Decode(os.Open(name))
}

Now consider the OpenImage functions below, _OpenImage2_ checks errors and _OpenImage1_ does not. The logical-flow of _OpenImage1_ is easier to read than _OpenImage2_.

func OpenImage1(name string) image.Image {
    f, _ := os.Open(name)
    img, _, _ := image.Decode(f)
    return img
}

func OpenImage2(name string) (image.Image, error) {
    f, err := os.Open(name)
    if err != nil {
        return nil, fmt.Errorf("OpenImage: %w", err)
    }
    img, _, err := image.Decode(f)
    if err != nil {
        return nil, fmt.Errorf("OpenImage: %w", err)
    }
    return img, nil
}

Most of the time, errors do need to be checked though and this causes the logical-flow of the code to be obstructed. Here is another example from here.

func (ds *GitDataSource) Fetch(from, to string) ([]string, error) {
    fmt.Printf("Fetching data from %s into %s...\n", from, to)
    if err := createFolderIfNotExist(to); err != nil {
        return nil, err
    }
    if err := clearFolder(to); err != nil {
        return nil, err
    }
    if err := cloneRepo(to, from); err != nil {
        return nil, err
    }
    dirs, err := getContentFolders(to)
    if err != nil {
        return nil, err
    }
    fmt.Print("Fetching complete.\n")
    return dirs, nil
}

Trying to read functions like this is jarring as the reader is interrupted by error checking at almost every step.

Proposal: selector-specific control flow

x.return [ ExpressionList ] .

For a value _x_ of type _error_, _x.return_ inside of function _F_ terminates the execution of _F_
if _x != nil_, and optionally provides one or more resulting error values. Any functions deferred by F are executed before F returns to its caller.
There are three ways to return values from a function with a result type:

  1. The return value or values may be explicitly listed in the "x.return" statement. Each expression must be single-valued and be assignable to a subsequent error-typed element of the function's result types, all non-error results are set to their zero values.
func OpenImage(name string) (image.Image, error) {
    f, err := os.Open(name);    err.return fmt.Errorf("failed to open image: %w", err) 
    img, _, err := image.Decode(f); err.return fmt.Errorf("failed to decode image: %w", err)

    return img, nil
}

func OpenImageSpecificErrors(name string) (img image.Image, openErr error, decodeErr error) {
    f, err := os.Open(name);    err.return fmt.Errorf("failed to open image: %w", err), nil
    img, _, err := image.Decode(f); err.return nil, fmt.Errorf("failed to decode image: %w", err)

    return img, nil, nil
}
  1. The expression list in the "return" statement may be a single call to a multi-valued function. The effect is as if each value returned from that function were assigned to a temporary variable with the type of the respective value, followed by a "return" statement listing these variables, at which point the rules of the previous case apply.
  2. The expression list may be empty if the function's result type contains only one error type. The "return" statement returns the selected error and all other results set to their zero values.
func OpenImage(name string) (image.Image, error) {
    f, err := os.Open(name);    err.return
    img, _, err := image.Decode(f); err.return

    return img, nil
}

Proposal: gofmt rules for error.return

  1. An error.return following an error value assignment should always be placed on the same line as the error assignment (or the same line as the last line of a multiline assignment).
  2. IfStmts that are on the subsequent or same line of an error assignment and are equivalent to an error.return statement should be replaced with an error.return statement that abides by the previous rule.
  3. If an error.return is both on the current line and on the previous line, the indentation of both should be set such that both error.return's are aligned. As shown in the selector-specific control flow examples.

Result

func OpenImage(name string) (image.Image, error) {
    wrap := func(in error) error {
        return fmt.Errorf("OpenImage: %w", in)
    }

    f, err := os.Open(name);    err.return wrap(err)
    img, _, err := image.Decode(f); err.return wrap(err)

    return img, nil
}

func (ds *GitDataSource) Fetch(from, to string) ([]string, error) {
    fmt.Printf("Fetching data from %s into %s...\n", from, to)

    err := createFolderIfNotExist(to);  err.return
    err = clearFolder(to);          err.return
    err = cloneRepo(to, from);      err.return
    dirs, err := getContentFolders(to); err.return

    fmt.Print("Fetching complete.\n")
    return dirs, nil
}

What impacts will this have on Go

Pros

  • Backwards-compatible with all existing Go code.
  • error.return acts like an annotation, complementing readability rather than interrupting it.
  • The control-flow is mostly explicit, branching behaviour is easily deduced.
  • Fixes the readability of logical-flow in functions that do more error-passing than error-handling.
  • Gofmt enforces single way of passing errors to the caller.
  • No manual changes needed to stdlib or existing Go software's error handling (gofmt will auto-migrate).
  • Not specific to any one pattern of function signature, supports multiple errors, errors that are not the last return type. This makes it easier to copy and paste code with error-passing between functions.
  • Allows error-filtering and error-wrapping patterns by applying a function to the values in an error.return ie filter(err).return and err.return wrap(err)
  • Errors are not hidden.
  • May encourage error wrapping over lazy errors.

Cons

  • Longer lines.
  • Behaviour of error.return and return differ slightly and this could be confusing.
  • Branching behaviour of err.return not explicit.
  • Not found in other programming languages, unfamiliar pattern.
  • May encourage error passing over handling.
Go2 LanguageChange Proposal Proposal-FinalCommentPeriod error-handling

Most helpful comment

It seems like having very long lines with error handling on the right will tend to obscure the error handling. I understand that that is kind of the point of this issue. But errors do need to be handled. Hiding the handling at the end of a long line doesn't clarify the function, it obscures it.

If we don't try to put the error handling at the end of a long line, we could put it on the next line. But then we aren't saving all that much with this approach. We're saving an if, a != nil, and a couple of braces. And admittedly a couple of line breaks.

I feel like the try proposal (#32437) tried to address error handling at a deeper level. That proposal was not accepted, but it had the advantage of trying to recast error handling. This proposal seems to be tweaking error handling a bit. Is the benefit worth the cost of a language change?

All 9 comments

Sorry for the misoperation

Has similarities to #33177, #32946, #38151, #39148, and likely others.

I agree that the error checking and returning in the simple case if verbose in go. On the other hand it forces the developer to think about the actual handling, i.e. to not only return the error but also implement additional cleanup tasks (closing files etc).

In the proposal here I'm unclear how return values for the non-error results would be passed back?

@andig
In regards to cleanup tasks, that is what _defer_ and wrapper functions are for.

I want to make the distinction between handling an error (where appropriate action is taken and the error is not returned) and not handling an error (the error is transformed in some way, clean-up tasks are run and the error is returned so that somewhere up the stack it can get handled).

In Go, errors are explicit and a decision to handle needs to be made every time an error value is recieved. This is one of Go's strengths.

Handling errors in Go is great. Returning errors are... not so great. It looks like the error is being handled and this obstructs the readabillity of the code.

In my experience, Go doesn't force you to handle errors, it can actually discourage you from doing so. "My function is incredibly long and unreadable from all this error checking, why would I want to make it worse, I'll just return the error".

An argument can be made that having a shorter way to return errors will encourage less error handling. However I would speculate that the inverse can be true, having shorter functions psychologically gives the developer more room for error-handling. Less is more!

Keep in mind that this proposal is not strictly about saving characters, it's about moving error-passing to the right to improve readability. Personally I'm not a fan of previous check/try/? proposals (not explicit enough, everything is hidden, etc).

To clarify the behavior of non-error results:
_error.return_ returns non-error results set to their zero value. If you need to return different values then you must use a standard return statement.

It seems like having very long lines with error handling on the right will tend to obscure the error handling. I understand that that is kind of the point of this issue. But errors do need to be handled. Hiding the handling at the end of a long line doesn't clarify the function, it obscures it.

If we don't try to put the error handling at the end of a long line, we could put it on the next line. But then we aren't saving all that much with this approach. We're saving an if, a != nil, and a couple of braces. And admittedly a couple of line breaks.

I feel like the try proposal (#32437) tried to address error handling at a deeper level. That proposal was not accepted, but it had the advantage of trying to recast error handling. This proposal seems to be tweaking error handling a bit. Is the benefit worth the cost of a language change?

Based on the comments above, and the lack of support in emoji voting, this proposal is a likely decline. Leaving open for four weeks for final comments.

No further comments.

Was this page helpful?
0 / 5 - 0 ratings