Verify objects before casts

Author: bpcreech

src/py_mini_racer/py_mini_racer.py

@@ -131,18 +131,6 @@ def __hash__(self) -> int:
         return self._ctx.get_identity_hash(self)
 
 
-class JSSymbol:
-    """JavaScript symbol."""
-
-    def __init__(
-        self,
-        ctx: _Context,
-        handle: _ValueHandle,
-    ):
-        self._ctx = ctx
-        self._handle = handle
-
-
 PythonJSConvertedTypes = Union[
     None,
     JSUndefinedType,
@@ -151,7 +139,6 @@ def __init__(
     float,
     str,
     JSObject,
-    JSSymbol,
     datetime,
     memoryview,
 ]
@@ -259,6 +246,10 @@ def __call__(
         return self._ctx.call_function(self, *args, this=this, timeout_sec=timeout_sec)
 
 
+class JSSymbol(JSMappedObject):
+    """JavaScript symbol."""
+
+
 class JSPromise(JSObject):
     """JavaScript Promise.
 
@@ -1008,10 +999,10 @@ def _wrap_raw_handle(self, raw: _RawValueHandleType) -> _ValueHandle:
         return _ValueHandle(self, raw)
 
     def _python_to_value_handle(self, obj: PythonJSConvertedTypes) -> _ValueHandle:
-        if isinstance(obj, (JSObject, JSSymbol)):
-            # JSObjects and JSSymbols originate from the V8 side. We can just send
-            # back the handle we originally got. (This also covers JSObject derived
-            # types JSFunction, JSPromise, and JSArray.)
+        if isinstance(obj, JSObject):
+            # JSObjects originate from the V8 side. We can just send back the handle
+            # we originally got. (This also covers derived types JSFunction, JSSymbol,
+            # JSPromise, and JSArray.)
             return obj._handle  # noqa: SLF001
 
         if obj is None:

src/v8_py_frontend/object_manipulator.cc

@@ -31,7 +31,7 @@ auto ObjectManipulator::GetIdentityHash(v8::Isolate* isolate,
   const v8::Context::Scope context_scope(local_context);
 
   const v8::Local<v8::Value> local_obj_val = obj_ptr->ToValue(local_context);
-  if (!local_obj_val->IsObject()) {
+  if (!local_obj_val->IsObject() && !local_obj_val->IsSymbol()) {
     return bv_factory_->New("Not an object", type_execute_exception);
   }
 
@@ -50,7 +50,7 @@ auto ObjectManipulator::GetOwnPropertyNames(v8::Isolate* isolate,
   const v8::Context::Scope context_scope(local_context);
 
   const v8::Local<v8::Value> local_obj_val = obj_ptr->ToValue(local_context);
-  if (!local_obj_val->IsObject()) {
+  if (!local_obj_val->IsObject() && !local_obj_val->IsSymbol()) {
     return bv_factory_->New("Not an object", type_execute_exception);
   }
 
@@ -71,7 +71,7 @@ auto ObjectManipulator::Get(v8::Isolate* isolate,
   const v8::Context::Scope context_scope(local_context);
 
   const v8::Local<v8::Value> local_obj_val = obj_ptr->ToValue(local_context);
-  if (!local_obj_val->IsObject()) {
+  if (!local_obj_val->IsObject() && !local_obj_val->IsSymbol()) {
     return bv_factory_->New("Not an object", type_execute_exception);
   }
 
@@ -98,7 +98,7 @@ auto ObjectManipulator::Set(v8::Isolate* isolate,
   const v8::Context::Scope context_scope(local_context);
 
   const v8::Local<v8::Value> local_obj_val = obj_ptr->ToValue(local_context);
-  if (!local_obj_val->IsObject()) {
+  if (!local_obj_val->IsObject() && !local_obj_val->IsSymbol()) {
     return bv_factory_->New("Not an object", type_execute_exception);
   }
 
@@ -120,7 +120,7 @@ auto ObjectManipulator::Del(v8::Isolate* isolate,
   const v8::Context::Scope context_scope(local_context);
 
   const v8::Local<v8::Value> local_obj_val = obj_ptr->ToValue(local_context);
-  if (!local_obj_val->IsObject()) {
+  if (!local_obj_val->IsObject() && !local_obj_val->IsSymbol()) {
     return bv_factory_->New("Not an object", type_execute_exception);
   }
 

tests/test_objects.py

@@ -250,6 +250,8 @@ def test_symbol(gc_check):
     )
 
     assert isinstance(obj, JSSymbol)
+    assert obj.__hash__()
+    assert tuple(obj.keys()) == ()
 
     del obj
     gc_check.check(mr)

tests/test_types.py

@@ -141,6 +141,8 @@ def test_symbol(gc_check):
     mr = MiniRacer()
     res = mr.eval('Symbol("my_symbol")')
     assert isinstance(res, JSSymbol)
+    assert res.__hash__() is not None
+    _test_round_trip(mr, res)
 
     del res
     gc_check.check(mr)