[CS2] Destructured parameter default evaluation order is broken

[connec]

Example:

current = 0
next    = -> ++current

foo = ({ a = next() }, b = next()) -> [ a, b ]

console.log foo {}

Expected output: [ 1, 2 ]

Output on master: [ 1, 2 ]

Output on 2: [ 2, 1 ]

---

This is caused by the 2 branch compiling defaults for simple parameters into the argument list, but handling complex parameters' defaults in the function body.

Output on master (simplified)

function (arg, b) {
  var a
  a = arg.a != null ? arg.a : next();
  if (b == null) {
    b = next();
  }
  return [a, b];
}

Output on 2 (simplified)

function (arg, b = next()) {
  var a;
  a = arg.a != null ? arg.a : next();
  return [a, b];
}

---

I encountered this whilst improving super handling in #4354 (coming soon).

I guess the way we want to fix this is to compile destructured parameters and their defaults to ES also, but I wanted to create an issue to make sure we don't forget about it!

[GeoffreyBooth]

Compiling destructured parameters and their defaults to ES means losing the ability to use this in parameters. That’s why I didn’t go down that road.

Perhaps another solution would be to detect if a function call is happening within parameters, and if so, do everything the “old way” (define the parameter variables within the function body).

[connec]

@GeoffreyBooth I've actually improved that on my branch for #4354 - @ parameters are now being pulled off destructured parameters and assigned separately. This probably means there's quite a bit of destructuring code to clean up...

Current output on my branch:

({ @a, b: { @b, c: { @c } } }) ->

(function(arg) {
  var a, b, c, ref, ref1;
  a = arg.a, (ref = arg.b, b = ref.b, (ref1 = ref.c, c = ref1.c));
  this.a = a;
  this.b = b;
  this.c = c;
});

[GeoffreyBooth]

So maybe we should fix this after #4354 is merged in? Or @connec if you have time to work on it, be my guest, I think you have more time than me 😄

[connec]

Lets do it after. I might get a chance to look at it then, but I don't want to have multiple PRs ongoing.

[connec]

There's another case not covered by the PR:

i = 0
({ a = ++i }, b = ++i) ->

It generates:

// Generated by CoffeeScript 2.0.0-alpha
var i;

i = 0;

(function(arg, b = ++i) { // 'b' will be set first (b === 1)
  var a, ref;
  a = (ref = arg.a) != null ? ref : ++i; // 'a' will be set second (a === 2)
});

[GeoffreyBooth]

This makes me more inspired to rename isComplex.

[connec]

I guess what it's really trying to capture is 'may have side effects'.

[GeoffreyBooth]

In 2.0.0-beta1 @connec’s example compiles to:

var i;

i = 0;

(function({a = ++i}, b = ++i) {});

which should be in the correct order. If there are further edge cases to fix, please open a new issue.