LibJS: Use new NativeFunction::create() in most places

Resolves one FIXME where we can now pass a realm, and sets the length
correctly in a bunch of places that previously didn't.
Also reduces the number of "format function name string from arbitrary
PropertyKey" implementations, although two more remain present in the
AST (used with ECMAScriptFunctionObjects, which is a different beast).

9 files changed, 96 insertions, 103 deletions

diff --git a/Userland/Libraries/LibJS/CyclicModule.cpp b/Userland/Libraries/LibJS/CyclicModule.cpp
index bac58e22e8..aebb290b9e 100644
--- a/Userland/Libraries/LibJS/CyclicModule.cpp
+++ b/Userland/Libraries/LibJS/CyclicModule.cpp
@@ -457,7 +457,7 @@ ThrowCompletionOr<void> CyclicModule::execute_async_module(VM& vm)
     };
 
     // 5. Let onFulfilled be ! CreateBuiltinFunction(fulfilledClosure, 0, "", « »).
-    auto* on_fulfilled = NativeFunction::create(global_object, "", move(fulfilled_closure));
+    auto* on_fulfilled = NativeFunction::create(global_object, move(fulfilled_closure), 0, "");
 
     // 6. Let rejectedClosure be a new Abstract Closure with parameters (error) that captures module and performs the following steps when called:
     auto rejected_closure = [&](VM& vm, GlobalObject&) -> ThrowCompletionOr<Value> {
@@ -470,8 +470,8 @@ ThrowCompletionOr<void> CyclicModule::execute_async_module(VM& vm)
         return js_undefined();
     };
 
-    auto* on_rejected = NativeFunction::create(global_object, "", move(rejected_closure));
     // 7. Let onRejected be ! CreateBuiltinFunction(rejectedClosure, 0, "", « »).
+    auto* on_rejected = NativeFunction::create(global_object, move(rejected_closure), 0, "");
 
     VERIFY(is<Promise>(*capability.promise));
 
diff --git a/Userland/Libraries/LibJS/Runtime/AsyncFromSyncIteratorPrototype.cpp b/Userland/Libraries/LibJS/Runtime/AsyncFromSyncIteratorPrototype.cpp
index 37ff916fe4..fb8b2820e2 100644
--- a/Userland/Libraries/LibJS/Runtime/AsyncFromSyncIteratorPrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/AsyncFromSyncIteratorPrototype.cpp
@@ -52,7 +52,7 @@ static ThrowCompletionOr<Object*> async_from_sync_iterator_continuation(GlobalOb
     };
 
     // 8. Let onFulfilled be ! CreateBuiltinFunction(unwrap, 1, "", « »).
-    auto on_fulfilled = NativeFunction::create(global_object, "", move(unwrap));
+    auto* on_fulfilled = NativeFunction::create(global_object, move(unwrap), 1, "");
     // 9. NOTE: onFulfilled is used when processing the "value" property of an IteratorResult object in order to wait for its value if it is a promise and re-package the result in a new "unwrapped" IteratorResult object.
     VERIFY(is<Promise>(value_wrapper));
     auto* value_wrapper_promise = static_cast<Promise*>(value_wrapper);
diff --git a/Userland/Libraries/LibJS/Runtime/Completion.cpp b/Userland/Libraries/LibJS/Runtime/Completion.cpp
index 0376b5a481..70ad51fe71 100644
--- a/Userland/Libraries/LibJS/Runtime/Completion.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Completion.cpp
@@ -63,7 +63,7 @@ ThrowCompletionOr<Value> await(GlobalObject& global_object, Value value)
     };
 
     // 4. Let onFulfilled be ! CreateBuiltinFunction(fulfilledClosure, 1, "", « »).
-    auto on_fulfilled = NativeFunction::create(global_object, "", move(fulfilled_closure));
+    auto* on_fulfilled = NativeFunction::create(global_object, move(fulfilled_closure), 1, "");
 
     // 5. Let rejectedClosure be a new Abstract Closure with parameters (reason) that captures asyncContext and performs the following steps when called:
     auto rejected_closure = [&success, &result](VM& vm, GlobalObject&) -> ThrowCompletionOr<Value> {
@@ -87,7 +87,7 @@ ThrowCompletionOr<Value> await(GlobalObject& global_object, Value value)
     };
 
     // 6. Let onRejected be ! CreateBuiltinFunction(rejectedClosure, 1, "", « »).
-    auto on_rejected = NativeFunction::create(global_object, "", move(rejected_closure));
+    auto* on_rejected = NativeFunction::create(global_object, move(rejected_closure), 1, "");
 
     // 7. Perform ! PerformPromiseThen(promise, onFulfilled, onRejected).
     auto* promise = verify_cast<Promise>(promise_object);
diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp
index 53c819a4a7..37a74c10e1 100644
--- a/Userland/Libraries/LibJS/Runtime/Object.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Object.cpp
@@ -1052,29 +1052,12 @@ void Object::set_prototype(Object* new_prototype)
 
 void Object::define_native_accessor(PropertyKey const& property_key, Function<ThrowCompletionOr<Value>(VM&, GlobalObject&)> getter, Function<ThrowCompletionOr<Value>(VM&, GlobalObject&)> setter, PropertyAttributes attribute)
 {
-    auto& vm = this->vm();
-    String formatted_property_key;
-    if (property_key.is_number()) {
-        formatted_property_key = property_key.to_string();
-    } else if (property_key.is_string()) {
-        formatted_property_key = property_key.as_string();
-    } else {
-        formatted_property_key = String::formatted("[{}]", property_key.as_symbol()->description());
-    }
     FunctionObject* getter_function = nullptr;
-    if (getter) {
-        auto name = String::formatted("get {}", formatted_property_key);
-        getter_function = NativeFunction::create(global_object(), name, move(getter));
-        getter_function->define_direct_property(vm.names.length, Value(0), Attribute::Configurable);
-        getter_function->define_direct_property(vm.names.name, js_string(vm, name), Attribute::Configurable);
-    }
+    if (getter)
+        getter_function = NativeFunction::create(global_object(), move(getter), 0, property_key, {}, {}, "get"sv);
     FunctionObject* setter_function = nullptr;
-    if (setter) {
-        auto name = String::formatted("set {}", formatted_property_key);
-        setter_function = NativeFunction::create(global_object(), name, move(setter));
-        setter_function->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-        setter_function->define_direct_property(vm.names.name, js_string(vm, name), Attribute::Configurable);
-    }
+    if (setter)
+        setter_function = NativeFunction::create(global_object(), move(setter), 1, property_key, {}, {}, "set"sv);
     return define_direct_accessor(property_key, getter_function, setter_function, attribute);
 }
 
@@ -1118,16 +1101,7 @@ Value Object::get_without_side_effects(const PropertyKey& property_key) const
 
 void Object::define_native_function(PropertyKey const& property_key, Function<ThrowCompletionOr<Value>(VM&, GlobalObject&)> native_function, i32 length, PropertyAttributes attribute)
 {
-    auto& vm = this->vm();
-    String function_name;
-    if (property_key.is_string()) {
-        function_name = property_key.as_string();
-    } else {
-        function_name = String::formatted("[{}]", property_key.as_symbol()->description());
-    }
-    auto* function = NativeFunction::create(global_object(), function_name, move(native_function));
-    function->define_direct_property(vm.names.length, Value(length), Attribute::Configurable);
-    function->define_direct_property(vm.names.name, js_string(vm, function_name), Attribute::Configurable);
+    auto* function = NativeFunction::create(global_object(), move(native_function), length, property_key);
     define_direct_property(property_key, function, attribute);
 }
 
diff --git a/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp b/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp
index c504672ca5..ebd1b18d6d 100644
--- a/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp
@@ -99,8 +99,7 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
     // 6. Else,
     else {
         // a. Let thenFinallyClosure be a new Abstract Closure with parameters (value) that captures onFinally and C and performs the following steps when called:
-        // b. Let thenFinally be ! CreateBuiltinFunction(thenFinallyClosure, 1, "", « »).
-        auto* then_finally_function = NativeFunction::create(global_object, "", [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+        auto then_finally_closure = [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
             auto& constructor = const_cast<FunctionObject&>(*constructor_handle.cell());
             auto& on_finally = const_cast<FunctionObject&>(*on_finally_handle.cell());
             auto value = vm.argument(0);
@@ -112,21 +111,23 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
             auto* promise = TRY(promise_resolve(global_object, constructor, result));
 
             // iii. Let returnValue be a new Abstract Closure with no parameters that captures value and performs the following steps when called:
-            // iv. Let valueThunk be ! CreateBuiltinFunction(returnValue, 0, "", « »).
-            auto* value_thunk = NativeFunction::create(global_object, "", [value](auto&, auto&) -> ThrowCompletionOr<Value> {
+            auto return_value = [value](auto&, auto&) -> ThrowCompletionOr<Value> {
                 // 1. Return value.
                 return value;
-            });
+            };
+
+            // iv. Let valueThunk be ! CreateBuiltinFunction(returnValue, 0, "", « »).
+            auto* value_thunk = NativeFunction::create(global_object, move(return_value), 0, "");
 
             // v. Return ? Invoke(promise, "then", « valueThunk »).
             return TRY(Value(promise).invoke(global_object, vm.names.then, value_thunk));
-        });
-        then_finally_function->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-        then_finally = Value(then_finally_function);
+        };
+
+        // b. Let thenFinally be ! CreateBuiltinFunction(thenFinallyClosure, 1, "", « »).
+        then_finally = NativeFunction::create(global_object, move(then_finally_closure), 1, "");
 
         // c. Let catchFinallyClosure be a new Abstract Closure with parameters (reason) that captures onFinally and C and performs the following steps when called:
-        // d. Let catchFinally be ! CreateBuiltinFunction(catchFinallyClosure, 1, "", « »).
-        auto* catch_finally_function = NativeFunction::create(global_object, "", [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+        auto catch_finally_closure = [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
             auto& constructor = const_cast<FunctionObject&>(*constructor_handle.cell());
             auto& on_finally = const_cast<FunctionObject&>(*on_finally_handle.cell());
             auto reason = vm.argument(0);
@@ -137,17 +138,21 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
             // ii. Let promise be ? PromiseResolve(C, result).
             auto* promise = TRY(promise_resolve(global_object, constructor, result));
 
-            // iv. Let thrower be ! CreateBuiltinFunction(throwReason, 0, "", « »).
-            auto* thrower = NativeFunction::create(global_object, "", [reason](auto&, auto&) -> ThrowCompletionOr<Value> {
+            // iii. Let throwReason be a new Abstract Closure with no parameters that captures reason and performs the following steps when called:
+            auto throw_reason = [reason](auto&, auto&) -> ThrowCompletionOr<Value> {
                 // 1. Return ThrowCompletion(reason).
                 return throw_completion(reason);
-            });
+            };
+
+            // iv. Let thrower be ! CreateBuiltinFunction(throwReason, 0, "", « »).
+            auto* thrower = NativeFunction::create(global_object, move(throw_reason), 0, "");
 
             // v. Return ? Invoke(promise, "then", « thrower »).
             return TRY(Value(promise).invoke(global_object, vm.names.then, thrower));
-        });
-        catch_finally_function->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-        catch_finally = Value(catch_finally_function);
+        };
+
+        // d. Let catchFinally be ! CreateBuiltinFunction(catchFinallyClosure, 1, "", « »).
+        catch_finally = NativeFunction::create(global_object, move(catch_finally_closure), 1, "");
     }
 
     // 7. Return ? Invoke(promise, "then", « thenFinally, catchFinally »).
diff --git a/Userland/Libraries/LibJS/Runtime/PromiseReaction.cpp b/Userland/Libraries/LibJS/Runtime/PromiseReaction.cpp
index 0905142740..436e6cabad 100644
--- a/Userland/Libraries/LibJS/Runtime/PromiseReaction.cpp
+++ b/Userland/Libraries/LibJS/Runtime/PromiseReaction.cpp
@@ -31,8 +31,7 @@ ThrowCompletionOr<PromiseCapability> new_promise_capability(GlobalObject& global
     } promise_capability_functions;
 
     // 4. Let executorClosure be a new Abstract Closure with parameters (resolve, reject) that captures promiseCapability and performs the following steps when called:
-    // 5. Let executor be ! CreateBuiltinFunction(executorClosure, 2, "", « »).
-    auto* executor = NativeFunction::create(global_object, "", [&promise_capability_functions](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+    auto executor_closure = [&promise_capability_functions](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
         auto resolve = vm.argument(0);
         auto reject = vm.argument(1);
 
@@ -53,9 +52,10 @@ ThrowCompletionOr<PromiseCapability> new_promise_capability(GlobalObject& global
 
         // e. Return undefined.
         return js_undefined();
-    });
-    executor->define_direct_property(vm.names.length, Value(2), Attribute::Configurable);
-    executor->define_direct_property(vm.names.name, js_string(vm, String::empty()), Attribute::Configurable);
+    };
+
+    // 5. Let executor be ! CreateBuiltinFunction(executorClosure, 2, "", « »).
+    auto* executor = NativeFunction::create(global_object, move(executor_closure), 2, "");
 
     // 6. Let promise be ? Construct(C, « executor »).
     auto* promise = TRY(construct(global_object, constructor.as_function(), executor));
diff --git a/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp b/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp
index 23fad447fc..0ec421ec66 100644
--- a/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp
@@ -60,25 +60,44 @@ ThrowCompletionOr<Object*> ProxyConstructor::construct(FunctionObject&)
 // 28.2.2.1 Proxy.revocable ( target, handler ), https://tc39.es/ecma262/#sec-proxy.revocable
 JS_DEFINE_NATIVE_FUNCTION(ProxyConstructor::revocable)
 {
+    // 1. Let p be ? ProxyCreate(target, handler).
     auto* proxy = TRY(proxy_create(global_object, vm.argument(0), vm.argument(1)));
 
-    // 28.2.2.1.1 Proxy Revocation Functions, https://tc39.es/ecma262/#sec-proxy-revocation-functions
-    auto* revoker = NativeFunction::create(global_object, "", [proxy_handle = make_handle(proxy)](auto&, auto&) -> ThrowCompletionOr<Value> {
+    // 2. Let revokerClosure be a new Abstract Closure with no parameters that captures nothing and performs the following steps when called:
+    auto revoker_closure = [proxy_handle = make_handle(proxy)](auto&, auto&) -> ThrowCompletionOr<Value> {
+        // a. Let F be the active function object.
+
+        // b. Let p be F.[[RevocableProxy]].
         auto& proxy = const_cast<ProxyObject&>(*proxy_handle.cell());
+
+        // c. If p is null, return undefined.
         if (proxy.is_revoked())
             return js_undefined();
-        // NOTE: The spec wants us to unset [[ProxyTarget]] and [[ProxyHandler]],
-        // which is their way of revoking the Proxy - this might affect GC-ability,
-        // but AFAICT not doing that should be ok compatibility-wise.
+
+        // d. Set F.[[RevocableProxy]] to null.
+        // e. Assert: p is a Proxy object.
+        // f. Set p.[[ProxyTarget]] to null.
+        // g. Set p.[[ProxyHandler]] to null.
         proxy.revoke();
+
+        // h. Return undefined.
         return js_undefined();
-    });
-    revoker->define_direct_property(vm.names.length, Value(0), Attribute::Configurable);
-    revoker->define_direct_property(vm.names.name, js_string(vm, String::empty()), Attribute::Configurable);
+    };
 
+    // 3. Let revoker be ! CreateBuiltinFunction(revokerClosure, 0, "", « [[RevocableProxy]] »).
+    // 4. Set revoker.[[RevocableProxy]] to p.
+    auto* revoker = NativeFunction::create(global_object, move(revoker_closure), 0, "");
+
+    // 5. Let result be ! OrdinaryObjectCreate(%Object.prototype%).
     auto* result = Object::create(global_object, global_object.object_prototype());
+
+    // 6. Perform ! CreateDataPropertyOrThrow(result, "proxy", p).
     MUST(result->create_data_property_or_throw(vm.names.proxy, proxy));
+
+    // 7. Perform ! CreateDataPropertyOrThrow(result, "revoke", revoker).
     MUST(result->create_data_property_or_throw(vm.names.revoke, revoker));
+
+    // 8. Return result.
     return result;
 }
 
diff --git a/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp b/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp
index 90bac0c2cc..0ca9657588 100644
--- a/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp
@@ -228,44 +228,39 @@ ThrowCompletionOr<Value> shadow_realm_import_value(GlobalObject& global_object,
     // NOTE: We don't support this concept yet.
 
     // 9. Let steps be the steps of an ExportGetter function as described below.
+    auto steps = [string = move(export_name_string)](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+        // 1. Assert: exports is a module namespace exotic object.
+        VERIFY(vm.argument(0).is_object());
+        auto& exports = vm.argument(0).as_object();
+        VERIFY(is<ModuleNamespaceObject>(exports));
+
+        // 2. Let f be the active function object.
+        auto* function = vm.running_execution_context().function;
+
+        // 3. Let string be f.[[ExportNameString]].
+        // 4. Assert: Type(string) is String.
+
+        // 5. Let hasOwn be ? HasOwnProperty(exports, string).
+        auto has_own = TRY(exports.has_own_property(string));
+
+        // 6. If hasOwn is false, throw a TypeError exception.
+        if (!has_own)
+            return vm.template throw_completion<TypeError>(global_object, ErrorType::MissingRequiredProperty, string);
+
+        // 7. Let value be ? Get(exports, string).
+        auto value = TRY(exports.get(string));
+
+        // 8. Let realm be f.[[Realm]].
+        auto* realm = function->realm();
+        VERIFY(realm);
+
+        // 9. Return ? GetWrappedValue(realm, value).
+        return get_wrapped_value(global_object, *realm, value);
+    };
+
     // 10. Let onFulfilled be ! CreateBuiltinFunction(steps, 1, "", « [[ExportNameString]] », callerRealm).
     // 11. Set onFulfilled.[[ExportNameString]] to exportNameString.
-    // FIXME: Support passing a realm to NativeFunction::create()
-    (void)caller_realm;
-    auto* on_fulfilled = NativeFunction::create(
-        global_object,
-        "",
-        [string = move(export_name_string)](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
-            // 1. Assert: exports is a module namespace exotic object.
-            VERIFY(vm.argument(0).is_object());
-            auto& exports = vm.argument(0).as_object();
-            VERIFY(is<ModuleNamespaceObject>(exports));
-
-            // 2. Let f be the active function object.
-            auto* function = vm.running_execution_context().function;
-
-            // 3. Let string be f.[[ExportNameString]].
-            // 4. Assert: Type(string) is String.
-
-            // 5. Let hasOwn be ? HasOwnProperty(exports, string).
-            auto has_own = TRY(exports.has_own_property(string));
-
-            // 6. If hasOwn is false, throw a TypeError exception.
-            if (!has_own)
-                return vm.template throw_completion<TypeError>(global_object, ErrorType::MissingRequiredProperty, string);
-
-            // 7. Let value be ? Get(exports, string).
-            auto value = TRY(exports.get(string));
-
-            // 8. Let realm be f.[[Realm]].
-            auto* realm = function->realm();
-            VERIFY(realm);
-
-            // 9. Return ? GetWrappedValue(realm, value).
-            return get_wrapped_value(global_object, *realm, value);
-        });
-    on_fulfilled->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-    on_fulfilled->define_direct_property(vm.names.name, js_string(vm, String::empty()), Attribute::Configurable);
+    auto* on_fulfilled = NativeFunction::create(global_object, move(steps), 1, "", &caller_realm);
 
     // 12. Let promiseCapability be ! NewPromiseCapability(%Promise%).
     auto promise_capability = MUST(new_promise_capability(global_object, global_object.promise_constructor()));
diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp
index 43e2625f94..2511e185ef 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.cpp
+++ b/Userland/Libraries/LibJS/Runtime/VM.cpp
@@ -997,7 +997,7 @@ void VM::finish_dynamic_import(ScriptOrModule referencing_script_or_module, Modu
     };
 
     // 2. Let onFulfilled be ! CreateBuiltinFunction(fulfilledClosure, 0, "", « »).
-    auto* on_fulfilled = NativeFunction::create(current_realm()->global_object(), "", move(fulfilled_closure));
+    auto* on_fulfilled = NativeFunction::create(current_realm()->global_object(), move(fulfilled_closure), 0, "");
 
     // 3. Let rejectedClosure be a new Abstract Closure with parameters (error) that captures promiseCapability and performs the following steps when called:
     auto rejected_closure = [rejected_function = make_handle(promise_capability.reject)](VM& vm, GlobalObject& global_object) -> ThrowCompletionOr<Value> {
@@ -1009,7 +1009,7 @@ void VM::finish_dynamic_import(ScriptOrModule referencing_script_or_module, Modu
     };
 
     // 4. Let onRejected be ! CreateBuiltinFunction(rejectedClosure, 0, "", « »).
-    auto* on_rejected = NativeFunction::create(current_realm()->global_object(), "", move(rejected_closure));
+    auto* on_rejected = NativeFunction::create(current_realm()->global_object(), move(rejected_closure), 0, "");
 
     // 5. Perform ! PerformPromiseThen(innerPromise, onFulfilled, onRejected).
     inner_promise->perform_then(on_fulfilled, on_rejected, {});