From fb21fbe14d239fa4072fb8889d0cfd3f064795ee Mon Sep 17 00:00:00 2001 From: Zdenko Vujasinovic Date: Wed, 16 Aug 2017 01:44:37 +0200 Subject: [PATCH 1/4] fix splat error with soak properties or expressions --- lib/coffee-script/nodes.js | 28 +++++++++++++++++++++++----- src/nodes.coffee | 14 ++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js index f4d6479008..db7174952d 100644 --- a/lib/coffee-script/nodes.js +++ b/lib/coffee-script/nodes.js @@ -535,7 +535,7 @@ }; Literal.prototype.toString = function() { - return " " + (this.isStatement() ? Literal.__super__.toString.apply(this, arguments) : this.constructor.name) + ": " + this.value; + return " " + (this.isStatement() ? Literal.__super__.toString.apply(this, (arguments)) : this.constructor.name) + ": " + this.value; }; return Literal; @@ -781,7 +781,7 @@ if (o.scope.parent == null) { this.error('yield can only occur inside functions'); } - return YieldReturn.__super__.compileNode.apply(this, arguments); + return YieldReturn.__super__.compileNode.apply(this, (arguments)); }; return YieldReturn; @@ -1037,7 +1037,7 @@ } delete this.needsUpdatedStartLocation; } - return Call.__super__.updateLocationDataIfMissing.apply(this, arguments); + return Call.__super__.updateLocationDataIfMissing.apply(this, (arguments)); }; Call.prototype.newInstance = function() { @@ -2725,7 +2725,25 @@ return this.name.assigns(name); }; + Splat.prototype.propHasSoak = function() { + var isSoak, j, len1, prop, ref3; + if (!this.name.properties) { + return false; + } + ref3 = this.name.properties; + for (j = 0, len1 = ref3.length; j < len1; j++) { + prop = ref3[j]; + if (prop.soak) { + isSoak = true; + } + } + return isSoak != null ? isSoak : false; + }; + Splat.prototype.compileToFragments = function(o) { + if (!(this.name instanceof Value) || (!(this.name.base instanceof Parens) && this.propHasSoak())) { + this.name = new Value(new Parens(this.name)); + } return this.name.compileToFragments(o); }; @@ -2818,7 +2836,7 @@ While.prototype.makeReturn = function(res) { if (res) { - return While.__super__.makeReturn.apply(this, arguments); + return While.__super__.makeReturn.apply(this, (arguments)); } else { this.returns = !this.jumps({ loop: true @@ -3371,7 +3389,7 @@ StringWithInterpolations.prototype.compileNode = function(o) { var element, elements, expr, fragments, j, len1, value; if (!o.inTaggedTemplateCall) { - return StringWithInterpolations.__super__.compileNode.apply(this, arguments); + return StringWithInterpolations.__super__.compileNode.apply(this, (arguments)); } expr = this.body.unwrap(); elements = []; diff --git a/src/nodes.coffee b/src/nodes.coffee index 6ef56a7bb6..7d91b0e72f 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1812,7 +1812,21 @@ exports.Splat = class Splat extends Base assigns: (name) -> @name.assigns name + propHasSoak: -> + return no unless @name.properties + isSoak = yes for prop in @name.properties when prop.soak + isSoak ? no + compileToFragments: (o) -> + # Check if @name is not an instance of `Value` or @name properties contains soak accessor, e.g. ?.b, + # and ensure correct compilation by wrapping the @name in `Parens`. + # Examples: + # [a?.b...] => [(a?.b)...] + # f(a.b?.c...) => f((a.b?.c)...) + # [a if b...] => [(a if b)...] + if not (@name instanceof Value) or + (not (@name.base instanceof Parens) and @propHasSoak()) + @name = new Value new Parens @name @name.compileToFragments o unwrap: -> @name From a302e53110ebb800b3d89fea15aec120d2f1b625 Mon Sep 17 00:00:00 2001 From: Zdenko Vujasinovic Date: Wed, 23 Aug 2017 06:33:30 +0200 Subject: [PATCH 2/4] Add test based on #4260 --- test/arrays.coffee | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/arrays.coffee b/test/arrays.coffee index bffd512787..43d72d4bea 100644 --- a/test/arrays.coffee +++ b/test/arrays.coffee @@ -36,9 +36,7 @@ test "array splat expansions with assignments", -> eq 4, b arrayEq [0,1,2,3,4], list - test "mixed shorthand objects in array lists", -> - arr = [ a:1 'b' @@ -58,7 +56,6 @@ test "mixed shorthand objects in array lists", -> eq arr[2].b, 1 eq arr[3], 'b' - test "array splats with nested arrays", -> nonce = {} a = [nonce] @@ -70,6 +67,11 @@ test "array splats with nested arrays", -> list = [1, 2, a...] arrayEq list, [1, 2, [nonce]] +test "#4260: splat after existential operator soak", -> + a = {b: [3]} + arrayEq [a?.b...], [3] + arrayEq [c?.b...], [] + test "#1274: `[] = a()` compiles to `false` instead of `a()`", -> a = false fn = -> a = true From 16f782b8048ae9887dfaec046f4815adc41642ed Mon Sep 17 00:00:00 2001 From: Zdenko Vujasinovic Date: Wed, 23 Aug 2017 07:26:34 +0200 Subject: [PATCH 3/4] Add test based on #1349 --- lib/coffee-script/nodes.js | 12 +++++++++++- src/nodes.coffee | 10 +++++++++- test/arrays.coffee | 7 +++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js index db7174952d..e2eb737b6a 100644 --- a/lib/coffee-script/nodes.js +++ b/lib/coffee-script/nodes.js @@ -2741,10 +2741,20 @@ }; Splat.prototype.compileToFragments = function(o) { + var fragment, fragments, ix, j, len1; if (!(this.name instanceof Value) || (!(this.name.base instanceof Parens) && this.propHasSoak())) { this.name = new Value(new Parens(this.name)); + fragments = this.name.compileToFragments(o); + for (ix = j = 0, len1 = fragments.length; j < len1; ix = ++j) { + fragment = fragments[ix]; + if (fragment.code === "void 0" && fragment.type === "If") { + fragments[ix].code = "[]"; + } + } + return fragments; + } else { + return this.name.compileToFragments(o); } - return this.name.compileToFragments(o); }; Splat.prototype.unwrap = function() { diff --git a/src/nodes.coffee b/src/nodes.coffee index 7d91b0e72f..e995b12ae3 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1827,7 +1827,15 @@ exports.Splat = class Splat extends Base if not (@name instanceof Value) or (not (@name.base instanceof Parens) and @propHasSoak()) @name = new Value new Parens @name - @name.compileToFragments o + # We need to replace `void 0` with `[]` in compiled fragments. + # Example: [a?.b...] + # slice.call((typeof a !== "undefined" && a !== null ? a.b : [])); + fragments = @name.compileToFragments o + for fragment, ix in fragments + fragments[ix].code = "[]" if fragment.code == "void 0" and fragment.type == "If" + fragments + else + @name.compileToFragments o unwrap: -> @name diff --git a/test/arrays.coffee b/test/arrays.coffee index 43d72d4bea..96531a2de9 100644 --- a/test/arrays.coffee +++ b/test/arrays.coffee @@ -72,6 +72,13 @@ test "#4260: splat after existential operator soak", -> arrayEq [a?.b...], [3] arrayEq [c?.b...], [] +test "#1349: trailing if after splat", -> + a = [3] + b = yes + c = null + arrayEq [a if b...], [3] + arrayEq [a if c...], [] + test "#1274: `[] = a()` compiles to `false` instead of `a()`", -> a = false fn = -> a = true From 17fb5cabf44cf3e74cb838f87d1434d649b553f4 Mon Sep 17 00:00:00 2001 From: Zdenko Vujasinovic Date: Sun, 27 Aug 2017 08:51:00 +0200 Subject: [PATCH 4/4] remove 'void 0' replacement; add Splat::compileNode --- lib/coffee-script/nodes.js | 32 ++------------------------------ src/nodes.coffee | 26 ++------------------------ test/arrays.coffee | 16 ++++++++++++++-- 3 files changed, 18 insertions(+), 56 deletions(-) diff --git a/lib/coffee-script/nodes.js b/lib/coffee-script/nodes.js index e2eb737b6a..103163c9e4 100644 --- a/lib/coffee-script/nodes.js +++ b/lib/coffee-script/nodes.js @@ -2725,36 +2725,8 @@ return this.name.assigns(name); }; - Splat.prototype.propHasSoak = function() { - var isSoak, j, len1, prop, ref3; - if (!this.name.properties) { - return false; - } - ref3 = this.name.properties; - for (j = 0, len1 = ref3.length; j < len1; j++) { - prop = ref3[j]; - if (prop.soak) { - isSoak = true; - } - } - return isSoak != null ? isSoak : false; - }; - - Splat.prototype.compileToFragments = function(o) { - var fragment, fragments, ix, j, len1; - if (!(this.name instanceof Value) || (!(this.name.base instanceof Parens) && this.propHasSoak())) { - this.name = new Value(new Parens(this.name)); - fragments = this.name.compileToFragments(o); - for (ix = j = 0, len1 = fragments.length; j < len1; ix = ++j) { - fragment = fragments[ix]; - if (fragment.code === "void 0" && fragment.type === "If") { - fragments[ix].code = "[]"; - } - } - return fragments; - } else { - return this.name.compileToFragments(o); - } + Splat.prototype.compileNode = function(o) { + return this.name.compileToFragments(o); }; Splat.prototype.unwrap = function() { diff --git a/src/nodes.coffee b/src/nodes.coffee index e995b12ae3..8625b74319 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1812,30 +1812,8 @@ exports.Splat = class Splat extends Base assigns: (name) -> @name.assigns name - propHasSoak: -> - return no unless @name.properties - isSoak = yes for prop in @name.properties when prop.soak - isSoak ? no - - compileToFragments: (o) -> - # Check if @name is not an instance of `Value` or @name properties contains soak accessor, e.g. ?.b, - # and ensure correct compilation by wrapping the @name in `Parens`. - # Examples: - # [a?.b...] => [(a?.b)...] - # f(a.b?.c...) => f((a.b?.c)...) - # [a if b...] => [(a if b)...] - if not (@name instanceof Value) or - (not (@name.base instanceof Parens) and @propHasSoak()) - @name = new Value new Parens @name - # We need to replace `void 0` with `[]` in compiled fragments. - # Example: [a?.b...] - # slice.call((typeof a !== "undefined" && a !== null ? a.b : [])); - fragments = @name.compileToFragments o - for fragment, ix in fragments - fragments[ix].code = "[]" if fragment.code == "void 0" and fragment.type == "If" - fragments - else - @name.compileToFragments o + compileNode: (o) -> + @name.compileToFragments o unwrap: -> @name diff --git a/test/arrays.coffee b/test/arrays.coffee index 96531a2de9..234c9ddcc9 100644 --- a/test/arrays.coffee +++ b/test/arrays.coffee @@ -69,15 +69,27 @@ test "array splats with nested arrays", -> test "#4260: splat after existential operator soak", -> a = {b: [3]} + foo = (a) -> [a] arrayEq [a?.b...], [3] - arrayEq [c?.b...], [] + arrayEq [c?.b ? []...], [] + arrayEq foo(a?.b...), [3] + arrayEq foo(c?.b ? []...), [undefined] + e = yes + f = null + arrayEq [(a if e)?.b...], [3] + arrayEq [(a if f)?.b ? []...], [] + arrayEq foo((a if e)?.b...), [3] + arrayEq foo((a if f)?.b ? []...), [undefined] test "#1349: trailing if after splat", -> a = [3] b = yes c = null + foo = (a) -> [a] arrayEq [a if b...], [3] - arrayEq [a if c...], [] + arrayEq [(a if c) ? []...], [] + arrayEq foo((a if b)...), [3] + arrayEq foo((a if c) ? []...), [undefined] test "#1274: `[] = a()` compiles to `false` instead of `a()`", -> a = false