From 99f9609e451ddf00318c8887545bfb77c2e59b4e Mon Sep 17 00:00:00 2001
From: Linus Groh <mail@linusgroh.de>
Date: Mon, 13 Sep 2021 17:44:08 +0100
Subject: [PATCH] LibJS: Evaluate function arguments before checking callee
 type

In the spec, this happens in the EvaluateCall abstract operation
(https://tc39.es/ecma262/#sec-evaluatecall), and the order is defined
as:

    3. Let argList be ? ArgumentListEvaluation of arguments.
    4. If Type(func) is not Object, throw a TypeError exception.
    5. If IsCallable(func) is false, throw a TypeError exception.

In LibJS this is handled by CallExpression::execute(), which had the
callee function check first and would therefore never evaluate the
arguments for a non-function callee.
---
 Userland/Libraries/LibJS/AST.cpp              | 10 +++----
 .../functions/function-evaluation-order.js    | 30 +++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)
 create mode 100644 Userland/Libraries/LibJS/Tests/functions/function-evaluation-order.js

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index 9945689bb1..1fb878c2be 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -239,16 +239,16 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj
 
     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 (!callee.is_function()) {
+        throw_type_error_for_callee(interpreter, global_object, callee, "function"sv);
+        return {};
+    }
+
     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()) {
diff --git a/Userland/Libraries/LibJS/Tests/functions/function-evaluation-order.js b/Userland/Libraries/LibJS/Tests/functions/function-evaluation-order.js
new file mode 100644
index 0000000000..15584202db
--- /dev/null
+++ b/Userland/Libraries/LibJS/Tests/functions/function-evaluation-order.js
@@ -0,0 +1,30 @@
+test("callee is evaluated before arguments", () => {
+    function foo() {}
+    const values = [];
+
+    [foo][(values.push("callee"), 0)](values.push("args"));
+
+    expect(values).toEqual(["callee", "args"]);
+});
+
+test("arguments are evaluated in order", () => {
+    function foo() {}
+    const values = [];
+
+    foo(values.push("arg1"), values.push("arg2"), values.push("arg3"));
+
+    expect(values).toEqual(["arg1", "arg2", "arg3"]);
+});
+
+test("arguments are evaluated before callee is checked for its type", () => {
+    const values = [];
+
+    expect(() => {
+        "foo"(values.push("args"));
+    }).toThrowWithMessage(TypeError, "foo is not a function");
+    expect(values).toEqual(["args"]);
+
+    expect(() => {
+        "foo"(bar);
+    }).toThrowWithMessage(ReferenceError, "'bar' is not defined");
+});