Bug 609832: Function statements should be banned (for now) in ES5 strict mode. (r=cdleary)
authorJim Blandy <jimb@mozilla.com>
Fri, 07 Jan 2011 17:56:22 -0800
changeset 60253 b0f047a6a9daeb63ea8fa206f95efe39b7dd96c3
parent 60252 08f2504e5f44f73caf794cab922763428e826e5a
child 60254 a213cb8ca3961466294da925ab0bddea1219cb29
push idunknown
push userunknown
push dateunknown
reviewerscdleary
bugs609832
milestone2.0b9pre
Bug 609832: Function statements should be banned (for now) in ES5 strict mode. (r=cdleary)
js/src/js.msg
js/src/jsparse.cpp
js/src/tests/ecma_5/extensions/jstests.list
js/src/tests/ecma_5/extensions/shell.js
js/src/tests/ecma_5/extensions/strict-function-statements.js
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -341,8 +341,9 @@ MSG_DEF(JSMSG_STRICT_CODE_LET_EXPR_STMT,
 MSG_DEF(JSMSG_CANT_CHANGE_EXTENSIBILITY, 259, 0, JSEXN_TYPEERR, "can't change object's extensibility")
 MSG_DEF(JSMSG_SC_BAD_SERIALIZED_DATA, 260, 1, JSEXN_INTERNALERR, "bad serialized structured data ({0})")
 MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE,    261, 0, JSEXN_TYPEERR, "unsupported type for structured data")
 MSG_DEF(JSMSG_SC_RECURSION,           262, 0, JSEXN_INTERNALERR, "recursive object")
 MSG_DEF(JSMSG_CANT_WRAP_XML_OBJECT,   263, 0, JSEXN_TYPEERR, "can't wrap XML objects")
 MSG_DEF(JSMSG_BAD_CLONE_VERSION,      264, 0, JSEXN_ERR, "unsupported structured clone version")
 MSG_DEF(JSMSG_CANT_CLONE_OBJECT,      265, 0, JSEXN_TYPEERR, "can't clone object")
 MSG_DEF(JSMSG_NON_NATIVE_SCOPE,       266, 0, JSEXN_TYPEERR, "non-native scope object")
+MSG_DEF(JSMSG_STRICT_FUNCTION_STATEMENT, 267, 0, JSEXN_SYNTAXERR, "in strict mode code, functions may only be declared at top level or immediately within another function")
--- a/js/src/jsparse.cpp
+++ b/js/src/jsparse.cpp
@@ -3239,16 +3239,23 @@ Parser::functionStmt()
     } else {
         if (context->options & JSOPTION_ANONFUNFIX) {
             /* Extension: accept unnamed function expressions as statements. */
             reportErrorNumber(NULL, JSREPORT_ERROR, JSMSG_SYNTAX_ERROR);
             return NULL;
         }
         tokenStream.ungetToken();
     }
+
+    /* We forbid function statements in strict mode code. */
+    if (!tc->atBodyLevel() && tc->inStrictMode()) {
+        reportErrorNumber(NULL, JSREPORT_STRICT_MODE_ERROR, JSMSG_STRICT_FUNCTION_STATEMENT);
+        return NULL;
+    }
+
     return functionDef(name, GENERAL, 0);
 }
 
 JSParseNode *
 Parser::functionExpr()
 {
     JSAtom *name = NULL;
     if (tokenStream.getToken(TSF_KEYWORD_IS_NAME) == TOK_NAME)
--- a/js/src/tests/ecma_5/extensions/jstests.list
+++ b/js/src/tests/ecma_5/extensions/jstests.list
@@ -16,8 +16,9 @@ script eval-native-callback-is-indirect.
 script regress-bug607284.js
 script Object-keys-and-object-ids.js
 fails script nested-delete-name-in-evalcode.js # bug 604301, at a minimum
 script bug352085.js
 script bug472534.js
 script bug496985.js
 script bug566661.js
 script iterator-in-catch.js
+script strict-function-statements.js
--- a/js/src/tests/ecma_5/extensions/shell.js
+++ b/js/src/tests/ecma_5/extensions/shell.js
@@ -1,37 +0,0 @@
-/* -*- 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/
- */
-
-/*
- * Return true if both of these return true:
- * - LENIENT_PRED applied to CODE
- * - STRICT_PRED applied to CODE with a use strict directive added to the front
- *
- * Run STRICT_PRED first, for testing code that affects the global environment
- * in loose mode, but fails in strict mode.
- */
-function testLenientAndStrict(code, lenient_pred, strict_pred) {
-  return (strict_pred("'use strict'; " + code) && 
-          lenient_pred(code));
-}
-
-/*
- * raisesException(EXCEPTION)(CODE) returns true if evaluating CODE (as eval
- * code) throws an exception object whose prototype is
- * EXCEPTION.prototype, and returns false if it throws any other error
- * or evaluates successfully. For example: raises(TypeError)("0()") ==
- * true.
- */
-function raisesException(exception) {
-  return function (code) {
-    try {
-      eval(code);
-      return false;
-    } catch (actual) {
-      return exception.prototype.isPrototypeOf(actual);
-    }
-  };
-};
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_5/extensions/strict-function-statements.js
@@ -0,0 +1,100 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+// Ordinary function definitions should be unaffected.
+assertEq(testLenientAndStrict("function f() { }",
+                              parsesSuccessfully,
+                              parsesSuccessfully),
+         true);
+
+// Function statements within blocks are forbidden in strict mode code.
+assertEq(testLenientAndStrict("{ function f() { } }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+
+// Lambdas are always permitted within blocks.
+assertEq(testLenientAndStrict("{ (function f() { }) }",
+                              parsesSuccessfully,
+                              parsesSuccessfully),
+         true);
+
+// Function statements within any sort of statement are forbidden in strict mode code.
+assertEq(testLenientAndStrict("if (true) function f() { }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("while (true) function f() { }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("do function f() { } while (true);",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("for(;;) function f() { }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("for(x in []) function f() { }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("with(o) function f() { }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("switch(1) { case 1: function f() { } }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("x: function f() { }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+assertEq(testLenientAndStrict("try { function f() { } } catch (x) { }",
+                              parsesSuccessfully,
+                              parseRaisesException(SyntaxError)),
+         true);
+
+// Lambdas are always permitted within any sort of statement.
+assertEq(testLenientAndStrict("if (true) (function f() { })",
+                              parsesSuccessfully,
+                              parsesSuccessfully),
+         true);
+
+// Function statements are permitted in blocks within lenient functions.
+assertEq(parsesSuccessfully("function f() { function g() { } }"),
+         true);
+
+// Function statements are permitted in any statement within lenient functions.
+assertEq(parsesSuccessfully("function f() { if (true) function g() { } }"),
+         true);
+
+assertEq(parseRaisesException(SyntaxError)
+         ("function f() { 'use strict'; if (true) function g() { } }"),
+         true);
+
+assertEq(parseRaisesException(SyntaxError)
+         ("function f() { 'use strict'; { function g() { } } }"),
+         true);
+
+assertEq(parsesSuccessfully("function f() { 'use strict'; if (true) (function g() { }) }"),
+         true);
+
+assertEq(parsesSuccessfully("function f() { 'use strict'; { (function g() { }) } }"),
+         true);
+
+// Eval should behave the same way. (The parse-only tests use the Function constructor.)
+assertEq(testLenientAndStrict("function f() { }",
+                              completesNormally,
+                              completesNormally),
+         true);
+assertEq(testLenientAndStrict("{ function f() { } }",
+                              completesNormally,
+                              raisesException(SyntaxError)),
+         true);
+
+reportCompare(true, true);