From 794f65fbd74ee9f3839a5f715116e71cf23e24c0 Mon Sep 17 00:00:00 2001 From: zdenko Date: Sat, 3 Feb 2018 22:35:41 +0100 Subject: [PATCH] Fix #4878: Compile error when using destructuring with a splat or expansion in an array (#4879) * fix #4878 * improvements * test * refactor --- lib/coffeescript/nodes.js | 20 ++++++++++---------- src/nodes.coffee | 20 ++++++++++---------- test/assignment.coffee | 40 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 20 deletions(-) diff --git a/lib/coffeescript/nodes.js b/lib/coffeescript/nodes.js index cbc21600..fbec324d 100644 --- a/lib/coffeescript/nodes.js +++ b/lib/coffeescript/nodes.js @@ -3575,8 +3575,8 @@ this.toVar}`, upperBound = `${this.fromVar} >= ${idx} && ${gt} ${this.toVar}`, ` // Sort 'splatsAndExpans' so we can show error at first disallowed token. objects[splatsAndExpans.sort()[1]].error("multiple splats/expansions are disallowed in an assignment"); } - isSplat = splats.length; - isExpans = expans.length; + isSplat = (splats != null ? splats.length : void 0) > 0; + isExpans = (expans != null ? expans.length : void 0) > 0; isObject = this.variable.isObject(); isArray = this.variable.isArray(); vvar = value.compileToFragments(o, LEVEL_LIST); @@ -3648,7 +3648,7 @@ this.toVar}`, upperBound = `${this.fromVar} >= ${idx} && ${gt} ${this.toVar}`, ` }; // "Complex" `objects` are processed in a loop. // Examples: [a, b, {c, r...}, d], [a, ..., {b, r...}, c, d] - loopObjects = (objs, vvarTxt) => { + loopObjects = (objs, vvar, vvarTxt) => { var acc, idx, j, len1, message, objSpreads, results, vval; objSpreads = hasObjSpreads(objs); results = []; @@ -3707,7 +3707,7 @@ this.toVar}`, upperBound = `${this.fromVar} >= ${idx} && ${gt} ${this.toVar}`, ` return results; }; // "Simple" `objects` can be split and compiled to arrays, [a, b, c] = arr, [a, b, c...] = arr - assignObjects = (objs, vvarTxt) => { + assignObjects = (objs, vvar, vvarTxt) => { var vval; vvar = new Value(new Arr(objs, true)); vval = vvarTxt instanceof Value ? vvarTxt : new Value(new Literal(vvarTxt)); @@ -3716,11 +3716,11 @@ this.toVar}`, upperBound = `${this.fromVar} >= ${idx} && ${gt} ${this.toVar}`, ` subpattern: true }).compileToFragments(o, LEVEL_LIST)); }; - processObjects = function(objs, vvarTxt) { + processObjects = function(objs, vvar, vvarTxt) { if (complexObjects(objs)) { - return loopObjects(objs, vvarTxt); + return loopObjects(objs, vvar, vvarTxt); } else { - return assignObjects(objs, vvarTxt); + return assignObjects(objs, vvar, vvarTxt); } }; // In case there is `Splat` or `Expansion` in `objects`, @@ -3739,7 +3739,7 @@ this.toVar}`, upperBound = `${this.fromVar} >= ${idx} && ${gt} ${this.toVar}`, ` leftObjs = objects.slice(0, expIdx + (isSplat ? 1 : 0)); rightObjs = objects.slice(expIdx + 1); if (leftObjs.length !== 0) { - processObjects(leftObjs, vvarText); + processObjects(leftObjs, vvar, vvarText); } if (rightObjs.length !== 0) { // Slice or splice `objects`. @@ -3756,11 +3756,11 @@ this.toVar}`, upperBound = `${this.fromVar} >= ${idx} && ${gt} ${this.toVar}`, ` refExp = o.scope.freeVariable('ref'); assigns.push([this.makeCode(refExp + ' = '), ...restVar.compileToFragments(o, LEVEL_LIST)]); } - processObjects(rightObjs, refExp); + processObjects(rightObjs, vvar, refExp); } } else { // There is no `Splat` or `Expansion` in `objects`. - processObjects(objects, vvarText); + processObjects(objects, vvar, vvarText); } if (!(top || this.subpattern)) { assigns.push(vvar); diff --git a/src/nodes.coffee b/src/nodes.coffee index f22ff748..0d09caef 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -2408,8 +2408,8 @@ exports.Assign = class Assign extends Base # Sort 'splatsAndExpans' so we can show error at first disallowed token. objects[splatsAndExpans.sort()[1]].error "multiple splats/expansions are disallowed in an assignment" - isSplat = splats.length - isExpans = expans.length + isSplat = splats?.length > 0 + isExpans = expans?.length > 0 isObject = @variable.isObject() isArray = @variable.isArray() @@ -2458,7 +2458,7 @@ exports.Assign = class Assign extends Base # "Complex" `objects` are processed in a loop. # Examples: [a, b, {c, r...}, d], [a, ..., {b, r...}, c, d] - loopObjects = (objs, vvarTxt) => + loopObjects = (objs, vvar, vvarTxt) => objSpreads = hasObjSpreads objs for obj, i in objs # `Elision` can be skipped. @@ -2488,16 +2488,16 @@ exports.Assign = class Assign extends Base assigns.push new Assign(vvar, vval, null, param: @param, subpattern: yes).compileToFragments o, LEVEL_LIST # "Simple" `objects` can be split and compiled to arrays, [a, b, c] = arr, [a, b, c...] = arr - assignObjects = (objs, vvarTxt) => + assignObjects = (objs, vvar, vvarTxt) => vvar = new Value new Arr(objs, yes) vval = if vvarTxt instanceof Value then vvarTxt else new Value new Literal(vvarTxt) assigns.push new Assign(vvar, vval, null, param: @param, subpattern: yes).compileToFragments o, LEVEL_LIST - processObjects = (objs, vvarTxt) -> + processObjects = (objs, vvar, vvarTxt) -> if complexObjects objs - loopObjects objs, vvarTxt + loopObjects objs, vvar, vvarTxt else - assignObjects objs, vvarTxt + assignObjects objs, vvar, vvarTxt # In case there is `Splat` or `Expansion` in `objects`, # we can split array in two simple subarrays. @@ -2514,7 +2514,7 @@ exports.Assign = class Assign extends Base expIdx = splatsAndExpans[0] leftObjs = objects.slice 0, expIdx + (if isSplat then 1 else 0) rightObjs = objects.slice expIdx + 1 - processObjects leftObjs, vvarText if leftObjs.length isnt 0 + processObjects leftObjs, vvar, vvarText if leftObjs.length isnt 0 if rightObjs.length isnt 0 # Slice or splice `objects`. refExp = switch @@ -2524,10 +2524,10 @@ exports.Assign = class Assign extends Base restVar = refExp refExp = o.scope.freeVariable 'ref' assigns.push [@makeCode(refExp + ' = '), restVar.compileToFragments(o, LEVEL_LIST)...] - processObjects rightObjs, refExp + processObjects rightObjs, vvar, refExp else # There is no `Splat` or `Expansion` in `objects`. - processObjects objects, vvarText + processObjects objects, vvar, vvarText assigns.push vvar unless top or @subpattern fragments = @joinFragmentArrays assigns, ', ' if o.level < LEVEL_LIST then fragments else @wrapInParentheses fragments diff --git a/test/assignment.coffee b/test/assignment.coffee index 38d837c9..5023d612 100644 --- a/test/assignment.coffee +++ b/test/assignment.coffee @@ -945,3 +945,43 @@ test "#4673: complex destructured object spread variables", -> {{g}...} = g: 1 eq g, 1 + +test "#4878: Compile error when using destructuring with a splat or expansion in an array", -> + arr = ['a', 'b', 'c', 'd'] + + f1 = (list) -> + [first, ..., last] = list + + f2 = (list) -> + [first..., last] = list + + f3 = (list) -> + ([first, ...] = list); first + + f4 = (list) -> + ([first, ...rest] = list); rest + + arrayEq f1(arr), arr + arrayEq f2(arr), arr + arrayEq f3(arr), 'a' + arrayEq f4(arr), ['b', 'c', 'd'] + + foo = (list) -> + ret = + if list?.length > 0 + [first, ..., last] = list + [first, last] + else + [] + + arrayEq foo(arr), ['a', 'd'] + + bar = (list) -> + ret = + if list?.length > 0 + [first, ...rest] = list + [first, rest] + else + [] + + arrayEq bar(arr), ['a', ['b', 'c', 'd']]