LibJS: Make References see into Environment's bindings as well

'bindings' is the spec-compliant version of 'variables', but we were
simply not even looking at them, which made things using bindings (such
as named function expressions) break in unexpected ways after the move
to using references in call expressions.

Co-Authored-By: davidot <david.tuin@gmail.com>

3 files changed, 25 insertions, 5 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
index 3ae2894f6d..7d6502917c 100644
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
@@ -98,7 +98,17 @@ void ObjectEnvironment::set_mutable_binding(GlobalObject& global_object, FlyStri
         global_object.vm().throw_exception<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
         return;
     }
-    m_binding_object.set(name, value, strict ? Object::ShouldThrowExceptions::Yes : Object::ShouldThrowExceptions::No);
+
+    auto result = 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 && strict) {
+        auto property = m_binding_object.internal_get_own_property(name);
+        if (property.has_value() && !property->writable.value_or(true)) {
+            vm.clear_exception();
+            vm.throw_exception<TypeError>(global_object, ErrorType::DescWriteNonWritable, name);
+        }
+    }
 }
 
 // 9.1.1.2.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-object-environment-records-getbindingvalue-n-s
diff --git a/Userland/Libraries/LibJS/Runtime/Reference.cpp b/Userland/Libraries/LibJS/Runtime/Reference.cpp
index bb6d3620b1..adae50a022 100644
--- a/Userland/Libraries/LibJS/Runtime/Reference.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Reference.cpp
@@ -50,6 +50,7 @@ void Reference::put_value(GlobalObject& global_object, Value value)
     }
 
     VERIFY(m_base_type == BaseType::Environment);
+    // FIXME: This entire following section is 100% not spec-compliant.
     auto existing_variable = m_base_environment->get_from_environment(m_name.as_string());
     Variable variable {
         .value = value,
@@ -66,10 +67,11 @@ void Reference::put_value(GlobalObject& global_object, Value value)
     if (vm.exception())
         return;
 
-    if (!succeeded && m_strict) {
-        // FIXME: This is a hack and will disappear when we support proper variable bindings.
-        vm.throw_exception<TypeError>(global_object, ErrorType::DescWriteNonWritable, m_name);
-        return;
+    if (!succeeded) {
+        if (m_base_environment->has_binding(m_name.as_string())) {
+            m_base_environment->set_mutable_binding(global_object, m_name.as_string(), value, is_strict());
+            return;
+        }
     }
 }
 
@@ -98,8 +100,11 @@ Value Reference::get_value(GlobalObject& global_object, bool throw_if_undefined)
     }
 
     VERIFY(m_base_type == BaseType::Environment);
+    // FIXME: This entire section is 100% not spec-compliant.
     auto value = m_base_environment->get_from_environment(m_name.as_string());
     if (!value.has_value()) {
+        if (m_base_environment->has_binding(m_name.as_string()))
+            return m_base_environment->get_binding_value(global_object, m_name.as_string(), is_strict());
         if (!throw_if_undefined) {
             // FIXME: This is an ad-hoc hack for the `typeof` operator until we support proper variable bindings.
             return js_undefined();
@@ -168,6 +173,9 @@ bool Reference::delete_(GlobalObject& global_object)
     VERIFY(m_base_type == BaseType::Environment);
 
     //    c. Return ? base.DeleteBinding(ref.[[ReferencedName]]).
+    if (m_base_environment->has_binding(m_name.as_string()))
+        return m_base_environment->delete_binding(global_object, m_name.as_string());
+
     // FIXME: This is ad-hoc, we should be calling DeleteBinding.
     return m_base_environment->delete_from_environment(m_name.as_string());
 }
diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp
index 9bbbce248b..d470470ab1 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.cpp
+++ b/Userland/Libraries/LibJS/Runtime/VM.cpp
@@ -417,6 +417,8 @@ Reference VM::get_identifier_reference(Environment* environment, FlyString name,
         auto possible_match = environment->get_from_environment(name);
         if (possible_match.has_value())
             return Reference { *environment, move(name), strict };
+        if (environment->has_binding(name))
+            return Reference { *environment, move(name), strict };
     }
 
     auto& global_environment = interpreter().realm().global_environment();