From 43a8b462037bcec57f1d9ddd1c83e48726cc74d1 Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Thu, 1 Sep 2011 14:47:10 -0400 Subject: [PATCH 1/8] fixes #1643: splatted accesses in destructuring assignments no longer create obj.key var declarations --- lib/coffee-script/nodes.js | 14 ++++++++------ src/nodes.coffee | 8 +++++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js index 7b7ff2cf..207d476c 100644 --- a/lib/coffee-script/nodes.js +++ b/lib/coffee-script/nodes.js @@ -1063,7 +1063,7 @@ return unfoldSoak(o, this, 'variable'); }; Assign.prototype.compileNode = function(o) { - var isValue, match, name, val, _ref2, _ref3, _ref4, _ref5; + var hasProps, isValue, match, name, val, _base, _base2, _ref2, _ref3, _ref4, _ref5; if (isValue = this.variable instanceof Value) { if (this.variable.isArray() || this.variable.isObject()) { return this.compilePatternMatch(o); @@ -1074,10 +1074,11 @@ } } name = this.variable.compile(o, LEVEL_LIST); - if (!(this.context || this.variable.isAssignable())) { + if (!(this.context || this.variable.unwrapAll().isAssignable())) { throw SyntaxError("\"" + (this.variable.compile(o)) + "\" cannot be assigned."); } - if (!(this.context || isValue && (this.variable.namespaced || this.variable.hasProperties()))) { + hasProps = (isValue && (typeof (_base = this.variable.unwrapAll()).hasProperties === "function" ? _base.hasProperties() : void 0)) || (this.variable instanceof Splat && (typeof (_base2 = this.variable.name.unwrapAll()).hasProperties === "function" ? _base2.hasProperties() : void 0)); + if (!(this.context || hasProps)) { if (this.param) { o.scope.add(name, 'var'); } else { @@ -1098,7 +1099,7 @@ } }; Assign.prototype.compilePatternMatch = function(o) { - var acc, assigns, code, i, idx, isObject, ivar, name, obj, objects, olen, ref, rest, splat, top, val, value, vvar, _len, _ref2, _ref3, _ref4, _ref5, _ref6; + var acc, assigns, code, i, idx, isObject, isSoak, ivar, name, obj, objects, olen, ref, rest, soakNode, splat, top, val, value, vvar, _len, _ref2, _ref3, _ref4, _ref5, _ref6, _ref7, _ref8, _ref9; top = o.level === LEVEL_TOP; value = this.value; objects = this.variable.base.objects; @@ -1183,9 +1184,10 @@ if ((name != null) && __indexOf.call(['arguments', 'eval'].concat(RESERVED), name) >= 0) { throw new SyntaxError("assignment to a reserved word: " + (obj.compile(o)) + " = " + (val.compile(o))); } - assigns.push(new Assign(obj, val, null, { + isSoak = (soakNode = obj.name || obj) && ((_ref7 = soakNode.base) != null ? _ref7.soak : void 0) || ((_ref8 = soakNode.properties) != null ? (_ref9 = _ref8[0]) != null ? _ref9.soak : void 0 : void 0); + assigns.push(new Assign(obj.name || obj, val, null, { param: this.param - }).compile(o, LEVEL_TOP)); + }).compile(o, isSoak ? LEVEL_ACCESS : LEVEL_TOP)); } if (!top) assigns.push(vvar); code = assigns.join(', '); diff --git a/src/nodes.coffee b/src/nodes.coffee index 4f3f2634..3b4c7dfb 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -940,9 +940,10 @@ exports.Assign = class Assign extends Base return @compileSplice o if @variable.isSplice() return @compileConditional o if @context in ['||=', '&&=', '?='] name = @variable.compile o, LEVEL_LIST - unless @context or @variable.isAssignable() + unless @context or @variable.unwrapAll().isAssignable() throw SyntaxError "\"#{ @variable.compile o }\" cannot be assigned." - unless @context or isValue and (@variable.namespaced or @variable.hasProperties()) + hasProps = (isValue and @variable.unwrapAll().hasProperties?()) or (@variable instanceof Splat and @variable.name.unwrapAll().hasProperties?()) + unless @context or hasProps if @param o.scope.add name, 'var' else @@ -1030,7 +1031,8 @@ exports.Assign = class Assign extends Base val = new Value new Literal(vvar), [new (if acc then Access else Index) idx] if name? and name in ['arguments','eval'].concat RESERVED throw new SyntaxError "assignment to a reserved word: #{obj.compile o} = #{val.compile o}" - assigns.push new Assign(obj, val, null, param: @param).compile o, LEVEL_TOP + isSoak = (soakNode = obj.name or obj) and soakNode.base?.soak or soakNode.properties?[0]?.soak + assigns.push new Assign(obj.name or obj, val, null, param: @param).compile o, if isSoak then LEVEL_ACCESS else LEVEL_TOP assigns.push vvar unless top code = assigns.join ', ' if o.level < LEVEL_LIST then code else "(#{code})" From 8ebda7ac022a53742e41da87dd6dd0352cbd860b Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Thu, 1 Sep 2011 14:47:30 -0400 Subject: [PATCH 2/8] tests for #1643: splatted accesses in destructuring assignments no longer create obj.key var declarations --- test/assignment.coffee | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/assignment.coffee b/test/assignment.coffee index dba200e5..e3b90850 100644 --- a/test/assignment.coffee +++ b/test/assignment.coffee @@ -306,3 +306,17 @@ test "#1591, #1101: splatted expressions in destructuring assignment must be ass nonce = {} for nonref in ['', '""', '0', 'f()', '(->)'].concat CoffeeScript.RESERVED eq nonce, (try CoffeeScript.compile "[#{nonref}...] = v" catch e then nonce) + +test "#1643: splatted accesses in destructuring assignments should not be declared as variables", -> + nonce = {} + accesses = ['o.a', 'C::a', 'C::', 'o["a"]', '(o.a)', 'f().a', '(o.a).a', '@o.a', 'o?.a', 'f?().a'] + for access in accesses + code = + """ + nonce = {}; @o = o = new (class C then a:{}); f = -> o + [#{access}...] = [nonce] + unless #{access}[0] is nonce then throw 'error' + """ + eq nonce, unless (try CoffeeScript.run code, bare: true catch e then true) then nonce + + From 6622f015abb4b43f998573d9272ae363edcfd280 Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Fri, 9 Sep 2011 18:55:08 -0400 Subject: [PATCH 3/8] #1643: Add unwrap method to Splat --- src/nodes.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nodes.coffee b/src/nodes.coffee index 3b4c7dfb..163a11b3 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1009,6 +1009,7 @@ exports.Assign = class Assign extends Base unless obj.name.unwrapAll().isAssignable() throw SyntaxError "\"#{ obj.name.compile(o) }\" cannot be assigned." name = obj.name.unwrap().value + obj = obj.unwrap() val = "#{olen} <= #{vvar}.length ? #{ utility 'slice' }.call(#{vvar}, #{i}" if rest = olen - i - 1 ivar = o.scope.freeVariable 'i' @@ -1032,7 +1033,7 @@ exports.Assign = class Assign extends Base if name? and name in ['arguments','eval'].concat RESERVED throw new SyntaxError "assignment to a reserved word: #{obj.compile o} = #{val.compile o}" isSoak = (soakNode = obj.name or obj) and soakNode.base?.soak or soakNode.properties?[0]?.soak - assigns.push new Assign(obj.name or obj, val, null, param: @param).compile o, if isSoak then LEVEL_ACCESS else LEVEL_TOP + assigns.push new Assign(obj, val, null, param: @param).compile o, if isSoak then LEVEL_ACCESS else LEVEL_TOP assigns.push vvar unless top code = assigns.join ', ' if o.level < LEVEL_LIST then code else "(#{code})" @@ -1177,6 +1178,8 @@ exports.Splat = class Splat extends Base compile: (o) -> if @index? then @compileParam o else @name.compile o + + unwrap: -> @name # Utility function that converts an arbitrary number of elements, mixed with # splats, to a proper array. From 6d0ba4b3bd2a5807a482e97c1254b4d11dca9a7b Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Fri, 9 Sep 2011 18:57:57 -0400 Subject: [PATCH 4/8] #1643: Clean up Assign#compile and Assign#compilePatternMatch --- src/nodes.coffee | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/nodes.coffee b/src/nodes.coffee index 163a11b3..8586a5e2 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -940,14 +940,14 @@ exports.Assign = class Assign extends Base return @compileSplice o if @variable.isSplice() return @compileConditional o if @context in ['||=', '&&=', '?='] name = @variable.compile o, LEVEL_LIST - unless @context or @variable.unwrapAll().isAssignable() - throw SyntaxError "\"#{ @variable.compile o }\" cannot be assigned." - hasProps = (isValue and @variable.unwrapAll().hasProperties?()) or (@variable instanceof Splat and @variable.name.unwrapAll().hasProperties?()) - unless @context or hasProps - if @param - o.scope.add name, 'var' - else - o.scope.find name + unless @context + unless (varBase = @variable.unwrapAll()).isAssignable() + throw SyntaxError "\"#{ @variable.compile o }\" cannot be assigned." + unless varBase.hasProperties?() + if @param + o.scope.add name, 'var' + else + o.scope.find name if @value instanceof Code and match = METHOD_DEF.exec name @value.klass = match[1] if match[1] @value.name = match[2] ? match[3] ? match[4] ? match[5] @@ -1032,8 +1032,7 @@ exports.Assign = class Assign extends Base val = new Value new Literal(vvar), [new (if acc then Access else Index) idx] if name? and name in ['arguments','eval'].concat RESERVED throw new SyntaxError "assignment to a reserved word: #{obj.compile o} = #{val.compile o}" - isSoak = (soakNode = obj.name or obj) and soakNode.base?.soak or soakNode.properties?[0]?.soak - assigns.push new Assign(obj, val, null, param: @param).compile o, if isSoak then LEVEL_ACCESS else LEVEL_TOP + assigns.push new Assign(obj, val, null, param: @param).compile o, LEVEL_LIST assigns.push vvar unless top code = assigns.join ', ' if o.level < LEVEL_LIST then code else "(#{code})" From 03372c9b29e7441a75e815df7fa7d2257b94042d Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Fri, 9 Sep 2011 18:58:59 -0400 Subject: [PATCH 5/8] #1643: Tag subpatterns in Assign#compilePatternMatch to prevent appending reference to RHS --- src/nodes.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/nodes.coffee b/src/nodes.coffee index 8586a5e2..5de4e9a6 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -918,6 +918,7 @@ exports.Class = class Class extends Base exports.Assign = class Assign extends Base constructor: (@variable, @value, @context, options) -> @param = options and options.param + @subpattern = options and options.subpattern children: ['variable', 'value'] @@ -1032,8 +1033,8 @@ exports.Assign = class Assign extends Base val = new Value new Literal(vvar), [new (if acc then Access else Index) idx] if name? and name in ['arguments','eval'].concat RESERVED throw new SyntaxError "assignment to a reserved word: #{obj.compile o} = #{val.compile o}" - assigns.push new Assign(obj, val, null, param: @param).compile o, LEVEL_LIST - assigns.push vvar unless top + assigns.push new Assign(obj, val, null, param: @param, subpattern: yes).compile o, LEVEL_LIST + assigns.push vvar unless top or @subpattern code = assigns.join ', ' if o.level < LEVEL_LIST then code else "(#{code})" From 7d4e693d20aeb118bdfa59ce93c5b9533768866c Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Fri, 9 Sep 2011 18:59:35 -0400 Subject: [PATCH 6/8] #1643: Updated tests --- test/assignment.coffee | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/test/assignment.coffee b/test/assignment.coffee index e3b90850..d0d3693e 100644 --- a/test/assignment.coffee +++ b/test/assignment.coffee @@ -309,14 +309,25 @@ test "#1591, #1101: splatted expressions in destructuring assignment must be ass test "#1643: splatted accesses in destructuring assignments should not be declared as variables", -> nonce = {} - accesses = ['o.a', 'C::a', 'C::', 'o["a"]', '(o.a)', 'f().a', '(o.a).a', '@o.a', 'o?.a', 'f?().a'] + accesses = ['o.a', 'o["a"]', '(o.a)', '(o.a).a', '@o.a', 'C::a', 'C::', 'f().a', 'o?.a', 'o?.a.b', 'f?().a'] for access in accesses - code = - """ - nonce = {}; @o = o = new (class C then a:{}); f = -> o - [#{access}...] = [nonce] - unless #{access}[0] is nonce then throw 'error' - """ - eq nonce, unless (try CoffeeScript.run code, bare: true catch e then true) then nonce - - + for i,j in [1,2,3] #position can matter + code = + """ + nonce = {}; nonce2 = {}; nonce3 = {}; + @o = o = new (class C then a:{}); f = -> o + [#{new Array(i).join('x,')}#{access}...] = [#{new Array(i).join('0,')}nonce, nonce2, nonce3] + unless #{access}[0] is nonce and #{access}[1] is nonce2 and #{access}[2] is nonce3 then throw new Error('[...]') + """ + eq nonce, unless (try CoffeeScript.run code, bare: true catch e then true) then nonce + # subpatterns like `[[a]...]` and `[{a}...]` + subpatterns = ['[sub, sub2, sub3]', '{0: sub, 1: sub2, 2: sub3}'] + for subpattern in subpatterns + for i,j in [1,2,3] + code = + """ + nonce = {}; nonce2 = {}; nonce3 = {}; + [#{new Array(i).join('x,')}#{subpattern}...] = [#{new Array(i).join('0,')}nonce, nonce2, nonce3] + unless sub is nonce and sub2 is nonce2 and sub3 is nonce3 then throw new Error('[sub...]') + """ + eq nonce, unless (try CoffeeScript.run code, bare: true catch e then true) then nonce From f0e276c63a6f9a359b86f7d6a9a5a2b43a6547e7 Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Fri, 9 Sep 2011 19:03:12 -0400 Subject: [PATCH 7/8] #1643: Remove superfluous assignable check from destructured Splats --- src/nodes.coffee | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nodes.coffee b/src/nodes.coffee index 5de4e9a6..b1611279 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1007,8 +1007,6 @@ exports.Assign = class Assign extends Base else idx = if obj.this then obj.properties[0].name else obj if not splat and obj instanceof Splat - unless obj.name.unwrapAll().isAssignable() - throw SyntaxError "\"#{ obj.name.compile(o) }\" cannot be assigned." name = obj.name.unwrap().value obj = obj.unwrap() val = "#{olen} <= #{vvar}.length ? #{ utility 'slice' }.call(#{vvar}, #{i}" From 447c3639e7dfda01e0615b15698924e325960278 Mon Sep 17 00:00:00 2001 From: Gerald Lewis Date: Fri, 9 Sep 2011 19:03:40 -0400 Subject: [PATCH 8/8] #1643: compiled JS --- lib/coffee-script/nodes.js | 51 ++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js index 207d476c..55feeba5 100644 --- a/lib/coffee-script/nodes.js +++ b/lib/coffee-script/nodes.js @@ -1051,6 +1051,7 @@ this.value = value; this.context = context; this.param = options && options.param; + this.subpattern = options && options.subpattern; } Assign.prototype.children = ['variable', 'value']; Assign.prototype.isStatement = function(o) { @@ -1063,7 +1064,7 @@ return unfoldSoak(o, this, 'variable'); }; Assign.prototype.compileNode = function(o) { - var hasProps, isValue, match, name, val, _base, _base2, _ref2, _ref3, _ref4, _ref5; + var isValue, match, name, val, varBase, _ref2, _ref3, _ref4, _ref5; if (isValue = this.variable instanceof Value) { if (this.variable.isArray() || this.variable.isObject()) { return this.compilePatternMatch(o); @@ -1074,15 +1075,16 @@ } } name = this.variable.compile(o, LEVEL_LIST); - if (!(this.context || this.variable.unwrapAll().isAssignable())) { - throw SyntaxError("\"" + (this.variable.compile(o)) + "\" cannot be assigned."); - } - hasProps = (isValue && (typeof (_base = this.variable.unwrapAll()).hasProperties === "function" ? _base.hasProperties() : void 0)) || (this.variable instanceof Splat && (typeof (_base2 = this.variable.name.unwrapAll()).hasProperties === "function" ? _base2.hasProperties() : void 0)); - if (!(this.context || hasProps)) { - if (this.param) { - o.scope.add(name, 'var'); - } else { - o.scope.find(name); + if (!this.context) { + if (!(varBase = this.variable.unwrapAll()).isAssignable()) { + throw SyntaxError("\"" + (this.variable.compile(o)) + "\" cannot be assigned."); + } + if (!(typeof varBase.hasProperties === "function" ? varBase.hasProperties() : void 0)) { + if (this.param) { + o.scope.add(name, 'var'); + } else { + o.scope.find(name); + } } } if (this.value instanceof Code && (match = METHOD_DEF.exec(name))) { @@ -1099,7 +1101,7 @@ } }; Assign.prototype.compilePatternMatch = function(o) { - var acc, assigns, code, i, idx, isObject, isSoak, ivar, name, obj, objects, olen, ref, rest, soakNode, splat, top, val, value, vvar, _len, _ref2, _ref3, _ref4, _ref5, _ref6, _ref7, _ref8, _ref9; + var acc, assigns, code, i, idx, isObject, ivar, name, obj, objects, olen, ref, rest, splat, top, val, value, vvar, _len, _ref2, _ref3, _ref4, _ref5, _ref6, _ref7, _ref8; top = o.level === LEVEL_TOP; value = this.value; objects = this.variable.base.objects; @@ -1114,10 +1116,10 @@ isObject = this.variable.isObject(); if (top && olen === 1 && !((obj = objects[0]) instanceof Splat)) { if (obj instanceof Assign) { - _ref2 = obj, idx = _ref2.variable.base, obj = _ref2.value; + _ref2 = obj, (_ref3 = _ref2.variable, idx = _ref3.base), obj = _ref2.value; } else { if (obj.base instanceof Parens) { - _ref3 = new Value(obj.unwrapAll()).cacheReference(o), obj = _ref3[0], idx = _ref3[1]; + _ref4 = new Value(obj.unwrapAll()).cacheReference(o), obj = _ref4[0], idx = _ref4[1]; } else { idx = isObject ? obj["this"] ? obj.properties[0].name : obj : new Literal(0); } @@ -1125,7 +1127,7 @@ acc = IDENTIFIER.test(idx.unwrap().value || 0); value = new Value(value); value.properties.push(new (acc ? Access : Index)(idx)); - if (_ref4 = obj.unwrap().value, __indexOf.call(['arguments', 'eval'].concat(RESERVED), _ref4) >= 0) { + if (_ref5 = obj.unwrap().value, __indexOf.call(['arguments', 'eval'].concat(RESERVED), _ref5) >= 0) { throw new SyntaxError("assignment to a reserved word: " + (obj.compile(o)) + " = " + (value.compile(o))); } return new Assign(obj, value, null, { @@ -1144,20 +1146,18 @@ idx = i; if (isObject) { if (obj instanceof Assign) { - _ref5 = obj, idx = _ref5.variable.base, obj = _ref5.value; + _ref6 = obj, (_ref7 = _ref6.variable, idx = _ref7.base), obj = _ref6.value; } else { if (obj.base instanceof Parens) { - _ref6 = new Value(obj.unwrapAll()).cacheReference(o), obj = _ref6[0], idx = _ref6[1]; + _ref8 = new Value(obj.unwrapAll()).cacheReference(o), obj = _ref8[0], idx = _ref8[1]; } else { idx = obj["this"] ? obj.properties[0].name : obj; } } } if (!splat && obj instanceof Splat) { - if (!obj.name.unwrapAll().isAssignable()) { - throw SyntaxError("\"" + (obj.name.compile(o)) + "\" cannot be assigned."); - } name = obj.name.unwrap().value; + obj = obj.unwrap(); val = "" + olen + " <= " + vvar + ".length ? " + (utility('slice')) + ".call(" + vvar + ", " + i; if (rest = olen - i - 1) { ivar = o.scope.freeVariable('i'); @@ -1184,12 +1184,12 @@ if ((name != null) && __indexOf.call(['arguments', 'eval'].concat(RESERVED), name) >= 0) { throw new SyntaxError("assignment to a reserved word: " + (obj.compile(o)) + " = " + (val.compile(o))); } - isSoak = (soakNode = obj.name || obj) && ((_ref7 = soakNode.base) != null ? _ref7.soak : void 0) || ((_ref8 = soakNode.properties) != null ? (_ref9 = _ref8[0]) != null ? _ref9.soak : void 0 : void 0); - assigns.push(new Assign(obj.name || obj, val, null, { - param: this.param - }).compile(o, isSoak ? LEVEL_ACCESS : LEVEL_TOP)); + assigns.push(new Assign(obj, val, null, { + param: this.param, + subpattern: true + }).compile(o, LEVEL_LIST)); } - if (!top) assigns.push(vvar); + if (!(top || this.subpattern)) assigns.push(vvar); code = assigns.join(', '); if (o.level < LEVEL_LIST) { return code; @@ -1374,6 +1374,9 @@ return this.name.compile(o); } }; + Splat.prototype.unwrap = function() { + return this.name; + }; Splat.compileSplattedArray = function(o, list, apply) { var args, base, code, i, index, node, _len; index = -1;