8022903: Enhance for-in and for-each for Lists and Maps

Reviewed-by: lagergren, sundar

nashorn/src/jdk/nashorn/internal/runtime/ScriptRuntime.java

@@ -37,7 +37,9 @@ import java.lang.invoke.MethodHandles;
 import java.lang.reflect.Array;
 import java.util.Collections;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Objects;
 import jdk.internal.dynalink.beans.StaticClass;
@@ -221,49 +223,71 @@ public final class ScriptRuntime {
     }
 
     /**
-     * Used to determine property iterator used in for in.
-     * @param obj Object to iterate on.
-     * @return Iterator.
+     * Returns an iterator over property identifiers used in the {@code for...in} statement. Note that the ECMAScript
+     * 5.1 specification, chapter 12.6.4. uses the terminology "property names", which seems to imply that the property
+     * identifiers are expected to be strings, but this is not actually spelled out anywhere, and Nashorn will in some
+     * cases deviate from this. Namely, we guarantee to always return an iterator over {@link String} values for any
+     * built-in JavaScript object. We will however return an iterator over {@link Integer} objects for native Java
+     * arrays and {@link List} objects, as well as arbitrary objects representing keys of a {@link Map}. Therefore, the
+     * expression {@code typeof i} within a {@code for(i in obj)} statement can return something other than
+     * {@code string} when iterating over native Java arrays, {@code List}, and {@code Map} objects.
+     * @param obj object to iterate on.
+     * @return iterator over the object's property names.
      */
-    public static Iterator<String> toPropertyIterator(final Object obj) {
+    public static Iterator<?> toPropertyIterator(final Object obj) {
         if (obj instanceof ScriptObject) {
             return ((ScriptObject)obj).propertyIterator();
         }
 
         if (obj != null && obj.getClass().isArray()) {
-            final int length = Array.getLength(obj);
-
-            return new Iterator<String>() {
-                private int index = 0;
-
-                @Override
-                public boolean hasNext() {
-                    return index < length;
-                }
-
-                @Override
-                public String next() {
-                    return "" + index++; //TODO numeric property iterator?
-                }
-
-                @Override
-                public void remove() {
-                    throw new UnsupportedOperationException();
-                }
-            };
+            return new RangeIterator(Array.getLength(obj));
         }
 
         if (obj instanceof ScriptObjectMirror) {
             return ((ScriptObjectMirror)obj).keySet().iterator();
         }
 
+        if (obj instanceof List) {
+            return new RangeIterator(((List<?>)obj).size());
+        }
+
+        if (obj instanceof Map) {
+            return ((Map<?,?>)obj).keySet().iterator();
+        }
+
         return Collections.emptyIterator();
     }
 
+    private static final class RangeIterator implements Iterator<Integer> {
+        private final int length;
+        private int index;
+
+        RangeIterator(int length) {
+            this.length = length;
+        }
+
+        @Override
+        public boolean hasNext() {
+            return index < length;
+        }
+
+        @Override
+        public Integer next() {
+            return index++;
+        }
+
+        @Override
+        public void remove() {
+            throw new UnsupportedOperationException();
+        }
+    }
+
     /**
-     * Used to determine property value iterator used in for each in.
-     * @param obj Object to iterate on.
-     * @return Iterator.
+     * Returns an iterator over property values used in the {@code for each...in} statement. Aside from built-in JS
+     * objects, it also operates on Java arrays, any {@link Iterable}, as well as on {@link Map} objects, iterating over
+     * map values.
+     * @param obj object to iterate on.
+     * @return iterator over the object's property values.
      */
     public static Iterator<?> toValueIterator(final Object obj) {
         if (obj instanceof ScriptObject) {
@@ -301,6 +325,10 @@ public final class ScriptRuntime {
             return ((ScriptObjectMirror)obj).values().iterator();
         }
 
+        if (obj instanceof Map) {
+            return ((Map<?,?>)obj).values().iterator();
+        }
+
         if (obj instanceof Iterable) {
             return ((Iterable<?>)obj).iterator();
         }

nashorn/test/script/basic/JDK-8022903.js

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ * 
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ * 
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ * 
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ * 
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8022903: Enhance for-in and for-each for Lists and Maps
+ *
+ * @test
+ * @run
+ */
+
+var colors = new java.util.ArrayList()
+colors.add("red")
+colors.add("purple")
+colors.add("pink")
+
+for(var index in colors) {
+    print("colors[" + index + "]=" + colors[index])
+}
+
+for each(var color in colors) {
+    print(color)
+}
+
+var capitals = new java.util.LinkedHashMap()
+capitals.Sweden = "Stockholm"
+capitals.Hungary = "Budapet"
+capitals.Croatia = "Zagreb"
+
+for(var key in capitals) {
+    print("capital of " + key + " is " + capitals[key])
+}
+
+for each(var capital in capitals) {
+    print(capital)
+}

nashorn/test/script/basic/JDK-8022903.js.EXPECTED

@@ -0,0 +1,12 @@
+colors[0]=red
+colors[1]=purple
+colors[2]=pink
+red
+purple
+pink
+capital of Sweden is Stockholm
+capital of Hungary is Budapet
+capital of Croatia is Zagreb
+Stockholm
+Budapet
+Zagreb