From 275da6fcc9274cd2e3ac0e8a89af8566820b7275 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 10 Apr 2021 18:35:29 +0200 Subject: [PATCH] LibJS: Update Object::define_accessor() to take both getter and setter This replaces the current 'function plus boolean indicating the type' API, which makes it easier to set both getter and setter at once. This was already possible before but required two calls of this function, which wasn't intuitive: define_accessor(name, getter, true, ...); define_accessor(name, setter, false, ...); Which now becomes: define_accessor(name, getter, setter, ...); --- Userland/Libraries/LibJS/AST.cpp | 49 ++++++++++++--------- Userland/Libraries/LibJS/Runtime/Object.cpp | 14 +++--- Userland/Libraries/LibJS/Runtime/Object.h | 2 +- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 1816115ed1..cd414e4ae0 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -814,21 +814,20 @@ Value ClassExpression::execute(Interpreter& interpreter, GlobalObject& global_ob auto& target = method.is_static() ? *class_constructor : class_prototype.as_object(); method_function.set_home_object(&target); - if (method.kind() == ClassMethod::Kind::Method) { + switch (method.kind()) { + case ClassMethod::Kind::Method: target.define_property(StringOrSymbol::from_value(global_object, key), method_value); - } else { - String accessor_name = [&] { - switch (method.kind()) { - case ClassMethod::Kind::Getter: - return String::formatted("get {}", get_function_name(global_object, key)); - case ClassMethod::Kind::Setter: - return String::formatted("set {}", get_function_name(global_object, key)); - default: - VERIFY_NOT_REACHED(); - } - }(); - update_function_name(method_value, accessor_name); - target.define_accessor(StringOrSymbol::from_value(global_object, key), method_function, method.kind() == ClassMethod::Kind::Getter, Attribute::Configurable | Attribute::Enumerable); + break; + case ClassMethod::Kind::Getter: + update_function_name(method_value, String::formatted("get {}", get_function_name(global_object, key))); + target.define_accessor(StringOrSymbol::from_value(global_object, key), &method_function, nullptr, Attribute::Configurable | Attribute::Enumerable); + break; + case ClassMethod::Kind::Setter: + update_function_name(method_value, String::formatted("set {}", get_function_name(global_object, key))); + target.define_accessor(StringOrSymbol::from_value(global_object, key), nullptr, &method_function, Attribute::Configurable | Attribute::Enumerable); + break; + default: + VERIFY_NOT_REACHED(); } if (interpreter.exception()) return {}; @@ -1675,16 +1674,24 @@ Value ObjectExpression::execute(Interpreter& interpreter, GlobalObject& global_o update_function_name(value, name); - if (property.type() == ObjectProperty::Type::Getter || property.type() == ObjectProperty::Type::Setter) { + switch (property.type()) { + case ObjectProperty::Type::Getter: VERIFY(value.is_function()); - object->define_accessor(PropertyName::from_value(global_object, key), value.as_function(), property.type() == ObjectProperty::Type::Getter, Attribute::Configurable | Attribute::Enumerable); - if (interpreter.exception()) - return {}; - } else { + object->define_accessor(PropertyName::from_value(global_object, key), &value.as_function(), nullptr, Attribute::Configurable | Attribute::Enumerable); + break; + case ObjectProperty::Type::Setter: + VERIFY(value.is_function()); + object->define_accessor(PropertyName::from_value(global_object, key), nullptr, &value.as_function(), Attribute::Configurable | Attribute::Enumerable); + break; + case ObjectProperty::Type::KeyValue: object->define_property(PropertyName::from_value(global_object, key), value); - if (interpreter.exception()) - return {}; + break; + case ObjectProperty::Type::Spread: + default: + VERIFY_NOT_REACHED(); } + if (interpreter.exception()) + return {}; } return object; } diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index aa42fda180..41a1b29070 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -536,7 +536,7 @@ bool Object::define_property(const PropertyName& property_name, Value value, Pro return put_own_property(property_name.to_string_or_symbol(), value, attributes, PutOwnPropertyMode::DefineProperty, throw_exceptions); } -bool Object::define_accessor(const PropertyName& property_name, Function& getter_or_setter, bool is_getter, PropertyAttributes attributes, bool throw_exceptions) +bool Object::define_accessor(const PropertyName& property_name, Function* getter, Function* setter, PropertyAttributes attributes, bool throw_exceptions) { VERIFY(property_name.is_valid()); @@ -548,18 +548,18 @@ bool Object::define_accessor(const PropertyName& property_name, Function& getter accessor = &existing_property.as_accessor(); } if (!accessor) { - accessor = Accessor::create(vm(), nullptr, nullptr); + accessor = Accessor::create(vm(), getter, setter); bool definition_success = define_property(property_name, accessor, attributes, throw_exceptions); if (vm().exception()) return {}; if (!definition_success) return false; + } else { + if (getter) + accessor->set_getter(getter); + if (setter) + accessor->set_setter(setter); } - if (is_getter) - accessor->set_getter(&getter_or_setter); - else - accessor->set_setter(&getter_or_setter); - return true; } diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index 7ad8c91a9c..5210670df2 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -110,7 +110,7 @@ public: virtual bool define_property(const StringOrSymbol& property_name, const Object& descriptor, bool throw_exceptions = true); bool define_property(const PropertyName&, Value value, PropertyAttributes attributes = default_attributes, bool throw_exceptions = true); bool define_property_without_transition(const PropertyName&, Value value, PropertyAttributes attributes = default_attributes, bool throw_exceptions = true); - bool define_accessor(const PropertyName&, Function& getter_or_setter, bool is_getter, PropertyAttributes attributes = default_attributes, bool throw_exceptions = true); + bool define_accessor(const PropertyName&, Function* getter, Function* setter, PropertyAttributes attributes = default_attributes, bool throw_exceptions = true); bool define_native_function(const StringOrSymbol& property_name, AK::Function, i32 length = 0, PropertyAttributes attributes = default_attributes); bool define_native_property(const StringOrSymbol& property_name, AK::Function getter, AK::Function setter, PropertyAttributes attributes = default_attributes);