General code clean-up and new lint addition (#1809)

This PR adds some Clippy lints. Mainly, it adds the list of pedantic lints excluding some lints that were causing too many warnings. I also denied some useful restriction and pedantic lints, to make sure we use `Self` all the possible times (for better maintainability), and that we pass elements by reference where possible, for example, or that the documentation is properly written.

This might even have some small performance gains.

I also added a perfect hash function for the CLI keywords, which should be more efficient than a `HashSet`. This is something we could use elsewhere too.

boa/src/bigint.rs

@@ -77,23 +77,23 @@ impl JsBigInt {
         self.inner.to_str_radix(radix)
     }
 
-    /// Converts the BigInt to a f64 type.
+    /// Converts the `BigInt` to a f64 type.
     ///
-    /// Returns `f64::INFINITY` if the BigInt is too big.
+    /// Returns `f64::INFINITY` if the `BigInt` is too big.
     #[inline]
     pub fn to_f64(&self) -> f64 {
         self.inner.to_f64().unwrap_or(f64::INFINITY)
     }
 
-    /// Converts a string to a BigInt with the specified radix.
+    /// Converts a string to a `BigInt` with the specified radix.
     #[inline]
     pub fn from_string_radix(buf: &str, radix: u32) -> Option<Self> {
         Some(Self {
             inner: Rc::new(RawBigInt::parse_bytes(buf.as_bytes(), radix)?),
         })
     }
 
-    /// This function takes a string and conversts it to BigInt type.
+    /// This function takes a string and conversts it to `BigInt` type.
     ///
     /// More information:
     ///  - [ECMAScript reference][spec]
@@ -104,7 +104,7 @@ impl JsBigInt {
         string = string.trim();
 
         if string.is_empty() {
-            return Some(JsBigInt::zero());
+            return Some(Self::zero());
         }
 
         let mut radix = 10;
@@ -149,7 +149,7 @@ impl JsBigInt {
 
     /// Checks for mathematical equality.
     ///
-    /// The abstract operation BigInt::equal takes arguments x (a `BigInt`) and y (a `BigInt`).
+    /// The abstract operation `BigInt::equal` takes arguments x (a `BigInt`) and y (a `BigInt`).
     /// It returns `true` if x and y have the same mathematical integer value and false otherwise.
     ///
     /// More information:

boa/src/builtins/array/array_iterator.rs

@@ -25,15 +25,15 @@ impl ArrayIterator {
     pub(crate) const NAME: &'static str = "ArrayIterator";
 
     fn new(array: JsObject, kind: PropertyNameKind) -> Self {
-        ArrayIterator {
+        Self {
             array,
             kind,
             next_index: 0,
             done: false,
         }
     }
 
-    /// CreateArrayIterator( array, kind )
+    /// `CreateArrayIterator( array, kind )`
     ///
     /// Creates a new iterator over the given array.
     ///
@@ -62,7 +62,7 @@ impl ArrayIterator {
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-%arrayiteratorprototype%.next
     pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult<JsValue> {
-        let mut array_iterator = this.as_object().map(|obj| obj.borrow_mut());
+        let mut array_iterator = this.as_object().map(JsObject::borrow_mut);
         let array_iterator = array_iterator
             .as_mut()
             .and_then(|obj| obj.as_array_iterator_mut())
@@ -111,7 +111,7 @@ impl ArrayIterator {
         }
     }
 
-    /// Create the %ArrayIteratorPrototype% object
+    /// Create the `%ArrayIteratorPrototype%` object
     ///
     /// More information:
     ///  - [ECMA reference][spec]

boa/src/builtins/array/mod.rs

@@ -142,16 +142,17 @@ impl Array {
         // 4. If numberOfArgs = 0, then
         if number_of_args == 0 {
             // 4.a. Return ! ArrayCreate(0, proto).
-            Ok(Array::array_create(0, Some(prototype), context)
+            Ok(Self::array_create(0, Some(prototype), context)
                 .unwrap()
                 .into())
         // 5. Else if numberOfArgs = 1, then
         } else if number_of_args == 1 {
             // a. Let len be values[0].
             let len = &args[0];
             // b. Let array be ! ArrayCreate(0, proto).
-            let array = Array::array_create(0, Some(prototype), context).unwrap();
+            let array = Self::array_create(0, Some(prototype), context).unwrap();
             // c. If Type(len) is not Number, then
+            #[allow(clippy::if_not_else)]
             let int_len = if !len.is_number() {
                 // i. Perform ! CreateDataPropertyOrThrow(array, "0", len).
                 array
@@ -179,7 +180,7 @@ impl Array {
             debug_assert!(number_of_args >= 2);
 
             // b. Let array be ? ArrayCreate(numberOfArgs, proto).
-            let array = Array::array_create(number_of_args, Some(prototype), context)?;
+            let array = Self::array_create(number_of_args, Some(prototype), context)?;
             // c. Let k be 0.
             // d. Repeat, while k < numberOfArgs,
             for (i, item) in args.iter().cloned().enumerate() {
@@ -304,13 +305,14 @@ impl Array {
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-get-array-@@species
     /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/@@species
+    #[allow(clippy::unnecessary_wraps)]
     fn get_species(this: &JsValue, _: &[JsValue], _: &mut Context) -> JsResult<JsValue> {
         // 1. Return the this value.
         Ok(this.clone())
     }
 
     /// Utility function used to specify the creation of a new Array object using a constructor
-    /// function that is derived from original_array.
+    /// function that is derived from `original_array`.
     ///
     /// see: <https://tc39.es/ecma262/#sec-arrayspeciescreate>
     pub(crate) fn array_species_create(
@@ -414,9 +416,7 @@ impl Array {
         context: &mut Context,
     ) -> JsResult<JsValue> {
         // 1. Return ? IsArray(arg).
-        args.get_or_undefined(0)
-            .is_array(context)
-            .map(|ok| ok.into())
+        args.get_or_undefined(0).is_array(context).map(Into::into)
     }
 
     /// `Array.of(...items)`
@@ -448,7 +448,7 @@ impl Array {
                 .ok_or_else(|| {
                     context.construct_type_error("object constructor didn't return an object")
                 })?,
-            _ => Array::array_create(len, None, context)?,
+            _ => Self::array_create(len, None, context)?,
         };
 
         // 6. Let k be 0.
@@ -574,7 +574,7 @@ impl Array {
                 // iii. Perform ? CreateDataPropertyOrThrow(A, ! ToString(𝔽(n)), E).
                 arr.create_data_property_or_throw(n, item, context)?;
                 // iv. Set n to n + 1.
-                n += 1
+                n += 1;
             }
         }
         // 6. Perform ? Set(A, "length", 𝔽(n), true).
@@ -1152,7 +1152,7 @@ impl Array {
         let mut k;
         if n >= 0 {
             // a. Let k be n.
-            k = n
+            k = n;
         // 9. Else,
         } else {
             // a. Let k be len + n.
@@ -1189,7 +1189,7 @@ impl Array {
     /// `Array.prototype.lastIndexOf( searchElement[, fromIndex ] )`
     ///
     ///
-    /// lastIndexOf compares searchElement to the elements of the array in descending order
+    /// `lastIndexOf` compares searchElement to the elements of the array in descending order
     /// using the Strict Equality Comparison algorithm, and if found at one or more indices,
     /// returns the largest such index; otherwise, -1 is returned.
     ///
@@ -1426,9 +1426,9 @@ impl Array {
 
     /// `Array.prototype.findLastIndex( predicate [ , thisArg ] )`
     ///
-    /// findLastIndex calls predicate once for each element of the array, in descending order,
-    /// until it finds one where predicate returns true. If such an element is found, findLastIndex
-    /// immediately returns the index of that element value. Otherwise, findLastIndex returns -1.
+    /// `findLastIndex` calls predicate once for each element of the array, in descending order,
+    /// until it finds one where predicate returns true. If such an element is found, `findLastIndex`
+    /// immediately returns the index of that element value. Otherwise, `findLastIndex` returns -1.
     ///
     /// More information:
     ///  - [ECMAScript proposal][spec]
@@ -1565,7 +1565,7 @@ impl Array {
             source_len as u64,
             0,
             1,
-            Some(mapper_function.clone()),
+            Some(mapper_function),
             args.get_or_undefined(1),
             context,
         )?;
@@ -1587,7 +1587,7 @@ impl Array {
         source_len: u64,
         start: u64,
         depth: u64,
-        mapper_function: Option<JsObject>,
+        mapper_function: Option<&JsObject>,
         this_arg: &JsValue,
         context: &mut Context,
     ) -> JsResult<u64> {
@@ -1618,7 +1618,7 @@ impl Array {
                 let mut element = source.get(p, context)?;
 
                 // ii. If mapperFunction is present, then
-                if let Some(ref mapper_function) = mapper_function {
+                if let Some(mapper_function) = mapper_function {
                     // 1. Set element to ? Call(mapperFunction, thisArg, <<element, sourceIndex, source>>)
                     element = mapper_function.call(
                         this_arg,
@@ -1781,7 +1781,7 @@ impl Array {
         let mut k;
         if n >= 0 {
             // a. Let k be n.
-            k = n
+            k = n;
         // 9. Else,
         } else {
             // a. Let k be len + n.

boa/src/builtins/array/tests.rs

@@ -818,7 +818,7 @@ fn map() {
 
     // 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\""))
+    assert_eq!(one_with_this, String::from("\"The answer to life is: 42\""));
 }
 
 #[test]

boa/src/builtins/array_buffer/mod.rs

@@ -105,6 +105,7 @@ impl ArrayBuffer {
     ///  - [ECMAScript reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-get-arraybuffer-@@species
+    #[allow(clippy::unnecessary_wraps)]
     fn get_species(this: &JsValue, _: &[JsValue], _: &mut Context) -> JsResult<JsValue> {
         // 1. Return the this value.
         Ok(this.clone())
@@ -116,6 +117,7 @@ impl ArrayBuffer {
     ///  - [ECMAScript reference][spec]
     ///
     /// [spec]: https://tc39.es/ecma262/#sec-arraybuffer.isview
+    #[allow(clippy::unnecessary_wraps)]
     fn is_view(_: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult<JsValue> {
         // 1. If Type(arg) is not Object, return false.
         // 2. If arg has a [[ViewedArrayBuffer]] internal slot, return true.
@@ -320,7 +322,7 @@ impl ArrayBuffer {
 
         // 3. Set obj.[[ArrayBufferData]] to block.
         // 4. Set obj.[[ArrayBufferByteLength]] to byteLength.
-        obj.borrow_mut().data = ObjectData::array_buffer(ArrayBuffer {
+        obj.borrow_mut().data = ObjectData::array_buffer(Self {
             array_buffer_data: Some(block),
             array_buffer_byte_length: byte_length,
             array_buffer_detach_key: JsValue::Undefined,
@@ -600,7 +602,7 @@ impl ArrayBuffer {
     /// [spec]: https://tc39.es/ecma262/#sec-numerictorawbytes
     fn numeric_to_raw_bytes(
         t: TypedArrayName,
-        value: JsValue,
+        value: &JsValue,
         is_little_endian: bool,
         context: &mut Context,
     ) -> JsResult<Vec<u8>> {
@@ -696,7 +698,7 @@ impl ArrayBuffer {
         &mut self,
         byte_index: usize,
         t: TypedArrayName,
-        value: JsValue,
+        value: &JsValue,
         _order: SharedMemoryOrder,
         is_little_endian: Option<bool>,
         context: &mut Context,
@@ -804,7 +806,7 @@ fn copy_data_block_bytes(
 
 // TODO: Allow unused variants until shared array buffers are implemented.
 #[allow(dead_code)]
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 pub(crate) enum SharedMemoryOrder {
     Init,
     SeqCst,

boa/src/builtins/array_buffer/tests.rs

@@ -4,12 +4,12 @@ use super::*;
 fn ut_sunnyy_day_create_byte_data_block() {
     let mut context = Context::default();
 
-    assert!(create_byte_data_block(100, &mut context).is_ok())
+    assert!(create_byte_data_block(100, &mut context).is_ok());
 }
 
 #[test]
 fn ut_rainy_day_create_byte_data_block() {
     let mut context = Context::default();
 
-    assert!(create_byte_data_block(usize::MAX, &mut context).is_err())
+    assert!(create_byte_data_block(usize::MAX, &mut context).is_err());
 }