Improve Array.prototype.copyWithin polyfill.

Closes #2902. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=230397978
google · Jan 23, 2019 · 6dbfbbd · 6dbfbbd
1 parent 877e304
commit 6dbfbbd
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 17 deletions.
diff --git a/src/com/google/javascript/jscomp/js/es6/array/copywithin.js b/src/com/google/javascript/jscomp/js/es6/array/copywithin.js
@@ -17,6 +17,9 @@
 'require util/polyfill';
 
 $jscomp.polyfill('Array.prototype.copyWithin', function(orig) {
+  // requires strict mode to throw for invalid `this` or params
+  'use strict';
+
   if (orig) return orig;
 
   /**
@@ -31,32 +34,46 @@ $jscomp.polyfill('Array.prototype.copyWithin', function(orig) {
    */
   var polyfill = function(target, start, opt_end) {
     var len = this.length;
-    target = Number(target);
-    start = Number(start);
-    opt_end = Number(opt_end != null ? opt_end : len);
-    if (target < start) {
-      opt_end = Math.min(opt_end, len);
-      while (start < opt_end) {
-        if (start in this) {
-          this[target++] = this[start++];
+    target = toInteger(target);
+    start = toInteger(start);
+    var end = opt_end === undefined ? len : toInteger(opt_end);
+    var to = target < 0 ? Math.max(len + target, 0) : Math.min(target, len);
+    var from = start < 0 ? Math.max(len + start, 0) : Math.min(start, len);
+    var final = end < 0 ? Math.max(len + end, 0) : Math.min(end, len);
+    if (to < from) {
+      while (from < final) {
+        if (from in this) {
+          this[to++] = this[from++];
         } else {
-          delete this[target++];
-          start++;
+          delete this[to++];
+          from++;
         }
       }
     } else {
-      opt_end = Math.min(opt_end, len + start - target);
-      target += opt_end - start;
-      while (opt_end > start) {
-        if (--opt_end in this) {
-          this[--target] = this[opt_end];
+      final = Math.min(final, len + from - to);
+      to += final - from;
+      while (final > from) {
+        if (--final in this) {
+          this[--to] = this[final];
         } else {
-          delete this[target];
+          delete this[--to];
         }
       }
     }
     return this;
   };
 
+  /**
+   * @param {number} arg
+   * @return {number}
+   */
+  function toInteger(arg) {
+    var n = Number(arg);
+    if (n === Infinity || n === -Infinity) {
+      return n;
+    }
+    return n | 0;
+  }
+
   return polyfill;
 }, 'es6', 'es3');
diff --git a/src/com/google/javascript/jscomp/resources.json b/src/com/google/javascript/jscomp/resources.json
diff --git a/test/com/google/javascript/jscomp/runtime_tests/polyfill_tests/array_copywithin_test.js b/test/com/google/javascript/jscomp/runtime_tests/polyfill_tests/array_copywithin_test.js
@@ -64,5 +64,57 @@ testSuite({
     arr = {length: 3, 1: 4, 3: 'unused'};
     assertEquals(arr, Array.prototype.copyWithin.call(noCheck(arr), 0, 1));
     assertObjectEquals({length: 3, 0: 4, 3: 'unused'}, arr);
+
+    arr = {length: 3, 1: 4, 3: 'unused'};
+    assertEquals(arr, Array.prototype.copyWithin.call(noCheck(arr), 1, 0));
+    assertObjectEquals({length: 3, 2: 4, 3: 'unused'}, arr);
+  },
+
+  testCopyWithin_coercingArgs() {
+    assertObjectEquals([1, 2, 3, 3], [0, 1, 2, 3].copyWithin(NaN, 1));
+    assertObjectEquals([1, 2, 3, 3], [0, 1, 2, 3].copyWithin(0.5, 1));
+    assertObjectEquals([0, 0, 1, 2], [0, 1, 2, 3].copyWithin(1.5, 0));
+
+    assertObjectEquals([0, 0, 1, 2], [0, 1, 2, 3].copyWithin(1, NaN));
+    assertObjectEquals([0, 0, 1, 2], [0, 1, 2, 3].copyWithin(1, 0.5));
+    assertObjectEquals([1, 2, 3, 3], [0, 1, 2, 3].copyWithin(0, 1.5));
+
+    assertObjectEquals([0, 1, 2, 3], [0, 1, 2, 3].copyWithin(1, 0, NaN));
+    assertObjectEquals([0, 0, 2, 3], [0, 1, 2, 3].copyWithin(1, 0, 1.5));
+    assertObjectEquals([0, 0, 1, 3], [0, 1, 2, 3].copyWithin(1, 0, -2.5));
+    assertObjectEquals([0, 0, 1, 2], [0, 1, 2, 3].copyWithin(1, 0, undefined));
+  },
+
+  testCopyWithin_negativeArgs() {
+    assertObjectEquals([1, 2, 3, 1, 2], [1, 2, 3, 4, 5].copyWithin(-2, 0));
+    assertObjectEquals([1, 3, 4, 5, 5], [1, 2, 3, 4, 5].copyWithin(1, -3));
+    assertObjectEquals([1, 3, 4, 4, 5], [1, 2, 3, 4, 5].copyWithin(1, 2, -1));
+  },
+
+  testCopyWithin_throwsIfNullish() {
+    // TODO(tjgq): requires strict mode, lost in transpilation (b/24413211)
+    // assertThrows(function() {
+    //   Array.prototype.copyWithin.call(null, 0, 0);
+    // });
+    // assertThrows(function() {
+    //   Array.prototype.copyWithin.call(undefined, 0, 0);
+    // });
+  },
+
+  testCopyWithin_throwIfFailToDeleteProperty() {
+    var obj = {
+      length: 5
+    };
+
+    Object.defineProperty(obj, '4', {
+      value: 'a',
+      configurable: false,
+      writable: true
+    });
+
+    // TODO(tjgq): requires strict mode, lost in transpilation (b/24413211)
+    // assertThrows(function() {
+    //   Array.prototype.copyWithin.call(noCheck(obj), 4, 0);
+    // });
   },
 });