LibJS: Convert delete_binding() to ThrowCompletionOr

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

8 files changed, 41 insertions, 12 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
index 97d593d989..214f1a28c6 100644
--- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
@@ -175,16 +175,24 @@ ThrowCompletionOr<Value> DeclarativeEnvironment::get_binding_value_direct(Global
 }
 
 // 9.1.1.1.7 DeleteBinding ( N ), https://tc39.es/ecma262/#sec-declarative-environment-records-deletebinding-n
-bool DeclarativeEnvironment::delete_binding(GlobalObject&, FlyString const& name)
+ThrowCompletionOr<bool> DeclarativeEnvironment::delete_binding(GlobalObject&, FlyString const& name)
 {
+    // 1. Assert: envRec has a binding for the name that is the value of N.
     auto it = m_names.find(name);
     VERIFY(it != m_names.end());
+
     auto& binding = m_bindings[it->value];
+
+    // 2. If the binding for N in envRec cannot be deleted, return false.
     if (!binding.can_be_deleted)
         return false;
+
+    // 3. Remove the binding for N from envRec.
     // NOTE: We keep the entry in m_bindings to avoid disturbing indices.
     binding = {};
     m_names.remove(it);
+
+    // 4. Return true.
     return true;
 }
 
diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
index c843663d49..934c14400d 100644
--- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
@@ -28,7 +28,7 @@ public:
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, FlyString const& name, Value) override;
     virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
     virtual ThrowCompletionOr<Value> get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
-    virtual bool delete_binding(GlobalObject&, FlyString const& name) override;
+    virtual ThrowCompletionOr<bool> delete_binding(GlobalObject&, FlyString const& name) override;
 
     void initialize_or_set_mutable_binding(Badge<ScopeNode>, GlobalObject& global_object, FlyString const& name, Value value);
 
diff --git a/Userland/Libraries/LibJS/Runtime/Environment.h b/Userland/Libraries/LibJS/Runtime/Environment.h
index b4fdd07f18..ec94a3b1e3 100644
--- a/Userland/Libraries/LibJS/Runtime/Environment.h
+++ b/Userland/Libraries/LibJS/Runtime/Environment.h
@@ -39,7 +39,7 @@ public:
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value) { return {}; }
     virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value, [[maybe_unused]] bool strict) { return {}; }
     virtual ThrowCompletionOr<Value> get_binding_value(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { return Value {}; }
-    virtual bool delete_binding(GlobalObject&, [[maybe_unused]] FlyString const& name) { return false; }
+    virtual ThrowCompletionOr<bool> delete_binding(GlobalObject&, [[maybe_unused]] FlyString const& name) { return false; }
 
     // [[OuterEnv]]
     Environment* outer_environment() { return m_outer_environment; }
diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
index 598c6df87e..d2dbf2bb25 100644
--- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
@@ -122,19 +122,38 @@ ThrowCompletionOr<Value> GlobalEnvironment::get_binding_value(GlobalObject& glob
 }
 
 // 9.1.1.4.7 DeleteBinding ( N ), https://tc39.es/ecma262/#sec-global-environment-records-deletebinding-n
-bool GlobalEnvironment::delete_binding(GlobalObject& global_object, FlyString const& name)
+ThrowCompletionOr<bool> GlobalEnvironment::delete_binding(GlobalObject& global_object, FlyString const& name)
 {
-    if (MUST(m_declarative_record->has_binding(name)))
+    // 1. Let DclRec be envRec.[[DeclarativeRecord]].
+    // 2. If DclRec.HasBinding(N) is true, then
+    if (MUST(m_declarative_record->has_binding(name))) {
+        // a. Return DclRec.DeleteBinding(N).
         return m_declarative_record->delete_binding(global_object, name);
+    }
 
-    bool existing_prop = TRY_OR_DISCARD(m_object_record->binding_object().has_own_property(name));
+    // 3. Let ObjRec be envRec.[[ObjectRecord]].
+    // 4. Let globalObject be ObjRec.[[BindingObject]].
+
+    // 5. Let existingProp be ? HasOwnProperty(globalObject, N).
+    bool existing_prop = TRY(m_object_record->binding_object().has_own_property(name));
+
+    // 6. If existingProp is true, then
     if (existing_prop) {
-        bool status = m_object_record->delete_binding(global_object, name);
+        // a. Let status be ? ObjRec.DeleteBinding(N).
+        bool status = TRY(m_object_record->delete_binding(global_object, name));
+
+        // b. If status is true, then
         if (status) {
+            // i. Let varNames be envRec.[[VarNames]].
+            // ii. If N is an element of varNames, remove that element from the varNames.
             m_var_names.remove_all_matching([&](auto& entry) { return entry == name; });
         }
+
+        // c. Return status.
         return status;
     }
+
+    // 7. Return true.
     return true;
 }
 
diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
index 27863b0178..8379fb326c 100644
--- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
@@ -25,7 +25,7 @@ public:
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, FlyString const& name, Value) override;
     virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
     virtual ThrowCompletionOr<Value> get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
-    virtual bool delete_binding(GlobalObject&, FlyString const& name) override;
+    virtual ThrowCompletionOr<bool> delete_binding(GlobalObject&, FlyString const& name) override;
 
     ObjectEnvironment& object_record() { return *m_object_record; }
     Object& global_this_value() { return *m_global_this_value; }
diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
index facbf7165a..b5d10d312e 100644
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
@@ -138,9 +138,11 @@ ThrowCompletionOr<Value> ObjectEnvironment::get_binding_value(GlobalObject& glob
 }
 
 // 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)
+ThrowCompletionOr<bool> ObjectEnvironment::delete_binding(GlobalObject&, FlyString const& name)
 {
-    return TRY_OR_DISCARD(m_binding_object.internal_delete(name));
+    // 1. Let bindingObject be envRec.[[BindingObject]].
+    // 2. Return ? bindingObject.[[Delete]](N).
+    return m_binding_object.internal_delete(name);
 }
 
 }
diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
index c69f27db96..6d451b5057 100644
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
@@ -26,7 +26,7 @@ public:
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, FlyString const& name, Value) override;
     virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
     virtual ThrowCompletionOr<Value> get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
-    virtual bool delete_binding(GlobalObject&, FlyString const& name) override;
+    virtual ThrowCompletionOr<bool> delete_binding(GlobalObject&, FlyString const& name) override;
 
     // 9.1.1.2.10 WithBaseObject ( ), https://tc39.es/ecma262/#sec-object-environment-records-withbaseobject
     virtual Object* with_base_object() const override
diff --git a/Userland/Libraries/LibJS/Runtime/Reference.cpp b/Userland/Libraries/LibJS/Runtime/Reference.cpp
index 3c7a92036f..10d07609bc 100644
--- a/Userland/Libraries/LibJS/Runtime/Reference.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Reference.cpp
@@ -144,7 +144,7 @@ bool Reference::delete_(GlobalObject& global_object)
     VERIFY(m_base_type == BaseType::Environment);
 
     //    c. Return ? base.DeleteBinding(ref.[[ReferencedName]]).
-    return m_base_environment->delete_binding(global_object, m_name.as_string());
+    return TRY_OR_DISCARD(m_base_environment->delete_binding(global_object, m_name.as_string()));
 }
 
 String Reference::to_string() const