Remove unnecessary wraps for non built-in functions (#1126)

boa/src/builtins/array/array_iterator.rs

@@ -49,14 +49,14 @@ impl ArrayIterator {
         context: &Context,
         array: Value,
         kind: ArrayIterationKind,
-    ) -> Result<Value> {
+    ) -> Value {
         let array_iterator = Value::new_object(context);
         array_iterator.set_data(ObjectData::ArrayIterator(Self::new(array, kind)));
         array_iterator
             .as_object()
             .expect("array iterator object")
             .set_prototype_instance(context.iterator_prototypes().array_iterator().into());
-        Ok(array_iterator)
+        array_iterator
     }
 
     /// %ArrayIteratorPrototype%.next( )

boa/src/builtins/array/mod.rs

@@ -120,7 +120,7 @@ impl Array {
             .unwrap_or_else(|| context.standard_objects().array_object().prototype());
         // Delegate to the appropriate constructor based on the number of arguments
         match args.len() {
-            0 => Array::construct_array_empty(prototype, context),
+            0 => Ok(Array::construct_array_empty(prototype, context)),
             1 => Array::construct_array_length(prototype, &args[0], context),
             _ => Array::construct_array_values(prototype, args, context),
         }
@@ -132,7 +132,7 @@ impl Array {
     ///  - [ECMAScript reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-array-constructor-array
-    fn construct_array_empty(proto: GcObject, context: &mut Context) -> Result<Value> {
+    fn construct_array_empty(proto: GcObject, context: &mut Context) -> Value {
         Array::array_create(0, Some(proto), context)
     }
 
@@ -147,7 +147,7 @@ impl Array {
         length: &Value,
         context: &mut Context,
     ) -> Result<Value> {
-        let array = Array::array_create(0, Some(prototype), context)?;
+        let array = Array::array_create(0, Some(prototype), context);
 
         if !length.is_number() {
             array.set_property(0, DataDescriptor::new(length, Attribute::all()));
@@ -174,7 +174,7 @@ impl Array {
         context: &mut Context,
     ) -> Result<Value> {
         let items_len = items.len().try_into().map_err(interror_to_value)?;
-        let array = Array::array_create(items_len, Some(prototype), context)?;
+        let array = Array::array_create(items_len, Some(prototype), context);
 
         for (k, item) in items.iter().enumerate() {
             array.set_property(k, DataDescriptor::new(item.clone(), Attribute::all()));
@@ -189,11 +189,7 @@ impl Array {
     ///  - [ECMAScript reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-arraycreate
-    fn array_create(
-        length: u32,
-        prototype: Option<GcObject>,
-        context: &mut Context,
-    ) -> Result<Value> {
+    fn array_create(length: u32, prototype: Option<GcObject>, context: &mut Context) -> Value {
         let prototype = match prototype {
             Some(prototype) => prototype,
             None => context.standard_objects().array_object().prototype(),
@@ -214,11 +210,11 @@ impl Array {
         );
         array.set_property("length", length);
 
-        Ok(array)
+        array
     }
 
     /// Creates a new `Array` instance.
-    pub(crate) fn new_array(context: &Context) -> Result<Value> {
+    pub(crate) fn new_array(context: &Context) -> Value {
         let array = Value::new_object(context);
         array.set_data(ObjectData::Array);
         array
@@ -230,7 +226,7 @@ impl Array {
             Attribute::WRITABLE | Attribute::NON_ENUMERABLE | Attribute::PERMANENT,
         );
         array.set_property("length", length);
-        Ok(array)
+        array
     }
 
     /// Utility function for creating array objects.
@@ -682,7 +678,7 @@ impl Array {
             return context.throw_range_error("Invalid array length");
         }
 
-        let new = Self::new_array(context)?;
+        let new = Self::new_array(context);
 
         let values = (0..length)
             .map(|idx| {
@@ -960,7 +956,7 @@ impl Array {
     /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.slice
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice
     pub(crate) fn slice(this: &Value, args: &[Value], context: &mut Context) -> Result<Value> {
-        let new_array = Self::new_array(context)?;
+        let new_array = Self::new_array(context);
 
         let len = this.get_field("length", context)?.to_length(context)?;
         let from = Self::get_relative_start(context, args.get(0), len)?;
@@ -1005,7 +1001,7 @@ impl Array {
 
         let length = this.get_field("length", context)?.to_length(context)?;
 
-        let new = Self::new_array(context)?;
+        let new = Self::new_array(context);
 
         let values = (0..length)
             .map(|idx| {
@@ -1245,7 +1241,11 @@ impl Array {
     /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.values
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/values
     pub(crate) fn values(this: &Value, _: &[Value], context: &mut Context) -> Result<Value> {
-        ArrayIterator::create_array_iterator(context, this.clone(), ArrayIterationKind::Value)
+        Ok(ArrayIterator::create_array_iterator(
+            context,
+            this.clone(),
+            ArrayIterationKind::Value,
+        ))
     }
 
     /// `Array.prototype.keys( )`
@@ -1259,7 +1259,11 @@ impl Array {
     /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.values
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/values
     pub(crate) fn keys(this: &Value, _: &[Value], context: &mut Context) -> Result<Value> {
-        ArrayIterator::create_array_iterator(context, this.clone(), ArrayIterationKind::Key)
+        Ok(ArrayIterator::create_array_iterator(
+            context,
+            this.clone(),
+            ArrayIterationKind::Key,
+        ))
     }
 
     /// `Array.prototype.entries( )`
@@ -1273,7 +1277,11 @@ impl Array {
     /// [spec]: https://tc39.es/ecma262/#sec-array.prototype.values
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/values
     pub(crate) fn entries(this: &Value, _: &[Value], context: &mut Context) -> Result<Value> {
-        ArrayIterator::create_array_iterator(context, this.clone(), ArrayIterationKind::KeyAndValue)
+        Ok(ArrayIterator::create_array_iterator(
+            context,
+            this.clone(),
+            ArrayIterationKind::KeyAndValue,
+        ))
     }
 
     /// Represents the algorithm to calculate `relativeStart` (or `k`) in array functions.

boa/src/builtins/array/tests.rs

@@ -1366,7 +1366,7 @@ fn get_relative_end() {
 fn array_length_is_not_enumerable() {
     let context = Context::new();
 
-    let array = Array::new_array(&context).unwrap();
+    let array = Array::new_array(&context);
     let desc = array.get_property("length").unwrap();
     assert!(!desc.enumerable());
 }

boa/src/builtins/date/mod.rs

@@ -371,7 +371,7 @@ impl Date {
         context: &mut Context,
     ) -> Result<Value> {
         if new_target.is_undefined() {
-            Self::make_date_string()
+            Ok(Self::make_date_string())
         } else {
             let prototype = new_target
                 .as_object()
@@ -386,7 +386,7 @@ impl Date {
             obj.set_prototype_instance(prototype.into());
             let this = obj.into();
             if args.is_empty() {
-                Self::make_date_now(&this)
+                Ok(Self::make_date_now(&this))
             } else if args.len() == 1 {
                 Self::make_date_single(&this, args, context)
             } else {
@@ -405,8 +405,8 @@ impl Date {
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-date-constructor
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/Date
-    pub(crate) fn make_date_string() -> Result<Value> {
-        Ok(Value::from(Local::now().to_rfc3339()))
+    pub(crate) fn make_date_string() -> Value {
+        Value::from(Local::now().to_rfc3339())
     }
 
     /// `Date()`
@@ -419,10 +419,10 @@ impl Date {
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-date-constructor
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/Date
-    pub(crate) fn make_date_now(this: &Value) -> Result<Value> {
+    pub(crate) fn make_date_now(this: &Value) -> Value {
         let date = Date::default();
         this.set_data(ObjectData::Date(date));
-        Ok(this.clone())
+        this.clone()
     }
 
     /// `Date(value)`

boa/src/builtins/function/mod.rs

@@ -119,7 +119,7 @@ impl Function {
         local_env: &Environment,
     ) {
         // Create array of values
-        let array = Array::new_array(context).unwrap();
+        let array = Array::new_array(context);
         Array::add_to_array_object(&array, &args_list[index..], context).unwrap();
 
         // Create binding

boa/src/builtins/map/map_iterator.rs

@@ -49,14 +49,14 @@ impl MapIterator {
         context: &Context,
         map: Value,
         kind: MapIterationKind,
-    ) -> Result<Value> {
+    ) -> Value {
         let map_iterator = Value::new_object(context);
         map_iterator.set_data(ObjectData::MapIterator(Self::new(map, kind)));
         map_iterator
             .as_object()
             .expect("map iterator object")
             .set_prototype_instance(context.iterator_prototypes().map_iterator().into());
-        Ok(map_iterator)
+        map_iterator
     }
 
     /// %MapIteratorPrototype%.next( )
@@ -104,7 +104,7 @@ impl MapIterator {
                                     }
                                     MapIterationKind::KeyAndValue => {
                                         let result = Array::construct_array(
-                                            &Array::new_array(context)?,
+                                            &Array::new_array(context),
                                             &[key.clone(), value.clone()],
                                             context,
                                         )?;

boa/src/builtins/map/mod.rs

@@ -158,7 +158,11 @@ impl Map {
     /// [spec]: https://www.ecma-international.org/ecma-262/11.0/index.html#sec-map.prototype.entries
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/entries
     pub(crate) fn entries(this: &Value, _: &[Value], context: &mut Context) -> Result<Value> {
-        MapIterator::create_map_iterator(context, this.clone(), MapIterationKind::KeyAndValue)
+        Ok(MapIterator::create_map_iterator(
+            context,
+            this.clone(),
+            MapIterationKind::KeyAndValue,
+        ))
     }
 
     /// `Map.prototype.keys()`
@@ -172,7 +176,11 @@ impl Map {
     /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.keys
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/keys
     pub(crate) fn keys(this: &Value, _: &[Value], context: &mut Context) -> Result<Value> {
-        MapIterator::create_map_iterator(context, this.clone(), MapIterationKind::Key)
+        Ok(MapIterator::create_map_iterator(
+            context,
+            this.clone(),
+            MapIterationKind::Key,
+        ))
     }
 
     /// Helper function to set the size property.
@@ -395,7 +403,11 @@ impl Map {
     /// [spec]: https://tc39.es/ecma262/#sec-map.prototype.values
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/values
     pub(crate) fn values(this: &Value, _: &[Value], context: &mut Context) -> Result<Value> {
-        MapIterator::create_map_iterator(context, this.clone(), MapIterationKind::Value)
+        Ok(MapIterator::create_map_iterator(
+            context,
+            this.clone(),
+            MapIterationKind::Value,
+        ))
     }
 
     /// Helper function to get a key-value pair from an array.

boa/src/builtins/object/for_in_iterator.rs

@@ -45,14 +45,14 @@ impl ForInIterator {
     ///  - [ECMA reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-createforiniterator
-    pub(crate) fn create_for_in_iterator(context: &Context, object: Value) -> Result<Value> {
+    pub(crate) fn create_for_in_iterator(context: &Context, object: Value) -> Value {
         let for_in_iterator = Value::new_object(context);
         for_in_iterator.set_data(ObjectData::ForInIterator(Self::new(object)));
         for_in_iterator
             .as_object()
             .expect("for in iterator object")
             .set_prototype_instance(context.iterator_prototypes().for_in_iterator().into());
-        Ok(for_in_iterator)
+        for_in_iterator
     }
 
     /// %ForInIteratorPrototype%.next( )

boa/src/builtins/object/mod.rs

@@ -164,7 +164,7 @@ impl Object {
             let key = key.to_property_key(context)?;
 
             if let Some(desc) = object.get_own_property(&key) {
-                return Ok(Self::from_property_descriptor(desc, context)?);
+                return Ok(Self::from_property_descriptor(desc, context));
             }
         }
 
@@ -197,7 +197,7 @@ impl Object {
                 let desc = object
                     .get_own_property(&key)
                     .expect("Expected property to be on object.");
-                Self::from_property_descriptor(desc, context)?
+                Self::from_property_descriptor(desc, context)
             };
 
             if !descriptor.is_undefined() {
@@ -216,7 +216,7 @@ impl Object {
     /// [ECMAScript reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-frompropertydescriptor
-    fn from_property_descriptor(desc: PropertyDescriptor, context: &mut Context) -> Result<Value> {
+    fn from_property_descriptor(desc: PropertyDescriptor, context: &mut Context) -> Value {
         let mut descriptor = ObjectInitializer::new(context);
 
         if let PropertyDescriptor::Data(data_desc) = &desc {
@@ -251,7 +251,7 @@ impl Object {
                 Attribute::all(),
             );
 
-        Ok(descriptor.build().into())
+        descriptor.build().into()
     }
 
     /// Uses the SameValue algorithm to check equality of objects

boa/src/builtins/string/mod.rs

@@ -910,11 +910,11 @@ impl String {
         max_length: i32,
         fill_string: Option<RcString>,
         at_start: bool,
-    ) -> Result<Value> {
+    ) -> Value {
         let primitive_length = primitive.len() as i32;
 
         if max_length <= primitive_length {
-            return Ok(Value::from(primitive));
+            return Value::from(primitive);
         }
 
         let filter = fill_string.as_deref().unwrap_or(" ");
@@ -929,9 +929,9 @@ impl String {
         let concat_fill_str: StdString = fill_str.chars().take(fill_len as usize).collect();
 
         if at_start {
-            Ok(Value::from(format!("{}{}", concat_fill_str, &primitive)))
+            Value::from(format!("{}{}", concat_fill_str, &primitive))
         } else {
-            Ok(Value::from(format!("{}{}", primitive, &concat_fill_str)))
+            Value::from(format!("{}{}", primitive, &concat_fill_str))
         }
     }
 
@@ -959,7 +959,7 @@ impl String {
 
         let fill_string = args.get(1).map(|arg| arg.to_string(context)).transpose()?;
 
-        Self::string_pad(primitive, max_length, fill_string, false)
+        Ok(Self::string_pad(primitive, max_length, fill_string, false))
     }
 
     /// `String.prototype.padStart( targetLength [, padString] )`
@@ -986,7 +986,7 @@ impl String {
 
         let fill_string = args.get(1).map(|arg| arg.to_string(context)).transpose()?;
 
-        Self::string_pad(primitive, max_length, fill_string, true)
+        Ok(Self::string_pad(primitive, max_length, fill_string, true))
     }
 
     /// String.prototype.trim()
@@ -1263,7 +1263,7 @@ impl String {
                 .collect(),
         };
 
-        let new = Array::new_array(context)?;
+        let new = Array::new_array(context);
         Array::construct_array(&new, &values, context)
     }
 

boa/src/lib.rs

@@ -42,6 +42,8 @@ This is an experimental Javascript lexer, parser and compiler written in Rust. C
     missing_doc_code_examples
 )]
 
+// builtins module has a lot of built-in functions that need unnecessary_wraps
+#[allow(clippy::unnecessary_wraps)]
 pub mod builtins;
 pub mod class;
 pub mod environment;

boa/src/syntax/ast/node/array/mod.rs

@@ -38,7 +38,7 @@ pub struct ArrayDecl {
 impl Executable for ArrayDecl {
     fn run(&self, context: &mut Context) -> Result<Value> {
         let _timer = BoaProfiler::global().start_event("ArrayDecl", "exec");
-        let array = Array::new_array(context)?;
+        let array = Array::new_array(context);
         let mut elements = Vec::new();
         for elem in self.as_ref() {
             if let Node::Spread(ref x) = elem {

boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs

@@ -84,7 +84,7 @@ impl Executable for ForInLoop {
             return Ok(result);
         }
         let object = object.to_object(context)?;
-        let for_in_iterator = ForInIterator::create_for_in_iterator(context, Value::from(object))?;
+        let for_in_iterator = ForInIterator::create_for_in_iterator(context, Value::from(object));
         let next_function = for_in_iterator
             .get_property("next")
             .map(|p| p.as_data_descriptor().unwrap().value())

boa/src/syntax/ast/node/template/mod.rs

@@ -88,8 +88,8 @@ impl Executable for TaggedTemplate {
     fn run(&self, context: &mut Context) -> Result<Value> {
         let _timer = BoaProfiler::global().start_event("TaggedTemplate", "exec");
 
-        let template_object = Array::new_array(context)?;
-        let raw_array = Array::new_array(context)?;
+        let template_object = Array::new_array(context);
+        let raw_array = Array::new_array(context);
 
         for (i, raw) in self.raws.iter().enumerate() {
             raw_array.set_field(i, Value::from(raw), context)?;

boa/src/syntax/parser/expression/assignment/mod.rs

@@ -217,6 +217,13 @@ where
 /// [spec]: https://tc39.es/ecma262/#sec-assignment-operators-static-semantics-early-errors
 #[inline]
 pub(crate) fn is_assignable(node: &Node) -> bool {
-    matches!(node, Node::GetConstField(_) | Node::GetField(_) | Node::Assign(_)
-                   | Node::Call(_) | Node::Identifier(_) | Node::Object(_))
+    matches!(
+        node,
+        Node::GetConstField(_)
+            | Node::GetField(_)
+            | Node::Assign(_)
+            | Node::Call(_)
+            | Node::Identifier(_)
+            | Node::Object(_)
+    )
 }

boa/src/syntax/parser/expression/primary/object_initializer/mod.rs

@@ -208,8 +208,10 @@ where
             idn @ "get" | idn @ "set"
                 if matches!(
                     cursor.peek(0)?.map(|t| t.kind()),
-                    Some(&TokenKind::Identifier(_)) | Some(&TokenKind::Keyword(_))
-                        | Some(&TokenKind::BooleanLiteral(_)) | Some(&TokenKind::NullLiteral)
+                    Some(&TokenKind::Identifier(_))
+                        | Some(&TokenKind::Keyword(_))
+                        | Some(&TokenKind::BooleanLiteral(_))
+                        | Some(&TokenKind::NullLiteral)
                         | Some(&TokenKind::NumericLiteral(_))
                 ) =>
             {

boa_tester/src/exec.rs

@@ -110,18 +110,27 @@ impl Test {
             && !IGNORED.contains_test(&self.name)
             && !IGNORED.contains_any_feature(&self.features)
             && (matches!(self.expected_outcome, Outcome::Positive)
-                || matches!(self.expected_outcome, Outcome::Negative {
-                    phase: Phase::Parse,
-                    error_type: _,
-                })
-                || matches!(self.expected_outcome, Outcome::Negative {
-                    phase: Phase::Early,
-                    error_type: _,
-                })
-                || matches!(self.expected_outcome, Outcome::Negative {
-                    phase: Phase::Runtime,
-                    error_type: _,
-                })) {
+                || matches!(
+                    self.expected_outcome,
+                    Outcome::Negative {
+                        phase: Phase::Parse,
+                        error_type: _,
+                    }
+                )
+                || matches!(
+                    self.expected_outcome,
+                    Outcome::Negative {
+                        phase: Phase::Early,
+                        error_type: _,
+                    }
+                )
+                || matches!(
+                    self.expected_outcome,
+                    Outcome::Negative {
+                        phase: Phase::Runtime,
+                        error_type: _,
+                    }
+                )) {
             let res = panic::catch_unwind(|| match self.expected_outcome {
                 Outcome::Positive => {
                     // TODO: implement async and add `harness/doneprintHandle.js` to the includes.