Bug 523846 - Assignments to a property that has a getter but not a setter should not throw a TypeError per ES5 (at least not until strict mode is supported). r=brendan

--HG--
extra : rebase_source : 7d47eb44d21f17bfa11aeadca2ff980336315eca

js/src/jsobj.cpp

@@ -4355,8 +4355,8 @@ js_NativeSet(JSContext *cx, JSObject *obj, JSScopeProperty *sprop, bool added,
          * to /dev/null.
          *
          * But we can't short-circuit if there's a scripted getter or setter
-         * since we might need to throw. In that case, we let SPROP_SET
-         * decide whether to throw an exception. See bug 478047.
+         * since we might need to throw. In that case, JSScopeProperty::set
+         * decides whether to throw an exception. See bug 478047.
          */
         if (!(sprop->attrs & JSPROP_GETTER) && SPROP_HAS_STUB_SETTER(sprop)) {
             JS_ASSERT(!(sprop->attrs & JSPROP_SETTER));
@@ -6084,17 +6084,19 @@ js_IsCallable(JSObject *obj, JSContext *cx)
     return callable;
 }
 
-void
+JSBool
 js_ReportGetterOnlyAssignment(JSContext *cx)
 {
-    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL,
-                         JSMSG_GETTER_ONLY, NULL);
+    return JS_ReportErrorFlagsAndNumber(cx,
+                                        JSREPORT_WARNING | JSREPORT_STRICT,
+                                        js_GetErrorMessage, NULL,
+                                        JSMSG_GETTER_ONLY);
 }
 
 JS_FRIEND_API(JSBool)
 js_GetterOnlyPropertyStub(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
 {
-    js_ReportGetterOnlyAssignment(cx);
+    JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_GETTER_ONLY);
     return JS_FALSE;
 }
 

js/src/jsobj.h

@@ -980,7 +980,7 @@ js_ComputeFilename(JSContext *cx, JSStackFrame *caller,
 extern JSBool
 js_IsCallable(JSObject *obj, JSContext *cx);
 
-void
+extern JSBool
 js_ReportGetterOnlyAssignment(JSContext *cx);
 
 extern JS_FRIEND_API(JSBool)

js/src/jsscope.h

@@ -691,10 +691,8 @@ JSScopeProperty::set(JSContext* cx, JSObject* obj, jsval* vp)
         return js_InternalGetOrSet(cx, obj, id, fval, JSACC_WRITE, 1, vp, vp);
     }
 
-    if (attrs & JSPROP_GETTER) {
-        js_ReportGetterOnlyAssignment(cx);
-        return false;
-    }
+    if (attrs & JSPROP_GETTER)
+        return !!js_ReportGetterOnlyAssignment(cx);
 
     /* See the comment in JSScopeProperty::get as to why we can check for With. */
     if (STOBJ_GET_CLASS(obj) == &js_WithClass)

js/src/tests/ecma_3_1/extensions/jstests.list

@@ -1,2 +1 @@
 url-prefix ../../jsreftest.html?test=ecma_3_1/extensions/
-script regress-478047.js

js/src/tests/ecma_3_1/extensions/regress-478047.js

@@ -1,87 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* ***** BEGIN LICENSE BLOCK *****
- * Version: MPL 1.1/GPL 2.0/LGPL 2.1
- *
- * The contents of this file are subject to the Mozilla Public License Version
- * 1.1 (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- * http://www.mozilla.org/MPL/
- *
- * Software distributed under the License is distributed on an "AS IS" basis,
- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
- * for the specific language governing rights and limitations under the
- * License.
- *
- * The Original Code is JavaScript Engine testing utilities.
- *
- * The Initial Developer of the Original Code is
- * Mozilla Foundation.
- * Portions created by the Initial Developer are Copyright (C) 2008
- * the Initial Developer. All Rights Reserved.
- *
- * Contributor(s): Norris Boyd
- *
- * Alternatively, the contents of this file may be used under the terms of
- * either the GNU General Public License Version 2 or later (the "GPL"), or
- * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
- * in which case the provisions of the GPL or the LGPL are applicable instead
- * of those above. If you wish to allow use of your version of this file only
- * under the terms of either the GPL or the LGPL, and not to allow others to
- * use your version of this file under the terms of the MPL, indicate your
- * decision by deleting the provisions above and replace them with the notice
- * and other provisions required by the GPL or the LGPL. If you do not delete
- * the provisions above, a recipient may use your version of this file under
- * the terms of any one of the MPL, the GPL or the LGPL.
- *
- * ***** END LICENSE BLOCK ***** */
-
-var gTestfile = 'regress-478047.js';
-//-----------------------------------------------------------------------------
-var BUGNUMBER = 478047;
-var summary = 'Assign to property with getter but no setter should throw ' +
-  'TypeError';
-var actual = '';
-var expect = '';
-
-
-//-----------------------------------------------------------------------------
-test();
-//-----------------------------------------------------------------------------
-
-function test()
-{
-  enterFunc ('test');
-  printBugNumber(BUGNUMBER);
-  printStatus (summary);
-
-  expect = 'TypeError: setting a property that has only a getter';
-  try
-  { 
-    var o = { get p() { return "a"; } };
-    o.p = "b";
-  }
-  catch(ex)
-  {
-    actual = ex + '';
-  }
-  reportCompare(expect, actual, summary);
-
-
-  actual = '';
-  try
-  {
-    o = { get p() { return "a"; } };
-    T = (function () {});
-    T.prototype = o;
-    y = new T();
-    y.p = "b";
-  }
-  catch(ex)
-  {
-    actual = ex + '';
-  }
-
-  reportCompare(expect, actual, summary);
-
-  exitFunc ('test');
-}

js/src/tests/ecma_5/Object/browser.js

@@ -0,0 +1 @@
+

js/src/tests/ecma_5/Types/8.12.5-01.js

@@ -0,0 +1,120 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ * Contributor:
+ *   Jason Orendorff
+ *   Jeff Walden <jwalden+code@mit.edu>
+ */
+
+var gTestfile = "template.js";
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 523846;
+var summary =
+  "Assignments to a property that has a getter but not a setter should not " +
+  "throw a TypeError per ES5 (at least not until strict mode is supported)";
+var actual = "Early failure";
+var expect = "No errors";
+
+
+printBugNumber(BUGNUMBER);
+printStatus(summary);
+
+var o = { get p() { return "a"; } };
+
+function test1()
+{
+  o.p = "b";
+  assertEq(o.p, "a");
+}
+
+function test2()
+{
+  function T() {}
+  T.prototype = o;
+  y = new T();
+  y.p = "b";
+  assertEq(y.p, "a");
+}
+
+function strictTest1()
+{
+  "use strict";
+
+  o.p = "b"; // strict-mode violation here
+  assertEq(o.p, "a");
+}
+
+function strictTest2()
+{
+  "use strict";
+
+  function T() {}
+  T.prototype = o;
+  y = new T;
+  y.p = "b";  // strict-mode violation here
+  assertEq(y.p, "a");
+}
+
+// Feel free to tweak this as necessary to preserve the effectiveness of not
+// running the strict-mode correctness tests, as long as we don't support
+// strict mode and as long as we haven't updated our get/set code to throw when
+// setting a property that only has a getter in strict mode.
+var strictModeSupported = (function() { "use strict"; return !this; })();
+
+var errors = [];
+try
+{
+  try
+  {
+    test1();
+  }
+  catch (e)
+  {
+    errors.push(e);
+  }
+
+  try
+  {
+    test2();
+  }
+  catch (e)
+  {
+    errors.push(e);
+  }
+
+  if (strictModeSupported)
+  {
+    try
+    {
+      strictTest1();
+      errors.push("strictTest1 didn't fail");
+    }
+    catch (e)
+    {
+      if (!(e instanceof TypeError))
+        errors.push("strictTest1 didn't fail with a TypeError: " + e);
+    }
+
+    try
+    {
+      strictTest2();
+      errors.push("strictTest2 didn't fail");
+    }
+    catch (e)
+    {
+      if (!(e instanceof TypeError))
+        errors.push("strictTest2 didn't fail with a TypeError: " + e);
+    }
+  }
+}
+catch (e)
+{
+  errors.push("Unexpected error: " + e);
+}
+finally
+{
+  actual = errors.length > 0 ? errors.join(", ") : "No errors";
+}
+
+reportCompare(expect, actual, summary);

js/src/tests/ecma_5/Types/browser.js

@@ -0,0 +1 @@
+

js/src/tests/ecma_5/Types/jstests.list

@@ -0,0 +1,2 @@
+url-prefix ../../jsreftest.html?test=ecma_5/Types/
+script 8.12.5-01.js

js/src/tests/ecma_5/Types/shell.js

@@ -0,0 +1 @@
+gTestsubsuite='Types';

js/src/tests/ecma_5/extensions/8.12.5-01.js

@@ -0,0 +1,98 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ * Contributor:
+ *   Jason Orendorff
+ *   Jeff Walden <jwalden+code@mit.edu>
+ */
+
+var gTestfile = "template.js";
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 523846;
+var summary =
+  "Assignments to a property that has a getter but not a setter should not " +
+  "throw a TypeError per ES5 (at least not until strict mode is supported)";
+var actual = "Early failure";
+var expect = "No errors";
+
+
+printBugNumber(BUGNUMBER);
+printStatus(summary);
+
+var o = { get p() { return "a"; } };
+
+function test1()
+{
+  o.p = "b"; // strict-mode violation here
+  assertEq(o.p, "a");
+}
+
+function test2()
+{
+  function T() {}
+  T.prototype = o;
+  y = new T();
+  y.p = "b";  // strict-mode violation here
+  assertEq(y.p, "a");
+}
+
+
+var errors = [];
+try
+{
+  try
+  {
+    test1();
+  }
+  catch (e)
+  {
+    errors.push(e);
+  }
+
+  try
+  {
+    test2();
+  }
+  catch (e)
+  {
+    errors.push(e);
+  }
+
+  options("strict");
+  options("werror");
+  try
+  {
+    test1();
+    errors.push("strict+werror didn't make test1 fail");
+  }
+  catch (e)
+  {
+    if (!(e instanceof TypeError))
+      errors.push("test1 with strict+werror failed without a TypeError: " + e);
+  }
+
+  try
+  {
+    test2();
+    errors.push("strict+werror didn't make test2 fail");
+  }
+  catch (e)
+  {
+    if (!(e instanceof TypeError))
+      errors.push("test2 with strict+werror failed without a TypeError: " + e);
+  }
+
+  options("strict");
+  options("werror");
+}
+catch (e)
+{
+  errors.push("Unexpected error: " + e);
+}
+finally
+{
+  actual = errors.length > 0 ? errors.join(", ") : "No errors";
+}
+
+reportCompare(expect, actual, summary);

js/src/tests/ecma_5/extensions/browser.js

@@ -0,0 +1 @@
+

js/src/tests/ecma_5/extensions/jstests.list

@@ -0,0 +1,2 @@
+url-prefix ../../jsreftest.html?test=ecma_5/extensions/
+script 8.12.5-01.js

js/src/tests/ecma_5/extensions/shell.js

@@ -0,0 +1 @@
+gTestsubsuite='extensions';

js/src/tests/js1_5/extensions/regress-452178.js

@@ -54,8 +54,6 @@ function test()
 
   jit(true);
 
-  expect = 'TypeError: setting a property that has only a getter';
-
   try
   {
     q getter= function(){}; for (var j = 0; j < 4; ++j) q = 1;

js/src/tests/js1_6/extensions/regress-414098.js

@@ -67,7 +67,7 @@ catch(ex)
 {
   actual = ex + '';
 }
-reportCompare("TypeError: setting a property that has only a getter", actual, "sort");
+reportCompare("1,54,54", actual, "sort");
 
 try
 {
@@ -77,7 +77,7 @@ catch(ex)
 {
   actual = ex + '';
 }
-reportCompare("TypeError: setting a property that has only a getter", actual, "setter");
+reportCompare("1,54,54", actual, "setter");
 
 actual = a.pop();
 reportCompare(3, actual, "pop");