Skip to content

Commit

Permalink
Fix #4843: bad output when assigning to @prop in destructuring assign…
Browse files Browse the repository at this point in the history
…ment with defaults (#4848)

* fix #4843

* improvements

* typo

* small fix
  • Loading branch information
zdenko authored and GeoffreyBooth committed Jan 16, 2018
1 parent c1283ea commit c8c8c16
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 17 deletions.
38 changes: 27 additions & 11 deletions lib/coffeescript/nodes.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 25 additions & 6 deletions src/nodes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2605,14 +2605,24 @@ exports.Code = class Code extends Base

# Check for duplicate parameters and separate `this` assignments.
paramNames = []
@eachParamName (name, node, param) ->
@eachParamName (name, node, param, obj) ->
node.error "multiple parameters named '#{name}'" if name in paramNames
paramNames.push name

if node.this
name = node.properties[0].name.value
name = "_#{name}" if name in JS_FORBIDDEN
target = new IdentifierLiteral o.scope.freeVariable name
param.renameParam node, target
target = new IdentifierLiteral o.scope.freeVariable name, reserve: no
# `Param` is object destructuring with a default value: ({@prop = 1}) ->
# In a case when the variable name is already reserved, we have to assign
# a new variable name to the destructured variable: ({prop:prop1 = 1}) ->
replacement =
if param.name instanceof Obj and obj instanceof Assign and
obj.operatorToken.value is '='
new Assign (new IdentifierLiteral name), target, 'object' #, operatorToken: new Literal ':'
else
target
param.renameParam node, replacement
thisAssignments.push new Assign node, target

# Parse the parameters, adding them to the list of parameters to put in the
Expand Down Expand Up @@ -2901,12 +2911,14 @@ exports.Param = class Param extends Base
# `name` is the name of the parameter and `node` is the AST node corresponding
# to that name.
eachName: (iterator, name = @name) ->
atParam = (obj) => iterator "@#{obj.properties[0].name.value}", obj, @
atParam = (obj, originalObj = null) => iterator "@#{obj.properties[0].name.value}", obj, @, originalObj
# * simple literals `foo`
return iterator name.value, name, @ if name instanceof Literal
# * at-params `@foo`
return atParam name if name instanceof Value
for obj in name.objects ? []
# Save original obj.
nObj = obj
# * destructured parameter with default value
if obj instanceof Assign and not obj.context?
obj = obj.variable
Expand All @@ -2928,7 +2940,7 @@ exports.Param = class Param extends Base
@eachName iterator, obj.base
# * at-params within destructured parameters `{@foo}`
else if obj.this
atParam obj
atParam obj, nObj
# * simple destructured parameters {foo}
else iterator obj.base.value, obj.base, @
else if obj instanceof Elision
Expand All @@ -2945,7 +2957,14 @@ exports.Param = class Param extends Base
if parent instanceof Obj
key = node
key = node.properties[0].name if node.this
new Assign new Value(key), newNode, 'object'
# No need to assign a new variable for the destructured variable if the variable isn't reserved.
# Examples:
# `({@foo}) ->` should compile to `({foo}) { this.foo = foo}`
# `foo = 1; ({@foo}) ->` should compile to `foo = 1; ({foo:foo1}) { this.foo = foo1 }`
if node.this and key.value is newNode.value
new Value newNode
else
new Assign new Value(key), newNode, 'object'
else
newNode

Expand Down
9 changes: 9 additions & 0 deletions test/function_invocation.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,15 @@ test "Simple Destructuring function arguments with same-named variables in scope
eq f([2]), 2
eq x, 1

test "#4843: Bad output when assigning to @prop in destructuring assignment with defaults", ->
works = "maybe"
drinks = "beer"
class A
constructor: ({@works = 'no', @drinks = 'wine'}) ->
a = new A {works: 'yes', drinks: 'coffee'}
eq a.works, 'yes'
eq a.drinks, 'coffee'

test "caching base value", ->

obj =
Expand Down

0 comments on commit c8c8c16

Please sign in to comment.