Fix #5013: return statement as an expression

[zdenko]

Fixes #5013 and return statement as an expression in general.

-> (return)
# (function() {});

-> (yield return)
# (function*() {});

-> (await return)
# (async function() {});

[GeoffreyBooth]

Why do we test these against generated JS?

Why is there no return in the JS?

[vendethiel]

yield return, await return are ways to mark a function as a generator/async without needing to use yield or await in the body.

[GeoffreyBooth]

Yes but it seems incorrect for there to be yield, await and return in the CoffeeScript and for none of those to appear in the generated JavaScript. They’re not CS-only keywords, they’re basically passthroughs. When else do we swallow a return in the CoffeeScript and not output it in the JavaScript?

I would think that -> (yield return) should generate:

(function*() {
  (yield return);
});

which is invalid JavaScript; but that’s okay. It’s what the user typed.

[vendethiel]

We have been slicing Returns off the end of Blocks since the very beginning(™).

[jashkenas]

I'm a little confused by this one. return is a statement. Why would we want to allow it as a part of an expression?

(return) doesn't make sense, because it returns from the function and never provides a value that can be used by the parenthesis. It's the same reason why we don't have return + 5, or call(return 3).

[zdenko]

It is confusing. Other statements in the expression are allowed as of #4849.

So, since (a(); break) for a in b, (break) for a in b and (break) compile, should we allow
(a(); return) if true, (return) if true and (return) as well?
Or, perhaps just the first case as others don't make much sense. And even the example from #5013 could be written as

->
  if true
    b = 1
    return
  b = 2

@GeoffreyBooth both function*() {} and async function() {} are valid ES syntax.

[jashkenas]

• (break) should not compile.
• (break) for a in b should not compile.
• (a(); break) for a in b should not compile.

The right way to write the latter, is:

for a in b
  a()
  break

Or:

for a in b then a(); break

Let's not loosen the rules to allow people to write clever, but syntactically nonsensical code. Let's keep things tidy.

[zdenko]

@jashkenas so #4849 should be reverted then? This was labeled as a bug since 2014.
In other words, statements in the expression are not allowed.

[jashkenas]

I would confirm with @lydell and @GeoffreyBooth first, but my opinion is yes: The whole point of statements is that they represent constructs that can't meaningfully be used as a part of a larger expression.

In CoffeeScript, we want as much as possible to be an expression (including things that aren't in JavaScript, like variable initialization). But some things just can't be expressions, and break and return are two of those things.

[GeoffreyBooth]

@GeoffreyBooth both function*() {} and async function() {} are valid ES syntax.

Yes, but my point was that the original CoffeeScript has a return in it (and/or a break or await) and those aren’t present in the output. That feels wrong to me.

I see no reason to disagree with @jashkenas here. #4849 was only merged in a few weeks ago, we can revert it. That would basically be fixing a regression, in that #4849 more or less introduced a bug in that statements are suddenly allowed where they shouldn’t be. #3441, the bug from 2014, would remain closed as per @jashkenas’ comment here it shouldn’t have compiled in the first place. At least now we can explain why.

[zdenko]

@GeoffreyBooth I'll make changes in this PR to revert to the previous state.
It's just a few lines of change. Plus, I'll add a couple of tests for statement compilation error message.

[vendethiel]

I don't really mind how it ends up anyway, I just feel like pointing one thing out:

I would confirm with @lydell and @GeoffreyBooth first, but my opinion is yes: The whole point of statements is that they represent constructs that can't meaningfully be used as a part of a larger expression.

This is less, imho, about statements vs expressions, that about simulating a JS block using parentheses and the semicolon operator, which is somewhat our alternative to JS' comma operator. The distinction between expression and statement is still present.

[jashkenas]

This is less, imho, about statements vs expressions, that about simulating a JS block using parentheses and the semicolon operator, which is somewhat our alternative to JS' comma operator.

I see that point of view, but it still feels like something that we shouldn't support and encourage. Instead of tricks using parentheses to jam a bunch of stuff on a single line — just write it the other way.

[rosenfeld]

I'm used to this syntax in Ruby since it's the only way to write early returns in a single line in Ruby. I use this pattern very often both in Ruby and CoffeeScript 1. For example (in Ruby):

(render 'error'; return) if error?
common_path

At least in Ruby, when discussing such early results statements once in a ticket Matz told that was the recommended approach for those cases, so he doesn't seem to find it weird. By the way, I use the same pattern in a case statement block as well (with break).