From fb89c324c5a577712881fb5d385f5f3d004c071f Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Tue, 3 Nov 2020 19:52:21 +0000 Subject: [PATCH] LibJS: Implement spec-compliant OrdinaryToPrimitive This renames Object::to_primitive() to Object::ordinary_to_primitive() for two reasons: - No confusion with Value::to_primitive() - To match the spec's name Also change existing uses of Object::to_primitive() to Value::to_primitive() when the spec uses the latter (which will still call Object::ordinary_to_primitive()). Object::to_string() has been removed as it's not needed anymore (and nothing the spec uses). This makes it possible to overwrite an object's toString and valueOf and have them provide results for anything that uses to_primitive() - e.g.: const o = { toString: undefined, valueOf: () => 42 }; Number(o) // 42, previously NaN ["foo", o].toString(); // "foo,42", previously "foo,[object Object]" ++o // 43, previously NaN etc. --- Libraries/LibJS/Runtime/Object.cpp | 54 +++++++------------ Libraries/LibJS/Runtime/Object.h | 3 +- Libraries/LibJS/Runtime/Value.cpp | 12 +++-- .../LibJS/Tests/ordinary-to-primitive.js | 23 ++++++++ 4 files changed, 50 insertions(+), 42 deletions(-) create mode 100644 Libraries/LibJS/Tests/ordinary-to-primitive.js diff --git a/Libraries/LibJS/Runtime/Object.cpp b/Libraries/LibJS/Runtime/Object.cpp index 5ed464624c..331f3b3704 100644 --- a/Libraries/LibJS/Runtime/Object.cpp +++ b/Libraries/LibJS/Runtime/Object.cpp @@ -836,48 +836,30 @@ bool Object::has_own_property(const PropertyName& property_name) const return shape().lookup(property_name.to_string_or_symbol()).has_value(); } -Value Object::to_primitive(Value::PreferredType preferred_type) const +Value Object::ordinary_to_primitive(Value::PreferredType preferred_type) const { - Value result = js_undefined(); + ASSERT(preferred_type == Value::PreferredType::String || preferred_type == Value::PreferredType::Number); - switch (preferred_type) { - case Value::PreferredType::Default: - case Value::PreferredType::Number: { - result = value_of(); - if (result.is_object()) { - result = to_string(); - } - break; - } - case Value::PreferredType::String: { - result = to_string(); - if (result.is_object()) - result = value_of(); - break; - } - } - - ASSERT(!result.is_object()); - return result; -} - -Value Object::to_string() const -{ auto& vm = this->vm(); - auto to_string_property = get(vm.names.toString); - if (to_string_property.is_function()) { - auto& to_string_function = to_string_property.as_function(); - auto to_string_result = vm.call(to_string_function, const_cast(this)); - if (to_string_result.is_object()) - vm.throw_exception(global_object(), ErrorType::Convert, "object", "string"); + + Vector method_names; + if (preferred_type == Value::PreferredType::String) + method_names = { vm.names.toString, vm.names.valueOf }; + else + method_names = { vm.names.valueOf, vm.names.toString }; + + for (auto& method_name : method_names) { + auto method = get(method_name); if (vm.exception()) return {}; - auto* string = to_string_result.to_primitive_string(global_object()); - if (vm.exception()) - return {}; - return string; + if (method.is_function()) { + auto result = vm.call(method.as_function(), const_cast(this)); + if (!result.is_object()) + return result; + } } - return js_string(vm, String::formatted("[object {}]", class_name())); + vm.throw_exception(global_object(), ErrorType::Convert, "object", preferred_type == Value::PreferredType::String ? "string" : "number"); + return {}; } Value Object::invoke(const StringOrSymbol& property_name, Optional arguments) diff --git a/Libraries/LibJS/Runtime/Object.h b/Libraries/LibJS/Runtime/Object.h index a1d76957a3..93e9daac69 100644 --- a/Libraries/LibJS/Runtime/Object.h +++ b/Libraries/LibJS/Runtime/Object.h @@ -140,8 +140,7 @@ public: virtual bool prevent_extensions(); virtual Value value_of() const { return Value(const_cast(this)); } - virtual Value to_primitive(Value::PreferredType preferred_type = Value::PreferredType::Default) const; - virtual Value to_string() const; + virtual Value ordinary_to_primitive(Value::PreferredType preferred_type) const; Value get_direct(size_t index) const { return m_storage[index]; } diff --git a/Libraries/LibJS/Runtime/Value.cpp b/Libraries/LibJS/Runtime/Value.cpp index 953a3b70ad..a0b6a4f96d 100644 --- a/Libraries/LibJS/Runtime/Value.cpp +++ b/Libraries/LibJS/Runtime/Value.cpp @@ -168,7 +168,7 @@ String Value::to_string(GlobalObject& global_object) const case Type::BigInt: return m_value.as_bigint->big_integer().to_base10(); case Type::Object: { - auto primitive_value = as_object().to_primitive(PreferredType::String); + auto primitive_value = to_primitive(PreferredType::String); if (global_object.vm().exception()) return {}; return primitive_value.to_string(global_object); @@ -205,8 +205,12 @@ bool Value::to_boolean() const Value Value::to_primitive(PreferredType preferred_type) const { - if (is_object()) - return as_object().to_primitive(preferred_type); + if (is_object()) { + // FIXME: Also support @@toPrimitive + if (preferred_type == PreferredType::Default) + preferred_type = PreferredType::Number; + return as_object().ordinary_to_primitive(preferred_type); + } return *this; } @@ -277,7 +281,7 @@ Value Value::to_number(GlobalObject& global_object) const global_object.vm().throw_exception(global_object, ErrorType::Convert, "BigInt", "number"); return {}; case Type::Object: { - auto primitive = m_value.as_object->to_primitive(PreferredType::Number); + auto primitive = to_primitive(PreferredType::Number); if (global_object.vm().exception()) return {}; return primitive.to_number(global_object); diff --git a/Libraries/LibJS/Tests/ordinary-to-primitive.js b/Libraries/LibJS/Tests/ordinary-to-primitive.js new file mode 100644 index 0000000000..176beadb76 --- /dev/null +++ b/Libraries/LibJS/Tests/ordinary-to-primitive.js @@ -0,0 +1,23 @@ +test("object with custom toString", () => { + const o = { toString: () => "foo" }; + expect(o + "bar").toBe("foobar"); + expect([o, "bar"].toString()).toBe("foo,bar"); +}); + +test("object with uncallable toString and custom valueOf", () => { + const o = { toString: undefined, valueOf: () => "foo" }; + expect(o + "bar").toBe("foobar"); + expect([o, "bar"].toString()).toBe("foo,bar"); +}); + +test("object with custom valueOf", () => { + const o = { valueOf: () => 42 }; + expect(Number(o)).toBe(42); + expect(o + 1).toBe(43); +}); + +test("object with uncallable valueOf and custom toString", () => { + const o = { valueOf: undefined, toString: () => "42" }; + expect(Number(o)).toBe(42); + expect(o + 1).toBe("421"); +});