AST: allow assignment to nontrailing splat (#5263)

* allow assignment to nontrailing splat

* allow assignment to empty array

* complex object splat

* add explanatory comment

Author: helixbass

lib/coffeescript/nodes.js

@@ -2442,7 +2442,7 @@ exports.Obj = class Obj extends Base
 
   children: ['properties']
 
-  isAssignable: ->
+  isAssignable: (opts) ->
     for prop in @properties
       # Check for reserved words.
       message = isUnassignable prop.unwrapAll().value
@@ -2451,7 +2451,7 @@ exports.Obj = class Obj extends Base
       prop = prop.value if prop instanceof Assign and
         prop.context is 'object' and
         prop.value?.base not instanceof Arr
-      return no unless prop.isAssignable()
+      return no unless prop.isAssignable opts
     yes
 
   shouldCache: ->
@@ -2594,7 +2594,7 @@ exports.Obj = class Obj extends Base
         # Shorthand property with default, e.g. `{a = 1} = b`.
         property.nestedLhs = yes
       else if property instanceof Splat
-        property.lhs = yes
+        property.propagateLhs yes
 
   astNode: (o) ->
     @getAndCheckSplatProps()
@@ -2661,11 +2661,11 @@ exports.Arr = class Arr extends Base
     return yes for obj in @objects when obj instanceof Elision
     no
 
-  isAssignable: ({allowExpansion} = {}) ->
-    return no unless @objects.length
+  isAssignable: ({allowExpansion, allowNontrailingSplat, allowEmptyArray = no} = {}) ->
+    return allowEmptyArray unless @objects.length
 
     for obj, i in @objects
-      return no if obj instanceof Splat and i + 1 isnt @objects.length
+      return no if not allowNontrailingSplat and obj instanceof Splat and i + 1 isnt @objects.length
       return no unless (allowExpansion and obj instanceof Expansion) or (obj.isAssignable() and (not obj.isAtomic or obj.isAtomic()))
     yes
 
@@ -3455,11 +3455,24 @@ exports.Assign = class Assign extends Base
   unfoldSoak: (o) ->
     unfoldSoak o, this, 'variable'
 
-  addScopeVariables: (o, {allowAssignmentToExpansion = no} = {}) ->
+  addScopeVariables: (o, {
+    # During AST generation, we need to allow assignment to these constructs
+    # that are considered “unassignable” during compile-to-JS, while still
+    # flagging things like `[null] = b`.
+    allowAssignmentToExpansion = no,
+    allowAssignmentToNontrailingSplat = no,
+    allowAssignmentToEmptyArray = no,
+    allowAssignmentToComplexSplat = no
+  } = {}) ->
     return unless not @context or @context is '**='
 
     varBase = @variable.unwrapAll()
-    if not varBase.isAssignable allowExpansion: allowAssignmentToExpansion
+    if not varBase.isAssignable {
+      allowExpansion: allowAssignmentToExpansion
+      allowNontrailingSplat: allowAssignmentToNontrailingSplat
+      allowEmptyArray: allowAssignmentToEmptyArray
+      allowComplexSplat: allowAssignmentToComplexSplat
+    }
       @variable.error "'#{@variable.compile o}' can't be assigned"
 
     varBase.eachName (name) =>
@@ -3805,7 +3818,7 @@ exports.Assign = class Assign extends Base
       variable = @variable.unwrap()
       if variable instanceof IdentifierLiteral and not o.scope.check variable.value
         @throwUnassignableConditionalError variable.value
-    @addScopeVariables o, allowAssignmentToExpansion: yes
+    @addScopeVariables o, allowAssignmentToExpansion: yes, allowAssignmentToNontrailingSplat: yes, allowAssignmentToEmptyArray: yes, allowAssignmentToComplexSplat: yes
     super o
 
   astType: ->
@@ -4403,8 +4416,8 @@ exports.Splat = class Splat extends Base
 
   shouldCache: -> no
 
-  isAssignable: ->
-    return no if @name instanceof Obj or @name instanceof Parens
+  isAssignable: ({allowComplexSplat = no} = {})->
+    return allowComplexSplat if @name instanceof Obj or @name instanceof Parens
     @name.isAssignable() and (not @name.isAtomic or @name.isAtomic())
 
   assigns: (name) ->
@@ -4417,6 +4430,11 @@ exports.Splat = class Splat extends Base
 
   unwrap: -> @name
 
+  propagateLhs: (setLhs) ->
+    @lhs = yes if setLhs
+    return unless @lhs
+    @name.propagateLhs? yes
+
   astType: ->
     if @jsx
       'JSXSpreadAttribute'

src/nodes.coffee

@@ -2275,6 +2275,85 @@ test "AST as expected for Assign node", ->
       operator: '?='
     ]
 
+  testExpression '[a..., b] = c',
+    type: 'AssignmentExpression'
+    left:
+      type: 'ArrayPattern'
+      elements: [
+        type: 'RestElement'
+        argument: ID 'a', declaration: yes
+        postfix: yes
+      ,
+        ID 'b'
+      ]
+    right:
+      ID 'c'
+
+  testExpression '[] = c',
+    type: 'AssignmentExpression'
+    left:
+      type: 'ArrayPattern'
+      elements: []
+    right:
+      ID 'c'
+
+  testExpression '{{a...}...} = b',
+    type: 'AssignmentExpression'
+    left:
+      type: 'ObjectPattern'
+      properties: [
+        type: 'RestElement'
+        argument:
+          type: 'ObjectPattern'
+          properties: [
+            type: 'RestElement'
+            argument: ID 'a'
+          ]
+        postfix: yes
+      ]
+    right: ID 'b'
+
+  testExpression '{a..., b} = c',
+    type: 'AssignmentExpression'
+    left:
+      type: 'ObjectPattern'
+      properties: [
+        type: 'RestElement'
+        argument: ID 'a'
+        postfix: yes
+      ,
+        type: 'ObjectProperty'
+      ]
+    right: ID 'c'
+
+  testExpression '{a.b...} = c',
+    type: 'AssignmentExpression'
+    left:
+      type: 'ObjectPattern'
+      properties: [
+        type: 'RestElement'
+        argument:
+          type: 'MemberExpression'
+        postfix: yes
+      ]
+    right: ID 'c'
+
+  testExpression '{{a}...} = b',
+    type: 'AssignmentExpression'
+    left:
+      type: 'ObjectPattern'
+      properties: [
+        type: 'RestElement'
+        argument:
+          type: 'ObjectPattern'
+          properties: [
+            type: 'ObjectProperty'
+            shorthand: yes
+          ]
+        postfix: yes
+      ]
+    right: ID 'b'
+
 test "AST as expected for Code node", ->
   testExpression '=>',
     type: 'ArrowFunctionExpression'