LibJS: Strict mode is now handled by Functions and Programs, not Blocks

Since blocks can't be strict by themselves, it makes no sense for them
to store whether or not they are strict. Strict-ness is now stored in
the Program and FunctionNode ASTNodes. Fixes issue #3641

10 files changed, 92 insertions, 37 deletions

diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp
index a8a1b44cdc..ccdb818d1b 100644
--- a/Libraries/LibJS/AST.cpp
+++ b/Libraries/LibJS/AST.cpp
@@ -89,6 +89,11 @@ Value ScopeNode::execute(Interpreter& interpreter, GlobalObject& global_object)
     return interpreter.execute_statement(global_object, *this);
 }
 
+Value Program::execute(Interpreter& interpreter, GlobalObject& global_object) const
+{
+    return interpreter.execute_statement(global_object, *this, {}, ScopeType::Block, m_is_strict_mode);
+}
+
 Value FunctionDeclaration::execute(Interpreter&, GlobalObject&) const
 {
     return js_undefined();
@@ -96,7 +101,7 @@ Value FunctionDeclaration::execute(Interpreter&, GlobalObject&) const
 
 Value FunctionExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const
 {
-    return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.current_environment(), m_is_arrow_function);
+    return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.current_environment(), is_strict_mode(), m_is_arrow_function);
 }
 
 Value ExpressionStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const
diff --git a/Libraries/LibJS/AST.h b/Libraries/LibJS/AST.h
index 057162b43f..cdefaf6f78 100644
--- a/Libraries/LibJS/AST.h
+++ b/Libraries/LibJS/AST.h
@@ -129,8 +129,6 @@ public:
     void add_functions(NonnullRefPtrVector<FunctionDeclaration>);
     const NonnullRefPtrVector<VariableDeclaration>& variables() const { return m_variables; }
     const NonnullRefPtrVector<FunctionDeclaration>& functions() const { return m_functions; }
-    bool in_strict_mode() const { return m_strict_mode; }
-    void set_strict_mode() { m_strict_mode = true; }
 
 protected:
     ScopeNode() { }
@@ -140,14 +138,20 @@ private:
     NonnullRefPtrVector<Statement> m_children;
     NonnullRefPtrVector<VariableDeclaration> m_variables;
     NonnullRefPtrVector<FunctionDeclaration> m_functions;
-    bool m_strict_mode { false };
 };
 
 class Program final : public ScopeNode {
 public:
     Program() { }
 
+    virtual Value execute(Interpreter&, GlobalObject&) const override;
+
+    bool is_strict_mode() const { return m_is_strict_mode; }
+    void set_strict_mode() { m_is_strict_mode = true; }
+
 private:
+    bool m_is_strict_mode { false };
+
     virtual bool is_program() const override { return true; }
     virtual const char* class_name() const override { return "Program"; }
 };
@@ -180,14 +184,16 @@ public:
     const Statement& body() const { return *m_body; }
     const Vector<Parameter>& parameters() const { return m_parameters; };
     i32 function_length() const { return m_function_length; }
+    bool is_strict_mode() const { return m_is_strict_mode; }
 
 protected:
-    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
+    FunctionNode(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_strict_mode)
         : m_name(name)
         , m_body(move(body))
         , m_parameters(move(parameters))
         , m_variables(move(variables))
         , m_function_length(function_length)
+        , m_is_strict_mode(is_strict_mode)
     {
     }
 
@@ -201,6 +207,7 @@ private:
     const Vector<Parameter> m_parameters;
     NonnullRefPtrVector<VariableDeclaration> m_variables;
     const i32 m_function_length;
+    bool m_is_strict_mode;
 };
 
 class FunctionDeclaration final
@@ -209,8 +216,8 @@ class FunctionDeclaration final
 public:
     static bool must_have_name() { return true; }
 
-    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables)
-        : FunctionNode(name, move(body), move(parameters), function_length, move(variables))
+    FunctionDeclaration(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_strict_mode = false)
+        : FunctionNode(name, move(body), move(parameters), function_length, move(variables), is_strict_mode)
     {
     }
 
@@ -227,8 +234,8 @@ class FunctionExpression final
 public:
     static bool must_have_name() { return false; }
 
-    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_arrow_function = false)
-        : FunctionNode(name, move(body), move(parameters), function_length, move(variables))
+    FunctionExpression(const FlyString& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, NonnullRefPtrVector<VariableDeclaration> variables, bool is_strict_mode, bool is_arrow_function = false)
+        : FunctionNode(name, move(body), move(parameters), function_length, move(variables), is_strict_mode)
         , m_is_arrow_function(is_arrow_function)
     {
     }
diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp
index 2f4e80be37..a1351e7c64 100644
--- a/Libraries/LibJS/Interpreter.cpp
+++ b/Libraries/LibJS/Interpreter.cpp
@@ -91,15 +91,15 @@ const GlobalObject& Interpreter::global_object() const
     return static_cast<const GlobalObject&>(*m_global_object.cell());
 }
 
-void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type, GlobalObject& global_object)
+void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type, GlobalObject& global_object, bool is_strict)
 {
     for (auto& declaration : scope_node.functions()) {
-        auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment());
+        auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment(), is_strict);
         vm().set_variable(declaration.name(), function, global_object);
     }
 
     if (scope_type == ScopeType::Function) {
-        m_scope_stack.append({ scope_type, scope_node, false });
+        push_scope({ scope_type, scope_node, false, is_strict });
         return;
     }
 
@@ -130,7 +130,7 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector argume
         pushed_lexical_environment = true;
     }
 
-    m_scope_stack.append({ scope_type, scope_node, pushed_lexical_environment });
+    push_scope({ scope_type, scope_node, pushed_lexical_environment, is_strict });
 }
 
 void Interpreter::exit_scope(const ScopeNode& scope_node)
@@ -148,13 +148,20 @@ void Interpreter::exit_scope(const ScopeNode& scope_node)
         vm().unwind(ScopeType::None);
 }
 
-Value Interpreter::execute_statement(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type)
+void Interpreter::push_scope(ScopeFrame frame)
+{
+    if (in_strict_mode())
+        frame.is_strict_mode = true;
+    m_scope_stack.append(move(frame));
+}
+
+Value Interpreter::execute_statement(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type, bool is_strict)
 {
     if (!statement.is_scope_node())
         return statement.execute(*this, global_object);
 
     auto& block = static_cast<const ScopeNode&>(statement);
-    enter_scope(block, move(arguments), scope_type, global_object);
+    enter_scope(block, move(arguments), scope_type, global_object, is_strict);
 
     if (block.children().is_empty())
         vm().set_last_value({}, js_undefined());
diff --git a/Libraries/LibJS/Interpreter.h b/Libraries/LibJS/Interpreter.h
index afdc9a667a..40c4cc42c4 100644
--- a/Libraries/LibJS/Interpreter.h
+++ b/Libraries/LibJS/Interpreter.h
@@ -88,10 +88,9 @@ public:
 
     bool in_strict_mode() const
     {
-        // FIXME: This implementation is bogus; strict mode is per-file or per-function, not per-block!
         if (m_scope_stack.is_empty())
-            return true;
-        return m_scope_stack.last().scope_node->in_strict_mode();
+            return false;
+        return m_scope_stack.last().is_strict_mode;
     }
 
     size_t argument_count() const { return vm().argument_count(); }
@@ -100,10 +99,10 @@ public:
     LexicalEnvironment* current_environment() { return vm().current_environment(); }
     const CallFrame& call_frame() { return vm().call_frame(); }
 
-    void enter_scope(const ScopeNode&, ArgumentVector, ScopeType, GlobalObject&);
+    void enter_scope(const ScopeNode&, ArgumentVector, ScopeType, GlobalObject&, bool is_strict = false);
     void exit_scope(const ScopeNode&);
 
-    Value execute_statement(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block);
+    Value execute_statement(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block, bool is_strict = false);
 
 private:
     explicit Interpreter(VM&);
@@ -113,11 +112,13 @@ private:
         return vm().call(function, this_value, move(arguments));
     }
 
+    void push_scope(ScopeFrame frame);
+
+    Vector<ScopeFrame> m_scope_stack;
+
     NonnullRefPtr<VM> m_vm;
 
     Handle<Object> m_global_object;
-
-    Vector<ScopeFrame> m_scope_stack;
 };
 
 template<>
diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp
index 6de165e6f4..7383277f3f 100644
--- a/Libraries/LibJS/Parser.cpp
+++ b/Libraries/LibJS/Parser.cpp
@@ -390,10 +390,12 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     if (function_length == -1)
         function_length = parameters.size();
 
-    auto function_body_result = [this]() -> RefPtr<BlockStatement> {
+    bool is_strict = false;
+
+    auto function_body_result = [&]() -> RefPtr<BlockStatement> {
         if (match(TokenType::CurlyOpen)) {
             // Parse a function body with statements
-            return parse_block_statement();
+            return parse_block_statement(is_strict);
         }
         if (match_expression()) {
             // Parse a function body which returns a single expression
@@ -414,7 +416,7 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
     if (!function_body_result.is_null()) {
         state_rollback_guard.disarm();
         auto body = function_body_result.release_nonnull();
-        return create_ast_node<FunctionExpression>("", move(body), move(parameters), function_length, m_parser_state.m_var_scopes.take_last(), true);
+        return create_ast_node<FunctionExpression>("", move(body), move(parameters), function_length, m_parser_state.m_var_scopes.take_last(), is_strict, true);
     }
 
     return nullptr;
@@ -562,9 +564,9 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
             constructor_body->append(create_ast_node<ExpressionStatement>(move(super_call)));
             constructor_body->add_variables(m_parser_state.m_var_scopes.last());
 
-            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector { FunctionNode::Parameter { "args", nullptr, true } }, 0, NonnullRefPtrVector<VariableDeclaration>());
+            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector { FunctionNode::Parameter { "args", nullptr, true } }, 0, NonnullRefPtrVector<VariableDeclaration>(), true);
         } else {
-            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector<FunctionNode::Parameter> {}, 0, NonnullRefPtrVector<VariableDeclaration>());
+            constructor = create_ast_node<FunctionExpression>(class_name, move(constructor_body), Vector<FunctionNode::Parameter> {}, 0, NonnullRefPtrVector<VariableDeclaration>(), true);
         }
     }
 
@@ -1204,6 +1206,12 @@ NonnullRefPtr<ReturnStatement> Parser::parse_return_statement()
 
 NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
 {
+    bool dummy = false;
+    return parse_block_statement(dummy);
+}
+
+NonnullRefPtr<BlockStatement> Parser::parse_block_statement(bool& is_strict)
+{
     ScopePusher scope(*this, ScopePusher::Let);
     auto block = create_ast_node<BlockStatement>();
     consume(TokenType::CurlyOpen);
@@ -1212,7 +1220,7 @@ NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
     bool initial_strict_mode_state = m_parser_state.m_strict_mode;
     if (initial_strict_mode_state) {
         m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
-        block->set_strict_mode();
+        is_strict = true;
     } else {
         m_parser_state.m_use_strict_directive = UseStrictDirectiveState::Looking;
     }
@@ -1225,7 +1233,7 @@ NonnullRefPtr<BlockStatement> Parser::parse_block_statement()
 
             if (first && !initial_strict_mode_state) {
                 if (m_parser_state.m_use_strict_directive == UseStrictDirectiveState::Found) {
-                    block->set_strict_mode();
+                    is_strict = true;
                     m_parser_state.m_strict_mode = true;
                 }
                 m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None;
@@ -1292,10 +1300,11 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(bool check_for_funct
     if (function_length == -1)
         function_length = parameters.size();
 
-    auto body = parse_block_statement();
+    bool is_strict = false;
+    auto body = parse_block_statement(is_strict);
     body->add_variables(m_parser_state.m_var_scopes.last());
     body->add_functions(m_parser_state.m_function_scopes.last());
-    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), function_length, NonnullRefPtrVector<VariableDeclaration>());
+    return create_ast_node<FunctionNodeType>(name, move(body), move(parameters), function_length, NonnullRefPtrVector<VariableDeclaration>(), is_strict);
 }
 
 NonnullRefPtr<VariableDeclaration> Parser::parse_variable_declaration(bool with_semicolon)
diff --git a/Libraries/LibJS/Parser.h b/Libraries/LibJS/Parser.h
index f6d45adca8..e3ee817c64 100644
--- a/Libraries/LibJS/Parser.h
+++ b/Libraries/LibJS/Parser.h
@@ -50,6 +50,7 @@ public:
 
     NonnullRefPtr<Statement> parse_statement();
     NonnullRefPtr<BlockStatement> parse_block_statement();
+    NonnullRefPtr<BlockStatement> parse_block_statement(bool& is_strict);
     NonnullRefPtr<ReturnStatement> parse_return_statement();
     NonnullRefPtr<VariableDeclaration> parse_variable_declaration(bool with_semicolon = true);
     NonnullRefPtr<Statement> parse_for_statement();
diff --git a/Libraries/LibJS/Runtime/ScriptFunction.cpp b/Libraries/LibJS/Runtime/ScriptFunction.cpp
index aea79317e8..76a001c131 100644
--- a/Libraries/LibJS/Runtime/ScriptFunction.cpp
+++ b/Libraries/LibJS/Runtime/ScriptFunction.cpp
@@ -47,18 +47,19 @@ static ScriptFunction* typed_this(VM& vm, GlobalObject& global_object)
     return static_cast<ScriptFunction*>(this_object);
 }
 
-ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_arrow_function)
+ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_strict, bool is_arrow_function)
 {
-    return global_object.heap().allocate<ScriptFunction>(global_object, global_object, name, body, move(parameters), m_function_length, parent_environment, *global_object.function_prototype(), is_arrow_function);
+    return global_object.heap().allocate<ScriptFunction>(global_object, global_object, name, body, move(parameters), m_function_length, parent_environment, *global_object.function_prototype(), is_strict, is_arrow_function);
 }
 
-ScriptFunction::ScriptFunction(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_arrow_function)
+ScriptFunction::ScriptFunction(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_strict, bool is_arrow_function)
     : Function(prototype, is_arrow_function ? vm().this_value(global_object) : Value(), {})
     , m_name(name)
     , m_body(body)
     , m_parameters(move(parameters))
     , m_parent_environment(parent_environment)
     , m_function_length(m_function_length)
+    , m_is_strict(is_strict)
     , m_is_arrow_function(is_arrow_function)
 {
 }
@@ -140,7 +141,7 @@ Value ScriptFunction::call()
         arguments.append({ parameter.name, value });
         vm().current_environment()->set(global_object(), parameter.name, { value, DeclarationKind::Var });
     }
-    return interpreter->execute_statement(global_object(), m_body, arguments, ScopeType::Function);
+    return interpreter->execute_statement(global_object(), m_body, arguments, ScopeType::Function, m_is_strict);
 }
 
 Value ScriptFunction::construct(Function&)
diff --git a/Libraries/LibJS/Runtime/ScriptFunction.h b/Libraries/LibJS/Runtime/ScriptFunction.h
index 3f0b2276ed..335ca441d5 100644
--- a/Libraries/LibJS/Runtime/ScriptFunction.h
+++ b/Libraries/LibJS/Runtime/ScriptFunction.h
@@ -35,9 +35,9 @@ class ScriptFunction final : public Function {
     JS_OBJECT(ScriptFunction, Function);
 
 public:
-    static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_arrow_function = false);
+    static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_strict, bool is_arrow_function = false);
 
-    ScriptFunction(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_arrow_function = false);
+    ScriptFunction(GlobalObject&, const FlyString& name, const Statement& body, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_strict, bool is_arrow_function = false);
     virtual void initialize(GlobalObject&) override;
     virtual ~ScriptFunction();
 
@@ -63,6 +63,7 @@ private:
     const Vector<FunctionNode::Parameter> m_parameters;
     LexicalEnvironment* m_parent_environment { nullptr };
     i32 m_function_length;
+    bool m_is_strict;
     bool m_is_arrow_function;
 };
 
diff --git a/Libraries/LibJS/Runtime/VM.h b/Libraries/LibJS/Runtime/VM.h
index a602febae7..045318e60b 100644
--- a/Libraries/LibJS/Runtime/VM.h
+++ b/Libraries/LibJS/Runtime/VM.h
@@ -50,6 +50,7 @@ struct ScopeFrame {
     ScopeType type;
     NonnullRefPtr<ScopeNode> scope_node;
     bool pushed_environment { false };
+    bool is_strict_mode { false };
 };
 
 struct CallFrame {
diff --git a/Libraries/LibJS/Tests/strict-mode-blocks.js b/Libraries/LibJS/Tests/strict-mode-blocks.js
new file mode 100644
index 0000000000..c1fe1a7d24
--- /dev/null
+++ b/Libraries/LibJS/Tests/strict-mode-blocks.js
@@ -0,0 +1,22 @@
+test("Issue #3641, strict mode should be function- or program-level, not block-level", () => {
+    function func() {
+        expect(isStrictMode()).toBeFalse();
+
+        {
+            "use strict";
+            expect(isStrictMode()).toBeFalse();
+        }
+
+        if (true) {
+            "use strict";
+            expect(isStrictMode()).toBeFalse();
+        }
+
+        do {
+            "use strict";
+            expect(isStrictMode()).toBeFalse();
+        } while (false);
+    }
+
+    func();
+});