Fix #4843: bad output when assigning to @prop in destructuring assignment with defaults

[zdenko]

Fixes #4843.

works = "maybe"
drinks = "beer"
class A
  constructor: ({@works = 'no', @drinks = 'wine'}) ->

a = new A {works: 'yes', drinks: 'coffee'}

Ouput

var A, a, drinks, works;

works = maybe;
drinks = beer;

A = class A {
  constructor({works: works1 = 'no', drinks: drinks1 = 'wine'}) {
    this.works = works1;
    this.drinks = drinks1;
  }

};

a = new A({
  works: 'yes',
  drinks: 'coffee'
});

[vendethiel]

why?

[zdenko]

In case of object destructuring parameter with the default value, e.g. ({@works = "no", @drinks}) ->, when replacement function is called from @renameParam, the parent is Assign in case of @works = "no", and Obj in case of @drinks.
Because of this @works = "no" is changed to works1 = "no" and @drinks to drinks:drinks1.

The best way, I could find, to solve this, was to pass Assign ({works:works1}) to @renameParam instead of Identifier (works1).
But, because Param::eachName passes obj.variable (@works in this case) to the iterator, I had to find a way to detect if param has a default value set in order to create a correct replacement. Passing original obj seems like a way to go.
Another way would be passing index and use it to check the argument in the Code::eachParamName (e.g. param.name.parameters[i]).

[vendethiel]

Ok, now I get it. I don't like that atParam magically references a variable from the for loop, tbh. Can you please make it explicit?

[zdenko]

You mean to pass it as an argument?

atParam = (obj, originalObject = no) => iterator "@#{obj.properties[0].name.value}", obj, @, originalObject

....

else if obj.this
  atParam obj, nObj

[vendethiel]

Yes.

[zdenko]

While working on this PR I noticed that object destructuring variables in parameters are always replaced, even if the variable isn't reserved in the scope.

class A
  constructor: ({@works}) ->

###
Output
var A;

A = class A {
  constructor({
      works: works
    }) {
    this.works = works;
  }
};
###

I don't think this is required, so I've changed the logic and now variables are replaced only if needed.

class A
  constructor: ({@works}) ->

###
A = class A {
  constructor({works}) {
    this.works = works;
  }
};
###

foo = 1
class B
  constructor: ({@foo}) ->

###
foo = 1;
B = class B {
  constructor({foo:foo1}) {
    this.foo = foo1;
  }
};
###

[Inve1951]

gj, thx @zdenko 👍