proposal: go/printer: don't reformat single line if statements

[ianlancetaylor]

This proposal is mainly due to @bradfitz and likely others. It's similar to #33113 but we could actually choose to adopt it.

I'm not sure that this is a good idea

The proposal is to not format certain if statements if they are written as a single line. This will permit more concise code for certain common error handling patterns.

Consider these two functions. They are the same sequence of tokens, formatted differently. However, gofmt today will not change either of them.

func F1() {
	go func() { fmt.Println("hi") }()
}

func F2() {
	go func() {
		fmt.Println("hi")
	}()
}

This is because go/printer permits a go statement with a simple function literal that is written as a single line to remain on a single line.

We could apply a similar rule to if statements. Basically, if

• an if statement is written on a single line, and
• the body of the if statement is a single return statement, and
• nothing in the return statement would normally be formatted on multiple lines (such as a composite literal), and
• there is no else clause

then go/printer would not modify the if statement: it would remain on a single line.

Here is what the example from https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md might look like if this were permitted:

func CopyFile(src, dst string) error {
	r, err := os.Open(src)
	if err != nil { return err }
	defer r.Close()

	w, err := os.Create(dst)
	if err != nil { return err }
	defer w.Close()

	if _, err := io.Copy(w, r); err != nil { return err }
	if err := w.Close(); err != nil { return err }
}

In other words, this proposal would permit more concise error handling without changing the language. It would be concise not in the sense that it would use fewer tokens, or that it would require less typing, because it wouldn't, but in the sense that it would be less obtrusive on the page.

This proposal would not affect any existing code. It would merely permit people to use a different formatting style for this specific case, and gofmt would preserve that formatting style.

Of course, that is also a disadvantage, in that it would introduce a variation in Go formatting, one that is more significant in practice than the formatting of go and defer statements. One could imagine arguments about the formatting style to use, which would go against the goal of gofmt.

As I said above, I'm not sure this is a good idea, but I wanted to write it down for discussion purposes.

[thepudds]

FWIW, there was a decent amount of discussion about allowing certain if statements to format on one line as part of the try discussion in #32437. If I recall correctly, it might have been @zeebo who first raised it there.

Here's a link to that group of comments:

https://swtch.com/try.html#gofmt1

(This proposal might be a bit different than what was previously discussed, including I don't recall whether "merely permit people to use a different formatting style" was discussed, but just trying to link to related discussion, including @zeebo and perhaps others provided some additional concrete examples there).

[ianlancetaylor]

@thepudds Thanks, it does look like this exact idea was expressed back then.

[beoran]

Though it was discussed before in the context of "try", we didn't discuss it as a standalone proposal yet. I think now as I thought then that this proposal is the easiest way to simplify error handling in Go.

This proposal makes it easier both for the writer to write a single error return, and for the reader to skim over the error handling lines, and has many other benefits as described in the discussion linked above. Also, since this is essentially opt in, if will have no effect on existing code. The only downside I can see is that it might lead to inconsistency and discussions about what style to use, but for new projects I would definitely use the single line style for all if err != nil { return err } statements.

[dominikh]

This proposal makes it easier both for the writer to write a single error return

I wouldn't consider the omission of two newlines to be considerably easier.

and for the reader to skim over the error handling lines

Which I think is an argument against this, not for it. The lack of indentation conceals control flow.

since this is essentially opt in

It is opt-in for the author, but not for the reader.

[thediveo]

As a reader I'm perfectly fine with reading the more compact format as opposed to the current baroque style. Please don't patronize me.

[alarbada]

It is opt-in for the author, but not for the reader.

It is weird to read that, considering that one can always write the following code:

type somelargeuserstruct struct {
    name1 string
    name2 string
    name3 string
    name4 string
    name5 string
    name6 string
}

func test() {
    _ = somelargeuserstruct{"name1","name2","name3","name4","name5","name6"}
}

and be completely fine about having that large struct being produced in one line (at least gofmt is is fine), but we consider the following:

	r, err := os.Open(src)
	if err != nil { return err }

hard to read. I know, it is subjective, but consider these accepted gofmt style examples:

type User struct {
	name string
	age  int
}

func test() {
	_ = User{name: "some name", age: 21}

	_ = User{
		name: "some name",
		age:  21}
	_ = User{
		name: "some name", age: 21,
	}
	_ = User{
		name: "some name",
		age:  21,
	}
}

It's matter of style using each one of those. That's quite liberal, and in my opinion that's good, gofmt tries to adapt to most programming styles. Why then should the general style be conservative with adapting if err != nil { return err } in one line?

To add to the discussion, I don't think I would ever write the CopyFile as currently written, but rather:

func CopyFile(src, dst string) error {
    r, err := os.Open(src)
    if err != nil { return err }
    defer r.Close()

    w, err := os.Create(dst)
    if err != nil { return err }
    defer w.Close()

    _, err := io.Copy(w, r)
    if err != nil { return err }

    err := w.Close()
    if err != nil { return err }
}

So it is easier to debug when using delve.

[beoran]

@dominikh I never type those two newlines in an if err != nil { return err } , on an average Go program this does save me some typing. I wish go fmt wouldn't insert them either.

I see here and in many other error related issues that many Go code readers want to skim the simpler error handling lines to be able to understand more easily what the "happy path" through the code is doing, and I agree with that

For parts of the code where errors need to be handled in detail, this proposal will not apply anyway, since more thorough error handling will not fit on a single line.

As @not-rusty said, for other code constructs, go fmt now allows us to choose which style we prefer, and that works great. I think that for simple if err != nil { return err } statements, there is very little harm and many benefits to allowing the developers to choose likewise.

[shasderias]

For those interested in experimenting with how this proposal would feel like if implemented, the IntelliJ IDEs (GoLand and IntelliJ Ultimate with the Go plugin) have effectively implemented this proposal by displaying return values inline when folding certain if blocks. Hope this helps with coming to a decision.

The CopyFile() example can be folded as such:

This bit of code,

func NewInvoiceCreateRequest(discountPct, taxPct decimal.Decimal, remarks string) (*InvoiceCreateRequest, error) {
	if discountPct.IsNegative() {
		return nil, errors.New("discount percentage cannot be negative")
	}

	if discountPct.GreaterThan(decimal.NewFromInt(1)) {
		return nil, fmt.Errorf("discount percentage greater than 1: %v", discountPct)
	}

	if taxPct.IsNegative() {
		return nil, fmt.Errorf("tax percentage cannot be negative: %v", taxPct)
	}

	if remarks == "" {
		return nil, ErrRemarksEmpty
	}

	if err := expensiveDBCall(); err != nil {
		return nil, errors.Wrap(err)
	}

	if err := expensiveDBCall2(); err != nil {
		return nil, err
	}

	return &InvoiceCreateRequest{
		DiscountPct: discountPct,
		TaxPct:      taxPct,
		Remarks:     remarks,
	}, nil
}

can be folded as such:

The second if block is not folded as the IDE does not show the return values inline when folded, likely due to the resulting line being too long.

Same example, with newlines removed.

Multiple return values with a switch block, perhaps relevant for #57667.

[beoran]

@shasderias The fact that they went through the effort of implementing such code folding shows that there are enough Go developers out there who prefer to read the single line style.

[dominikh]

I never type those two newlines in an if err != nil { return err } , on an average Go program this does save me some typing. I wish go fmt wouldn't insert them either.

@beoran I never type them, either, because I rely on gofmt to insert them for me.

[beoran]

@dominikh I see. I have to admit that for those who want to keep the three line style even for a simple return err, this proposal has the overhead of having to type two additional newlines. Of course, this overhead seems acceptable in the case of @not-rusty 's examples, and it seems to me many go developers will prefer the single line style, so I am not convinced this is a serious problem.

[shasderias]

@beoran That's true, but besides the point. I think the fundamental question here is: gofmt respects the writer's intent of where to place line breaks in certain circumstances and disregards it in others. Does a single line if statement fall under the former or the latter?

Taking one step back, editors alter and augment the presentation of source code to assist the programmer. Given that an editor can present a single line if statement in its compact form much more effectively than gofmt can - IntelliJ elides braces and the return keyword when folding a single line if statement, is it still necessary to place this responsibility on the language?

[beoran]

@shasderias I think that, like for #57667, now go fmt is often overzealous in inserting newlines in simple one line blocks. It has the effect that simple code looks much more voluminous than is warranted. For this specific instance of newline insertion, I think go fmt should respect the writer's intent. Of course, no one forces me to use go fmt, and I could probably fork it, but I would rather not.

As for editors or IDE, sure they can do this, but I think that that is irrelevant to deciding on this proposal for go fmt by itself. Actually I think that relying on what editors or IDE can do will not lead to a good language, or in this case, tool design.

[ulikunitz]

The proposal allows two formatting styles for typical error handling code. The pattern

if err != nil {
        return  /* [ <zero value>, ] */ err
}

is part of every code base I have seen. The proposal will split the Go community in two camps. It has the potential to become the tabs vs spaces issue for the Go community. If there are benefits in compacting those statements, it should become mandatory.

However I don't think the change would be beneficial, because even this proposal has the weakness, that all proposals about error handling have; they want to make error handling disappear on various levels. I always thought, that making code flow -- particularly conditional code -- visually explicit, was a strength of the Go code formatting style. I don't want to lose this strength.

[beoran]

I don't think this proposal will be that divisive, when looking at @not-rusty's examples, which also did not split the community.

Actually I would say that the benefit of this proposal compared to most other error handling proposals is that it does not hide error handling, but just makes it easier to read the "happy path".

[alarbada]

I always thought, that making code flow -- particularly conditional code -- visually explicit, was a strength of the Go code formatting style. I don't want to lose this strength.

Mmm, I believe for people that care a lot about having an extremely consistent looking codebase style wise there's gofumpt, the stricter gofmt. That tool could also format all if err != nil statements into 3 lines.

At the end, it is up to the author whether they make an error handling block 3 lines or one. For example:

// this
if err != nil { return fmt.Errorf("user error: some really long description with params and wrapping %w", err) }

// vs this
if err != nil {
    return fmt.Errorf("user error: some really long description with params and wrapping %w", err)
}

I think most would agree to write the second form. gofmt is even more flexible, it allows you to do this:

if err != nil {
    return fmt.Errorf(
        "user error: user %v with age %v had an authentication error %w",
        user.Name, user.Age, err,
    )
}

and there are plenty of more cases where gofmt is flexible about how many lines an expression can take:

func SomeFunc(name string, age int, street string, isCool bool, isAwesome bool, otherThings string) (int, int, error) {
    // ...
}

// vs this

func SomeFunc(
    name string, age int, street string,
    isCool bool, isAwesome bool,
    otherThings string,
) (int, int, error) {
    // ...
}

// vs this

func SomeFunc(
    name string,
    age int,
    street string,
    isCool bool,
    isAwesome bool,
    otherThings string,
) (int, int, error) {
    // ...
}

// vs more combinations

Wouldn't you agree aswell that if the community was to be extremely splitted that easily, it could happen with how functions are defined, for example?

[robpike]

I am of the opinion that control flow should not be buried in the middle of the line, so the return keyword should be the first token on the line.

There are exceptions, the most common in my own code being the one-line functions satisfying sort.Interface. But I am unsure whether error handling, although a common pattern, should follow that lead by hiding itself mid-line. There are details in a handled error that should not be shrouded.

[beoran]

@robpike It seems to me that the 10 years long discussion on error handling in Go can be resumed somewhat simplified as being about two conflicting ideals, namely explicitness and readability.

Go, as it is now, has both error handling and control flow explicit. However, the problem is that this significantly reduces the readability of the "happy path". This is the reason why error handling in Go is such a much debated topic. It is difficult to have both explicitness and readability. Exception based programming languages increase readability at the cost of error handling being implicit to the point where it becomes an afterthought. In Go, the trade off is different. Error handling is explicit to the point where it harms readability.

With this proposal, error handling and control flow remain explicit, but the author can now opt to make the "happy path" easier to read by reducing the visual space taken by error handling. I think that this is probably the only way to increase readability while only sacrificing a minimal amount of explicitness.

But if even not inserting a newline is seen as too much of a sacrifice on the side of visibility, then I think there is no solution for the error handling readability problem in Go. Any other solution is by logical necessity going to be more implicit than simply not inserting newlines.

[robpike]

@beoran Your response captures my objection. Yes, it's good for the non-error path to be nice to read, but it's also important that the error path be visible.

if there is a problem in error handling in Go, it's not the readability, it's the belief that all one writes is

if err != nil {
   return err
}

That belief has led to a lot of bad code and bad thinking. The error path should actually handle the errors, not just blow them off.

As I wrote, the details of error handling should not be hidden. But this proposal not only obscures them, it encourages the return err approach to handling errors by making it easier to write bad error handling that just passes the buck. Keeping the handling clearly separated makes it easier to see when the error handling is insufficient.

[beoran]

@robpike yes, I agree this proposal makes the return err somewhat easier, but I don't think it is necessarily a bad thing.

In large, multi layered software, the middle level functions often do not have enough information to truly handle the error. The best they can do is decorate or wrap the error with return fmt.Errorf("decorations: %w", err), but in many cases, the low level error is good enough and can be passed on to the higher level functions which decide what to do and truly handle the error. If we look at existing go code the return err pattern, is extremely common for such reasons. Some "bucks" really need to be passed.

[bradfitz]

This proposal is mainly due to @bradfitz and likely others. It's similar to #33113 but we could actually choose to adopt it.

For the record, I'm not really pushing for this (but am not opposed to it). My comment to @ianlancetaylor and @griesemer was basically, "There sure are a bunch of error handling language change proposals. I bet 90% of them would never have been filed if if-err-not-nil-return was all on one line since that'd take the wind out of most the proposals' cited benefits."

And I pointed out that gofmt leaves plenty of style choices up the the author (like funcs, as Ian showed), and gofmt leaving it alone if one one line is how #33113 could've been made non-invasive, without reformatting the world.

[beoran]

@bradfitz Most error handling proposals have the implicit or explicit goal to reduce error handling to a single line, in order to improve the readability of the "happy path". As long as that important point is not addressed, new proposals will keep on being filed. Now we have generics, this point is one of the points blocking the wider acceptance of Go. This proposal is a small concession towards that point, but it will be be sufficient in many cases, without much negative side effects.

[apparentlymart]

My big concern with this proposal is that the if err != nil { return <something derived from err> } pattern is so fundamental to practical Go programs that it's very likely that any given Go program will include examples of every possible style that gofmt allows.

Other situations I can think of where gofmt is unopinionated are situations where I expect there are far fewer examples of them in a typical program, although of course that's just a hunch and I don't have any data to back it.

Because the error handling pattern is something that every Go developer will need to learn and recognize, I think it's particularly important for it to have a very similar shape in every Go program, so that our pattern-matching brains can quickly recognize it when scanning code.

If gofmt had always formatted this kind of statement in this way, or if it were feasible to retroactively change gofmt to force this new style on all existing programs (which I don't think is true) then I would be fine with it, but I'm specifically concerned about there being two allowable ways to present this pattern, and so that is why I voted 👎 .

[thediveo]

[...] Now we have generics, this point is one of the points blocking the wider acceptance of Go. This proposal is a small concession towards that point, but it will be be sufficient in many cases, without much negative side effects.

My impression is that acceptance of Go isn't blocked by this (at least not in itself). However, I agree with the second sentence here. This is to humbly disagree with Rob Pike because I write a lot of Go packages that are deep inside the stack, so "blowing off" errors is the only rational thing to do, optionally augmenting the error, where possible.

I can't see how supporting this new pattern will blow the pattern memory of Go programmers. As I said before, this smacks like patronizing to me; more so, as the people claiming this (including Rob Pike, sorry) don't have any real world scientific data on this. In that this is the unfortunate situation where the community is split, and thus no action is being taking in the end. Angela Merkel gives all her thumbs up, she's an expert at applying this technique in politics for decades 😁

[rsc]

There's definitely a real problem here with how to reduce the pain and verbosity of error handling without encouraging bad error handling. It's unclear that single-line if statements does that. I still think we should move toward something like 'try' if we can figure out how to make it work (we got hung up on interaction with defer and other control flow issues last time).

I wrote this code earlier this week:

func writeZip(name string, a *Archive) {
	f, err := os.Create(name)
	if err != nil {
		log.Fatal(err)
	}
	defer func() {
		if err := recover(); err != nil {
			log.Fatalf("writing %s: %v", name, err)
		}
	}()

	zw := zip.NewWriter(f)
	zw.RegisterCompressor(zip.Deflate, func(out io.Writer) (io.WriteCloser, error) {
		return flate.NewWriter(out, flate.BestCompression)
	})
	for _, f := range a.Files {
		h := check(zip.FileInfoHeader(f.Info()))
		h.Name = f.Name
		h.Method = zip.Deflate
		w := check(zw.CreateHeader(h))
		r := check(os.Open(f.Src))
		check(io.Copy(w, r))
		check1(r.Close())
	}
	check1(zw.Close())
}

func check[T any](x T, err error) T {
	check1(err)
	return x
}

func check1(err error) {
	if err != nil {
		panic(err)
	}
}

This is not what I'd recommend in general but it did reduce the error handling quite a lot.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[thepudds]

FWIW, that style with simple generic check (or must or try or ...) seems to be growing, either by typing the short amount of code each time, or copy/pasting from a gist, or similar.

Personally, I find it pleasant, especially when doing a quick one-off or when initially starting something (and then removing it later if/when graduating to a more "real" project).

My main reason for commenting here though is just to link to https://github.com/dsnet/try, which is a nicely thought out variation of that.

[earthboundkid]

See BradFitz's proposal for a "must" package with Do/Get: #54297

[beoran]

@rsc Maybe a generics approach like this could be added to the standard library. It would have the downside that it is easy to forget the defer to handle the error, so it would be best to add some way to enforce this to the language. Maybe "checked panics" like checked exceptions in Java?

Error handling in Go is a bit of an Excalibur which no one has been able to pull from the stone yet. This proposal is "the simplest thing that could work" and doesn't preclude any more complex solutions to be added later.

[DeedleFake]

@beoran

One issue with that is that it would require separate functions for every possible number of returns from a function. Variadic generics could fix that problem, but they seem unlikely any time soon.

[beoran]

@DeedleFake Actually that is an interesting idea. Adding variadic returns to Go, like, say, is possible with Ruby, even for non generic functions might also help with error handling. But that would be a different proposal.

[DeedleFake]

But that would be a different proposal.

Indeed. In fact, it would be #56462.

[rsc]

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

[beoran]

Well, at least this makes it clear that this also does not seem to be the desired solution for error handling in Go. Let's hope some Arthur passes by with a better idea.

[rsc]

No change in consensus, so declined.
— rsc for the proposal review group