fix object spread destructuring bug: #4651

lib/coffeescript/nodes.js

@@ -3234,7 +3234,7 @@
       // Check object destructuring variable for rest elements;
       // can be removed once ES proposal hits Stage 4.
       compileObjectDestruct(o) {
-        var fragments, getPropKey, getPropName, j, len1, restElement, restElements, result, setScopeVar, traverseRest, value, valueRef;
+        var fragments, getPropKey, getPropName, j, len1, restElement, restElements, result, setScopeVar, shouldCache, traverseRest, value, valueRef;
         // Per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Assignment_without_declaration,
         // if we’re destructuring without declaring, the destructuring assignment
         // must be wrapped in parentheses: `({a, b} = obj)`. Helper function
@@ -3301,9 +3301,13 @@
                 [prop.value.value, nestedSourceDefault] = prop.value.value.cache(o);
               }
               if (nestedProperties) {
-                nestedSource = new Value(source.base, source.properties.concat([new Access(getPropKey(prop))]));
-                if (nestedSourceDefault) {
-                  nestedSource = new Value(new Op('?', nestedSource, nestedSourceDefault));
+                if (source.properties) {
+                  nestedSource = new Value(source.base, source.properties.concat([new Access(getPropKey(prop))]));
+                  if (nestedSourceDefault) {
+                    nestedSource = new Value(new Op('?', nestedSource, nestedSourceDefault));
+                  }
+                } else {
+                  nestedSource = source;
                 }
                 restElements.push(...traverseRest(nestedProperties, nestedSource));
               }
@@ -3335,8 +3339,18 @@
           }
           return restElements;
         };
-        // Cache the value for reuse with rest elements
-        [this.value, valueRef] = this.value.cache(o);
+        // Cache the value for reuse with rest elements.
+        // `Obj` should be always cached.
+        // Examples:
+        //   {a, r...} = {a:1, b:2, c:3}
+        //   {a, r...} = {a:1, obj...}
+        shouldCache = (value) => {
+          if (value.base instanceof Obj) {
+            return true;
+          }
+          return value.shouldCache();
+        };
+        [this.value, valueRef] = this.value.cache(o, false, shouldCache);
         // Find all rest elements.
         restElements = traverseRest(this.variable.base.properties, valueRef);
         result = new Block([this]);

src/nodes.coffee

@@ -2229,8 +2229,11 @@ exports.Assign = class Assign extends Base
             nestedProperties = prop.value.variable.base.properties
             [prop.value.value, nestedSourceDefault] = prop.value.value.cache o
           if nestedProperties
-            nestedSource = new Value source.base, source.properties.concat [new Access getPropKey prop]
-            nestedSource = new Value new Op '?', nestedSource, nestedSourceDefault if nestedSourceDefault
+            if source.properties
+              nestedSource = new Value source.base, source.properties.concat [new Access getPropKey prop]
+              nestedSource = new Value new Op '?', nestedSource, nestedSourceDefault if nestedSourceDefault
+            else
+              nestedSource = source
             restElements.push traverseRest(nestedProperties, nestedSource)...
         else if prop instanceof Splat
           prop.error "multiple rest elements are disallowed in object destructuring" if restIndex?
@@ -2247,8 +2250,16 @@ exports.Assign = class Assign extends Base
 
       restElements
 
-    # Cache the value for reuse with rest elements
-    [@value, valueRef] = @value.cache o
+    # Cache the value for reuse with rest elements.
+    # `Obj` should be always cached.
+    # Examples:
+    #   {a, r...} = {a:1, b:2, c:3}
+    #   {a, r...} = {a:1, obj...}
+    shouldCache = (value) =>
+      return yes if value.base instanceof Obj
+      value.shouldCache()
+
+    [@value, valueRef] = @value.cache o, false, shouldCache
 
     # Find all rest elements.
     restElements = traverseRest @variable.base.properties, valueRef

test/assignment.coffee

@@ -296,6 +296,27 @@ test "destructuring assignment with multiple splats in different objects", ->
   deepEqual a, val: 1
   deepEqual b, val: 2
 
+  o = {
+    props: {
+      p: {
+        n: 1
+        m: 5
+      }
+      s: 6
+    }
+  }
+  {p: {m}, r...} = o.props
+  eq m, o.props.p.m
+  deepEqual r, s: 6
+
+  @props = o.props
+  {p: {m}, r...} = @props
+  eq m, @props.p.m
+  deepEqual r, s: 6
+
+  {p: {m}, r...} = {o.props..., p:{m:9}}
+  eq m, 9
+
   # Should not trigger implicit call, e.g. rest ... => rest(...)
   {
     a: {