array: fixes slice, unshift, shift

Author: kaelzhang

package.json

@@ -37,7 +37,7 @@
   "ava": {
     "babel": false,
     "files": [
-      "test/*.test.js"
+      "test/array.test.js"
     ]
   },
   "author": "kaelzhang",

src/array.js

@@ -133,7 +133,9 @@ class CommentArray extends Array {
   // - deleteCount + items.length
 
   // We should avoid `splice(begin, deleteCount, ...items)`,
-  // because `splice(0, undefined)` is not equivalent to `splice(0)`
+  // because `splice(0, undefined)` is not equivalent to `splice(0)`,
+  // as well as:
+  // - slice
   splice (...args) {
     const {length} = this
     const ret = super.splice(...args)
@@ -177,30 +179,43 @@ class CommentArray extends Array {
     return ret
   }
 
-  slice (begin, before) {
-    const array = new CommentArray(...super.slice(begin, before))
-    if (begin < 0 && before === UNDEFINED) {
+  slice (...args) {
+    const {length} = this
+    const array = super.slice(...args)
+    if (!array.length) {
       return new CommentArray()
     }
 
+    let [begin, before] = args
+
+    // Ref:
+    // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice
     if (before === UNDEFINED) {
-      before = this.length
+      before = length
+    } else if (before < 0) {
+      before += length
+    }
 
-      if (begin < 0) {
-        begin += before
-      }
+    if (begin < 0) {
+      begin += length
+    } else if (begin === UNDEFINED) {
+      begin = 0
     }
 
-    move_comments(array, this, begin, before - begin, begin)
+    move_comments(array, this, begin, before - begin, - begin)
+
     return array
   }
 
   unshift (...items) {
+    const {length} = this
     const ret = super.unshift(...items)
-    const {length} = items
+    const {
+      length: items_length
+    } = items
 
-    if (length > 0) {
-      move_comments(this, this, length, this.length, length, true)
+    if (items_length > 0) {
+      move_comments(this, this, 0, length, items_length, true)
     }
 
     return ret
@@ -210,7 +225,7 @@ class CommentArray extends Array {
     const ret = super.shift()
     const {length} = this
 
-    move_comments(this, this, 1, length - 1, - 1, true)
+    move_comments(this, this, 1, length, - 1, true)
 
     return ret
   }

test/array.test.js

@@ -1,5 +1,7 @@
 const test = require('ava')
-const {isFunction} = require('core-util-is')
+const {
+  isFunction, isObject, isString, isArray
+} = require('core-util-is')
 const {parse, stringify} = require('../src')
 
 const a1 = `[
@@ -18,6 +20,27 @@ const a2 = `[
   2
 ]`
 
+// ret: return value to expect
+const texpect = (t, ret, str, r, rr) => {
+  if (isObject(ret)) {
+    t.deepEqual(r, ret)
+  } else {
+    t.is(r, ret)
+  }
+
+  if (isString(rr)) {
+    t.is(rr, str)
+  } else {
+    t.is(stringify(rr, null, 2), str)
+  }
+}
+
+const slice = (ret, str) =>
+  (t, r, _, __, rr) => texpect(t, ret, str, r, rr)
+
+const unshift = (ret, str) =>
+  (t, r, s) => texpect(t, ret, str, r, s)
+
 const CASES = [
   [
     'splice(0, 1)',
@@ -46,7 +69,88 @@ const CASES = [
     array => array.splice(0, undefined),
     [],
     a1
-  ]
+  ],
+  [
+    'slice(0)',
+    a1,
+    array => array.slice(0),
+    [0, 1, 2],
+    a1
+  ],
+  [
+    'slice(-1)',
+    a1,
+    array => array.slice(- 1),
+    slice([2], `[
+  // 2
+  2
+]`)
+  ],
+  [
+    'slice(3)',
+    a1,
+    array => array.slice(3),
+    slice([], '[]')
+  ],
+  [
+    'slice(undefined, undefined)',
+    a1,
+    array => array.slice(),
+    slice([0, 1, 2], a1)
+  ],
+  [
+    'slice(0, - 2)',
+    a1,
+    array => array.slice(0, - 2),
+    slice([0], `[
+  // 0
+  0
+]`)
+  ],
+  [
+    'unshift()',
+    a1,
+    array => array.unshift(),
+    unshift(3, a1)
+  ],
+  [
+    'unshift(- 1)',
+    a1,
+    array => array.unshift(- 1),
+    unshift(4, `[
+  -1,
+  // 0
+  0,
+  // 1
+  1,
+  // 2
+  2
+]`)
+  ],
+  [
+    'shift',
+    a1,
+    array => array.shift(),
+    unshift(0, `[
+  // 1
+  1,
+  // 2
+  2
+]`)
+  ],
+//   [
+//     'reverse',
+//     a1,
+//     array => array.reverse(),
+//     unshift([2, 1, 0], `[
+//   // 2
+//   2,
+//   // 1
+//   1,
+//   // 0
+//   0
+// ]`)
+//   ]
 ]
 
 CASES.forEach(([d, a, run, e, s]) => {
@@ -61,6 +165,12 @@ CASES.forEach(([d, a, run, e, s]) => {
         tt.is(str, s)
       }
 
-    expect(t, [...ret], stringify(parsed, null, 2), parsed, ret)
+    expect(
+      t,
+      isArray(ret)
+        // clean ret
+        ? [...ret]
+        : ret,
+      stringify(parsed, null, 2), parsed, ret)
   })
 })