[CS2] Fix #4631: Expansion that becomes rest parameter causes runtime error (#4634)

* expansion-rest bug fix

* tests; improved implicit call recognition with dots on the left in the `rewriter`

* whitespace cleanup

* more tests

Author: zdenko

lib/coffeescript/rewriter.js

@@ -267,7 +267,8 @@ exports.Rewriter = class Rewriter
       # Added support for spread dots on the left side: f ...a
       if (tag in IMPLICIT_FUNC and token.spaced or
           tag is '?' and i > 0 and not tokens[i - 1].spaced) and
-         (nextTag in IMPLICIT_CALL or nextTag is '...' or
+         (nextTag in IMPLICIT_CALL or
+         (nextTag is '...' and @tag(i + 2) in IMPLICIT_CALL and not @findTagsBackwards(i, ['INDEX_START', '['])) or
           nextTag in IMPLICIT_UNSPACED_CALL and
           not nextToken.spaced and not nextToken.newLine)
         tag = token[0] = 'FUNC_EXIST' if tag is '?'

src/rewriter.coffee

@@ -185,6 +185,12 @@ test "destructuring assignment with splats", ->
   arrayEq [b,c,d], y
   eq e, z
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  [x,y ...,z] = [a,b,c,d,e]
+  eq a, x
+  arrayEq [b,c,d], y
+  eq e, z
+
 test "deep destructuring assignment with splats", ->
   a={}; b={}; c={}; d={}; e={}; f={}; g={}; h={}; i={}
   [u, [v, w..., x], y..., z] = [a, [b, c, d, e], f, g, h, i]
@@ -229,6 +235,11 @@ test "destructuring assignment with objects and splats", ->
   eq a, y
   arrayEq [b,c,d], z
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  {a: b: [y, z ...]} = obj
+  eq a, y
+  arrayEq [b,c,d], z
+
 test "destructuring assignment against an expression", ->
   a={}; b={}
   [y, z] = if true then [a, b] else [b, a]
@@ -263,6 +274,15 @@ test "destructuring assignment with splats and default values", ->
   eq b, 1
   deepEqual d, {}
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  {
+    a: {b} = c
+    d ...
+  } = obj
+
+  eq b, 1
+  deepEqual d, {}
+
 test "destructuring assignment with splat with default value", ->
   obj = {}
   c = {val: 1}
@@ -276,6 +296,18 @@ test "destructuring assignment with multiple splats in different objects", ->
   deepEqual a, val: 1
   deepEqual b, val: 2
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  {
+    a: {
+      a ...
+    }
+    b: {
+      b ...
+    }
+  } = obj
+  deepEqual a, val: 1
+  deepEqual b, val: 2
+
 test "destructuring assignment with dynamic keys and splats", ->
   i = 0
   foo = -> ++i
@@ -299,6 +331,15 @@ test "destructuring assignment with objects and splats: Babel tests", ->
   n = { x, y, z... }
   deepEqual n, { x: 1, y: 2, a: 3, b: 4 }
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  { x, y, z ... } = { x: 1, y: 2, a: 3, b: 4 }
+  eq x, 1
+  eq y, 2
+  deepEqual z, { a: 3, b: 4 }
+
+  n = { x, y, z ... }
+  deepEqual n, { x: 1, y: 2, a: 3, b: 4 }
+
 test "deep destructuring assignment with objects: ES2015", ->
   a1={}; b1={}; c1={}; d1={}
   obj = {
@@ -320,6 +361,13 @@ test "deep destructuring assignment with objects: ES2015", ->
   eq bb, b1
   eq r2.b2, obj.b2
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  {a: w, b: {c: {d: {b1: bb, r1 ...}}}, r2 ...} = obj
+  eq r1.e, c1
+  eq r2.b, undefined
+  eq bb, b1
+  eq r2.b2, obj.b2
+
 test "deep destructuring assignment with defaults: ES2015", ->
   obj =
     b: { c: 1, baz: 'qux' }
@@ -343,16 +391,55 @@ test "deep destructuring assignment with defaults: ES2015", ->
   eq hello, 'world'
   deepEqual h, some: 'prop'
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  {
+    a ...
+    b: {
+      c,
+      d ...
+    }
+    e: {
+      f: hello
+      g: {
+        h ...
+      } = i
+    } = j
+  } = obj
+
+  deepEqual a, foo: 'bar'
+  eq c, 1
+  deepEqual d, baz: 'qux'
+  eq hello, 'world'
+  deepEqual h, some: 'prop'
+
 test "object spread properties: ES2015", ->
   obj = {a: 1, b: 2, c: 3, d: 4, e: 5}
   obj2 = {obj..., c:9}
   eq obj2.c, 9
   eq obj.a, obj2.a
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  obj2 = {
+    obj ...
+    c:9
+  }
+  eq obj2.c, 9
+  eq obj.a, obj2.a
+
   obj2 = {obj..., a: 8, c: 9, obj...}
   eq obj2.c, 3
   eq obj.a, obj2.a
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  obj2 = {
+    obj ...
+    a: 8
+    c: 9
+    obj ...
+  }
+  eq obj2.c, 3
+  eq obj.a, obj2.a
+
   obj3 = {obj..., b: 7, g: {obj2..., c: 1}}
   eq obj3.g.c, 1
   eq obj3.b, 7
@@ -370,10 +457,42 @@ test "object spread properties: ES2015", ->
   eq obj4.f.g, 5
   deepEqual obj4.f, obj.c.f
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  (({
+    a
+    b
+    r ...
+    }) ->
+    eq 1, a
+    deepEqual r, {c: 3, d: 44, e: 55}
+  ) {
+    obj2 ...
+    d: 44
+    e: 55
+  }
+
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  obj4 = {
+    a: 10
+    obj.c ...
+  }
+  eq obj4.a, 10
+  eq obj4.d, 3
+  eq obj4.f.g, 5
+  deepEqual obj4.f, obj.c.f
+
   obj5 = {obj..., ((k) -> {b: k})(99)...}
   eq obj5.b, 99
   deepEqual obj5.c, obj.c
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  obj5 = {
+    obj ...
+    ((k) -> {b: k})(99) ...
+  }
+  eq obj5.b, 99
+  deepEqual obj5.c, obj.c
+
   fn = -> {c: {d: 33, e: 44, f: {g: 55}}}
   obj6 = {obj..., fn()...}
   eq obj6.c.d, 33
@@ -382,7 +501,16 @@ test "object spread properties: ES2015", ->
   obj7 = {obj..., fn()..., {c: {d: 55, e: 66, f: {77}}}...}
   eq obj7.c.d, 55
   deepEqual obj6.c, {d: 33, e: 44, f: {g: 55}}
-  
+
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  obj7 = {
+    obj ...
+    fn() ...
+    {c: {d: 55, e: 66, f: {77}}} ...
+  }
+  eq obj7.c.d, 55
+  deepEqual obj6.c, {d: 33, e: 44, f: {g: 55}}
+
   obj =
     a:
       b:

test/assignment.coffee

@@ -347,12 +347,26 @@ test "passing splats to functions", ->
   arrayEq [2..6], others
   eq 7, last
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  arrayEq [0..4], id id [0..4] ...
+  fn = (a, b, c ..., d) -> [a, b, c, d]
+  range = [0..3]
+  [first, second, others, last] = fn range ..., 4, [5 ... 8] ...
+  eq 0, first
+  eq 1, second
+  arrayEq [2..6], others
+  eq 7, last
+
 test "splat variables are local to the function", ->
   outer = "x"
   clobber = (avar, outer...) -> outer
   clobber "foo", "bar"
   eq "x", outer
 
+test "Issue 4631: left and right spread dots with preceding space", ->
+  a = []
+  f = (a) -> a
+  eq yes, (f ...a) is (f ... a) is (f a...) is (f a ...) is f(a...) is f(...a) is f(a ...) is f(... a)
 
 test "Issue 894: Splatting against constructor-chained functions.", ->
 
@@ -387,19 +401,38 @@ test "splats with super() within classes.", ->
       super nums...
   ok (new Child).meth().join(' ') is '3 2 1'
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  class Parent
+    meth: (args ...) ->
+      args
+  class Child extends Parent
+    meth: ->
+      nums = [3, 2, 1]
+      super nums ...
+  ok (new Child).meth().join(' ') is '3 2 1'
+
 
 test "#1011: passing a splat to a method of a number", ->
   eq '1011', 11.toString [2]...
   eq '1011', (31).toString [3]...
   eq '1011', 69.0.toString [4]...
   eq '1011', (131.0).toString [5]...
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  eq '1011', 11.toString [2] ...
+  eq '1011', (31).toString [3] ...
+  eq '1011', 69.0.toString [4] ...
+  eq '1011', (131.0).toString [5] ...
 
 test "splats and the `new` operator: functions that return `null` should construct their instance", ->
   args = []
   child = new (constructor = -> null) args...
   ok child instanceof constructor
 
+  # Should not trigger implicit call, e.g. rest ... => rest(...)
+  child = new (constructor = -> null) args ...
+  ok child instanceof constructor
+
 test "splats and the `new` operator: functions that return functions should construct their return value", ->
   args = []
   fn = ->