Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary wraps for non built-in functions #1126

Merged
merged 3 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions boa/src/builtins/array/array_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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( )
Expand Down
44 changes: 26 additions & 18 deletions boa/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand All @@ -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)
}

Expand All @@ -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()));
Expand 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()));
Expand 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(),
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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| {
Expand Down Expand Up @@ -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( )`
Expand All @@ -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( )`
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion boa/src/builtins/array/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
12 changes: 6 additions & 6 deletions boa/src/builtins/date/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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 {
Expand All @@ -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()`
Expand All @@ -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)`
Expand Down
2 changes: 1 addition & 1 deletion boa/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions boa/src/builtins/map/map_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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( )
Expand Down Expand Up @@ -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,
)?;
Expand Down
18 changes: 15 additions & 3 deletions boa/src/builtins/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()`
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions boa/src/builtins/object/for_in_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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( )
Expand Down
8 changes: 4 additions & 4 deletions boa/src/builtins/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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() {
Expand All @@ -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 {
Expand Down Expand Up @@ -251,7 +251,7 @@ impl Object {
Attribute::all(),
);

Ok(descriptor.build().into())
descriptor.build().into()
}

/// Uses the SameValue algorithm to check equality of objects
Expand Down
14 changes: 7 additions & 7 deletions boa/src/builtins/string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(" ");
Expand All @@ -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))
}
}

Expand Down Expand Up @@ -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] )`
Expand All @@ -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()
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 2 additions & 0 deletions boa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading