Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Support inline "if" statement syntax #32860

Closed
andreynering opened this issue Jun 29, 2019 · 17 comments
Closed

Proposal: Support inline "if" statement syntax #32860

andreynering opened this issue Jun 29, 2019 · 17 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Milestone

Comments

@andreynering
Copy link

This is a brainstorming idea that came from #32825 (comment).

To be honest, I'm not sure if a agree with my own proposal, since it goes against some Go conventions, which disallow one-line-ifs (#27135) or the ternary operator, for example.

Even then, I think it's worth posting the idea here and raise some discussion.


Proposal

I propose that Go could allow inline if statements, just like Ruby does. This means that a code like this:

if value < 0 {
	value = 0
}

Could be written like this:

value = 0 if value < 0

Or, to give a more common example:

if err != nil {
	return errors.Wrap(err, "oops")
}

Could be written as:

return errors.Wrap(err, "oops") if err != nil

Some experiences from Ruby

On the Ruby world, inline if statements are used a lot. Some Rubysts think inline if`s shouldn't be used for non-return statements, though, for a similar reason why Go requires blocks on if statements: it makes it harder to follow code paths.

Based on that, it's also a possibility to only allow inline if on return statements:

// this would work
return foo.thing() if foo != nil

// this would raise a compiler error
foo.doSomething() if foo != nil

This restriction may prevent users to misuse this feature. Requiring a return in the beginning of the line would mitigate the raise on complexity while reading code searching for code paths.

Talking about errors

While this proposal has no relation to errors per se, it's true that the most common use case would be reducing the number of lines of code required to do error handling. In theory, in most cases this would allow 1 line to check + return an error instead of 3.

Pros

I think this proposal is way saner then #32437 which proposes the try built-in:

  • try is black magic, and will be likely very confusing for beginners, while return is clear
  • try is implicit, returns are explicit
  • try disallow doing anything different than just returning the error, while with a plain return ... if people still have control to return a wrapped error or to modify the if statement:
    • return err if err != nil && err != context.Cancelled

I also think this is better than most errors proposals out there (including handle, etc).

Cons

  • Harder to follow code paths, since if blocks are easier find while scanning code
  • Can encourage programmers to write long lines (80+ characters) which reduces readability
  • This breaks with existing Go conventions (but not more than try or handle, IMHO)
@henvic
Copy link
Contributor

henvic commented Jun 29, 2019

if err != nil return err

Why would your proposal be better than this example?

I don't like either, though.

@networkimprov
Copy link

Would it also allow single-line case, for, else, var () ? I'd like them all, please ;-)

See also #32611 which suggests: on err, <single_statement>

@lootch
Copy link

lootch commented Jun 30, 2019

Well, the obvious sequitur seems to me only a little stretch to Go syntax; not beautiful, but that is in the eye of the beholder:

return err != nil; arg1, arg2, arg3, fmt.Errorf("failed: %v", err)

as in - for the case in point:

return err != nil; err

Which then might just lead to the totally heretical:

return arg1, arg2, arg3, fmt.Errorf("failed: %v", err); err != nil

and an incompatible idiom for returning the now "special" error argument:

func action(a int) (arg1, arg2, arg3 string; error) { ... }

(note that I intentionally omitted the "err" return argument name to suggest that the elements either side of the semicolon may be treated differently), but that is starting to look like an unsightly mess.

Also, maybe the return needs parentheses to "escape" the semicolon:
return (err != nil ; arg1, arg2, arg3, fmt.Errorf("failure: %v", err))

or

return (arg1, arg2, arg3, fmt.Errorf("failure: %v", err); err != nil)

I leave that to the referees.

Well, one needs to turn every stone...

@ianlancetaylor ianlancetaylor added v2 A language change or incompatible library change LanguageChange labels Jun 30, 2019
@andreynering
Copy link
Author

andreynering commented Jun 30, 2019

if err != nil return err

Why would your proposal be better than this example?

@henvic I never saw a language that allow that. IMO that's harder to read, since you don't have a keyword to separate the if statement to the rest of the code. Yes, you have return, but the inline if could be used for anything, so this would look strange:

if value < 0 value = 0

Having the if after ensure you have the if to separate the code from the check. Ruby also does that.

Would it also allow single-line case, for, else, var () ? I'd like them all, please ;-)

See also #32611 which suggests: on err, <single_statement>

@networkimprov I disagree with supporting inline else and for, I think that would hurt too much the readability.

About inline case or var, I'm not even sure what you mean here.

Well, the obvious sequitur seems to me only a little stretch to Go syntax; not beautiful, but that is in the eye of the beholder:

return err != nil; arg1, arg2, arg3, fmt.Errorf("failed: %v", err)

[...]

@lootch This falls in the same problem other proposals do, it more complicates then simplify things. That statement is less readable than a plain if statement, and I'm sure it could be confusing to new Gophers.

Inline ifs are just code, as we've already been writing, but inline to save a few lines of code.

@mvdan
Copy link
Member

mvdan commented Jun 30, 2019

If you really wanted to collapse error checks into a single line, a much smaller change would be for gofmt to allow:

if err != nil { return err }

That opens its own list of questions, such as "what about for". But at least it doesn't introduce new syntax.

@networkimprov
Copy link

@mvdan, ppl keep repeating that, but it's not actually trivial.

if err == io.EOF { continue }     // if this
else if err != nil { return err } // why not this?

if t { x() } else { y() } // or this?
v := a; if t { v = b }    // or that?

And would the new go fmt rewrite much existing error code?

@mvdan
Copy link
Member

mvdan commented Jun 30, 2019

I'm not saying it's a good idea. I'm saying that introducing new syntax generally has a higher cost.

@networkimprov
Copy link

Generally. But in this case, probably not.

@alanfo
Copy link

alanfo commented Jun 30, 2019

I've always been an advocate of go fmt allowing if statements which contain a single statement to be written on one line. These are very common and are often written like this in many other languages.

However, I'd leave it at that as doing more would open up a can of worms. In particular, I wouldn't allow it if there was an else clause and I wouldn't allow it for for statements either.

@andreynering
Copy link
Author

However, I'd leave it at that as doing more would open up a can of worms. In particular, I wouldn't allow it if there was an else clause and I wouldn't allow it for for statements either.

I think that's the point: by having the inline if we could keep the current behavior of gofmt, disallowing single line for else and for and other usage like that.

To be honest, I'm fine with our current state, and have upvoted #32825 to reflect that. 🙂

But if we're going to make a change, I think it may be worth paying the cost to have a better and flexible syntax. A lot of other proposals also requires deeper changes, but this one is simpler.

@bcmills
Copy link
Contributor

bcmills commented Jul 1, 2019

I think this proposal would make it too easy to bury important side-effects.

	x = thingThatNeedsToHappenForSideEffect() if false  // Easy not to notice the 'if`'.
	x = y if thingThatModifiesY()  // Does x use the value of y from before the mutation, or after?

@andreynering
Copy link
Author

andreynering commented Jul 1, 2019

@bcmills I agree that code like that shouldn't be written.

But I think that while we should in general reduce the possibility of bad code, we can't 100% guarantee that the user won't shoot their own foot.

I mean: functions with side effects are almost always bad and should be avoided. The inline if itself don't encourage code like that.

@ianlancetaylor
Copy link
Contributor

In my opinion

return fmt.Errorf("my error: %v", err) if err != nil

is harder to read, because it buries the important part. When skimming through the code, it looks like a return statement, so you think "wait, the function returns now? What is all this other code after the return?" Then you realize that this is actually a backward if statement.

I don't understand the benefit that is worth the cost. The "pros" listed above are all about how try is bad. But this proposal is not about try. If we don't adopt the try proposal, this proposal still has to stand on its own merits. And right now I don't see the benefit.

@andreynering
Copy link
Author

@ianlancetaylor

I don't understand the benefit that is worth the cost. The "pros" listed above are all about how try is bad. But this proposal is not about try. If we don't adopt the try proposal, this proposal still has to stand on its own merits. And right now I don't see the benefit.

To be honest, this is indeed more a counter-proposal to try than something I'd propose if the try discussion wasn't a thing. I actually agree with you that having blocks for ifs makes it way easier to read. I wouldn't be disappointed if this proposal get rejected, I'm just doing some brainstorming here. 😉

That said, the only real benefit here is reducing lines of code and boilerplate. (Actually, that's also the only benefit of try).

In my opinion

return fmt.Errorf("my error: %v", err) if err != nil

is harder to read, because it buries the important part. When skimming through the code, it looks like a return statement, so you think "wait, the function returns now? What is all this other code after the return?" Then you realize that this is actually a backward if statement.

Well, I think this would be specially surprising for people that are not aware of this syntax, but perhaps people would get used to it. On the Ruby world, most people doesn't complain about it because they're used to see code like this.

func WriteToFile() error {
	file, err := os.Open("file.txt")
	return errors.Wrap(err, "couldn't open file") if err != nil
	defer file.Close()

	_, err = file.Write([]byte("Writing something to this file"))
	return errors.Wrap(err, "couldn't write to file.txt") if err != nil

	return nil
}

While reading the third line of code above, you kinda know the return should have an if because you see more code below in the same function. That code would be dead code if the return was a simple return, and Go wouldn't even allow that since dead code triggers a compilation error.

That's how I read Ruby code with early returns: if I see code in the method after the return statement, then I know it might be a conditional return.

@mvndaai
Copy link

mvndaai commented Jul 18, 2019

I think the problem with this proposal is that you lose the visual cue that something special is happening. Look at a current if in go:

if foo {
    return bar
}

Since everything in the if block is indented, it can be ignored when trying to understand the regular flow of the function. See Mat Ryer's presentation on why he doesn't use else.

@donaldnevermore
Copy link

donaldnevermore commented Jul 20, 2019

I think breaking the convention of the if statement is an overhead because your logic will be not from top to bottom and left to right.

You don't know the if condition at first sight. You will be surprised when you finish reading the whole statement.

@ianlancetaylor
Copy link
Contributor

As noted above, the benefits of this proposal seem to be: try is bad. But we aren't implementing try, so this proposal does not seem to bring enough benefits for the cost.

Making this orthogonal would mean that every statement can have an optional condition, which is a poor fit for the language as it exists today. As @bcmills said above, it is easy to bury important side-effects.

-- for @golang/proposal-review

@golang golang locked and limited conversation to collaborators Jul 29, 2020
@seankhliao seankhliao added the error-handling Language & library change proposals that are about error handling. label May 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge LanguageChange Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests