8006984: Introducing local into a function inside with statement confuses its scope

Reviewed-by: jlaskey, lagergren, sundar

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

@@ -628,14 +628,15 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
      * @return FindPropertyData or null if not found.
      */
     public final FindProperty findProperty(final String key, final boolean deep) {
-        return findProperty(key, deep, false);
+        return findProperty(key, deep, false, this);
     }
 
     /**
      * Low level property API (not using property descriptors)
      * <p>
-     * Find a property in the prototype hierarchy. Note: this is final and not
-     * a good idea to override. If you have to, use
+     * Find a property in the prototype hierarchy. Note: this is not a good idea
+     * to override except as it was done in {@link WithObject}.
+     * If you have to, use
      * {jdk.nashorn.internal.objects.NativeArray{@link #getProperty(String)} or
      * {jdk.nashorn.internal.objects.NativeArray{@link #getPropertyDescriptor(String)} as the
      * overriding way to find array properties
@@ -645,27 +646,29 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
      * @param key  Property key.
      * @param deep Whether the search should look up proto chain.
      * @param stopOnNonScope should a deep search stop on the first non-scope object?
+     * @param start the object on which the lookup was originally initiated
      *
      * @return FindPropertyData or null if not found.
      */
-    public final FindProperty findProperty(final String key, final boolean deep, final boolean stopOnNonScope) {
-        for (ScriptObject self = this; self != null; self = self.getProto()) {
-            // if doing deep search, stop search on the first non-scope object if asked to do so
-            if (stopOnNonScope && self != this && !self.isScope()) {
-                return null;
-            }
+    FindProperty findProperty(final String key, final boolean deep, final boolean stopOnNonScope, final ScriptObject start) {
+        // if doing deep search, stop search on the first non-scope object if asked to do so
+        if (stopOnNonScope && start != this && !isScope()) {
+            return null;
+        }
 
-            final PropertyMap selfMap  = self.getMap();
-            final Property    property = selfMap.findProperty(key);
+        final PropertyMap selfMap  = getMap();
+        final Property    property = selfMap.findProperty(key);
 
-            if (property != null) {
-                return new FindProperty(this, self, property);
-            }
+        if (property != null) {
+            return new FindProperty(start, this, property);
+        }
 
-            if (!deep) {
-                return null;
+        if (deep) {
+            final ScriptObject proto = getProto();
+            if(proto != null) {
+                return proto.findProperty(key, deep, stopOnNonScope, start);
             }
-         }
+        }
 
         return null;
     }
@@ -1781,12 +1784,12 @@ public abstract class ScriptObject extends PropertyListenerManager implements Pr
         final boolean scope = isScope();
         /*
          * If doing property set on a scope object, we should stop proto search on the first
-         * non-scope object. Without this, for exmaple, when assigning "toString" on global scope,
+         * non-scope object. Without this, for example, when assigning "toString" on global scope,
          * we'll end up assigning it on it's proto - which is Object.prototype.toString !!
          *
          * toString = function() { print("global toString"); } // don't affect Object.prototype.toString
          */
-        FindProperty find = findProperty(name, true, scope);
+        FindProperty find = findProperty(name, true, scope, this);
         // If it's not a scope search, then we don't want any inherited properties except those with user defined accessors.
         if (!scope && find != null && find.isInherited() && !(find.getProperty() instanceof UserAccessorProperty)) {
             // We should still check if inherited data property is not writable

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

@@ -184,6 +184,28 @@ public final class WithObject extends ScriptObject implements Scope {
         return null;
     }
 
+    /**
+     * Overridden to try to find the property first in the expression object (and its prototypes), and only then in this
+     * object (and its prototypes).
+     *
+     * @param key  Property key.
+     * @param deep Whether the search should look up proto chain.
+     * @param stopOnNonScope should a deep search stop on the first non-scope object?
+     * @param start the object on which the lookup was originally initiated
+     *
+     * @return FindPropertyData or null if not found.
+     */
+    @Override
+    FindProperty findProperty(final String key, final boolean deep, final boolean stopOnNonScope, final ScriptObject start) {
+        if (expression instanceof ScriptObject) {
+            final FindProperty exprProperty = ((ScriptObject)expression).findProperty(key, deep, stopOnNonScope, start);
+            if(exprProperty != null) {
+                return exprProperty;
+            }
+        }
+        return super.findProperty(key, deep, stopOnNonScope, start);
+    }
+
     @Override
     public void setSplitState(final int state) {
         getNonWithParent().setSplitState(state);

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

@@ -0,0 +1,44 @@
+/*
+ * 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.
+ */
+
+/**
+ * findProperty on WithObject was not considering its object argument
+ * 
+ * @test
+ * @run
+ */
+
+var guiPkgs = { JFrame: function() { print("created"); } }; 
+
+with (guiPkgs) { 
+     var main = function() { 
+        var frame; // <---- this local variable caused scope to be not set properly prior to fix
+
+        function createFrame() { 
+            frame = new JFrame(); 
+        } 
+
+        createFrame(); 
+    } 
+} 
+main();

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

@@ -0,0 +1 @@
+created