LibJS: Convert internal_delete() to ThrowCompletionOr

Author: https://github.com/linusg
Commit: https://github.com/SerenityOS/serenity/commit/fbfb0bb908f
Pull-request: https://github.com/SerenityOS/serenity/pull/10266
Reviewed-by: https://github.com/IdanHo ✅

Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator.cpp

@@ -1418,7 +1418,7 @@ public:
     virtual JS::ThrowCompletionOr<Optional<JS::PropertyDescriptor>> internal_get_own_property(JS::PropertyName const&) const override;
     virtual JS::ThrowCompletionOr<bool> internal_set(JS::PropertyName const&, JS::Value, JS::Value) override;
     virtual JS::ThrowCompletionOr<bool> internal_define_own_property(JS::PropertyName const&, JS::PropertyDescriptor const&) override;
-    virtual bool internal_delete(JS::PropertyName const&) override;
+    virtual JS::ThrowCompletionOr<bool> internal_delete(JS::PropertyName const&) override;
     virtual JS::ThrowCompletionOr<bool> internal_prevent_extensions() override;
     virtual JS::MarkedValueList internal_own_property_keys() const override;
 )~~~");
@@ -2178,7 +2178,7 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_define_own_property(JS::Prope
 
         // 3.9.4. [[Delete]], https://heycam.github.io/webidl/#legacy-platform-object-delete
         scoped_generator.append(R"~~~(
-bool @class_name@::internal_delete(JS::PropertyName const& property_name)
+JS::ThrowCompletionOr<bool> @class_name@::internal_delete(JS::PropertyName const& property_name)
 {
     [[maybe_unused]] auto& vm = this->vm();
     auto& global_object = this->global_object();
@@ -2228,8 +2228,9 @@ bool @class_name@::internal_delete(JS::PropertyName const& property_name)
                     scoped_generator.append(R"~~~(
         // 1. Perform the steps listed in the interface description to delete an existing named property with P as the name.
         auto result = throw_dom_exception_if_needed(vm, global_object, [&] { return impl().delete_existing_named_property(property_name_string); });
+        // FIXME: Make this nicer for ThrowCompletionOr.
         if (should_return_empty(result))
-            return {};
+            return JS::throw_completion(vm.exception()->value());
 
         bool succeeded = result.release_value();
 
@@ -2245,8 +2246,9 @@ bool @class_name@::internal_delete(JS::PropertyName const& property_name)
                     function_scoped_generator.append(R"~~~(
         // 1. Perform method steps of operation with O as this and « P » as the argument values.
         auto result = throw_dom_exception_if_needed(vm, global_object, [&] { return impl().@function.cpp_name@(property_name_string); });
+        // FIXME: Make this nicer for ThrowCompletionOr.
         if (should_return_empty(result))
-            return {};
+            return JS::throw_completion(vm.exception()->value());
 )~~~");
 
                     // 2. If operation was declared with a return type of boolean and the steps returned false, then return false.
@@ -2272,7 +2274,7 @@ bool @class_name@::internal_delete(JS::PropertyName const& property_name)
 
         scoped_generator.append(R"~~~(
     // 3. If O has an own property with name P, then:
-    auto own_property_named_p_descriptor = TRY_OR_DISCARD(Object::internal_get_own_property(property_name));
+    auto own_property_named_p_descriptor = TRY(Object::internal_get_own_property(property_name));
 
     if (own_property_named_p_descriptor.has_value()) {
         // 1. If the property is not configurable, then return false.

Userland/Libraries/LibJS/Runtime/ArgumentsObject.cpp

@@ -81,21 +81,21 @@ ThrowCompletionOr<bool> ArgumentsObject::internal_set(PropertyName const& proper
 }
 
 // 10.4.4.5 [[Delete]] ( P ), https://tc39.es/ecma262/#sec-arguments-exotic-objects-delete-p
-bool ArgumentsObject::internal_delete(PropertyName const& property_name)
+ThrowCompletionOr<bool> ArgumentsObject::internal_delete(PropertyName const& property_name)
 {
     // 1. Let map be args.[[ParameterMap]].
     auto& map = parameter_map();
+
     // 2. Let isMapped be ! HasOwnProperty(map, P).
     bool is_mapped = map.has_own_property(property_name);
+
     // 3. Let result be ? OrdinaryDelete(args, P).
-    bool result = Object::internal_delete(property_name);
-    if (vm().exception())
-        return false;
+    bool result = TRY(Object::internal_delete(property_name));
 
     // 4. If result is true and isMapped is true, then
     if (result && is_mapped) {
         // a. Call map.[[Delete]](P).
-        map.internal_delete(property_name);
+        (void)map.internal_delete(property_name);
     }
 
     // 5. Return result.
@@ -157,7 +157,7 @@ ThrowCompletionOr<bool> ArgumentsObject::internal_define_own_property(PropertyNa
         // a. If IsAccessorDescriptor(Desc) is true, then
         if (descriptor.is_accessor_descriptor()) {
             // i. Call map.[[Delete]](P).
-            map.internal_delete(property_name);
+            (void)map.internal_delete(property_name);
         } else {
             // i. If Desc.[[Value]] is present, then
             if (descriptor.value.has_value()) {
@@ -169,7 +169,7 @@ ThrowCompletionOr<bool> ArgumentsObject::internal_define_own_property(PropertyNa
             // ii. If Desc.[[Writable]] is present and its value is false, then
             if (descriptor.writable == false) {
                 // 1. Call map.[[Delete]](P).
-                map.internal_delete(property_name);
+                (void)map.internal_delete(property_name);
             }
         }
     }

Userland/Libraries/LibJS/Runtime/ArgumentsObject.h

@@ -27,7 +27,7 @@ public:
     virtual ThrowCompletionOr<bool> internal_define_own_property(PropertyName const&, PropertyDescriptor const&) override;
     virtual ThrowCompletionOr<Value> internal_get(PropertyName const&, Value receiver) const override;
     virtual ThrowCompletionOr<bool> internal_set(PropertyName const&, Value value, Value receiver) override;
-    virtual bool internal_delete(PropertyName const&) override;
+    virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&) override;
 
     // [[ParameterMap]]
     Object& parameter_map() { return *m_parameter_map; }

Userland/Libraries/LibJS/Runtime/Array.cpp

@@ -217,7 +217,7 @@ ThrowCompletionOr<bool> Array::internal_define_own_property(PropertyName const&
 }
 
 // NON-STANDARD: Used to reject deletes to ephemeral (non-configurable) length property
-bool Array::internal_delete(PropertyName const& property_name)
+ThrowCompletionOr<bool> Array::internal_delete(PropertyName const& property_name)
 {
     auto& vm = this->vm();
     if (property_name.is_string() && property_name.as_string() == vm.names.length.as_string())

Userland/Libraries/LibJS/Runtime/Array.h

@@ -40,7 +40,7 @@ public:
 
     virtual ThrowCompletionOr<Optional<PropertyDescriptor>> internal_get_own_property(PropertyName const&) const override;
     virtual ThrowCompletionOr<bool> internal_define_own_property(PropertyName const&, PropertyDescriptor const&) override;
-    virtual bool internal_delete(PropertyName const&) override;
+    virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&) override;
     virtual MarkedValueList internal_own_property_keys() const override;
 
     [[nodiscard]] bool length_is_writable() const { return m_length_writable; };

Userland/Libraries/LibJS/Runtime/JSONObject.cpp

@@ -472,32 +472,30 @@ Value JSONObject::internalize_json_property(GlobalObject& global_object, Object*
         auto is_array = TRY_OR_DISCARD(value.is_array(global_object));
 
         auto& value_object = value.as_object();
-        auto process_property = [&](const PropertyName& key) {
+        auto process_property = [&](const PropertyName& key) -> ThrowCompletionOr<void> {
             auto element = internalize_json_property(global_object, &value_object, key, reviver);
-            if (vm.exception())
-                return;
-            if (element.is_undefined())
-                value_object.internal_delete(key);
-            else
+            if (auto* exception = vm.exception())
+                return throw_completion(exception->value());
+            if (element.is_undefined()) {
+                TRY(value_object.internal_delete(key));
+            } else {
                 value_object.create_data_property(key, element);
+                if (auto* exception = vm.exception())
+                    return throw_completion(exception->value());
+            }
+            return {};
         };
 
         if (is_array) {
             auto length = TRY_OR_DISCARD(length_of_array_like(global_object, value_object));
-            for (size_t i = 0; i < length; ++i) {
-                process_property(i);
-                if (vm.exception())
-                    return {};
-            }
+            for (size_t i = 0; i < length; ++i)
+                TRY_OR_DISCARD(process_property(i));
         } else {
             auto property_list = value_object.enumerable_own_property_names(Object::PropertyKind::Key);
             if (vm.exception())
                 return {};
-            for (auto& property_name : property_list) {
-                process_property(property_name.as_string().string());
-                if (vm.exception())
-                    return {};
-            }
+            for (auto& property_name : property_list)
+                TRY_OR_DISCARD(process_property(property_name.as_string().string()));
         }
     }
 

Userland/Libraries/LibJS/Runtime/Object.cpp

@@ -235,9 +235,7 @@ bool Object::delete_property_or_throw(PropertyName const& property_name)
     VERIFY(property_name.is_valid());
 
     // 3. Let success be ? O.[[Delete]](P).
-    auto success = internal_delete(property_name);
-    if (vm.exception())
-        return {};
+    auto success = TRY_OR_DISCARD(internal_delete(property_name));
 
     // 4. If success is false, throw a TypeError exception.
     if (!success) {
@@ -796,13 +794,13 @@ bool Object::ordinary_set_with_own_descriptor(PropertyName const& property_name,
 }
 
 // 10.1.10 [[Delete]] ( P ), https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-delete-p
-bool Object::internal_delete(PropertyName const& property_name)
+ThrowCompletionOr<bool> Object::internal_delete(PropertyName const& property_name)
 {
     // 1. Assert: IsPropertyKey(P) is true.
     VERIFY(property_name.is_valid());
 
     // 2. Let desc be ? O.[[GetOwnProperty]](P).
-    auto descriptor = TRY_OR_DISCARD(internal_get_own_property(property_name));
+    auto descriptor = TRY(internal_get_own_property(property_name));
 
     // 3. If desc is undefined, return true.
     if (!descriptor.has_value())

Userland/Libraries/LibJS/Runtime/Object.h

@@ -100,7 +100,7 @@ public:
     virtual ThrowCompletionOr<bool> internal_has_property(PropertyName const&) const;
     virtual ThrowCompletionOr<Value> internal_get(PropertyName const&, Value receiver) const;
     virtual ThrowCompletionOr<bool> internal_set(PropertyName const&, Value value, Value receiver);
-    virtual bool internal_delete(PropertyName const&);
+    virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&);
     virtual MarkedValueList internal_own_property_keys() const;
 
     bool ordinary_set_with_own_descriptor(PropertyName const&, Value, Value, Optional<PropertyDescriptor>);

Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp

@@ -39,7 +39,7 @@ bool ObjectEnvironment::put_into_environment(FlyString const& name, Variable var
 
 bool ObjectEnvironment::delete_from_environment(FlyString const& name)
 {
-    return m_binding_object.internal_delete(name);
+    return TRY_OR_DISCARD(m_binding_object.internal_delete(name));
 }
 
 // 9.1.1.2.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-object-environment-records-hasbinding-n
@@ -134,7 +134,7 @@ Value ObjectEnvironment::get_binding_value(GlobalObject& global_object, FlyStrin
 // 9.1.1.2.7 DeleteBinding ( N ), https://tc39.es/ecma262/#sec-object-environment-records-deletebinding-n
 bool ObjectEnvironment::delete_binding(GlobalObject&, FlyString const& name)
 {
-    return m_binding_object.internal_delete(name);
+    return TRY_OR_DISCARD(m_binding_object.internal_delete(name));
 }
 
 }

Userland/Libraries/LibJS/Runtime/ProxyObject.cpp

@@ -603,7 +603,7 @@ ThrowCompletionOr<bool> ProxyObject::internal_set(PropertyName const& property_n
 }
 
 // 10.5.10 [[Delete]] ( P ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-delete-p
-bool ProxyObject::internal_delete(PropertyName const& property_name)
+ThrowCompletionOr<bool> ProxyObject::internal_delete(PropertyName const& property_name)
 {
     auto& vm = this->vm();
     auto& global_object = this->global_object();
@@ -614,16 +614,14 @@ bool ProxyObject::internal_delete(PropertyName const& property_name)
     // 2. Let handler be O.[[ProxyHandler]].
 
     // 3. If handler is null, throw a TypeError exception.
-    if (m_is_revoked) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyRevoked);
-        return {};
-    }
+    if (m_is_revoked)
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyRevoked);
 
     // 4. Assert: Type(handler) is Object.
     // 5. Let target be O.[[ProxyTarget]].
 
     // 6. Let trap be ? GetMethod(handler, "deleteProperty").
-    auto trap = TRY_OR_DISCARD(Value(&m_handler).get_method(global_object, vm.names.deleteProperty));
+    auto trap = TRY(Value(&m_handler).get_method(global_object, vm.names.deleteProperty));
 
     // 7. If trap is undefined, then
     if (!trap) {
@@ -632,35 +630,31 @@ bool ProxyObject::internal_delete(PropertyName const& property_name)
     }
 
     // 8. Let booleanTrapResult be ! ToBoolean(? Call(trap, handler, « target, P »)).
-    auto trap_result = TRY_OR_DISCARD(vm.call(*trap, &m_handler, &m_target, property_name_to_value(vm, property_name))).to_boolean();
+    auto trap_result = TRY(vm.call(*trap, &m_handler, &m_target, property_name_to_value(vm, property_name))).to_boolean();
 
     // 9. If booleanTrapResult is false, return false.
     if (!trap_result)
         return false;
 
     // 10. Let targetDesc be ? target.[[GetOwnProperty]](P).
-    auto target_descriptor = TRY_OR_DISCARD(m_target.internal_get_own_property(property_name));
+    auto target_descriptor = TRY(m_target.internal_get_own_property(property_name));
 
     // 11. If targetDesc is undefined, return true.
     if (!target_descriptor.has_value())
         return true;
 
     // 12. If targetDesc.[[Configurable]] is false, throw a TypeError exception.
-    if (!*target_descriptor->configurable) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyDeleteNonConfigurable);
-        return {};
-    }
+    if (!*target_descriptor->configurable)
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyDeleteNonConfigurable);
 
     // 13. Let extensibleTarget be ? IsExtensible(target).
     auto extensible_target = m_target.is_extensible();
-    if (vm.exception())
-        return {};
+    if (auto* exception = vm.exception())
+        return throw_completion(exception->value());
 
     // 14. If extensibleTarget is false, throw a TypeError exception.
-    if (!extensible_target) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyDeleteNonExtensible);
-        return {};
-    }
+    if (!extensible_target)
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyDeleteNonExtensible);
 
     // 15. Return true.
     return true;

Userland/Libraries/LibJS/Runtime/ProxyObject.h

@@ -44,7 +44,7 @@ public:
     virtual ThrowCompletionOr<bool> internal_has_property(PropertyName const&) const override;
     virtual ThrowCompletionOr<Value> internal_get(PropertyName const&, Value receiver) const override;
     virtual ThrowCompletionOr<bool> internal_set(PropertyName const&, Value value, Value receiver) override;
-    virtual bool internal_delete(PropertyName const&) override;
+    virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&) override;
     virtual MarkedValueList internal_own_property_keys() const override;
 
 private:

Userland/Libraries/LibJS/Runtime/Reference.cpp

@@ -153,9 +153,7 @@ bool Reference::delete_(GlobalObject& global_object)
         VERIFY(base_obj);
 
         // d. Let deleteStatus be ? baseObj.[[Delete]](ref.[[ReferencedName]]).
-        bool delete_status = base_obj->internal_delete(m_name);
-        if (vm.exception())
-            return {};
+        bool delete_status = TRY_OR_DISCARD(base_obj->internal_delete(m_name));
 
         // e. If deleteStatus is false and ref.[[Strict]] is true, throw a TypeError exception.
         if (!delete_status && m_strict) {

Userland/Libraries/LibJS/Runtime/ReflectObject.cpp

@@ -143,7 +143,7 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::delete_property)
         return {};
 
     // 3. Return ? target.[[Delete]](key).
-    return Value(target.as_object().internal_delete(key));
+    return Value(TRY_OR_DISCARD(target.as_object().internal_delete(key)));
 }
 
 // 28.1.5 Reflect.get ( target, propertyKey [ , receiver ] ), https://tc39.es/ecma262/#sec-reflect.get

Userland/Libraries/LibJS/Runtime/TypedArray.h

@@ -369,7 +369,7 @@ public:
     }
 
     // 10.4.5.6 [[Delete]] ( P ), https://tc39.es/ecma262/#sec-integer-indexed-exotic-objects-delete-p
-    virtual bool internal_delete(PropertyName const& property_name) override
+    virtual ThrowCompletionOr<bool> internal_delete(PropertyName const& property_name) override
     {
         // 1. Assert: IsPropertyKey(P) is true.
         VERIFY(property_name.is_valid());

Userland/Services/WebContent/ConsoleGlobalObject.cpp

@@ -88,7 +88,7 @@ JS::ThrowCompletionOr<bool> ConsoleGlobalObject::internal_set(JS::PropertyName c
     return m_window_object->internal_set(property_name, value, (receiver == this) ? m_window_object : receiver);
 }
 
-bool ConsoleGlobalObject::internal_delete(JS::PropertyName const& property_name)
+JS::ThrowCompletionOr<bool> ConsoleGlobalObject::internal_delete(JS::PropertyName const& property_name)
 {
     return m_window_object->internal_delete(property_name);
 }

Userland/Services/WebContent/ConsoleGlobalObject.h

@@ -32,7 +32,7 @@ public:
     virtual JS::ThrowCompletionOr<bool> internal_has_property(JS::PropertyName const& name) const override;
     virtual JS::ThrowCompletionOr<JS::Value> internal_get(JS::PropertyName const&, JS::Value) const override;
     virtual JS::ThrowCompletionOr<bool> internal_set(JS::PropertyName const&, JS::Value value, JS::Value receiver) override;
-    virtual bool internal_delete(JS::PropertyName const& name) override;
+    virtual JS::ThrowCompletionOr<bool> internal_delete(JS::PropertyName const& name) override;
     virtual JS::MarkedValueList internal_own_property_keys() const override;
 
     virtual void initialize_global_object() override;