From aa507f3c50c6b6da1cad09bdeed9b51d29301475 Mon Sep 17 00:00:00 2001 From: neeldug <5161147+neeldug@users.noreply.github.com> Date: Sun, 25 Jul 2021 20:52:36 +0100 Subject: [PATCH] style(boa): clippy lints and cleanup of old todos (#1383) - removes needless double references - removes ignored tests/sections --- boa/src/builtins/array/mod.rs | 8 ++++---- boa/src/builtins/array/tests.rs | 14 ++++++-------- boa/src/builtins/bigint/mod.rs | 2 +- boa/src/builtins/date/mod.rs | 2 +- boa/src/builtins/function/mod.rs | 2 +- boa/src/builtins/number/mod.rs | 2 +- boa/src/builtins/string/mod.rs | 2 +- boa/src/builtins/string/tests.rs | 3 --- boa/src/builtins/symbol/mod.rs | 2 +- boa/src/environment/global_environment_record.rs | 8 ++++---- boa/src/environment/object_environment_record.rs | 2 +- boa/src/object/gcobject.rs | 6 +++--- boa/src/object/internal_methods.rs | 10 +++++----- boa/src/syntax/ast/node/conditional/if_node/mod.rs | 2 +- .../node/declaration/arrow_function_decl/mod.rs | 2 +- boa/src/syntax/ast/node/iteration/mod.rs | 1 - boa/src/syntax/ast/node/new/mod.rs | 4 ++-- boa/src/syntax/ast/node/object/mod.rs | 2 +- boa/src/syntax/ast/node/return_smt/mod.rs | 2 +- boa/src/syntax/lexer/string.rs | 6 +++--- boa/src/value/display.rs | 2 +- boa/src/value/mod.rs | 4 ++-- boa/src/value/operations.rs | 11 +++++++---- 23 files changed, 48 insertions(+), 51 deletions(-) diff --git a/boa/src/builtins/array/mod.rs b/boa/src/builtins/array/mod.rs index 9bebb05a27..daaa5d1086 100644 --- a/boa/src/builtins/array/mod.rs +++ b/boa/src/builtins/array/mod.rs @@ -1271,7 +1271,7 @@ impl Array { let len = Self::flatten_into_array( context, &new_array, - &this, + this, source_len, 0, depth, @@ -1331,7 +1331,7 @@ impl Array { if !mapper_function.is_undefined() { // 1. Set element to Call(mapperFunction, thisArg, <>) let args = [element, Value::from(source_index), target.clone()]; - element = context.call(&mapper_function, &this_arg, &args)?; + element = context.call(mapper_function, this_arg, &args)?; } let element_as_object = element.as_object(); @@ -1689,7 +1689,7 @@ impl Array { Value::from(k), this.clone(), ]; - accumulator = context.call(&callback, &Value::undefined(), &arguments)?; + accumulator = context.call(callback, &Value::undefined(), &arguments)?; /* We keep track of possibly shortened length in order to prevent unnecessary iteration. It may also be necessary to do this since shortening the array length does not delete array elements. See: https://github.com/boa-dev/boa/issues/557 */ @@ -1770,7 +1770,7 @@ impl Array { Value::from(k), this.clone(), ]; - accumulator = context.call(&callback, &Value::undefined(), &arguments)?; + accumulator = context.call(callback, &Value::undefined(), &arguments)?; /* We keep track of possibly shortened length in order to prevent unnecessary iteration. It may also be necessary to do this since shortening the array length does not delete array elements. See: https://github.com/boa-dev/boa/issues/557 */ diff --git a/boa/src/builtins/array/tests.rs b/boa/src/builtins/array/tests.rs index ab354188b7..5a79325f79 100644 --- a/boa/src/builtins/array/tests.rs +++ b/boa/src/builtins/array/tests.rs @@ -767,12 +767,11 @@ fn map() { var one = ["x"]; var many = ["x", "y", "z"]; - // TODO: uncomment when `this` has been implemented - // var _this = { answer: 42 }; + var _this = { answer: 42 }; - // function callbackThatUsesThis() { - // return 'The answer to life is: ' + this.answer; - // } + function callbackThatUsesThis() { + return 'The answer to life is: ' + this.answer; + } var empty_mapped = empty.map(v => v + '_'); var one_mapped = one.map(v => '_' + v); @@ -818,10 +817,9 @@ fn map() { String::from("\"_x__y__z_\"") ); - // TODO: uncomment when `this` has been implemented // One but it uses `this` inside the callback - // let one_with_this = forward(&mut context, "one.map(callbackThatUsesThis, _this)[0];"); - // assert_eq!(one_with_this, String::from("The answer to life is: 42")) + let one_with_this = forward(&mut context, "one.map(callbackThatUsesThis, _this)[0];"); + assert_eq!(one_with_this, String::from("\"The answer to life is: 42\"")) } #[test] diff --git a/boa/src/builtins/bigint/mod.rs b/boa/src/builtins/bigint/mod.rs index ff6fc2f9af..7840b71e77 100644 --- a/boa/src/builtins/bigint/mod.rs +++ b/boa/src/builtins/bigint/mod.rs @@ -78,7 +78,7 @@ impl BigInt { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt/BigInt fn constructor(_: &Value, args: &[Value], context: &mut Context) -> Result { let data = match args.get(0) { - Some(ref value) => value.to_bigint(context)?, + Some(value) => value.to_bigint(context)?, None => JsBigInt::zero(), }; Ok(Value::from(data)) diff --git a/boa/src/builtins/date/mod.rs b/boa/src/builtins/date/mod.rs index 8153076fc8..3d39bf4667 100644 --- a/boa/src/builtins/date/mod.rs +++ b/boa/src/builtins/date/mod.rs @@ -444,7 +444,7 @@ impl Date { let tv = match this_time_value(value, context) { Ok(dt) => dt.0, _ => match value.to_primitive(context, PreferredType::Default)? { - Value::String(ref str) => match chrono::DateTime::parse_from_rfc3339(&str) { + Value::String(ref str) => match chrono::DateTime::parse_from_rfc3339(str) { Ok(dt) => Some(dt.naive_utc()), _ => None, }, diff --git a/boa/src/builtins/function/mod.rs b/boa/src/builtins/function/mod.rs index 584359ea6b..c8d2cd3a1e 100644 --- a/boa/src/builtins/function/mod.rs +++ b/boa/src/builtins/function/mod.rs @@ -120,7 +120,7 @@ impl Function { ) { // Create array of values let array = Array::new_array(context); - Array::add_to_array_object(&array, &args_list.get(index..).unwrap_or_default(), context) + Array::add_to_array_object(&array, args_list.get(index..).unwrap_or_default(), context) .unwrap(); // Create binding diff --git a/boa/src/builtins/number/mod.rs b/boa/src/builtins/number/mod.rs index b4d52bf9e0..1178932166 100644 --- a/boa/src/builtins/number/mod.rs +++ b/boa/src/builtins/number/mod.rs @@ -160,7 +160,7 @@ impl Number { context: &mut Context, ) -> Result { let data = match args.get(0) { - Some(ref value) => value.to_numeric_number(context)?, + Some(value) => value.to_numeric_number(context)?, None => 0.0, }; if new_target.is_undefined() { diff --git a/boa/src/builtins/string/mod.rs b/boa/src/builtins/string/mod.rs index a6933f95e0..2cd948b01e 100644 --- a/boa/src/builtins/string/mod.rs +++ b/boa/src/builtins/string/mod.rs @@ -169,7 +169,7 @@ impl String { .expect("'Symbol::to_string' returns 'Value::String'") .clone() } - Some(ref value) => value.to_string(context)?, + Some(value) => value.to_string(context)?, None => JsString::default(), }; diff --git a/boa/src/builtins/string/tests.rs b/boa/src/builtins/string/tests.rs index 4dcc124247..d9da4b23e1 100644 --- a/boa/src/builtins/string/tests.rs +++ b/boa/src/builtins/string/tests.rs @@ -1,8 +1,6 @@ use crate::{forward, forward_val, Context}; -///TODO: re-enable when getProperty() is finished; #[test] -#[ignore] fn length() { //TEST262: https://github.com/tc39/test262/blob/master/test/built-ins/String/length.js let mut context = Context::new(); @@ -16,7 +14,6 @@ fn length() { let a = forward(&mut context, "a.length"); assert_eq!(a, "1"); let b = forward(&mut context, "b.length"); - // TODO: fix this // unicode surrogate pair length should be 1 // utf16/usc2 length should be 2 // utf8 length should be 4 diff --git a/boa/src/builtins/symbol/mod.rs b/boa/src/builtins/symbol/mod.rs index 13629f07ef..8ae2cf469f 100644 --- a/boa/src/builtins/symbol/mod.rs +++ b/boa/src/builtins/symbol/mod.rs @@ -169,7 +169,7 @@ impl Symbol { return context.throw_type_error("Symbol is not a constructor"); } let description = match args.get(0) { - Some(ref value) if !value.is_undefined() => Some(value.to_string(context)?), + Some(value) if !value.is_undefined() => Some(value.to_string(context)?), _ => None, }; diff --git a/boa/src/environment/global_environment_record.rs b/boa/src/environment/global_environment_record.rs index 8228622bff..15a9a56ac8 100644 --- a/boa/src/environment/global_environment_record.rs +++ b/boa/src/environment/global_environment_record.rs @@ -185,7 +185,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn initialize_binding(&self, name: &str, value: Value, context: &mut Context) -> Result<()> { - if self.declarative_record.has_binding(&name) { + if self.declarative_record.has_binding(name) { return self .declarative_record .initialize_binding(name, value, context); @@ -205,7 +205,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { strict: bool, context: &mut Context, ) -> Result<()> { - if self.declarative_record.has_binding(&name) { + if self.declarative_record.has_binding(name) { return self .declarative_record .set_mutable_binding(name, value, strict, context); @@ -215,7 +215,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn get_binding_value(&self, name: &str, strict: bool, context: &mut Context) -> Result { - if self.declarative_record.has_binding(&name) { + if self.declarative_record.has_binding(name) { return self .declarative_record .get_binding_value(name, strict, context); @@ -224,7 +224,7 @@ impl EnvironmentRecordTrait for GlobalEnvironmentRecord { } fn delete_binding(&self, name: &str) -> bool { - if self.declarative_record.has_binding(&name) { + if self.declarative_record.has_binding(name) { return self.declarative_record.delete_binding(name); } diff --git a/boa/src/environment/object_environment_record.rs b/boa/src/environment/object_environment_record.rs index f2e4d81880..8add8e3aae 100644 --- a/boa/src/environment/object_environment_record.rs +++ b/boa/src/environment/object_environment_record.rs @@ -88,7 +88,7 @@ impl EnvironmentRecordTrait for ObjectEnvironmentRecord { // We should never need to check if a binding has been created, // As all calls to create_mutable_binding are followed by initialized binding // The below is just a check. - debug_assert!(self.has_binding(&name)); + debug_assert!(self.has_binding(name)); self.set_mutable_binding(name, value, false, context) } diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index 82a693f506..bc0f786644 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -292,7 +292,7 @@ impl GcObject { match body { FunctionBody::BuiltInConstructor(function) if construct => { - function(&this_target, args, context) + function(this_target, args, context) } FunctionBody::BuiltInConstructor(function) => { function(&Value::undefined(), args, context) @@ -378,7 +378,7 @@ impl GcObject { // a recursive structure // We can follow v8 & SpiderMonkey's lead and return a default value for the hint in this situation // (see https://repl.it/repls/IvoryCircularCertification#index.js) - let recursion_limiter = RecursionLimiter::new(&self); + let recursion_limiter = RecursionLimiter::new(self); if recursion_limiter.live { // we're in a recursive object, bail return Ok(match hint { @@ -1023,7 +1023,7 @@ impl RecursionLimiter { impl Debug for GcObject { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { - let limiter = RecursionLimiter::new(&self); + let limiter = RecursionLimiter::new(self); // Typically, using `!limiter.live` would be good enough here. // However, the JS object hierarchy involves quite a bit of repitition, and the sheer amount of data makes diff --git a/boa/src/object/internal_methods.rs b/boa/src/object/internal_methods.rs index ce0197b99b..010079cfee 100644 --- a/boa/src/object/internal_methods.rs +++ b/boa/src/object/internal_methods.rs @@ -273,7 +273,7 @@ impl GcObject { pub(crate) fn __delete__(&self, key: &PropertyKey) -> bool { match self.__get_own_property__(key) { Some(desc) if desc.configurable() => { - self.remove(&key); + self.remove(key); true } Some(_) => false, @@ -458,13 +458,13 @@ impl GcObject { // 8. if !current.configurable() { if let (Some(current_get), Some(desc_get)) = (current.getter(), desc.getter()) { - if !GcObject::equals(¤t_get, &desc_get) { + if !GcObject::equals(current_get, desc_get) { return false; } } if let (Some(current_set), Some(desc_set)) = (current.setter(), desc.setter()) { - if !GcObject::equals(¤t_set, &desc_set) { + if !GcObject::equals(current_set, desc_set) { return false; } } @@ -685,7 +685,7 @@ impl GcObject { pub fn ordinary_get_own_property(&self, key: &PropertyKey) -> Option { let object = self.borrow(); let property = match key { - PropertyKey::Index(index) => object.indexed_properties.get(&index), + PropertyKey::Index(index) => object.indexed_properties.get(index), PropertyKey::String(ref st) => object.string_properties.get(st), PropertyKey::Symbol(ref symbol) => object.symbol_properties.get(symbol), }; @@ -939,7 +939,7 @@ impl Object { #[inline] pub(crate) fn remove(&mut self, key: &PropertyKey) -> Option { match key { - PropertyKey::Index(index) => self.indexed_properties.remove(&index), + PropertyKey::Index(index) => self.indexed_properties.remove(index), PropertyKey::String(ref string) => self.string_properties.remove(string), PropertyKey::Symbol(ref symbol) => self.symbol_properties.remove(symbol), } diff --git a/boa/src/syntax/ast/node/conditional/if_node/mod.rs b/boa/src/syntax/ast/node/conditional/if_node/mod.rs index 63906833a8..a38de0b4da 100644 --- a/boa/src/syntax/ast/node/conditional/if_node/mod.rs +++ b/boa/src/syntax/ast/node/conditional/if_node/mod.rs @@ -82,7 +82,7 @@ impl Executable for If { fn run(&self, context: &mut Context) -> Result { Ok(if self.cond().run(context)?.to_boolean() { self.body().run(context)? - } else if let Some(ref else_e) = self.else_node() { + } else if let Some(else_e) = self.else_node() { else_e.run(context)? } else { Value::undefined() diff --git a/boa/src/syntax/ast/node/declaration/arrow_function_decl/mod.rs b/boa/src/syntax/ast/node/declaration/arrow_function_decl/mod.rs index 43c57b482a..5645727d72 100644 --- a/boa/src/syntax/ast/node/declaration/arrow_function_decl/mod.rs +++ b/boa/src/syntax/ast/node/declaration/arrow_function_decl/mod.rs @@ -50,7 +50,7 @@ impl ArrowFunctionDecl { /// Gets the body of the arrow function. pub(crate) fn body(&self) -> &[Node] { - &self.body.items() + self.body.items() } /// Implements the display formatting with indentation. diff --git a/boa/src/syntax/ast/node/iteration/mod.rs b/boa/src/syntax/ast/node/iteration/mod.rs index 362e2cb4d0..693bc7e2ea 100644 --- a/boa/src/syntax/ast/node/iteration/mod.rs +++ b/boa/src/syntax/ast/node/iteration/mod.rs @@ -9,7 +9,6 @@ pub use self::{ mod tests; // Checking labels for break and continue is the same operation for `ForLoop`, `While` and `DoWhile` -#[macro_use] macro_rules! handle_state_with_labels { ($self:ident, $label:ident, $interpreter:ident, $state:tt) => {{ if let Some(brk_label) = $label { diff --git a/boa/src/syntax/ast/node/new/mod.rs b/boa/src/syntax/ast/node/new/mod.rs index 59ce787ee7..48603ee8e5 100644 --- a/boa/src/syntax/ast/node/new/mod.rs +++ b/boa/src/syntax/ast/node/new/mod.rs @@ -38,12 +38,12 @@ pub struct New { impl New { /// Gets the name of the function call. pub fn expr(&self) -> &Node { - &self.call.expr() + self.call.expr() } /// Retrieves the arguments passed to the function. pub fn args(&self) -> &[Node] { - &self.call.args() + self.call.args() } } diff --git a/boa/src/syntax/ast/node/object/mod.rs b/boa/src/syntax/ast/node/object/mod.rs index f51e69a87f..f469a98838 100644 --- a/boa/src/syntax/ast/node/object/mod.rs +++ b/boa/src/syntax/ast/node/object/mod.rs @@ -75,7 +75,7 @@ impl Object { MethodDefinitionKind::Ordinary => (), } write!(f, "{}(", key)?; - join_nodes(f, &node.parameters())?; + join_nodes(f, node.parameters())?; write!(f, ") ")?; node.display_block(f, indent + 1)?; writeln!(f, ",")?; diff --git a/boa/src/syntax/ast/node/return_smt/mod.rs b/boa/src/syntax/ast/node/return_smt/mod.rs index adb0f71f4a..f22ae4a13f 100644 --- a/boa/src/syntax/ast/node/return_smt/mod.rs +++ b/boa/src/syntax/ast/node/return_smt/mod.rs @@ -63,7 +63,7 @@ impl Return { impl Executable for Return { fn run(&self, context: &mut Context) -> Result { let result = match self.expr() { - Some(ref v) => v.run(context), + Some(v) => v.run(context), None => Ok(Value::undefined()), }; // Set flag for return diff --git a/boa/src/syntax/lexer/string.rs b/boa/src/syntax/lexer/string.rs index 6c9d0a1465..0f306e3816 100644 --- a/boa/src/syntax/lexer/string.rs +++ b/boa/src/syntax/lexer/string.rs @@ -257,7 +257,7 @@ impl StringLiteral { .ok() .and_then(|code_point_str| { // The `code_point_str` should represent a single unicode codepoint, convert to u32 - u32::from_str_radix(&code_point_str, 16).ok() + u32::from_str_radix(code_point_str, 16).ok() }) .ok_or_else(|| { Error::syntax("malformed Unicode character escape sequence", start_pos) @@ -281,7 +281,7 @@ impl StringLiteral { // Convert to u16 let code_point = str::from_utf8(&code_point_utf8_bytes) .ok() - .and_then(|code_point_str| u16::from_str_radix(&code_point_str, 16).ok()) + .and_then(|code_point_str| u16::from_str_radix(code_point_str, 16).ok()) .ok_or_else(|| Error::syntax("invalid Unicode escape sequence", start_pos))?; Ok(code_point as u32) @@ -300,7 +300,7 @@ impl StringLiteral { cursor.fill_bytes(&mut code_point_utf8_bytes)?; let code_point = str::from_utf8(&code_point_utf8_bytes) .ok() - .and_then(|code_point_str| u16::from_str_radix(&code_point_str, 16).ok()) + .and_then(|code_point_str| u16::from_str_radix(code_point_str, 16).ok()) .ok_or_else(|| Error::syntax("invalid Hexadecimal escape sequence", start_pos))?; Ok(code_point as u32) diff --git a/boa/src/value/display.rs b/boa/src/value/display.rs index 157e328b05..ce7c53e81e 100644 --- a/boa/src/value/display.rs +++ b/boa/src/value/display.rs @@ -178,7 +178,7 @@ pub(crate) fn log_string_from(x: &Value, print_internals: bool, print_children: format!("Set({})", size) } } - _ => display_obj(&x, print_internals), + _ => display_obj(x, print_internals), } } Value::Symbol(ref symbol) => symbol.to_string(), diff --git a/boa/src/value/mod.rs b/boa/src/value/mod.rs index 56e3dcbd16..9c9a26a6de 100644 --- a/boa/src/value/mod.rs +++ b/boa/src/value/mod.rs @@ -564,7 +564,7 @@ impl Value { PreferredType::Default => "default", } .into(); - let result = exotic_to_prim.call(&self, &[hint], context)?; + let result = exotic_to_prim.call(self, &[hint], context)?; return if result.is_object() { Err(context.construct_type_error("Symbol.toPrimitive cannot return an object")) } else { @@ -897,7 +897,7 @@ impl Value { /// See: pub fn to_numeric_number(&self, context: &mut Context) -> Result { let primitive = self.to_primitive(context, PreferredType::Number)?; - if let Some(ref bigint) = primitive.as_bigint() { + if let Some(bigint) = primitive.as_bigint() { return Ok(bigint.to_f64()); } primitive.to_number(context) diff --git a/boa/src/value/operations.rs b/boa/src/value/operations.rs index 7439d363e4..87fe39c413 100644 --- a/boa/src/value/operations.rs +++ b/boa/src/value/operations.rs @@ -11,9 +11,12 @@ impl Value { (Self::Integer(x), Self::Rational(y)) => Self::rational(f64::from(*x) + y), (Self::Rational(x), Self::Integer(y)) => Self::rational(x + f64::from(*y)), + (Self::String(ref x), Self::String(ref y)) => Self::string(format!("{}{}", x, y)), + (Self::String(ref x), y) => Self::string(format!("{}{}", x, y.to_string(context)?)), + (x, Self::String(ref y)) => Self::string(format!("{}{}", x.to_string(context)?, y)), (Self::String(ref x), Self::String(ref y)) => Self::from(JsString::concat(x, y)), - (Self::String(ref x), ref y) => Self::from(JsString::concat(x, y.to_string(context)?)), - (ref x, Self::String(ref y)) => Self::from(JsString::concat(x.to_string(context)?, y)), + (Self::String(ref x), y) => Self::from(JsString::concat(x, y.to_string(context)?)), + (x, Self::String(ref y)) => Self::from(JsString::concat(x.to_string(context)?, y)), (Self::BigInt(ref n1), Self::BigInt(ref n2)) => { Self::bigint(n1.as_inner().clone() + n2.as_inner().clone()) } @@ -487,14 +490,14 @@ impl Value { unreachable!() } (Self::BigInt(ref x), Self::String(ref y)) => { - if let Some(y) = JsBigInt::from_string(&y) { + if let Some(y) = JsBigInt::from_string(y) { (*x < y).into() } else { AbstractRelation::Undefined } } (Self::String(ref x), Self::BigInt(ref y)) => { - if let Some(x) = JsBigInt::from_string(&x) { + if let Some(x) = JsBigInt::from_string(x) { (x < *y).into() } else { AbstractRelation::Undefined