LibJS: Convert set_mutable_binding() to ThrowCompletionOr

Also add spec step comments to it while we're here.

1 files changed, 18 insertions, 15 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
index 4e24a85300..a43ec7facc 100644
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
@@ -79,38 +79,41 @@ ThrowCompletionOr<void> ObjectEnvironment::create_immutable_binding(GlobalObject
 ThrowCompletionOr<void> ObjectEnvironment::initialize_binding(GlobalObject& global_object, FlyString const& name, Value value)
 {
     // 1. Return ? envRec.SetMutableBinding(N, V, false).
-    set_mutable_binding(global_object, name, value, false);
-    if (auto* exception = vm().exception())
-        return throw_completion(exception->value());
-    return {};
+    return set_mutable_binding(global_object, name, value, false);
 }
 
 // 9.1.1.2.5 SetMutableBinding ( N, V, S ), https://tc39.es/ecma262/#sec-object-environment-records-setmutablebinding-n-v-s
-void ObjectEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
+ThrowCompletionOr<void> ObjectEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
 {
     auto& vm = this->vm();
-    auto still_exists_or_error = m_binding_object.has_property(name);
-    if (still_exists_or_error.is_error())
-        return;
-    auto still_exists = still_exists_or_error.release_value();
-    if (!still_exists && strict) {
-        global_object.vm().throw_exception<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
-        return;
-    }
 
+    // 1. Let bindingObject be envRec.[[BindingObject]].
+    // 2. Let stillExists be ? HasProperty(bindingObject, N).
+    auto still_exists = TRY(m_binding_object.has_property(name));
+
+    // 3. If stillExists is false and S is true, throw a ReferenceError exception.
+    if (!still_exists && strict)
+        return vm.throw_completion<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
+
+    // 4. Return ? Set(bindingObject, N, V, S).
     auto result_or_error = m_binding_object.set(name, value, strict ? Object::ShouldThrowExceptions::Yes : Object::ShouldThrowExceptions::No);
 
     // Note: Nothing like this in the spec, this is here to produce nicer errors instead of the generic one thrown by Object::set().
     if (result_or_error.is_error() && strict) {
         auto property_or_error = m_binding_object.internal_get_own_property(name);
+        // Return the initial error instead of masking it with the new error
         if (property_or_error.is_error())
-            return;
+            return result_or_error.release_error();
         auto property = property_or_error.release_value();
         if (property.has_value() && !property->writable.value_or(true)) {
             vm.clear_exception();
-            vm.throw_exception<TypeError>(global_object, ErrorType::DescWriteNonWritable, name);
+            return vm.throw_completion<TypeError>(global_object, ErrorType::DescWriteNonWritable, name);
         }
     }
+
+    if (result_or_error.is_error())
+        return result_or_error.release_error();
+    return {};
 }
 
 // 9.1.1.2.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-object-environment-records-getbindingvalue-n-s