style(boa): clippy lints and cleanup of old todos (#1383)

- removes needless double references
- removes ignored tests/sections

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, <<element, sourceIndex, source>>)
                     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 */

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]

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<Value> {
         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))

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,
                 },

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

boa/src/builtins/number/mod.rs

@@ -160,7 +160,7 @@ impl Number {
         context: &mut Context,
     ) -> Result<Value> {
         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() {

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(),
         };
 

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

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,
         };
 

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<Value> {
-        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);
         }
 

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)
     }
 

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

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(&current_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(&current_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<PropertyDescriptor> {
         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<PropertyDescriptor> {
         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),
         }

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<Value> {
         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()

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.

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 {

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()
     }
 }
 

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, ",")?;

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<Value> {
         let result = match self.expr() {
-            Some(ref v) => v.run(context),
+            Some(v) => v.run(context),
             None => Ok(Value::undefined()),
         };
         // Set flag for return

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)

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(),

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: <https://tc39.es/ecma262/#sec-tonumeric>
     pub fn to_numeric_number(&self, context: &mut Context) -> Result<f64> {
         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)

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