fix(builtins/function/mod,function/arguments,function/bound,object/mod): convert panics to EngineError::Panic using js_expect (#5001)

Part of #3241.

It changes the following:
* `core/engine/src/builtins/function/arguments.rs`: converted 7 panics
(7× `js_expect` on `downcast_ref::<MappedArguments>()` exotic method
calls); `binding_indices` retains `.expect("binding must exist")` as a
documented
compiler invariant — binding existence is guaranteed by
`bound_names(formals)`
* `core/engine/src/builtins/function/mod.rs`: converted 10 panics (2×
`downcast_ref::<OrdinaryFunction>()`, 3× `vm.frames.last()` frame
existence, 1× `to_integer_or_infinity`, 1× `define_property_or_throw`
length, 1× `define_property_or_throw` name, 1× `new.target` object cast,
1× `set_function_name`) to use `js_expect`
* `core/engine/src/builtins/function/bound.rs`: converted 2 panics (2×
`downcast_ref::<BoundFunction>()` exotic method calls) to use
`js_expect`
* `core/engine/src/builtins/object/mod.rs`: converted 10 panics (6×
`CreateDataPropertyOrThrow`
* `from_property_descriptor`, 1× `create_data_property_or_throw` in
`group_by`,1× `create_data_property_or_throw` in
`get_own_property_descriptors`,1× `toObject` in `to_string`, 1×
`ToObject` in `assign`) to use `js_expect`

Author: KaustubhOG

core/engine/src/builtins/function/arguments.rs

@@ -1,5 +1,5 @@
 use crate::{
-    Context, JsData, JsResult, JsValue,
+    Context, JsData, JsExpect, JsResult, JsValue,
     bytecompiler::ToJsString,
     environments::DeclarativeEnvironment,
     object::{
@@ -296,7 +296,7 @@ pub(crate) fn arguments_exotic_get_own_property(
     if let PropertyKey::Index(index) = key
         && let Some(value) = obj
             .downcast_ref::<MappedArguments>()
-            .expect("arguments exotic method must only be callable from arguments objects")
+            .js_expect("arguments exotic method must only be callable from arguments objects")?
             .get(index.get())
     {
         // a. Set desc.[[Value]] to Get(map, P).
@@ -331,7 +331,7 @@ pub(crate) fn arguments_exotic_define_own_property(
     let mapped = if let &PropertyKey::Index(index) = &key {
         // 1. Let map be args.[[ParameterMap]].
         obj.downcast_ref::<MappedArguments>()
-            .expect("arguments exotic method must only be callable from arguments objects")
+            .js_expect("arguments exotic method must only be callable from arguments objects")?
             .get(index.get())
             .map(|value| (index, value))
     } else {
@@ -376,7 +376,7 @@ pub(crate) fn arguments_exotic_define_own_property(
         // 1. Let map be args.[[ParameterMap]].
         let mut map = obj
             .downcast_mut::<MappedArguments>()
-            .expect("arguments exotic method must only be callable from arguments objects");
+            .js_expect("arguments exotic method must only be callable from arguments objects")?;
 
         // a. If IsAccessorDescriptor(Desc) is true, then
         if desc.is_accessor_descriptor() {
@@ -425,7 +425,7 @@ pub(crate) fn arguments_exotic_try_get(
     if let PropertyKey::Index(index) = key
         && let Some(value) = obj
             .downcast_ref::<MappedArguments>()
-            .expect("arguments exotic method must only be callable from arguments objects")
+            .js_expect("arguments exotic method must only be callable from arguments objects")?
             .get(index.get())
     {
         // a. Assert: map contains a formal parameter mapping for P.
@@ -455,7 +455,7 @@ pub(crate) fn arguments_exotic_get(
     if let PropertyKey::Index(index) = key
         && let Some(value) = obj
             .downcast_ref::<MappedArguments>()
-            .expect("arguments exotic method must only be callable from arguments objects")
+            .js_expect("arguments exotic method must only be callable from arguments objects")?
             .get(index.get())
     {
         // a. Assert: map contains a formal parameter mapping for P.
@@ -493,7 +493,7 @@ pub(crate) fn arguments_exotic_set(
         // a. Let setStatus be Set(map, P, V, false).
         // b. Assert: setStatus is true because formal parameters mapped by argument objects are always writable.
         obj.downcast_ref::<MappedArguments>()
-            .expect("arguments exotic method must only be callable from arguments objects")
+            .js_expect("arguments exotic method must only be callable from arguments objects")?
             .set(index.get(), &value);
     }
 
@@ -521,7 +521,7 @@ pub(crate) fn arguments_exotic_delete(
         // 4. If result is true and isMapped is true, then
         // a. Call map.[[Delete]](P).
         obj.downcast_mut::<MappedArguments>()
-            .expect("arguments exotic method must only be callable from arguments objects")
+            .js_expect("arguments exotic method must only be callable from arguments objects")?
             .delete(index.get());
     }
 

core/engine/src/builtins/function/bound.rs

@@ -1,7 +1,7 @@
 use boa_gc::{Finalize, Trace};
 
 use crate::{
-    Context, JsObject, JsResult, JsValue,
+    Context, JsExpect, JsObject, JsResult, JsValue,
     object::{
         JsData,
         internal_methods::{
@@ -108,9 +108,9 @@ fn bound_function_exotic_call(
     argument_count: usize,
     context: &mut InternalMethodCallContext<'_>,
 ) -> JsResult<CallValue> {
-    let bound_function = obj
-        .downcast_ref::<BoundFunction>()
-        .expect("bound function exotic method should only be callable from bound function objects");
+    let bound_function = obj.downcast_ref::<BoundFunction>().js_expect(
+        "bound function exotic method should only be callable from bound function objects",
+    )?;
 
     // 1. Let target be F.[[BoundTargetFunction]].
     let target = bound_function.target_function();
@@ -155,9 +155,9 @@ fn bound_function_exotic_construct(
 
     debug_assert!(new_target.is_object(), "new.target should be an object");
 
-    let bound_function = function_object
-        .downcast_ref::<BoundFunction>()
-        .expect("bound function exotic method should only be callable from bound function objects");
+    let bound_function = function_object.downcast_ref::<BoundFunction>().js_expect(
+        "bound function exotic method should only be callable from bound function objects",
+    )?;
 
     // 1. Let target be F.[[BoundTargetFunction]].
     let target = bound_function.target_function();

core/engine/src/builtins/function/mod.rs

@@ -12,7 +12,7 @@
 //! [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function
 
 use crate::{
-    Context, JsArgs, JsResult, JsStr, JsString, JsValue, SpannedSourceText,
+    Context, JsArgs, JsExpect, JsResult, JsStr, JsString, JsValue, SpannedSourceText,
     builtins::{
         BuiltInBuilder, BuiltInConstructor, BuiltInObject, IntrinsicObject, OrdinaryObject,
     },
@@ -762,7 +762,7 @@ impl BuiltInFunctionObject {
                 // 1. Let targetLenAsInt be ! ToIntegerOrInfinity(targetLen).
                 match target_len
                     .to_integer_or_infinity(context)
-                    .expect("to_integer_or_infinity cannot fail for a number")
+                    .js_expect("to_integer_or_infinity cannot fail for a number")?
                 {
                     // i. If targetLen is +∞𝔽, set L to +∞.
                     IntegerOrInfinity::PositiveInfinity => l = f64::INFINITY.into(),
@@ -789,7 +789,7 @@ impl BuiltInFunctionObject {
                 .configurable(true),
             context,
         )
-        .expect("defining the `length` property for a new object should not fail");
+        .js_expect("defining the `length` property for a new object should not fail")?;
 
         // 8. Let targetName be ? Get(Target, "name").
         let target_name = target.get(js_string!("name"), context)?;
@@ -798,7 +798,7 @@ impl BuiltInFunctionObject {
         let target_name = target_name.as_string().unwrap_or_default();
 
         // 10. Perform SetFunctionName(F, targetName, "bound").
-        set_function_name(&f, &target_name.into(), Some(js_str!("bound")), context);
+        set_function_name(&f, &target_name.into(), Some(js_str!("bound")), context)?;
 
         // 11. Return F.
         Ok(f.into())
@@ -922,7 +922,7 @@ pub(crate) fn set_function_name(
     name: &PropertyKey,
     prefix: Option<JsStr<'_>>,
     context: &mut Context,
-) {
+) -> JsResult<()> {
     // 1. Assert: F is an extensible object that does not have a "name" own property.
     // 2. If Type(name) is Symbol, then
     let mut name = match name {
@@ -967,7 +967,9 @@ pub(crate) fn set_function_name(
                 .configurable(true),
             context,
         )
-        .expect("defining the `name` property must not fail per the spec");
+        .js_expect("defining the `name` property must not fail per the spec")?;
+
+    Ok(())
 }
 
 /// Call this object.
@@ -990,7 +992,7 @@ pub(crate) fn function_call(
 
     let function = function_object
         .downcast_ref::<OrdinaryFunction>()
-        .expect("not a function");
+        .js_expect("not a function")?;
     let realm = function.realm().clone();
 
     if function.code.is_class_constructor() {
@@ -1041,18 +1043,18 @@ pub(crate) fn function_call(
             context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
             let this: JsValue = context.realm().global_this().clone().into();
             context.vm.stack.set_this(
-                context.vm.frames.last().expect("frame must exist"),
+                context.vm.frames.last().js_expect("frame must exist")?,
                 this.clone(),
             );
             ThisBindingStatus::Initialized(this)
         } else {
             let this: JsValue = this
                 .to_object(context)
-                .expect("conversion cannot fail")
+                .js_expect("conversion cannot fail")?
                 .into();
             context.vm.frame_mut().flags |= CallFrameFlags::THIS_VALUE_CACHED;
             context.vm.stack.set_this(
-                context.vm.frames.last().expect("frame must exist"),
+                context.vm.frames.last().js_expect("frame must exist")?,
                 this.clone(),
             );
             ThisBindingStatus::Initialized(this)
@@ -1106,7 +1108,7 @@ fn function_construct(
 
     let function = this_function_object
         .downcast_ref::<OrdinaryFunction>()
-        .expect("not a function");
+        .js_expect("not a function")?;
     let realm = function.realm().clone();
 
     debug_assert!(
@@ -1198,7 +1200,7 @@ fn function_construct(
                 Some(
                     new_target
                         .as_object()
-                        .expect("new.target should be an object")
+                        .js_expect("new.target should be an object")?
                         .clone(),
                 ),
             ),
@@ -1208,7 +1210,7 @@ fn function_construct(
 
     let context = context.context();
     context.vm.stack.set_this(
-        context.vm.frames.last().expect("frame must exist"),
+        context.vm.frames.last().js_expect("frame must exist")?,
         this.map(JsValue::new).unwrap_or_default(),
     );
 

core/engine/src/builtins/object/mod.rs

@@ -19,7 +19,7 @@ use super::{
 use crate::builtins::function::arguments::{MappedArguments, UnmappedArguments};
 use crate::value::JsVariant;
 use crate::{
-    Context, JsArgs, JsData, JsResult, JsString,
+    Context, JsArgs, JsData, JsExpect, JsResult, JsString,
     builtins::{BuiltInObject, iterable::IteratorHint, map},
     context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
     error::JsNativeError,
@@ -169,7 +169,7 @@ impl BuiltInConstructor for OrdinaryObject {
                     .unwrap_or_else(|| context.intrinsics().constructors().object().constructor())
                     .into()
         {
-            //     a. Return ? OrdinaryCreateFromConstructor(NewTarget, "%Object.prototype%").
+            //     a. Return ? OrdinaryCreateFromConstructor(NewTarget, "%Object.prototype%").
             let prototype =
                 get_prototype_from_constructor(new_target, StandardConstructors::object, context)?;
             let object = JsObject::from_proto_and_data_with_shared_shape(
@@ -186,7 +186,7 @@ impl BuiltInConstructor for OrdinaryObject {
         if value.is_null_or_undefined() {
             Ok(JsObject::with_object_proto(context.intrinsics()).into())
         } else {
-            // 3. Return ! ToObject(value).
+            // 3. Return ! ToObject(value).
             value.to_object(context).map(JsValue::from)
         }
     }
@@ -511,7 +511,7 @@ impl OrdinaryObject {
             obj.__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?;
 
         // 4. Return FromPropertyDescriptor(desc).
-        Ok(Self::from_property_descriptor(desc, context))
+        Self::from_property_descriptor(desc, context)
     }
 
     /// `Object.getOwnPropertyDescriptors( object )`
@@ -547,14 +547,14 @@ impl OrdinaryObject {
                 obj.__get_own_property__(&key, &mut InternalMethodPropertyContext::new(context))?;
 
             // b. Let descriptor be FromPropertyDescriptor(desc).
-            let descriptor = Self::from_property_descriptor(desc, context);
+            let descriptor = Self::from_property_descriptor(desc, context)?;
 
             // c. If descriptor is not undefined,
             //    perform ! CreateDataPropertyOrThrow(descriptors, key, descriptor).
             if !descriptor.is_undefined() {
                 descriptors
                     .create_data_property_or_throw(key, descriptor, context)
-                    .expect("should not fail according to spec");
+                    .js_expect("should not fail according to spec")?;
             }
         }
 
@@ -570,10 +570,10 @@ impl OrdinaryObject {
     pub(crate) fn from_property_descriptor(
         desc: Option<PropertyDescriptor>,
         context: &mut Context,
-    ) -> JsValue {
+    ) -> JsResult<JsValue> {
         // 1. If Desc is undefined, return undefined.
         let Some(desc) = desc else {
-            return JsValue::undefined();
+            return Ok(JsValue::undefined());
         };
 
         // 2. Let obj be ! OrdinaryObjectCreate(%Object.prototype%).
@@ -584,46 +584,46 @@ impl OrdinaryObject {
         if let Some(value) = desc.value() {
             // a. Perform ! CreateDataPropertyOrThrow(obj, "value", Desc.[[Value]]).
             obj.create_data_property_or_throw(js_string!("value"), value.clone(), context)
-                .expect("CreateDataPropertyOrThrow cannot fail here");
+                .js_expect("CreateDataPropertyOrThrow cannot fail here")?;
         }
 
         // 5. If Desc has a [[Writable]] field, then
         if let Some(writable) = desc.writable() {
             // a. Perform ! CreateDataPropertyOrThrow(obj, "writable", Desc.[[Writable]]).
             obj.create_data_property_or_throw(js_string!("writable"), writable, context)
-                .expect("CreateDataPropertyOrThrow cannot fail here");
+                .js_expect("CreateDataPropertyOrThrow cannot fail here")?;
         }
 
         // 6. If Desc has a [[Get]] field, then
         if let Some(get) = desc.get() {
             // a. Perform ! CreateDataPropertyOrThrow(obj, "get", Desc.[[Get]]).
             obj.create_data_property_or_throw(js_string!("get"), get.clone(), context)
-                .expect("CreateDataPropertyOrThrow cannot fail here");
+                .js_expect("CreateDataPropertyOrThrow cannot fail here")?;
         }
 
         // 7. If Desc has a [[Set]] field, then
         if let Some(set) = desc.set() {
             // a. Perform ! CreateDataPropertyOrThrow(obj, "set", Desc.[[Set]]).
             obj.create_data_property_or_throw(js_string!("set"), set.clone(), context)
-                .expect("CreateDataPropertyOrThrow cannot fail here");
+                .js_expect("CreateDataPropertyOrThrow cannot fail here")?;
         }
 
         // 8. If Desc has an [[Enumerable]] field, then
         if let Some(enumerable) = desc.enumerable() {
             // a. Perform ! CreateDataPropertyOrThrow(obj, "enumerable", Desc.[[Enumerable]]).
             obj.create_data_property_or_throw(js_string!("enumerable"), enumerable, context)
-                .expect("CreateDataPropertyOrThrow cannot fail here");
+                .js_expect("CreateDataPropertyOrThrow cannot fail here")?;
         }
 
         // 9. If Desc has a [[Configurable]] field, then
         if let Some(configurable) = desc.configurable() {
             // a. Perform ! CreateDataPropertyOrThrow(obj, "configurable", Desc.[[Configurable]]).
             obj.create_data_property_or_throw(js_string!("configurable"), configurable, context)
-                .expect("CreateDataPropertyOrThrow cannot fail here");
+                .js_expect("CreateDataPropertyOrThrow cannot fail here")?;
         }
 
         // 10. Return obj.
-        obj.into()
+        Ok(obj.into())
     }
 
     /// Uses the `SameValue` algorithm to check equality of objects
@@ -841,7 +841,9 @@ impl OrdinaryObject {
             return Ok(js_string!("[object Null]").into());
         }
         // 3. Let O be ! ToObject(this value).
-        let o = this.to_object(context).expect("toObject cannot fail here");
+        let o = this
+            .to_object(context)
+            .js_expect("toObject cannot fail here")?;
 
         //  4. Let isArray be ? IsArray(O).
         //  5. If isArray is true, let builtinTag be "Array".
@@ -993,7 +995,7 @@ impl OrdinaryObject {
                 // 3.a.i. Let from be ! ToObject(nextSource).
                 let from = source
                     .to_object(context)
-                    .expect("this ToObject call must not fail");
+                    .js_expect("this ToObject call must not fail")?;
                 // 3.a.ii. Let keys be ? from.[[OwnPropertyKeys]]().
                 let keys =
                     from.__own_property_keys__(&mut InternalMethodPropertyContext::new(context))?;
@@ -1446,7 +1448,7 @@ impl OrdinaryObject {
 
             // b. Perform ! CreateDataPropertyOrThrow(obj, g.[[Key]], elements).
             obj.create_data_property_or_throw(key, elements, context)
-                .expect("cannot fail for a newly created object");
+                .js_expect("cannot fail for a newly created object")?;
         }
 
         // 4. Return obj.

core/engine/src/builtins/proxy/mod.rs

@@ -612,7 +612,7 @@ pub(crate) fn proxy_exotic_define_own_property(
     };
 
     // 7. Let descObj be FromPropertyDescriptor(Desc).
-    let desc_obj = OrdinaryObject::from_property_descriptor(Some(desc.clone()), context);
+    let desc_obj = OrdinaryObject::from_property_descriptor(Some(desc.clone()), context)?;
 
     // 8. Let booleanTrapResult be ! ToBoolean(? Call(trap, handler, « target, P, descObj »)).
     // 9. If booleanTrapResult is false, return false.

core/engine/src/object/operations.rs

@@ -2,7 +2,7 @@ use super::internal_methods::InternalMethodPropertyContext;
 use crate::js_error;
 use crate::value::JsVariant;
 use crate::{
-    Context, JsResult, JsSymbol, JsValue,
+    Context, JsExpect, JsResult, JsSymbol, JsValue,
     builtins::{
         Array, Proxy,
         function::{BoundFunction, ClassFieldDefinition, OrdinaryFunction, set_function_name},
@@ -1238,11 +1238,11 @@ impl JsObject {
                     set_function_name(
                         &init_value
                             .as_object()
-                            .expect("init value must be a function object"),
+                            .js_expect("init value must be a function object")?,
                         function_name,
                         None,
                         context,
-                    );
+                    )?;
                 }
 
                 // a. Assert: IsPropertyKey(fieldName) is true.