From 0217ed55055928cee4a13927c646745e4fdcceab Mon Sep 17 00:00:00 2001 From: zdenko Date: Tue, 16 Jan 2018 05:03:01 +0100 Subject: [PATCH] Fix #1726: expression in property access causes unexpected results (#4851) * fix #1726 * Explain what's happening, rather than just linking to an issue * Updated output * Optimization --- lib/coffeescript/nodes.js | 14 ++++++++++---- src/nodes.coffee | 8 ++++++-- test/slicing_and_splicing.coffee | 6 ++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/coffeescript/nodes.js b/lib/coffeescript/nodes.js index 504a5333..4a1876ef 100644 --- a/lib/coffeescript/nodes.js +++ b/lib/coffeescript/nodes.js @@ -849,7 +849,7 @@ // generates output indented by two spaces; so all we need to do is // search for a `code` property that begins with at least two spaces. fragmentIndent = ''; - ref1 = fragments.slice(0, fragmentIndex + 1); + ref1 = fragments.slice(0, (fragmentIndex + 1)); for (k = ref1.length - 1; k >= 0; k += -1) { pastFragment = ref1[k]; indent = /^ {2,}/m.exec(pastFragment.code); @@ -874,7 +874,7 @@ } return results; })()).join(`\n${fragmentIndent}`).replace(/^(\s*)$/gm, ''); - ref2 = fragments.slice(0, fragmentIndex + 1); + ref2 = fragments.slice(0, (fragmentIndex + 1)); for (pastFragmentIndex = l = ref2.length - 1; l >= 0; pastFragmentIndex = l += -1) { pastFragment = ref2[pastFragmentIndex]; newLineIndex = pastFragment.code.lastIndexOf('\n'); @@ -2097,8 +2097,14 @@ compileNode(o) { var compiled, compiledText, from, fromCompiled, to, toStr; ({to, from} = this.range); - fromCompiled = from && from.compileToFragments(o, LEVEL_PAREN) || [this.makeCode('0')]; - // TODO: jwalton - move this into the 'if'? + // Handle an expression in the property access, e.g. `a[!b in c..]`. + if (from != null ? from.shouldCache() : void 0) { + from = new Value(new Parens(from)); + } + if (to != null ? to.shouldCache() : void 0) { + to = new Value(new Parens(to)); + } + fromCompiled = (from != null ? from.compileToFragments(o, LEVEL_PAREN) : void 0) || [this.makeCode('0')]; if (to) { compiled = to.compileToFragments(o, LEVEL_PAREN); compiledText = fragmentsToText(compiled); diff --git a/src/nodes.coffee b/src/nodes.coffee index 8ca12ee9..7a459a4b 100644 --- a/src/nodes.coffee +++ b/src/nodes.coffee @@ -1406,8 +1406,12 @@ exports.Slice = class Slice extends Base # `9e9` should be safe because `9e9` > `2**32`, the max array length. compileNode: (o) -> {to, from} = @range - fromCompiled = from and from.compileToFragments(o, LEVEL_PAREN) or [@makeCode '0'] - # TODO: jwalton - move this into the 'if'? + # Handle an expression in the property access, e.g. `a[!b in c..]`. + if from?.shouldCache() + from = new Value new Parens from + if to?.shouldCache() + to = new Value new Parens to + fromCompiled = from?.compileToFragments(o, LEVEL_PAREN) or [@makeCode '0'] if to compiled = to.compileToFragments o, LEVEL_PAREN compiledText = fragmentsToText compiled diff --git a/test/slicing_and_splicing.coffee b/test/slicing_and_splicing.coffee index 65bd3485..ffbabc66 100644 --- a/test/slicing_and_splicing.coffee +++ b/test/slicing_and_splicing.coffee @@ -165,3 +165,9 @@ test "#2953: methods on endpoints in assignment from array splice literal", -> delete Number.prototype.same arrayEq [0, 5, 9], list + +test "#1726: `Op` expression in property access causes unexpected results", -> + a = [0..2] + arrayEq a, a[(!1 in a)..] + arrayEq a, a[!1 in a..] + arrayEq a[(!1 in a)..], a[(!1 in a)..]