LibJS: Convert has_binding() to ThrowCompletionOr

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

11 files changed, 50 insertions, 29 deletions

diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp
index aef848cffc..afa9b3fe15 100644
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@@ -3190,7 +3190,7 @@ void ScopeNode::block_declaration_instantiation(GlobalObject& global_object, Env
             if (is_constant_declaration) {
                 environment->create_immutable_binding(global_object, name, true);
             } else {
-                if (!environment->has_binding(name))
+                if (!MUST(environment->has_binding(name)))
                     environment->create_mutable_binding(global_object, name, false);
             }
         });
diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
index f2189206ae..fa6e1b59d4 100644
--- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
+++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
@@ -558,7 +558,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, GlobalObject& glo
         while (this_environment != variable_environment) {
             if (!is<ObjectEnvironment>(*this_environment)) {
                 program.for_each_var_declared_name([&](auto const& name) {
-                    if (this_environment->has_binding(name)) {
+                    if (MUST(this_environment->has_binding(name))) {
                         vm.throw_exception<SyntaxError>(global_object, ErrorType::FixmeAddAnErrorStringWithMessage, "Var already declared lexically");
                         return IterationDecision::Break;
                     }
@@ -605,7 +605,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, GlobalObject& glo
             auto* this_environment = lexical_environment;
 
             while (this_environment != variable_environment) {
-                if (!is<ObjectEnvironment>(*this_environment) && this_environment->has_binding(function_name))
+                if (!is<ObjectEnvironment>(*this_environment) && MUST(this_environment->has_binding(function_name)))
                     return IterationDecision::Continue;
 
                 this_environment = this_environment->outer_environment();
@@ -628,7 +628,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, GlobalObject& glo
                     if (vm.exception())
                         return IterationDecision::Break;
                 } else {
-                    if (!variable_environment->has_binding(function_name)) {
+                    if (!MUST(variable_environment->has_binding(function_name))) {
                         variable_environment->create_mutable_binding(global_object, function_name, true);
                         variable_environment->initialize_binding(global_object, function_name, js_undefined());
                         VERIFY(!vm.exception());
@@ -700,7 +700,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, GlobalObject& glo
             if (auto* exception = vm.exception())
                 return throw_completion(exception->value());
         } else {
-            auto binding_exists = variable_environment->has_binding(declaration.name());
+            auto binding_exists = MUST(variable_environment->has_binding(declaration.name()));
 
             if (!binding_exists) {
                 variable_environment->create_mutable_binding(global_object, declaration.name(), true);
@@ -721,7 +721,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, GlobalObject& glo
             if (auto* exception = vm.exception())
                 return throw_completion(exception->value());
         } else {
-            auto binding_exists = variable_environment->has_binding(var_name);
+            auto binding_exists = MUST(variable_environment->has_binding(var_name));
 
             if (!binding_exists) {
                 variable_environment->create_mutable_binding(global_object, var_name, true);
diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
index 5c4d83d8e4..417f1040d1 100644
--- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
@@ -35,7 +35,7 @@ void DeclarativeEnvironment::visit_edges(Visitor& visitor)
 }
 
 // 9.1.1.1.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-declarative-environment-records-hasbinding-n
-bool DeclarativeEnvironment::has_binding(FlyString const& name, Optional<size_t>* out_index) const
+ThrowCompletionOr<bool> DeclarativeEnvironment::has_binding(FlyString const& name, Optional<size_t>* out_index) const
 {
     auto it = m_names.find(name);
     if (it == m_names.end())
diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
index 16993e5c49..a2eb60c191 100644
--- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
@@ -8,6 +8,7 @@
 
 #include <AK/FlyString.h>
 #include <AK/HashMap.h>
+#include <LibJS/Runtime/Completion.h>
 #include <LibJS/Runtime/Environment.h>
 #include <LibJS/Runtime/Value.h>
 
@@ -21,7 +22,7 @@ public:
     explicit DeclarativeEnvironment(Environment* parent_scope);
     virtual ~DeclarativeEnvironment() override;
 
-    virtual bool has_binding(FlyString const& name, Optional<size_t>* = nullptr) const override;
+    virtual ThrowCompletionOr<bool> has_binding(FlyString const& name, Optional<size_t>* = nullptr) const override;
     virtual void create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override;
     virtual void create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override;
     virtual void initialize_binding(GlobalObject&, FlyString const& name, Value) override;
diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
index d65080792f..1bdd1386da 100644
--- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
@@ -357,7 +357,7 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
     }
 
     for (auto const& parameter_name : parameter_names) {
-        if (environment->has_binding(parameter_name))
+        if (MUST(environment->has_binding(parameter_name)))
             continue;
 
         environment->create_mutable_binding(global_object(), parameter_name, false);
diff --git a/Userland/Libraries/LibJS/Runtime/Environment.h b/Userland/Libraries/LibJS/Runtime/Environment.h
index e0864c427a..bfab780217 100644
--- a/Userland/Libraries/LibJS/Runtime/Environment.h
+++ b/Userland/Libraries/LibJS/Runtime/Environment.h
@@ -33,7 +33,7 @@ public:
 
     virtual Object* with_base_object() const { return nullptr; }
 
-    virtual bool has_binding([[maybe_unused]] FlyString const& name, [[maybe_unused]] Optional<size_t>* out_index = nullptr) const { return false; }
+    virtual ThrowCompletionOr<bool> has_binding([[maybe_unused]] FlyString const& name, [[maybe_unused]] Optional<size_t>* out_index = nullptr) const { return false; }
     virtual void create_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool can_be_deleted) { }
     virtual void create_immutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { }
     virtual void initialize_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value) { }
diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
index 1d7b4504a4..5e50373e81 100644
--- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
@@ -39,17 +39,22 @@ ThrowCompletionOr<Value> GlobalEnvironment::get_this_binding(GlobalObject&) cons
 }
 
 // 9.1.1.4.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-global-environment-records-hasbinding-n
-bool GlobalEnvironment::has_binding(FlyString const& name, Optional<size_t>*) const
+ThrowCompletionOr<bool> GlobalEnvironment::has_binding(FlyString const& name, Optional<size_t>*) const
 {
-    if (m_declarative_record->has_binding(name))
+    // 1. Let DclRec be envRec.[[DeclarativeRecord]].
+    // 2. If DclRec.HasBinding(N) is true, return true.
+    if (MUST(m_declarative_record->has_binding(name)))
         return true;
+
+    // 3. Let ObjRec be envRec.[[ObjectRecord]].
+    // 4. Return ? ObjRec.HasBinding(N).
     return m_object_record->has_binding(name);
 }
 
 // 9.1.1.4.2 CreateMutableBinding ( N, D ), https://tc39.es/ecma262/#sec-global-environment-records-createmutablebinding-n-d
 void GlobalEnvironment::create_mutable_binding(GlobalObject& global_object, FlyString const& name, bool can_be_deleted)
 {
-    if (m_declarative_record->has_binding(name)) {
+    if (MUST(m_declarative_record->has_binding(name))) {
         global_object.vm().throw_exception<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
         return;
     }
@@ -59,7 +64,7 @@ void GlobalEnvironment::create_mutable_binding(GlobalObject& global_object, FlyS
 // 9.1.1.4.3 CreateImmutableBinding ( N, S ), https://tc39.es/ecma262/#sec-global-environment-records-createimmutablebinding-n-s
 void GlobalEnvironment::create_immutable_binding(GlobalObject& global_object, FlyString const& name, bool strict)
 {
-    if (m_declarative_record->has_binding(name)) {
+    if (MUST(m_declarative_record->has_binding(name))) {
         global_object.vm().throw_exception<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
         return;
     }
@@ -69,7 +74,7 @@ void GlobalEnvironment::create_immutable_binding(GlobalObject& global_object, Fl
 // 9.1.1.4.4 InitializeBinding ( N, V ), https://tc39.es/ecma262/#sec-global-environment-records-initializebinding-n-v
 void GlobalEnvironment::initialize_binding(GlobalObject& global_object, FlyString const& name, Value value)
 {
-    if (m_declarative_record->has_binding(name)) {
+    if (MUST(m_declarative_record->has_binding(name))) {
         m_declarative_record->initialize_binding(global_object, name, value);
         return;
     }
@@ -79,7 +84,7 @@ void GlobalEnvironment::initialize_binding(GlobalObject& global_object, FlyStrin
 // 9.1.1.4.5 SetMutableBinding ( N, V, S ), https://tc39.es/ecma262/#sec-global-environment-records-setmutablebinding-n-v-s
 void GlobalEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
 {
-    if (m_declarative_record->has_binding(name)) {
+    if (MUST(m_declarative_record->has_binding(name))) {
         m_declarative_record->set_mutable_binding(global_object, name, value, strict);
         return;
     }
@@ -90,7 +95,7 @@ void GlobalEnvironment::set_mutable_binding(GlobalObject& global_object, FlyStri
 // 9.1.1.4.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-global-environment-records-getbindingvalue-n-s
 Value GlobalEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict)
 {
-    if (m_declarative_record->has_binding(name))
+    if (MUST(m_declarative_record->has_binding(name)))
         return m_declarative_record->get_binding_value(global_object, name, strict);
     return m_object_record->get_binding_value(global_object, name, strict);
 }
@@ -98,7 +103,7 @@ Value GlobalEnvironment::get_binding_value(GlobalObject& global_object, FlyStrin
 // 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)
 {
-    if (m_declarative_record->has_binding(name))
+    if (MUST(m_declarative_record->has_binding(name)))
         return m_declarative_record->delete_binding(global_object, name);
 
     bool existing_prop = TRY_OR_DISCARD(m_object_record->binding_object().has_own_property(name));
@@ -121,7 +126,7 @@ bool GlobalEnvironment::has_var_declaration(FlyString const& name) const
 // 9.1.1.4.13 HasLexicalDeclaration ( N ), https://tc39.es/ecma262/#sec-haslexicaldeclaration
 bool GlobalEnvironment::has_lexical_declaration(FlyString const& name) const
 {
-    return m_declarative_record->has_binding(name);
+    return MUST(m_declarative_record->has_binding(name));
 }
 
 // 9.1.1.4.14 HasRestrictedGlobalProperty ( N ), https://tc39.es/ecma262/#sec-hasrestrictedglobalproperty
diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
index 394d4cc6c9..7fc10f53ca 100644
--- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
@@ -19,7 +19,7 @@ public:
     virtual bool has_this_binding() const final { return true; }
     virtual ThrowCompletionOr<Value> get_this_binding(GlobalObject&) const final;
 
-    virtual bool has_binding(FlyString const& name, Optional<size_t>* = nullptr) const override;
+    virtual ThrowCompletionOr<bool> has_binding(FlyString const& name, Optional<size_t>* = nullptr) const override;
     virtual void create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override;
     virtual void create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override;
     virtual void initialize_binding(GlobalObject&, FlyString const& name, Value) override;
diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
index 3aa0fc2040..301c398885 100644
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
@@ -25,20 +25,37 @@ void ObjectEnvironment::visit_edges(Cell::Visitor& visitor)
 }
 
 // 9.1.1.2.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-object-environment-records-hasbinding-n
-bool ObjectEnvironment::has_binding(FlyString const& name, Optional<size_t>*) const
+ThrowCompletionOr<bool> ObjectEnvironment::has_binding(FlyString const& name, Optional<size_t>*) const
 {
     auto& vm = this->vm();
-    bool found_binding = TRY_OR_DISCARD(m_binding_object.has_property(name));
+
+    // 1. Let bindingObject be envRec.[[BindingObject]].
+
+    // 2. Let foundBinding be ? HasProperty(bindingObject, N).
+    bool found_binding = TRY(m_binding_object.has_property(name));
+
+    // 3. If foundBinding is false, return false.
     if (!found_binding)
         return false;
+
+    // 4. If envRec.[[IsWithEnvironment]] is false, return true.
     if (!m_with_environment)
         return true;
-    auto unscopables = TRY_OR_DISCARD(m_binding_object.get(*vm.well_known_symbol_unscopables()));
+
+    // 5. Let unscopables be ? Get(bindingObject, @@unscopables).
+    auto unscopables = TRY(m_binding_object.get(*vm.well_known_symbol_unscopables()));
+
+    // 6. If Type(unscopables) is Object, then
     if (unscopables.is_object()) {
-        auto blocked = TRY_OR_DISCARD(unscopables.as_object().get(name));
-        if (blocked.to_boolean())
+        // a. Let blocked be ! ToBoolean(? Get(unscopables, N)).
+        auto blocked = TRY(unscopables.as_object().get(name)).to_boolean();
+
+        // b. If blocked is true, return false.
+        if (blocked)
             return false;
     }
+
+    // 7. Return true.
     return true;
 }
 
diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
index 7cfe00fdbc..951955649c 100644
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
@@ -20,7 +20,7 @@ public:
     };
     ObjectEnvironment(Object& binding_object, IsWithEnvironment, Environment* outer_environment);
 
-    virtual bool has_binding(FlyString const& name, Optional<size_t>* = nullptr) const override;
+    virtual ThrowCompletionOr<bool> has_binding(FlyString const& name, Optional<size_t>* = nullptr) const override;
     virtual void create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override;
     virtual void create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override;
     virtual void initialize_binding(GlobalObject&, FlyString const& name, Value) override;
diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp
index b0f37553af..02fb5e0a2f 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.cpp
+++ b/Userland/Libraries/LibJS/Runtime/VM.cpp
@@ -445,9 +445,7 @@ Reference VM::get_identifier_reference(Environment* environment, FlyString name,
     }
 
     Optional<size_t> index;
-    auto exists = environment->has_binding(name, &index);
-    if (exception())
-        return {};
+    auto exists = TRY_OR_DISCARD(environment->has_binding(name, &index));
 
     Optional<EnvironmentCoordinate> environment_coordinate;
     if (index.has_value())