Allow iteration over the 'arguments' object even if it escapes and cannot be detected. Note that this causes some incorrect code to "work" when transpiled.

tbreisacher · blickly · commit 6a402120a118 · 2016-03-01T16:40:21.000-08:00
let obj = { 0: 'x', 1: 'y', length: 2, }; for (let x of obj) {} should throw a TypeError, but will not when transpiled. However, if typechecking is enabled there will be a compiler error. Fixes #1303 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=116058485
diff --git a/externs/es6.js b/externs/es6.js
@@ -142,7 +142,7 @@ Generator.prototype.return = function(value) {};
 Generator.prototype.throw = function(exception) {};
 
 
-// TODO(johnlenz): Array should be Iterable.
+// TODO(johnlenz): Array and Arguments should be Iterable.
 
 
 
diff --git a/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java b/src/com/google/javascript/jscomp/Es6RewriteDestructuring.java
@@ -294,7 +294,7 @@ private void visitArrayPattern(NodeTraversal t, Node arrayPattern, Node parent)
     String tempVarName = DESTRUCTURING_TEMP_VAR + (destructuringVarCounter++);
     Node tempDecl = IR.var(
         IR.name(tempVarName),
-        makeIterator(t, compiler, rhs.detachFromParent()));
+        makeIterator(compiler, rhs.detachFromParent()));
     tempDecl.useSourceInfoIfMissingFromForTree(arrayPattern);
     nodeToDetach.getParent().addChildBefore(tempDecl, nodeToDetach);
 
diff --git a/src/com/google/javascript/jscomp/Es6RewriteGenerators.java b/src/com/google/javascript/jscomp/Es6RewriteGenerators.java
@@ -179,7 +179,7 @@ private void visitYieldFor(NodeTraversal t, Node n, Node parent) {
     Node enclosingStatement = NodeUtil.getEnclosingStatement(n);
     Node generator = IR.var(
         IR.name(GENERATOR_YIELD_ALL_NAME),
-        makeIterator(t, compiler, n.removeFirstChild()));
+        makeIterator(compiler, n.removeFirstChild()));
     Node entryDecl = IR.var(IR.name(GENERATOR_YIELD_ALL_ENTRY));
     Node assignIterResult =
         IR.assign(
diff --git a/src/com/google/javascript/jscomp/Es6ToEs3Converter.java b/src/com/google/javascript/jscomp/Es6ToEs3Converter.java
@@ -259,7 +259,7 @@ private void visitForOf(NodeTraversal t, Node node, Node parent) {
     Node iterResult = IR.name(ITER_RESULT + variableName);
     iterResult.makeNonIndexable();
 
-    Node init = IR.var(iterName.cloneTree(), makeIterator(t, compiler, iterable));
+    Node init = IR.var(iterName.cloneTree(), makeIterator(compiler, iterable));
     Node initIterResult = iterResult.cloneTree();
     initIterResult.addChildToFront(getNext.cloneTree());
     init.addChildToBack(initIterResult);
@@ -397,7 +397,7 @@ private void visitArrayLitOrCallWithSpread(NodeTraversal t, Node node, Node pare
           currGroup = null;
         }
         compiler.needsEs6Runtime = true;
-        groups.add(arrayFromIterable(t, compiler, currElement.removeFirstChild()));
+        groups.add(arrayFromIterable(compiler, currElement.removeFirstChild()));
       } else {
         if (currGroup == null) {
           currGroup = IR.arraylit();
@@ -937,36 +937,17 @@ private void cannotConvertYet(Node n, String feature) {
   }
 
   /**
-   * Returns a call to {@code $jscomp.makeIterator} with {@code iterable} as its argument, unless
-   * {@code iterable} is the special {@code arguments} variable, in which case the returned Node is:
-   * {@code $jscomp.makeIterator($jscomp.arrayFromArguments(iterable))}.
+   * Returns a call to {@code $jscomp.makeIterator} with {@code iterable} as its argument.
    */
-  static Node makeIterator(NodeTraversal t, AbstractCompiler compiler, Node iterable) {
-    if (iterable.isName()) {
-      Var var = t.getScope().getVar(iterable.getString());
-      if (var != null && var.isArguments()) {
-        iterable = IR.call(
-            NodeUtil.newQName(compiler, "$jscomp.arrayFromArguments"),
-            iterable);
-      }
-    }
+  static Node makeIterator(AbstractCompiler compiler, Node iterable) {
     return callEs6RuntimeFunction(compiler, iterable, "makeIterator");
   }
 
   /**
-   * Returns a call to $jscomp.arrayFromIterable with {@code iterable} as its argument, unless
-   * {@code iterable} is the special {@code arguments} variable, in which case
-   * {@code $jscomp.arrayFromArguments} is called instead.
+   * Returns a call to $jscomp.arrayFromIterable with {@code iterable} as its argument.
    */
-  private static Node arrayFromIterable(NodeTraversal t, AbstractCompiler compiler, Node iterable) {
-    String fnName = "arrayFromIterable";
-    if (iterable.isName()) {
-      Var var = t.getScope().getVar(iterable.getString());
-      if (var != null && var.isArguments()) {
-        fnName = "arrayFromArguments";
-      }
-    }
-    return callEs6RuntimeFunction(compiler, iterable, fnName);
+  private static Node arrayFromIterable(AbstractCompiler compiler, Node iterable) {
+    return callEs6RuntimeFunction(compiler, iterable, "arrayFromIterable");
   }
 
   private static Node callEs6RuntimeFunction(
diff --git a/src/com/google/javascript/jscomp/js/es6/runtime.js b/src/com/google/javascript/jscomp/js/es6/runtime.js
@@ -90,7 +90,7 @@ $jscomp.initSymbolIterator = function() {
 /**
  * Creates an iterator for the given iterable.
  *
- * @param {string|!Array<T>|!Iterable<T>|!Iterator<T>} iterable
+ * @param {string|!Array<T>|!Iterable<T>|!Iterator<T>|!Arguments<T>} iterable
  * @return {!Iterator<T>}
  * @template T
  * @suppress {reportUnknownTypes}
@@ -101,10 +101,7 @@ $jscomp.makeIterator = function(iterable) {
   if (iterable[$jscomp.global.Symbol.iterator]) {
     return iterable[$jscomp.global.Symbol.iterator]();
   }
-  if (!(iterable instanceof Array) && typeof iterable != 'string' &&
-      !(iterable instanceof String)) {
-    throw new TypeError(iterable + ' is not iterable');
-  }
+
   let index = 0;
   return /** @type {!Iterator} */ ({
     next() {
@@ -142,7 +139,7 @@ $jscomp.arrayFromIterator = function(iterator) {
 
 /**
  * Copies the values from an Iterable into an Array.
- * @param {string|!Array<T>|!Iterable<T>} iterable
+ * @param {string|!Array<T>|!Iterable<T>|!Arguments<T>} iterable
  * @return {!Array<T>}
  * @template T
  */
@@ -155,20 +152,6 @@ $jscomp.arrayFromIterable = function(iterable) {
 };
 
 
-/**
- * Copies the values from an Arguments object into an Array.
- * @param {!Arguments} args
- * @return {!Array}
- */
-$jscomp.arrayFromArguments = function(args) {
-  const result = [];
-  for (let i = 0; i < args.length; i++) {
-    result.push(args[i]);
-  }
-  return result;
-};
-
-
 /**
  * Inherit the prototype methods and static methods from one constructor
  * into another.
diff --git a/src/com/google/javascript/jscomp/js/es6_runtime.js b/src/com/google/javascript/jscomp/js/es6_runtime.js
@@ -50,15 +50,12 @@
 /**@suppress {reportUnknownTypes}
 @template T
 
-@param {(string|!Array<T>|!Iterable<T>|!Iterator<T>)} iterable
+@param {(string|!Array<T>|!Iterable<T>|!Iterator<T>|!Arguments<T>)} iterable
 @return {!Iterator<T>} */$jscomp.makeIterator = function(iterable) {
   $jscomp.initSymbolIterator();
   if (iterable[$jscomp.global.Symbol.iterator]) {
     return iterable[$jscomp.global.Symbol.iterator]();
   }
-  if (!(iterable instanceof Array) && typeof iterable != "string" && !(iterable instanceof String)) {
-    throw new TypeError(iterable + " is not iterable");
-  }
   var index = 0;
   return /**@type {!Iterator} */({next:function() {
     if (index == iterable.length) {
@@ -81,7 +78,7 @@
 };
 /**@template T
 
-@param {(string|!Array<T>|!Iterable<T>)} iterable
+@param {(string|!Array<T>|!Iterable<T>|!Arguments<T>)} iterable
 @return {!Array<T>} */$jscomp.arrayFromIterable = function(iterable) {
   if (iterable instanceof Array) {
     return iterable;
@@ -90,15 +87,6 @@
   }
 };
 /**
-@param {!Arguments} args
-@return {!Array} */$jscomp.arrayFromArguments = function(args) {
-  /**@const */var result = [];
-  for (var i = 0;i < args.length;i++) {
-    result.push(args[i]);
-  }
-  return result;
-};
-/**
 @param {!Function} childCtor
 @param {!Function} parentCtor */$jscomp.inherits = function(childCtor, parentCtor) {
   /**@constructor */function tempCtor() {
diff --git a/test/com/google/javascript/jscomp/Es6RewriteDestructuringTest.java b/test/com/google/javascript/jscomp/Es6RewriteDestructuringTest.java
@@ -362,8 +362,7 @@ public void testArrayDestructuringArguments() {
     "function f() { var [x, y] = arguments; }",
     LINE_JOINER.join(
         "function f() {",
-        "  var $jscomp$destructuring$var0 = $jscomp.makeIterator(",
-        "      $jscomp.arrayFromArguments(arguments));",
+        "  var $jscomp$destructuring$var0 = $jscomp.makeIterator(arguments);",
         "  var x = $jscomp$destructuring$var0.next().value;",
         "  var y = $jscomp$destructuring$var0.next().value;",
         "}"));
diff --git a/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java b/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java

Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ Generator.prototype.return = function(value) {};`
`142`	`142`	`Generator.prototype.throw = function(exception) {};`
`143`	`143`
`144`	`144`
`145`		`-// TODO(johnlenz): Array should be Iterable.`
	`145`	`+// TODO(johnlenz): Array and Arguments should be Iterable.`
`146`	`146`
`147`	`147`
`148`	`148`