kotovalexarian-likes-github/jashkenas--coffeescript
1
0
Fork 0
mirror of https://github.com/jashkenas/coffeescript.git synced 2022-11-09 12:23:24 -05:00
Code Releases Activity

[CS2] Fix destructuring bugs #4673 and #4657 (#4683)

Browse source 
* destructuring fixes [Fixes #4673] [Fixes #4657]

* test for destructured @prop

* Add another test to cover #4657 cases

* don't declare actual params
This commit is contained in:
Julian Rosse 2017-09-07 13:06:35 -04:00 • committed by Geoffrey Booth
parent 63d3b699d7
commit 1b8f1af287
4 changed files with 83 additions and 78 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

72
lib/coffeescript/nodes.js
View file

@ -2109,15 +2109,15 @@
// Check if object contains splat.
hasSplat() {
var j, len1, prop, ref1, splat;
var j, len1, prop, ref1;
ref1 = this.properties;
for (j = 0, len1 = ref1.length; j < len1; j++) {
prop = ref1[j];
if (prop instanceof Splat) {
splat = true;
return true;
}
}
return splat != null ? splat : false;
return false;
}
compileNode(o) {
@ -2240,7 +2240,7 @@
}
// Object spread properties. https://github.com/tc39/proposal-object-rest-spread/blob/master/Spread.md
// `obj2 = {a: 1, obj..., c: 3, d: 4}` → `obj2 = Object.assign({}, {a: 1}, obj, {c: 3, d: 4})`
// `obj2 = {a: 1, obj..., c: 3, d: 4}` → `obj2 = _extends({}, {a: 1}, obj, {c: 3, d: 4})`
compileSpread(o) {
var _extends, addSlice, j, len1, prop, propSlices, props, slices, splatSlice;
props = this.properties;
@ -3200,6 +3200,8 @@
this.checkAssignability(o, name);
if (this.moduleDeclaration) {
return o.scope.add(name.value, this.moduleDeclaration);
} else if (this.param) {
return o.scope.add(name.value, this.param === 'alwaysDeclare' ? 'var' : 'param');
} else {
return o.scope.find(name.value);
}
@ -3230,7 +3232,7 @@
answer = compiledName.concat(this.makeCode(` ${this.context || '='} `), val);
// Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Assignment_without_declaration,
// if we’re destructuring without declaring, the destructuring assignment must be wrapped in parentheses.
if (o.level > LEVEL_LIST || (o.level === LEVEL_TOP && isValue && this.variable.base instanceof Obj && !this.nestedLhs && !this.param)) {
if (o.level > LEVEL_LIST || o.level === LEVEL_TOP && isValue && this.variable.base instanceof Obj && !this.nestedLhs && !(this.param === true)) {
return this.wrapInParentheses(answer);
} else {
return answer;
@ -3240,31 +3242,7 @@
// Check object destructuring variable for rest elements;
// can be removed once ES proposal hits Stage 4.
compileObjectDestruct(o) {
var fragments, getPropKey, getPropName, j, len1, restElement, restElements, result, setScopeVar, traverseRest, value, valueRef, valueRefTemp;
// Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Assignment_without_declaration,
// if we’re destructuring without declaring, the destructuring assignment
// must be wrapped in parentheses: `({a, b} = obj)`. Helper function
// `setScopeVar()` declares variables `a` and `b` at the top of the
// current scope.
setScopeVar = function(prop) {
var newVar;
newVar = false;
if (prop instanceof Assign && prop.value.base instanceof Obj) {
return;
}
if (prop instanceof Assign) {
if (prop.value.base instanceof IdentifierLiteral) {
newVar = prop.value.base.compileWithoutComments(o);
} else {
newVar = prop.variable.base.compileWithoutComments(o);
}
} else {
newVar = prop.compileWithoutComments(o);
}
if (newVar) {
return o.scope.add(newVar, 'var', true);
}
};
var fragments, getPropKey, getPropName, j, len1, restElement, restElements, result, traverseRest, value, valueRef, valueRefTemp;
// Returns a safe (cached) reference to the key for a given property
getPropKey = function(prop) {
var key;
@ -3300,7 +3278,6 @@
for (index = j = 0, len1 = properties.length; j < len1; index = ++j) {
prop = properties[index];
nestedSourceDefault = nestedSource = nestedProperties = null;
setScopeVar(prop.unwrap());
if (prop instanceof Assign) {
if (typeof (base1 = prop.value).isObject === "function" ? base1.isObject() : void 0) {
if (prop.context !== 'object') {
@ -3350,13 +3327,9 @@
return restElements;
};
// Cache the value for reuse with rest elements.
if (this.value.shouldCache()) {
valueRefTemp = new IdentifierLiteral(o.scope.freeVariable('ref', {
reserve: false
}));
} else {
valueRefTemp = this.value.base;
}
valueRefTemp = this.value.shouldCache() ? new IdentifierLiteral(o.scope.freeVariable('ref', {
reserve: false
})) : this.value.base;
// Find all rest elements.
restElements = traverseRest(this.variable.base.properties, valueRefTemp);
if (!(restElements && restElements.length > 0)) {
@ -3367,7 +3340,9 @@
for (j = 0, len1 = restElements.length; j < len1; j++) {
restElement = restElements[j];
value = new Call(new Value(new Literal(utility('objectWithoutKeys', o))), [restElement.source, restElement.excludeProps]);
result.push(new Assign(restElement.name, value));
result.push(new Assign(new Value(restElement.name), value, null, {
param: this.param ? 'alwaysDeclare' : null
}));
}
fragments = result.compileToFragments(o);
if (o.level === LEVEL_TOP) {
@ -3802,7 +3777,9 @@
ifTrue = new Assign(new Value(param.name), param.value);
exprs.push(new If(condition, ifTrue));
} else {
exprs.push(new Assign(new Value(param.name), param.asReference(o)));
exprs.push(new Assign(new Value(param.name), param.asReference(o), null, {
param: 'alwaysDeclare'
}));
}
}
// If this parameter comes before the splat or expansion, it will go
@ -3827,22 +3804,25 @@
if (param.name instanceof Arr || param.name instanceof Obj) {
// This parameter is destructured.
param.name.lhs = true;
param.name.eachName(function(prop) {
return o.scope.parameter(prop.value);
});
// Compile `foo({a, b...}) ->` to `foo(arg) -> {a, b...} = arg`.
// Can be removed once ES proposal hits Stage 4.
if (param.name instanceof Obj && param.name.hasSplat()) {
splatParamName = o.scope.freeVariable('arg');
o.scope.parameter(splatParamName);
ref = new Value(new IdentifierLiteral(splatParamName));
exprs.push(new Assign(new Value(param.name), ref));
exprs.push(new Assign(new Value(param.name), ref, null, {
param: 'alwaysDeclare'
}));
// Compile `foo({a, b...} = {}) ->` to `foo(arg = {}) -> {a, b...} = arg`.
if ((param.value != null) && !param.assignedInBody) {
ref = new Assign(ref, param.value, null, {
param: true
});
}
} else if (!param.shouldCache()) {
param.name.eachName(function(prop) {
return o.scope.parameter(prop.value);
});
}
} else {
// This compilation of the parameter is only to get its name to add
@ -4077,12 +4057,12 @@
// as well as be a splat, gathering up a group of parameters into an array.
exports.Param = Param = (function() {
class Param extends Base {
constructor(name1, value1, splat1) {
constructor(name1, value1, splat) {
var message, token;
super();
this.name = name1;
this.value = value1;
this.splat = splat1;
this.splat = splat;
message = isUnassignable(this.name.unwrapAll().value);
if (message) {
this.name.error(message);

54
src/nodes.coffee
View file

@ -1427,8 +1427,8 @@ exports.Obj = class Obj extends Base
# Check if object contains splat.
hasSplat: ->
splat = yes for prop in @properties when prop instanceof Splat
splat ? no
return yes for prop in @properties when prop instanceof Splat
no
compileNode: (o) ->
props = @properties
@ -1510,7 +1510,7 @@ exports.Obj = class Obj extends Base
prop.eachName iterator if prop.eachName?
# Object spread properties. https://github.com/tc39/proposal-object-rest-spread/blob/master/Spread.md
# `obj2 = {a: 1, obj..., c: 3, d: 4}` → `obj2 = Object.assign({}, {a: 1}, obj, {c: 3, d: 4})`
# `obj2 = {a: 1, obj..., c: 3, d: 4}` → `obj2 = _extends({}, {a: 1}, obj, {c: 3, d: 4})`
compileSpread: (o) ->
props = @properties
# Store object spreads.
@ -2144,6 +2144,12 @@ exports.Assign = class Assign extends Base
@checkAssignability o, name
if @moduleDeclaration
o.scope.add name.value, @moduleDeclaration
else if @param
o.scope.add name.value,
if @param is 'alwaysDeclare'
'var'
else
'param'
else
o.scope.find name.value
@ -2167,7 +2173,7 @@ exports.Assign = class Assign extends Base
answer = compiledName.concat @makeCode(" #{ @context or '=' } "), val
# Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Assignment_without_declaration,
# if we’re destructuring without declaring, the destructuring assignment must be wrapped in parentheses.
if o.level > LEVEL_LIST or (o.level is LEVEL_TOP and isValue and @variable.base instanceof Obj and not @nestedLhs and not @param)
if o.level > LEVEL_LIST or o.level is LEVEL_TOP and isValue and @variable.base instanceof Obj and not @nestedLhs and not (@param is yes)
@wrapInParentheses answer
else
answer
@ -2175,23 +2181,6 @@ exports.Assign = class Assign extends Base
# Check object destructuring variable for rest elements;
# can be removed once ES proposal hits Stage 4.
compileObjectDestruct: (o) ->
# Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Assignment_without_declaration,
# if we’re destructuring without declaring, the destructuring assignment
# must be wrapped in parentheses: `({a, b} = obj)`. Helper function
# `setScopeVar()` declares variables `a` and `b` at the top of the
# current scope.
setScopeVar = (prop) ->
newVar = false
return if prop instanceof Assign and prop.value.base instanceof Obj
if prop instanceof Assign
if prop.value.base instanceof IdentifierLiteral
newVar = prop.value.base.compileWithoutComments o
else
newVar = prop.variable.base.compileWithoutComments o
else
newVar = prop.compileWithoutComments o
o.scope.add(newVar, 'var', true) if newVar
# Returns a safe (cached) reference to the key for a given property
getPropKey = (prop) ->
if prop instanceof Assign
@ -2220,7 +2209,6 @@ exports.Assign = class Assign extends Base
for prop, index in properties
nestedSourceDefault = nestedSource = nestedProperties = null
setScopeVar prop.unwrap()
if prop instanceof Assign
# prop is `k: expr`, we need to check `expr` for nested splats
if prop.value.isObject?()
@ -2252,10 +2240,11 @@ exports.Assign = class Assign extends Base
restElements
# Cache the value for reuse with rest elements.
if @value.shouldCache()
valueRefTemp = new IdentifierLiteral o.scope.freeVariable 'ref', reserve: false
else
valueRefTemp = @value.base
valueRefTemp =
if @value.shouldCache()
new IdentifierLiteral o.scope.freeVariable 'ref', reserve: false
else
@value.base
# Find all rest elements.
restElements = traverseRest @variable.base.properties, valueRefTemp
@ -2266,7 +2255,7 @@ exports.Assign = class Assign extends Base
for restElement in restElements
value = new Call new Value(new Literal utility 'objectWithoutKeys', o), [restElement.source, restElement.excludeProps]
result.push new Assign restElement.name, value
result.push new Assign new Value(restElement.name), value, null, param: if @param then 'alwaysDeclare' else null
fragments = result.compileToFragments o
if o.level is LEVEL_TOP
@ -2600,7 +2589,7 @@ exports.Code = class Code extends Base
ifTrue = new Assign new Value(param.name), param.value
exprs.push new If condition, ifTrue
else
exprs.push new Assign new Value(param.name), param.asReference(o)
exprs.push new Assign new Value(param.name), param.asReference(o), null, param: 'alwaysDeclare'
# If this parameter comes before the splat or expansion, it will go
# in the function definition parameter list.
@ -2620,18 +2609,19 @@ exports.Code = class Code extends Base
if param.name instanceof Arr or param.name instanceof Obj
# This parameter is destructured.
param.name.lhs = yes
param.name.eachName (prop) ->
o.scope.parameter prop.value
# Compile `foo({a, b...}) ->` to `foo(arg) -> {a, b...} = arg`.
# Can be removed once ES proposal hits Stage 4.
if param.name instanceof Obj and param.name.hasSplat()
splatParamName = o.scope.freeVariable 'arg'
o.scope.parameter splatParamName
ref = new Value new IdentifierLiteral splatParamName
exprs.push new Assign new Value(param.name), ref
exprs.push new Assign new Value(param.name), ref, null, param: 'alwaysDeclare'
# Compile `foo({a, b...} = {}) ->` to `foo(arg = {}) -> {a, b...} = arg`.
if param.value? and not param.assignedInBody
if param.value? and not param.assignedInBody
ref = new Assign ref, param.value, null, param: yes
else unless param.shouldCache()
param.name.eachName (prop) ->
o.scope.parameter prop.value
else
# This compilation of the parameter is only to get its name to add
# to the scope name tracking; since the compilation output here

12
test/assignment.coffee
View file

@ -915,3 +915,15 @@ test "#4674: _extends utility for object spreads 2", ->
e = {a..., c...}
eq e.b, 1
eq e.d, 2
test "#4673: complex destructured object spread variables", ->
b = c: 1
{{a...}...} = b
eq a.c, 1
d = {}
{d.e...} = f: 1
eq d.e.f, 1
{{g}...} = g: 1
eq g, 1

23
test/functions.coffee
View file

@ -549,3 +549,26 @@ test "#4413: expressions in function parameters that create generated variables
g = (a = foo() ? bar()) -> a + 1
eq f(), 33
eq g(), 34
test "#4673: complex destructured object spread variables", ->
f = ({{a...}...}) ->
a
eq f(c: 1).c, 1
g = ({@y...}) ->
eq @y.b, 1
g b: 1
test "#4657: destructured array param declarations", ->
a = 1
b = 2
f = ([a..., b]) ->
f [3, 4, 5]
eq a, 1
eq b, 2
test "#4657: destructured array parameters", ->
f = ([a..., b]) -> {a, b}
result = f [1, 2, 3, 4]
arrayEq result.a, [1, 2, 3]
eq result.b, 4