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.

[earthboundkid]

If we also had ? for != zero and ... for return zero or more zero values, then error handling is solved forever. :-)

f, err := os.Open("whatever.txt")
if err? { return ..., err }
defer f.Close()

As I've written some years ago, I think those are the only possible improvements to error handling.

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