LibJS: Split out NewExpression evaluation from CallExpression

This patch adds an override for NewExpression::execute() in the AST
interpreter to separate the logic from CallExpression. As a result,
both evaluation functions are simplified.

Both expressions are still largely non-conforming, but this makes
it easier to work on improving that since we can now deal with them
separately. :^)

2 files changed, 66 insertions, 44 deletions

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index 15ff3b8e33..20e8294fbd 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -192,78 +192,93 @@ static void argument_list_evaluation(Interpreter& interpreter, GlobalObject& glo
     }
 }
 
-Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const
+Value NewExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const
 {
     InterpreterNodeScope node_scope { interpreter, *this };
     auto& vm = interpreter.vm();
+
     auto [this_value, callee] = compute_this_and_callee(interpreter, global_object);
     if (vm.exception())
         return {};
 
-    VERIFY(!callee.is_empty());
+    if (!callee.is_function() || (is<NativeFunction>(callee.as_object()) && !static_cast<NativeFunction&>(callee.as_object()).has_constructor())) {
+        throw_type_error_for_callee(interpreter, global_object, callee, "constructor"sv);
+        return {};
+    }
 
-    if (!callee.is_function()
-        || (is<NewExpression>(*this) && (is<NativeFunction>(callee.as_object()) && !static_cast<NativeFunction&>(callee.as_object()).has_constructor()))) {
-        String error_message;
-        auto call_type = is<NewExpression>(*this) ? "constructor" : "function";
-        if (is<Identifier>(*m_callee) || is<MemberExpression>(*m_callee)) {
-            String expression_string;
-            if (is<Identifier>(*m_callee)) {
-                expression_string = static_cast<Identifier const&>(*m_callee).string();
-            } else {
-                expression_string = static_cast<MemberExpression const&>(*m_callee).to_string_approximation();
-            }
-            vm.throw_exception<TypeError>(global_object, ErrorType::IsNotAEvaluatedFrom, callee.to_string_without_side_effects(), call_type, expression_string);
+    MarkedValueList arg_list(vm.heap());
+    argument_list_evaluation(interpreter, global_object, m_arguments, arg_list);
+    if (interpreter.exception())
+        return {};
+
+    auto& function = callee.as_function();
+    return vm.construct(function, function, move(arg_list));
+}
+
+void CallExpression::throw_type_error_for_callee(Interpreter& interpreter, GlobalObject& global_object, Value callee_value, StringView call_type) const
+{
+    auto& vm = interpreter.vm();
+    if (is<Identifier>(*m_callee) || is<MemberExpression>(*m_callee)) {
+        String expression_string;
+        if (is<Identifier>(*m_callee)) {
+            expression_string = static_cast<Identifier const&>(*m_callee).string();
         } else {
-            vm.throw_exception<TypeError>(global_object, ErrorType::IsNotA, callee.to_string_without_side_effects(), call_type);
+            expression_string = static_cast<MemberExpression const&>(*m_callee).to_string_approximation();
         }
-        return {};
+        vm.throw_exception<TypeError>(global_object, ErrorType::IsNotAEvaluatedFrom, callee_value.to_string_without_side_effects(), call_type, expression_string);
+    } else {
+        vm.throw_exception<TypeError>(global_object, ErrorType::IsNotA, callee_value.to_string_without_side_effects(), call_type);
     }
+}
 
-    auto& function = callee.as_function();
+Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const
+{
+    InterpreterNodeScope node_scope { interpreter, *this };
+    auto& vm = interpreter.vm();
+    auto [this_value, callee] = compute_this_and_callee(interpreter, global_object);
+    if (vm.exception())
+        return {};
+
+    VERIFY(!callee.is_empty());
+
+    if (!callee.is_function()) {
+        throw_type_error_for_callee(interpreter, global_object, callee, "function"sv);
+        return {};
+    }
 
     MarkedValueList arg_list(vm.heap());
     argument_list_evaluation(interpreter, global_object, m_arguments, arg_list);
     if (interpreter.exception())
         return {};
 
-    if (!is<NewExpression>(*this) && is<Identifier>(*m_callee) && static_cast<Identifier const&>(*m_callee).string() == vm.names.eval.as_string() && &callee.as_function() == global_object.eval_function()) {
+    auto& function = callee.as_function();
+
+    if (is<Identifier>(*m_callee) && static_cast<Identifier const&>(*m_callee).string() == vm.names.eval.as_string() && &function == global_object.eval_function()) {
         auto script_value = arg_list.size() == 0 ? js_undefined() : arg_list[0];
         return perform_eval(script_value, global_object, vm.in_strict_mode() ? CallerMode::Strict : CallerMode::NonStrict, EvalMode::Direct);
     }
 
-    vm.running_execution_context().current_node = interpreter.current_node();
-    Object* new_object = nullptr;
     Value result;
-    if (is<NewExpression>(*this)) {
-        result = vm.construct(function, function, move(arg_list));
-        if (result.is_object())
-            new_object = &result.as_object();
-    } else if (is<SuperExpression>(*m_callee)) {
-        auto* super_constructor = get_super_constructor(interpreter.vm());
-        // FIXME: Functions should track their constructor kind.
-        if (!super_constructor || !super_constructor->is_function()) {
-            vm.throw_exception<TypeError>(global_object, ErrorType::NotAConstructor, "Super constructor");
-            return {};
-        }
-        result = vm.construct(static_cast<FunctionObject&>(*super_constructor), function, move(arg_list));
-        if (vm.exception())
-            return {};
 
-        auto& this_er = get_this_environment(interpreter.vm());
-        verify_cast<FunctionEnvironment>(this_er).bind_this_value(global_object, result);
-    } else {
-        result = vm.call(function, this_value, move(arg_list));
+    if (!is<SuperExpression>(*m_callee))
+        return vm.call(function, this_value, move(arg_list));
+
+    auto* super_constructor = get_super_constructor(interpreter.vm());
+    // FIXME: Functions should track their constructor kind.
+    if (!super_constructor || !super_constructor->is_function()) {
+        vm.throw_exception<TypeError>(global_object, ErrorType::NotAConstructor, "Super constructor");
+        return {};
     }
+    result = vm.construct(static_cast<FunctionObject&>(*super_constructor), function, move(arg_list));
+    if (vm.exception())
+        return {};
+
+    auto& this_er = get_this_environment(interpreter.vm());
+    verify_cast<FunctionEnvironment>(this_er).bind_this_value(global_object, result);
 
     if (vm.exception())
         return {};
 
-    if (is<NewExpression>(*this)) {
-        if (result.is_object())
-            return result;
-        return new_object;
-    }
     return result;
 }
 
diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h
index 121e15ae90..7009e8314b 100644
--- a/Userland/Libraries/LibJS/AST.h
+++ b/Userland/Libraries/LibJS/AST.h
@@ -922,11 +922,16 @@ public:
     virtual void dump(int indent) const override;
     virtual void generate_bytecode(Bytecode::Generator&) const override;
 
-private:
+    Expression const& callee() const { return m_callee; }
+
+protected:
+    void throw_type_error_for_callee(Interpreter&, GlobalObject&, Value callee_value, StringView call_type) const;
+
     struct ThisAndCallee {
         Value this_value;
         Value callee;
     };
+
     ThisAndCallee compute_this_and_callee(Interpreter&, GlobalObject&) const;
 
     NonnullRefPtr<Expression> m_callee;
@@ -940,6 +945,8 @@ public:
     {
     }
 
+    virtual Value execute(Interpreter&, GlobalObject&) const override;
+
     virtual bool is_new_expression() const override { return true; }
 };