new(function(){})// Valid JavaScriptnew(()=>{})// Runtime error.

Both tests should be removed, in my opinion.

Are there no tests that include both fat arrows and parameter splats? I'd expect that to fail, since splats are implemented witharguments, which is not available in fat arrows.

a= (args...)=> args

Are there no tests that include both fat arrows and parameter splats? I'd expect that to fail, since splats are implemented with arguments, which is not available in fat arrows.

There's probably a whole debate to be had about whether we reallywant to output ES2015 arrow functions. I guess we do, but perhaps we should add special handling forarguments? Perhaps not since it's very 'magical'. But then users need to remember that there's a concrete difference between-> and=> beyond the value ofthis.

Just another thought (forgive me if this is already handled in the PR): would we want to special-case single-JS-statement fat arrow functions and generate shorthand methods in that case?

a==>console.log'hello'# var a = () => console.log('hello')a==>console.log'hello' ;console.log'world'# var a = () => {#   console.log('hello')#   console.log('world')# }

@connecarguments is not a problem. Here’s a lazy solution:

a=(..._arguments)=>{  ...}

OK, I guess I was just wondering aloud whether or not we should continue to deal witharguments in fat arrow functions or whether we should adopt ES6 behaviour (arguments is undefined).

E.g. what should this CS produce:

()=>arguments

Currently it compiles to:

function(){returnarguments;}

With this change:

()=>{returnarguments;}

Ultimately, would we want to catch calls toarguments in bound functions and set them up to instead output:

(..._arguments)=>{return_arguments;}

I think it makes sense to simply disallowarguments in fat arrow functions.

I vote no for outputting es6 fat arrows. Original CS fat arrows are objectively better, and there is no advantage to having es6 arrows in the compiled code.

Can everyone please stop trying to add es6 things to the compiled output just because they're new?

@bjmiller I would be very interesting to hear what makes CS fat arrows better! :)

Re the one-line syntax for arrow functions, I think we shouldn't bother. We don't outputif foo then bar() ora = { b: 1 } as one-liners.

Rearguments, one of the goals of the 2.x effort is to align more closely with ECMAScript. Part of that involves no more sugar to “correct” ECMAScript’s oversights as we may perceive them, primarily because ES is evolving so fast and they might fix said oversight themselves soon enough and we don't want our polyfill to behave differently than their new syntax. In the case ofarguments specifically, we should just follow the spec: if they left it out, presumably they have their reasons, and it's no more difficult in CoffeeScript to need to know that arrow functions lackarguments than it is to know the same in JavaScript.

My intuition of CoffeeScript functions is that-> has a freethis but=> has a boundthis. That's it. It's kind of weird forarguments to depend on the whether this was bound or not. If you are going to banarguments then ban it from both or neither.

I wouldn't mind "banning"arguments altogether – in fact, I don't think I'veever used in in CoffeeScript, since we have splat parameters.

Ipersonally would not mourn the loss ofarguments, however removing it has a high potential to break existing code. ¯_(ツ)_/¯

My view is that it's desirable for-> and=> in CoffeeScript to behave as identically as possible, and not to introduce too many surprises or gotchas. The current (ES3) implementation performs perfectly in this regard. It bindsthis and otherwise works exactly the same as->.

In some ways, ES6 arrows are worse. You can'tyield inside an ES6 arrow, so CoffeeScript would have to fall back to the current implementation to continue to support that combination of features.

So I agree with@bjmiller there's really nothing wrong with the way things currently work, and it has the added benefit of being compatible with ES3 and ES5 as well.

If you want to create a bound generator function after this change, you would do so the same way in CoffeeScript as you would in ECMAScript: just declareself = this in the enclosing function, like the old days. This doesn’t strike me as terrible. Likewise, having to write(args...) => instead of referencing the magicarguments object also doesn’t strike me as terrible.

What’s “wrong” with current arrow functions is that they unnecessarily deviate from the ES2015 spec. Now that JavaScript has arrow functions, we should output arrow functions. Anything else is counterintuitive.

I was confused in my previous comments.arguments is notdisallowed in ES arrow functions. They refer to the outsidearguments (just likethis).

$ ./bin/coffee -e 'console.log(((a...) -> a) 1, 2, 3)'[ 1, 2, 3 ]$ ./bin/coffee -e 'console.log(((a...) => a) 1, 2, 3)'[]

The second example returns an empty array, because of thearguments from CoffeeScript's IIFE wrapper.

I'm surprised that there were no tests for this! (Both examples above should of course log the same thing:[1, 2, 3]).

To complete@lydell’s example, here’s the ES2015 equivalent:

console.log(((...a)=>a)(1,2,3))// [1, 2, 3]

So thereis a way to access a limitless number of arguments in an arrow function, so long as we make sure to compile... into ES2015... in any parameters (e.g.args... to...args).See also.

That just leaves the generator issue. Is it really so common to make a bound function generator? Isself = this in the outer scope not good enough for what I presume to be an uncommon use case?

Okay@lydell, I did some work on splats/rest parameters. I could do a lot more to eliminate now-unnecessary code, but I just want to get this working first. Anyway, as of my latest commit your example:

a= (args...)=> args

becomes

vara;a=(...args)=>{returnargs;};

Since we’re not convertingargs... into ES5 code involvingarguments anymore, we needn’t worry aboutarguments within arrow functions. This also fixes your other example:

./bin/coffee -e'console.log(((a...) => a) 1, 2, 3)'[ 1, 2, 3 ]

An interesting disadvantage of the ES2015 rest parameter syntax is that the rest parameter must be thelast parameter. So now we have two failing tests:

  test/function_invocation.coffee:173    mult = function(x, ...mids, y) {                              ^SyntaxError: Rest parameter must be last formal parameter  test/functions.coffee:124    arrayEq([0, 1], (function(...splat, _, _1) {                                      ^SyntaxError: Rest parameter must be last formal parameter

ThatSyntaxError is thrown by Node. I guess this should just become part of our syntax as well that you can’t use... except in the last parameter?

This also raises the question of what to do about expansions in parameter lists:

./bin/coffee -bpe '(a, ..., b) => splat'(() => {  var a, b;  a = arguments[0], b = arguments[arguments.length - 1];  return splat;});

Yes, splats have to be thelast thing in ES. My first idea for handling that (rather than dropping support) was to do something like:

f= (a,b,args...,c,d)->

f=function(a,b,c,d, ...args){[c,d]=args.slice(-2);args=args.slice(0,-2);};

Also, could you mergemaster into2 and then rebase this branch on2? It’s impossible to review this PR now with so much crap in the diffs.

The diff should be sensible now.

Your example doesn’t work for me:

f=function(a,b,c,d, ...args){[c,d]=args.slice(-2);args=args.slice(0,-2);console.log(a,b,args[0],c,d);};f(1,2,3,4,5)// 1 2 undefined 5 undefined

args, at the start of the function, is just[5]. Soargs.slice(-2) returns[5], thereby settingd to undefined. We can’t useargs as a substitute forarguments.

We can, however, recreatearguments (this works for arrow functions too):

f=function(a,b,c,d, ...args){letarguments=[a,b,c,d].concat(args);[c,d]=arguments.slice(-2);args=arguments.slice(2,-2);console.log(a,b, ...args,c,d);};f(1,2,3,4,5)// 1 2 3 4 5f(1,2,3,4,5,6,7)// 1 2 3 4 5 6 7

I guess the question is whether we should. This strikes me as a lot of extra JavaScript to be outputting. I suppose we should do it to prevent breaking backward compatibility, but it feels messy. It’s also so much more difficult to figure out: theslice lines require knowing how many arguments precede and follow the splat, so that you know the indices to slice by. The[c, d] part requires knowing which arguments follow the splat. It’s just a lot of processing, prone to edge cases I can’t imagine, for a feature that I can’t imagine people use all that often.

Better version without intermediate variable:

f= (a,b,args...,c,d)=>console.log a, b, args..., c, d

becomes:

f=(...args)=>{[a,b]=args.slice(0,2);[c,d]=args.slice(-2);args=args.slice(2,-2);console.log(a,b, ...args,c,d);}

Andf(1, 2, 3, 4, 5, 6, 7) still prints1 2 3 4 5 6 7.

This is quite complicated to produce, so the question still stands: is this a feature we want to support?

Likewise with expanding parameters:

f= (a,...,b)=>

This isn’t even mentioned in the docs, though it’s in the grammar. Are we keeping this too?

Sorry about that broken compilation example. It sure was a "something like." Here's a version that I've actually tested:

varf;f=function(a,b,c,d, ...args){[c,d, ...args]=args.slice(-2).concat([c,d],args.slice(0,-2))}

Regarding that it is more complicated to compile: Yes, it is. But at the same time, that's what we've been doing here for like half a decade now.

Regarding whether the feature is useful or not: I can't really think of a case (from the top of my mind) when I've used a splat parameter somewhere else than at the end. However, Ihave used it in[first, middle..., last] = someArray.

Regardinga, ..., b: Yes, it is missing from the documentation. That's a shame. The feature is not very old (it was added in 1.7.0), and is useful for getting the last item of an array:[..., last] = someArray. That's used quite a bit in CoffeeScript's source code. When it comes to compiling, it's no different thana, args..., b, except thatargs won't be available for the user to use.

Regarding your suggested compilation:(...args) => means that we loose.length of the function. That's true for todays' compiled output too, which is a shame. Redux preserves.length. I think we should, too, when we have the chance.

Summary:

• I've never missed non-last-splat-parameter when writing ES2015, but Ihave missed it in array destructuring.
• I believe that dropping support for splats-anywhere-in-the-list will be a major backwards incompatibility hurdle for people.
• We already have lots of code for dealing with it.
• My suggested output is just one extra line of code.

Here's an example with parameter defaults:

// f = (a = 1, b = 2, args..., c = 3, d = 4) ->varf;f=function(a=1,b=2,c,d, ...args){[c=3,d=4, ...args]=args.slice(-2).concat([c,d],args.slice(0,-2))console.log(a,b,args,c,d)}

And here's an example with destructuring:

// f = ({a = 1} = {}, b, args..., c, d) ->varf;f=function({a=1}={},b,c,d, ...args){[c,{d=4}={}, ...args]=args.slice(-2).concat([c,d],args.slice(0,-2))console.log(a,b,args,c,d)}

I was only referring to function parameters. Limiting splats to the last argument in a function parameter list, the way ES does, doesn’t mean we would need to stop using splats in any position in destructuring.

Assuming splats elsewhere are untouched, are you still sure we want to support splats at any function parameter position? And expansions at any function parameter position?

I've never been sure :)

Personally, I wouldn't mind dropping splats at any position in parameter lists. And dropping expansion in parameter lists.

The only "bad" thing is that array destructuring and "parameter list destructuring" would no longer be equivalent. The following would no longer be true:

f= (<params>)-># equivalent to:f= (args...)->  [<params>]= args

Couldn't we use the same code in both cases?

Another route we could take, is to:

• Remove expansion.
• Make splats work like in ES everywhere.
• Flip the... to the ES side (...args instead ofargs... – this regularly trips me up when switching between the languages ;) )

@connec@jashkenas@lydell Not to short-circuit the discussion incoffeescript6/discuss#51, but can we merge in this PR as is? And if we ultimately decide to go the other way regardingnull, I can submit another PR. I don’t feel strongly either way on that question, but I want to get this change merged in so that the classes PR can start using it.

If merging this helps you who actually work on the 2 branch, then go for it, I'd say. The 2 branch doesn't need be be as stable as master.

So I just tried running the tests in this branch under--harmony mode, and I got a surprise. This test fails only in harmony (generators.coffee:55):

test"bound generator",->obj=bound:->do=>yieldthisunbound:->do->yieldthisnested:->do=>yielddo=>yielddo=>yieldthis

It only fails in harmony mode because we’re not runninggenerators.coffee except in harmony mode, because back when generators were added to CoffeeScript that’s the only way they could be run. Now with Node 6 the--harmony is unnecessary, and will be removed as soon as#3757 lands.

Anyway the reason this test fails is because of theyield this inside a bound function. For a simpler example, this:

f==>yieldthis

compiles (via this PR) to this:

f=()=>{return(yieldthis);};

which Node doesn’t like:

return(yieldthis);^^^^SyntaxError:Unexpectedtokenthis

becausearrow functions cannot be generators. So this is another breaking change as a result of this PR. And I need to find a way to check thatyield is inside a bound function as opposed to a regular function, and throw an error in the former case.

The workaround, ironically enough, is to just use a regular function and the oldself = this hack:

self=thisf=->yield self

Fixed. Everyone okay with this change? I suppose the alternative is to fall back to the CS 1.x output for bound functions in the event of a bound generator function, but I feel like that will cause more debugging trouble than it’s worth, by potentially introducing inconsistencies between bound functions with and withoutyield. Hopefully bound generator functions aren’t so common, and we’re only aligning ourselves with ES here by forcing people to do theself = this rigamarole for bound generator functions. I’ve added this to thebreaking changes wiki document.

@lydell, others, is there anything I’m missing here? If not, please let me know and I’ll merge this PR into2.

Updated. Now it only underlines theyield line (assuming I understand whato.scope.method refers to):

./bin/coffee -bpe 'f = =>  1  2  yield this'[stdin]:4:3: error: yield cannot occur inside bound (fat arrow) functions  yield this  ^^^^^^^^^^

@lydell any other notes?

No :)