From 72313a8da58ccbeda4198f7bb561fed504ec6361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Borges?= Date: Tue, 16 Feb 2021 19:56:03 +0000 Subject: [PATCH] Remove unnecessary wraps for non built-in functions (#1126) --- boa/src/builtins/array/array_iterator.rs | 4 +- boa/src/builtins/array/mod.rs | 44 +++++++++++-------- boa/src/builtins/array/tests.rs | 2 +- boa/src/builtins/date/mod.rs | 12 ++--- boa/src/builtins/function/mod.rs | 2 +- boa/src/builtins/map/map_iterator.rs | 6 +-- boa/src/builtins/map/mod.rs | 18 ++++++-- boa/src/builtins/object/for_in_iterator.rs | 4 +- boa/src/builtins/object/mod.rs | 8 ++-- boa/src/builtins/string/mod.rs | 14 +++--- boa/src/lib.rs | 2 + boa/src/syntax/ast/node/array/mod.rs | 2 +- .../ast/node/iteration/for_in_loop/mod.rs | 2 +- boa/src/syntax/ast/node/template/mod.rs | 4 +- .../parser/expression/assignment/mod.rs | 11 ++++- .../primary/object_initializer/mod.rs | 6 ++- boa_tester/src/exec.rs | 33 +++++++++----- 17 files changed, 107 insertions(+), 67 deletions(-) diff --git a/boa/src/builtins/array/array_iterator.rs b/boa/src/builtins/array/array_iterator.rs index 33c9e84b05..3934d5979a 100644 --- a/boa/src/builtins/array/array_iterator.rs +++ b/boa/src/builtins/array/array_iterator.rs @@ -49,14 +49,14 @@ impl ArrayIterator { context: &Context, array: Value, kind: ArrayIterationKind, - ) -> Result { + ) -> 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( ) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 32b734ff07..3d4a4c56ca 100644 --- a/boa/src/builtins/array/mod.rs +++ b/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 { + 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 { - 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 { 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, - context: &mut Context, - ) -> Result { + fn array_create(length: u32, prototype: Option, 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 { + 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 { - 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 { - 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 { - 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 { - 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. diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index dac5b842b4..8ae8e1119e 100644 --- a/boa/src/builtins/array/tests.rs +++ b/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()); } diff --git a/boa/src/builtins/date/mod.rs b/boa/src/builtins/date/mod.rs index 3f49aa6ae3..40556ac0c6 100644 --- a/boa/src/builtins/date/mod.rs +++ b/boa/src/builtins/date/mod.rs @@ -371,7 +371,7 @@ impl Date { context: &mut Context, ) -> Result { 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 { - 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 { + 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)` diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 0f3e7ab90d..b9b8ae9dc4 100644 --- a/boa/src/builtins/function/mod.rs +++ b/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 diff --git a/boa/src/builtins/map/map_iterator.rs b/boa/src/builtins/map/map_iterator.rs index fe2c933c65..13766475a7 100644 --- a/boa/src/builtins/map/map_iterator.rs +++ b/boa/src/builtins/map/map_iterator.rs @@ -49,14 +49,14 @@ impl MapIterator { context: &Context, map: Value, kind: MapIterationKind, - ) -> Result { + ) -> 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, )?; diff --git a/boa/src/builtins/map/mod.rs b/boa/src/builtins/map/mod.rs index 3b9c293743..3e4e56a455 100644 --- a/boa/src/builtins/map/mod.rs +++ b/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 { - 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 { - 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 { - 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. diff --git a/boa/src/builtins/object/for_in_iterator.rs b/boa/src/builtins/object/for_in_iterator.rs index 61c1d3bfa1..deeb9c2284 100644 --- a/boa/src/builtins/object/for_in_iterator.rs +++ b/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 { + 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( ) diff --git a/boa/src/builtins/object/mod.rs b/boa/src/builtins/object/mod.rs index fd6f423914..11a2ebbc20 100644 --- a/boa/src/builtins/object/mod.rs +++ b/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 { + 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 diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index b7ee8b07b4..1e309ada8b 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -910,11 +910,11 @@ impl String { max_length: i32, fill_string: Option, at_start: bool, - ) -> Result { + ) -> 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) } diff --git a/boa/src/lib.rs b/boa/src/lib.rs index 0540130fc3..25c54e44e7 100644 --- a/boa/src/lib.rs +++ b/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; diff --git a/boa/src/syntax/ast/node/array/mod.rs b/boa/src/syntax/ast/node/array/mod.rs index 7096770918..8bb7f5ac32 100644 --- a/boa/src/syntax/ast/node/array/mod.rs +++ b/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 { 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 { diff --git a/boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs b/boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs index 9841aedb3c..faf1124be5 100644 --- a/boa/src/syntax/ast/node/iteration/for_in_loop/mod.rs +++ b/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()) diff --git a/boa/src/syntax/ast/node/template/mod.rs b/boa/src/syntax/ast/node/template/mod.rs index 896c754118..2e6a747a84 100644 --- a/boa/src/syntax/ast/node/template/mod.rs +++ b/boa/src/syntax/ast/node/template/mod.rs @@ -88,8 +88,8 @@ impl Executable for TaggedTemplate { fn run(&self, context: &mut Context) -> Result { 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)?; diff --git a/boa/src/syntax/parser/expression/assignment/mod.rs b/boa/src/syntax/parser/expression/assignment/mod.rs index 0a7cf4c4d0..be30d52eeb 100644 --- a/boa/src/syntax/parser/expression/assignment/mod.rs +++ b/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(_) + ) } diff --git a/boa/src/syntax/parser/expression/primary/object_initializer/mod.rs b/boa/src/syntax/parser/expression/primary/object_initializer/mod.rs index 0ae11c5dad..9a450c8cef 100644 --- a/boa/src/syntax/parser/expression/primary/object_initializer/mod.rs +++ b/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(_)) ) => { diff --git a/boa_tester/src/exec.rs b/boa_tester/src/exec.rs index aa011deec6..b362f92356 100644 --- a/boa_tester/src/exec.rs +++ b/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.