LibJS: Support deleting local variables with operator delete

To make this cleaner i also moved the logic into Reference::delete_.

Userland/Libraries/LibJS/AST.cpp

@@ -685,16 +685,7 @@ Value UnaryExpression::execute(Interpreter& interpreter, GlobalObject& global_ob
         auto reference = m_lhs->to_reference(interpreter, global_object);
         if (interpreter.exception())
             return {};
-        if (reference.is_unresolvable())
+        return Value(reference.delete_(global_object));
-            return Value(true);
-        // FIXME: Support deleting locals
-        VERIFY(!reference.is_local_variable());
-        if (reference.is_global_variable())
-            return Value(global_object.delete_property(reference.name()));
-        auto* base_object = reference.base().to_object(global_object);
-        if (!base_object)
-            return {};
-        return Value(base_object->delete_property(reference.name()));
     }
 
     Value lhs_result;

Userland/Libraries/LibJS/Runtime/ErrorTypes.h

@@ -130,6 +130,7 @@
                                         "target is non-extensible")                                                                     \
     M(ProxyTwoArguments, "Proxy constructor requires at least two arguments")                                                           \
     M(ReduceNoInitial, "Reduce of empty array with no initial value")                                                                   \
+    M(ReferenceNullishDeleteProperty, "Cannot delete property '{}' of {}")                                                              \
     M(ReferenceNullishGetProperty, "Cannot get property '{}' of {}")                                                                    \
     M(ReferenceNullishSetProperty, "Cannot set property '{}' of {}")                                                                    \
     M(ReferencePrimitiveSetProperty, "Cannot set property '{}' of {} '{}'")                                                             \

Userland/Libraries/LibJS/Runtime/GlobalObject.cpp

@@ -290,6 +290,11 @@ void GlobalObject::put_to_scope(const FlyString& name, Variable variable)
     put(name, variable.value);
 }
 
+bool GlobalObject::delete_from_scope(FlyString const& name)
+{
+    return delete_property(name);
+}
+
 bool GlobalObject::has_this_binding() const
 {
     return true;

Userland/Libraries/LibJS/Runtime/GlobalObject.h

@@ -23,6 +23,7 @@ public:
 
     virtual Optional<Variable> get_from_scope(const FlyString&) const override;
     virtual void put_to_scope(const FlyString&, Variable) override;
+    virtual bool delete_from_scope(FlyString const&) override;
     virtual bool has_this_binding() const override;
     virtual Value get_this_binding(GlobalObject&) const override;
 

Userland/Libraries/LibJS/Runtime/LexicalEnvironment.cpp

@@ -62,6 +62,11 @@ void LexicalEnvironment::put_to_scope(const FlyString& name, Variable variable)
     m_variables.set(name, variable);
 }
 
+bool LexicalEnvironment::delete_from_scope(FlyString const& name)
+{
+    return m_variables.remove(name);
+}
+
 bool LexicalEnvironment::has_super_binding() const
 {
     return m_environment_record_type == EnvironmentRecordType::Function && this_binding_status() != ThisBindingStatus::Lexical && m_home_object.is_object();

Userland/Libraries/LibJS/Runtime/LexicalEnvironment.h

@@ -39,6 +39,7 @@ public:
     // ^ScopeObject
     virtual Optional<Variable> get_from_scope(const FlyString&) const override;
     virtual void put_to_scope(const FlyString&, Variable) override;
+    virtual bool delete_from_scope(FlyString const&) override;
     virtual bool has_this_binding() const override;
     virtual Value get_this_binding(GlobalObject&) const override;
 

Userland/Libraries/LibJS/Runtime/Reference.cpp

@@ -98,4 +98,32 @@ Value Reference::get(GlobalObject& global_object)
     return object->get(m_name).value_or(js_undefined());
 }
 
+bool Reference::delete_(GlobalObject& global_object)
+{
+    if (is_unresolvable())
+        return true;
+
+    auto& vm = global_object.vm();
+
+    if (is_local_variable() || is_global_variable()) {
+        if (is_local_variable())
+            return vm.delete_variable(m_name.to_string());
+        else
+            return global_object.delete_property(m_name);
+    }
+
+    auto base = this->base();
+
+    if (base.is_nullish()) {
+        // This will always fail the to_object() call below, let's throw the TypeError ourselves with a nice message instead.
+        vm.throw_exception<TypeError>(global_object, ErrorType::ReferenceNullishDeleteProperty, m_name.to_value(vm).to_string_without_side_effects(), base.to_string_without_side_effects());
+        return false;
+    }
+
+    auto* object = base.to_object(global_object);
+    VERIFY(object);
+
+    return object->delete_property(m_name);
+}
+
 }

Userland/Libraries/LibJS/Runtime/Reference.h

@@ -67,6 +67,7 @@ public:
 
     void put(GlobalObject&, Value);
     Value get(GlobalObject&);
+    bool delete_(GlobalObject&);
 
 private:
     void throw_reference_error(GlobalObject&);

Userland/Libraries/LibJS/Runtime/ScopeObject.h

@@ -21,6 +21,7 @@ class ScopeObject : public Object {
 public:
     virtual Optional<Variable> get_from_scope(const FlyString&) const = 0;
     virtual void put_to_scope(const FlyString&, Variable) = 0;
+    virtual bool delete_from_scope(FlyString const&) = 0;
     virtual bool has_this_binding() const = 0;
     virtual Value get_this_binding(GlobalObject&) const = 0;
 

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -161,6 +161,29 @@ void VM::set_variable(const FlyString& name, Value value, GlobalObject& global_o
     global_object.put(name, value);
 }
 
+bool VM::delete_variable(FlyString const& name)
+{
+    ScopeObject* specific_scope = nullptr;
+    Optional<Variable> possible_match;
+    if (!m_call_stack.is_empty()) {
+        for (auto* scope = current_scope(); scope; scope = scope->parent()) {
+            possible_match = scope->get_from_scope(name);
+            if (possible_match.has_value()) {
+                specific_scope = scope;
+                break;
+            }
+        }
+    }
+
+    if (!possible_match.has_value())
+        return false;
+    if (possible_match.value().declaration_kind == DeclarationKind::Const)
+        return false;
+
+    VERIFY(specific_scope);
+    return specific_scope->delete_from_scope(name);
+}
+
 void VM::assign(const FlyString& target, Value value, GlobalObject& global_object, bool first_assignment, ScopeObject* specific_scope)
 {
     set_variable(target, move(value), global_object, first_assignment, specific_scope);

Userland/Libraries/LibJS/Runtime/VM.h

@@ -187,6 +187,7 @@ public:
 
     Value get_variable(const FlyString& name, GlobalObject&);
     void set_variable(const FlyString& name, Value, GlobalObject&, bool first_assignment = false, ScopeObject* specific_scope = nullptr);
+    bool delete_variable(FlyString const& name);
     void assign(const Variant<NonnullRefPtr<Identifier>, NonnullRefPtr<BindingPattern>>& target, Value, GlobalObject&, bool first_assignment = false, ScopeObject* specific_scope = nullptr);
     void assign(const FlyString& target, Value, GlobalObject&, bool first_assignment = false, ScopeObject* specific_scope = nullptr);
     void assign(const NonnullRefPtr<BindingPattern>& target, Value, GlobalObject&, bool first_assignment = false, ScopeObject* specific_scope = nullptr);

Userland/Libraries/LibJS/Runtime/WithScope.cpp

@@ -34,6 +34,11 @@ void WithScope::put_to_scope(const FlyString& name, Variable variable)
     m_object.put(name, variable.value);
 }
 
+bool WithScope::delete_from_scope(FlyString const& name)
+{
+    return m_object.delete_property(name);
+}
+
 bool WithScope::has_this_binding() const
 {
     return parent()->has_this_binding();

Userland/Libraries/LibJS/Runtime/WithScope.h

@@ -18,6 +18,7 @@ public:
 
     virtual Optional<Variable> get_from_scope(const FlyString&) const override;
     virtual void put_to_scope(const FlyString&, Variable) override;
+    virtual bool delete_from_scope(FlyString const&) override;
     virtual bool has_this_binding() const override;
     virtual Value get_this_binding(GlobalObject&) const override;
 

Userland/Libraries/LibJS/Tests/operators/delete-local-variable.js

@@ -0,0 +1,8 @@
+test("basic functionality", () => {
+    let a = 5;
+    expect(delete a).toBeTrue();
+
+    expect(() => {
+        a;
+    }).toThrowWithMessage(ReferenceError, "'a' is not defined");
+});