LibJS: Replace StringOrSymbol::from_value with Value::to_property_key

This is a more specification compliant implementation of the
abstract operation 7.1.19 ToPropertyKey which should handle boxed
symbols correctly.

Userland/Libraries/LibJS/AST.cpp

@@ -824,15 +824,15 @@ Value ClassExpression::execute(Interpreter& interpreter, GlobalObject& global_ob
 
         switch (method.kind()) {
         case ClassMethod::Kind::Method:
-            target.define_property(StringOrSymbol::from_value(global_object, key), method_value);
+            target.define_property(key.to_property_key(global_object), method_value);
             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);
+            target.define_accessor(key.to_property_key(global_object), &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);
+            target.define_accessor(key.to_property_key(global_object), nullptr, &method_function, Attribute::Configurable | Attribute::Enumerable);
             break;
         default:
             VERIFY_NOT_REACHED();

Userland/Libraries/LibJS/Forward.h

@@ -130,6 +130,7 @@ class ScopeNode;
 class ScopeObject;
 class Shape;
 class Statement;
+class StringOrSymbol;
 class Symbol;
 class Token;
 class VM;

Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp

@@ -202,14 +202,14 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::define_property_)
         vm.throw_exception<TypeError>(global_object, ErrorType::NotAnObject, "Object argument");
         return {};
     }
+    auto property_key = vm.argument(1).to_property_key(global_object);
+    if (vm.exception())
+        return {};
     if (!vm.argument(2).is_object()) {
         vm.throw_exception<TypeError>(global_object, ErrorType::NotAnObject, "Descriptor argument");
         return {};
     }
     auto& object = vm.argument(0).as_object();
-    auto property_key = StringOrSymbol::from_value(global_object, vm.argument(1));
-    if (vm.exception())
-        return {};
     auto& descriptor = vm.argument(2).as_object();
     if (!object.define_property(property_key, descriptor)) {
         if (!vm.exception()) {
@@ -316,10 +316,10 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::has_own)
     auto* object = vm.argument(0).to_object(global_object);
     if (vm.exception())
         return {};
-    auto string_or_symbol = StringOrSymbol::from_value(global_object, vm.argument(1));
+    auto property_key = vm.argument(1).to_property_key(global_object);
     if (vm.exception())
         return {};
-    return Value(object->has_own_property(string_or_symbol));
+    return Value(object->has_own_property(property_key));
 }
 
 }

Userland/Libraries/LibJS/Runtime/ObjectPrototype.cpp

@@ -43,13 +43,13 @@ ObjectPrototype::~ObjectPrototype()
 
 JS_DEFINE_NATIVE_FUNCTION(ObjectPrototype::has_own_property)
 {
+    auto property_key = vm.argument(0).to_property_key(global_object);
+    if (vm.exception())
+        return {};
     auto* this_object = vm.this_value(global_object).to_object(global_object);
     if (!this_object)
         return {};
-    auto string_or_symbol = StringOrSymbol::from_value(global_object, vm.argument(0));
-    if (vm.exception())
-        return {};
-    return Value(this_object->has_own_property(string_or_symbol));
+    return Value(this_object->has_own_property(property_key));
 }
 
 JS_DEFINE_NATIVE_FUNCTION(ObjectPrototype::to_string)

Userland/Libraries/LibJS/Runtime/ReflectObject.cpp

@@ -124,13 +124,13 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::define_property)
     auto* target = get_target_object_from(global_object, "defineProperty");
     if (!target)
         return {};
+    auto property_key = vm.argument(1).to_property_key(global_object);
+    if (vm.exception())
+        return {};
     if (!vm.argument(2).is_object()) {
         vm.throw_exception<TypeError>(global_object, ErrorType::ReflectBadDescriptorArgument);
         return {};
     }
-    auto property_key = StringOrSymbol::from_value(global_object, vm.argument(1));
-    if (vm.exception())
-        return {};
     auto& descriptor = vm.argument(2).as_object();
     auto success = target->define_property(property_key, descriptor, false);
     if (vm.exception())

Userland/Libraries/LibJS/Runtime/StringOrSymbol.h

@@ -15,18 +15,6 @@ namespace JS {
 
 class StringOrSymbol {
 public:
-    static StringOrSymbol from_value(GlobalObject& global_object, Value value)
-    {
-        if (value.is_empty())
-            return {};
-        if (value.is_symbol())
-            return &value.as_symbol();
-        auto string = value.to_string(global_object);
-        if (string.is_null())
-            return {};
-        return string;
-    }
-
     StringOrSymbol() = default;
 
     StringOrSymbol(const char* chars)

Userland/Libraries/LibJS/Runtime/Value.cpp

@@ -543,6 +543,16 @@ double Value::to_double(GlobalObject& global_object) const
     return number.as_double();
 }
 
+StringOrSymbol Value::to_property_key(GlobalObject& global_object) const
+{
+    auto key = to_primitive(global_object, PreferredType::String);
+    if (global_object.vm().exception())
+        return {};
+    if (key.is_symbol())
+        return &key.as_symbol();
+    return to_string(global_object);
+}
+
 i32 Value::to_i32_slow_case(GlobalObject& global_object) const
 {
     VERIFY(type() != Type::Int32);
@@ -1008,10 +1018,10 @@ Value in(GlobalObject& global_object, Value lhs, Value rhs)
         global_object.vm().throw_exception<TypeError>(global_object, ErrorType::InOperatorWithObject);
         return {};
     }
-    auto lhs_string_or_symbol = StringOrSymbol::from_value(global_object, lhs);
+    auto lhs_property_key = lhs.to_property_key(global_object);
     if (global_object.vm().exception())
         return {};
-    return Value(rhs.as_object().has_property(lhs_string_or_symbol));
+    return Value(rhs.as_object().has_property(lhs_property_key));
 }
 
 Value instance_of(GlobalObject& global_object, Value lhs, Value rhs)

Userland/Libraries/LibJS/Runtime/Value.h

@@ -260,6 +260,7 @@ public:
     Value to_number(GlobalObject&) const;
     BigInt* to_bigint(GlobalObject&) const;
     double to_double(GlobalObject&) const;
+    StringOrSymbol to_property_key(GlobalObject&) const;
     i32 to_i32(GlobalObject& global_object) const
     {
         if (m_type == Type::Int32)