Remove `toInteger` and document the `string` builtin (#1884)

The ECMAScript 2022 specification removes the `toInteger` method, and replaces it with `toIntegerOrInfinity`, which is arguably better for us since the `JsValue::toInteger` returns an `f64`, which is pretty confusing at times.

This pull request removes the `JsValue::to_integer` method, replaces all its calls by `JsValue::to_integer_or_infinity` or others per the spec and documents several methods from the `string` builtin.

boa_engine/src/builtins/console/mod.rs

@@ -20,7 +20,7 @@ use crate::{
     builtins::{BuiltIn, JsArgs},
     object::ObjectInitializer,
     property::Attribute,
-    value::{display::display_obj, JsValue},
+    value::{display::display_obj, JsValue, Numeric},
     Context, JsResult, JsString,
 };
 use boa_profiler::Profiler;
@@ -71,21 +71,16 @@ pub fn formatter(data: &[JsValue], context: &mut Context) -> JsResult<String> {
                     match fmt {
                         /* integer */
                         'd' | 'i' => {
-                            let arg = data
-                                .get(arg_index)
-                                .cloned()
-                                .unwrap_or_default()
-                                .to_integer(context)?;
-                            formatted.push_str(&arg.to_string());
+                            let arg = match data.get_or_undefined(arg_index).to_numeric(context)? {
+                                Numeric::Number(r) => (r.floor() + 0.0).to_string(),
+                                Numeric::BigInt(int) => int.to_string(),
+                            };
+                            formatted.push_str(&arg);
                             arg_index += 1;
                         }
                         /* float */
                         'f' => {
-                            let arg = data
-                                .get(arg_index)
-                                .cloned()
-                                .unwrap_or_default()
-                                .to_number(context)?;
+                            let arg = data.get_or_undefined(arg_index).to_number(context)?;
                             formatted.push_str(&format!("{arg:.6}"));
                             arg_index += 1;
                         }

boa_engine/src/builtins/number/mod.rs

@@ -192,7 +192,7 @@ impl Number {
         let precision = match args.get(0) {
             None | Some(JsValue::Undefined) => None,
             // 2. Let f be ? ToIntegerOrInfinity(fractionDigits).
-            Some(n) => Some(n.to_integer(context)? as i32),
+            Some(n) => Some(n.to_integer_or_infinity(context)?),
         };
         // 4. If x is not finite, return ! Number::toString(x).
         if !this_num.is_finite() {
@@ -200,15 +200,17 @@ impl Number {
         }
         // Get rid of the '-' sign for -0.0
         let this_num = if this_num == 0. { 0. } else { this_num };
-        let this_str_num = if let Some(precision) = precision {
+        let this_str_num = match precision {
+            None => f64_to_exponential(this_num),
+            Some(IntegerOrInfinity::Integer(precision)) if (0..=100).contains(&precision) =>
             // 5. If f < 0 or f > 100, throw a RangeError exception.
-            if !(0..=100).contains(&precision) {
+            {
+                f64_to_exponential_with_precision(this_num, precision as usize)
+            }
+            _ => {
                 return context
-                    .throw_range_error("toExponential() argument must be between 0 and 100");
+                    .throw_range_error("toExponential() argument must be between 0 and 100")
             }
-            f64_to_exponential_with_precision(this_num, precision as usize)
-        } else {
-            f64_to_exponential(this_num)
         };
         Ok(JsValue::new(this_str_num))
     }
@@ -231,19 +233,19 @@ impl Number {
     ) -> JsResult<JsValue> {
         // 1. Let this_num be ? thisNumberValue(this value).
         let this_num = Self::this_number_value(this, context)?;
-        let precision = match args.get(0) {
+
         // 2. Let f be ? ToIntegerOrInfinity(fractionDigits).
-            Some(n) => match n.to_integer(context)? as i32 {
-                0..=100 => n.to_integer(context)? as usize,
+        // 3. Assert: If fractionDigits is undefined, then f is 0.
+        let precision = args.get_or_undefined(0).to_integer_or_infinity(context)?;
+
         // 4, 5. If f < 0 or f > 100, throw a RangeError exception.
-                _ => {
-                    return context
-                        .throw_range_error("toFixed() digits argument must be between 0 and 100")
-                }
-            },
-            // 3. If fractionDigits is undefined, then f is 0.
-            None => 0,
-        };
+        let precision = precision
+            .as_integer()
+            .filter(|i| (0..=100).contains(i))
+            .ok_or_else(|| {
+                context.construct_range_error("toFixed() digits argument must be between 0 and 100")
+            })? as usize;
+
         // 6. If x is not finite, return ! Number::toString(x).
         if !this_num.is_finite() {
             Ok(JsValue::new(Self::to_native_string(this_num)))
@@ -642,21 +644,23 @@ impl Number {
         // 1. Let x be ? thisNumberValue(this value).
         let x = Self::this_number_value(this, context)?;
 
-        // 2. If radix is undefined, let radixNumber be 10.
         let radix = args.get_or_undefined(0);
         let radix_number = if radix.is_undefined() {
-            10.0
-        // 3. Else, let radixNumber be ? ToInteger(radix).
+            // 2. If radix is undefined, let radixNumber be 10.
+            10
         } else {
-            radix.to_integer(context)?
-        };
-
+            // 3. Else, let radixMV be ? ToIntegerOrInfinity(radix).
+            radix
+                .to_integer_or_infinity(context)?
+                .as_integer()
                 // 4. If radixNumber < 2 or radixNumber > 36, throw a RangeError exception.
-        if !(2.0..=36.0).contains(&radix_number) {
-            return context
-                .throw_range_error("radix must be an integer at least 2 and no greater than 36");
-        }
-        let radix_number = radix_number as u8;
+                .filter(|i| (2..=36).contains(i))
+                .ok_or_else(|| {
+                    context.construct_range_error(
+                        "radix must be an integer at least 2 and no greater than 36",
+                    )
+                })?
+        } as u8;
 
         // 5. If radixNumber = 10, return ! ToString(x).
         if radix_number == 10 {

boa_engine/src/builtins/regexp/mod.rs

@@ -1707,8 +1707,7 @@ fn advance_string_index(s: &JsString, index: usize, unicode: bool) -> usize {
     }
 
     // 5. Let cp be ! CodePointAt(S, index).
-    let (_, offset, _) =
-        crate::builtins::string::code_point_at(s, index as i64).expect("Failed to get code point");
+    let (_, offset, _) = crate::builtins::string::code_point_at(s, index);
 
     index + offset as usize
 }

boa_engine/src/builtins/string/string_iterator.rs

@@ -57,8 +57,7 @@ impl StringIterator {
                 context,
             ));
         }
-        let (_, code_unit_count, _) = code_point_at(&native_string, i64::from(position))
-            .expect("Invalid code point position");
+        let (_, code_unit_count, _) = code_point_at(&native_string, position as usize);
         string_iterator.next_index += i32::from(code_unit_count);
         let result_string = crate::builtins::string::String::substring(
             &string_iterator.string,

boa_engine/src/builtins/string/tests.rs

@@ -147,7 +147,8 @@ fn repeat_throws_when_count_is_negative() {
         }
     "#
         ),
-        "\"RangeError: repeat count cannot be a negative number\""
+        "\"RangeError: repeat count must be a positive finite number \
+        that doesn't overflow the maximum string length (2^32 - 1)\""
     );
 }
 
@@ -166,7 +167,8 @@ fn repeat_throws_when_count_is_infinity() {
         }
     "#
         ),
-        "\"RangeError: repeat count cannot be infinity\""
+        "\"RangeError: repeat count must be a positive finite number \
+        that doesn't overflow the maximum string length (2^32 - 1)\""
     );
 }
 
@@ -185,7 +187,8 @@ fn repeat_throws_when_count_overflows_max_length() {
         }
     "#
         ),
-        "\"RangeError: repeat count must not overflow maximum string length\""
+        "\"RangeError: repeat count must be a positive finite number \
+        that doesn't overflow the maximum string length (2^32 - 1)\""
     );
 }
 

boa_engine/src/builtins/symbol/mod.rs

@@ -210,8 +210,11 @@ impl Symbol {
         _: &[JsValue],
         context: &mut Context,
     ) -> JsResult<JsValue> {
+        // 1. Let sym be ? thisSymbolValue(this value).
         let symbol = Self::this_symbol_value(this, context)?;
-        Ok(symbol.to_string().into())
+
+        // 2. Return SymbolDescriptiveString(sym).
+        Ok(symbol.descriptive_string().into())
     }
 
     /// `Symbol.prototype.valueOf()`

boa_engine/src/symbol.rs

@@ -289,6 +289,16 @@ impl JsSymbol {
     pub fn hash(&self) -> u64 {
         self.inner.hash
     }
+
+    /// Abstract operation `SymbolDescriptiveString ( sym )`
+    ///
+    /// More info:
+    /// - [ECMAScript reference][spec]
+    ///
+    /// [spec]: https://tc39.es/ecma262/#sec-symboldescriptivestring
+    pub fn descriptive_string(&self) -> JsString {
+        self.to_string().into()
+    }
 }
 
 impl Finalize for JsSymbol {}

boa_engine/src/tests.rs

@@ -1,5 +1,6 @@
 use crate::{
-    builtins::Number, check_output, exec, forward, forward_val, Context, JsValue, TestAction,
+    builtins::Number, check_output, exec, forward, forward_val, value::IntegerOrInfinity, Context,
+    JsValue, TestAction,
 };
 
 #[test]
@@ -942,40 +943,49 @@ fn to_index() {
 }
 
 #[test]
-fn to_integer() {
+fn to_integer_or_infinity() {
     let mut context = Context::default();
 
-    assert!(Number::equal(
-        JsValue::nan().to_integer(&mut context).unwrap(),
-        0.0
-    ));
-    assert!(Number::equal(
+    assert_eq!(
+        JsValue::nan().to_integer_or_infinity(&mut context).unwrap(),
+        0
+    );
+    assert_eq!(
         JsValue::new(f64::NEG_INFINITY)
-            .to_integer(&mut context)
+            .to_integer_or_infinity(&mut context)
             .unwrap(),
-        f64::NEG_INFINITY
-    ));
-    assert!(Number::equal(
+        IntegerOrInfinity::NegativeInfinity
+    );
+    assert_eq!(
         JsValue::new(f64::INFINITY)
-            .to_integer(&mut context)
+            .to_integer_or_infinity(&mut context)
             .unwrap(),
-        f64::INFINITY
-    ));
-    assert!(Number::equal(
-        JsValue::new(0.0).to_integer(&mut context).unwrap(),
-        0.0
-    ));
-    let number = JsValue::new(-0.0).to_integer(&mut context).unwrap();
-    assert!(!number.is_sign_negative());
-    assert!(Number::equal(number, 0.0));
-    assert!(Number::equal(
-        JsValue::new(20.9).to_integer(&mut context).unwrap(),
-        20.0
-    ));
-    assert!(Number::equal(
-        JsValue::new(-20.9).to_integer(&mut context).unwrap(),
-        -20.0
-    ));
+        IntegerOrInfinity::PositiveInfinity
+    );
+    assert_eq!(
+        JsValue::new(0.0)
+            .to_integer_or_infinity(&mut context)
+            .unwrap(),
+        0
+    );
+    assert_eq!(
+        JsValue::new(-0.0)
+            .to_integer_or_infinity(&mut context)
+            .unwrap(),
+        0
+    );
+    assert_eq!(
+        JsValue::new(20.9)
+            .to_integer_or_infinity(&mut context)
+            .unwrap(),
+        20
+    );
+    assert_eq!(
+        JsValue::new(-20.9)
+            .to_integer_or_infinity(&mut context)
+            .unwrap(),
+        -20
+    );
 }
 
 #[test]

boa_engine/src/value/integer.rs

@@ -0,0 +1,69 @@
+use std::cmp::Ordering;
+
+/// Represents the result of `ToIntegerOrInfinity` operation
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+pub enum IntegerOrInfinity {
+    PositiveInfinity,
+    Integer(i64),
+    NegativeInfinity,
+}
+
+impl IntegerOrInfinity {
+    /// Clamps an `IntegerOrInfinity` between two `i64`, effectively converting
+    /// it to an i64.
+    pub fn clamp_finite(self, min: i64, max: i64) -> i64 {
+        assert!(min <= max);
+
+        match self {
+            IntegerOrInfinity::Integer(i) => i.clamp(min, max),
+            IntegerOrInfinity::PositiveInfinity => max,
+            IntegerOrInfinity::NegativeInfinity => min,
+        }
+    }
+
+    /// Gets the wrapped `i64` if the variant is an `Integer`.
+    pub fn as_integer(self) -> Option<i64> {
+        match self {
+            IntegerOrInfinity::Integer(i) => Some(i),
+            _ => None,
+        }
+    }
+}
+
+impl PartialEq<i64> for IntegerOrInfinity {
+    fn eq(&self, other: &i64) -> bool {
+        match self {
+            IntegerOrInfinity::Integer(i) => i == other,
+            _ => false,
+        }
+    }
+}
+
+impl PartialEq<IntegerOrInfinity> for i64 {
+    fn eq(&self, other: &IntegerOrInfinity) -> bool {
+        match other {
+            IntegerOrInfinity::Integer(i) => i == other,
+            _ => false,
+        }
+    }
+}
+
+impl PartialOrd<i64> for IntegerOrInfinity {
+    fn partial_cmp(&self, other: &i64) -> Option<Ordering> {
+        match self {
+            IntegerOrInfinity::PositiveInfinity => Some(Ordering::Greater),
+            IntegerOrInfinity::Integer(i) => i.partial_cmp(other),
+            IntegerOrInfinity::NegativeInfinity => Some(Ordering::Less),
+        }
+    }
+}
+
+impl PartialOrd<IntegerOrInfinity> for i64 {
+    fn partial_cmp(&self, other: &IntegerOrInfinity) -> Option<Ordering> {
+        match other {
+            IntegerOrInfinity::PositiveInfinity => Some(Ordering::Less),
+            IntegerOrInfinity::Integer(i) => self.partial_cmp(i),
+            IntegerOrInfinity::NegativeInfinity => Some(Ordering::Greater),
+        }
+    }
+}

boa_engine/src/value/mod.rs

@@ -32,6 +32,7 @@ mod conversions;
 pub(crate) mod display;
 mod equality;
 mod hash;
+mod integer;
 mod operations;
 mod serde_json;
 mod r#type;
@@ -40,6 +41,7 @@ pub use conversions::*;
 pub use display::ValueDisplay;
 pub use equality::*;
 pub use hash::*;
+pub use integer::IntegerOrInfinity;
 pub use operations::*;
 pub use r#type::Type;
 
@@ -76,14 +78,6 @@ pub enum JsValue {
     Symbol(JsSymbol),
 }
 
-/// Represents the result of `ToIntegerOrInfinity` operation
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum IntegerOrInfinity {
-    Integer(i64),
-    PositiveInfinity,
-    NegativeInfinity,
-}
-
 impl JsValue {
     /// Create a new [`JsValue`].
     #[inline]
@@ -614,7 +608,7 @@ impl JsValue {
         }
 
         // 3. Let int be the mathematical value whose sign is the sign of number and whose magnitude is floor(abs(ℝ(number))).
-        let int = number.floor() as i64;
+        let int = number.abs().floor().copysign(number) as i64;
 
         // 4. Let int8bit be int modulo 2^8.
         let int_8_bit = int % 2i64.pow(8);
@@ -643,7 +637,7 @@ impl JsValue {
         }
 
         // 3. Let int be the mathematical value whose sign is the sign of number and whose magnitude is floor(abs(ℝ(number))).
-        let int = number.floor() as i64;
+        let int = number.abs().floor().copysign(number) as i64;
 
         // 4. Let int8bit be int modulo 2^8.
         let int_8_bit = int % 2i64.pow(8);
@@ -715,7 +709,7 @@ impl JsValue {
         }
 
         // 3. Let int be the mathematical value whose sign is the sign of number and whose magnitude is floor(abs(ℝ(number))).
-        let int = number.floor() as i64;
+        let int = number.abs().floor().copysign(number) as i64;
 
         // 4. Let int16bit be int modulo 2^16.
         let int_16_bit = int % 2i64.pow(16);
@@ -744,7 +738,7 @@ impl JsValue {
         }
 
         // 3. Let int be the mathematical value whose sign is the sign of number and whose magnitude is floor(abs(ℝ(number))).
-        let int = number.floor() as i64;
+        let int = number.abs().floor().copysign(number) as i64;
 
         // 4. Let int16bit be int modulo 2^16.
         let int_16_bit = int % 2i64.pow(16);
@@ -796,21 +790,29 @@ impl JsValue {
     ///
     /// See: <https://tc39.es/ecma262/#sec-toindex>
     pub fn to_index(&self, context: &mut Context) -> JsResult<usize> {
+        // 1. If value is undefined, then
         if self.is_undefined() {
+            // a. Return 0.
             return Ok(0);
         }
 
-        let integer_index = self.to_integer(context)?;
+        // 2. Else,
+        // a. Let integer be ? ToIntegerOrInfinity(value).
+        let integer = self.to_integer_or_infinity(context)?;
 
-        if integer_index < 0.0 {
-            return context.throw_range_error("Integer index must be >= 0");
-        }
+        // b. Let clamped be ! ToLength(𝔽(integer)).
+        let clamped = integer.clamp_finite(0, Number::MAX_SAFE_INTEGER as i64);
 
-        if integer_index > Number::MAX_SAFE_INTEGER {
-            return context.throw_range_error("Integer index must be less than 2**(53) - 1");
+        // c. If ! SameValue(𝔽(integer), clamped) is false, throw a RangeError exception.
+        if integer != clamped {
+            return context.throw_range_error("Index must be between 0 and  2^53 - 1");
         }
 
-        Ok(integer_index as usize)
+        // d. Assert: 0 ≤ integer ≤ 2^53 - 1.
+        debug_assert!(0 <= clamped && clamped <= Number::MAX_SAFE_INTEGER as i64);
+
+        // e. Return integer.
+        Ok(clamped as usize)
     }
 
     /// Converts argument to an integer suitable for use as the length of an array-like object.
@@ -818,37 +820,43 @@ impl JsValue {
     /// See: <https://tc39.es/ecma262/#sec-tolength>
     pub fn to_length(&self, context: &mut Context) -> JsResult<usize> {
         // 1. Let len be ? ToInteger(argument).
-        let len = self.to_integer(context)?;
-
         // 2. If len ≤ +0, return +0.
-        if len < 0.0 {
-            return Ok(0);
-        }
-
         // 3. Return min(len, 2^53 - 1).
-        Ok(len.min(Number::MAX_SAFE_INTEGER) as usize)
+        Ok(self
+            .to_integer_or_infinity(context)?
+            .clamp_finite(0, Number::MAX_SAFE_INTEGER as i64) as usize)
     }
 
-    /// Converts a value to an integral Number value.
+    /// Abstract operation `ToIntegerOrInfinity ( argument )`
+    ///
+    /// This method converts a `Value` to an integer representing its `Number` value with
+    /// fractional part truncated, or to +∞ or -∞ when that `Number` value is infinite.
+    ///
+    /// More information:
+    /// - [ECMAScript reference][spec]
     ///
-    /// See: <https://tc39.es/ecma262/#sec-tointeger>
-    pub fn to_integer(&self, context: &mut Context) -> JsResult<f64> {
+    /// [spec]: https://tc39.es/ecma262/#sec-tointegerorinfinity
+    pub fn to_integer_or_infinity(&self, context: &mut Context) -> JsResult<IntegerOrInfinity> {
         // 1. Let number be ? ToNumber(argument).
         let number = self.to_number(context)?;
 
-        // 2. If number is +∞ or -∞, return number.
-        if !number.is_finite() {
-            // 3. If number is NaN, +0, or -0, return +0.
-            if number.is_nan() {
-                return Ok(0.0);
-            }
-            return Ok(number);
-        }
+        if number.is_nan() || number == 0.0 {
+            // 2. If number is NaN, +0𝔽, or -0𝔽, return 0.
+            Ok(IntegerOrInfinity::Integer(0))
+        } else if number == f64::INFINITY {
+            // 3. If number is +∞𝔽, return +∞.
+            Ok(IntegerOrInfinity::PositiveInfinity)
+        } else if number == f64::NEG_INFINITY {
+            // 4. If number is -∞𝔽, return -∞.
+            Ok(IntegerOrInfinity::NegativeInfinity)
+        } else {
+            // 5. Let integer be floor(abs(ℝ(number))).
+            // 6. If number < +0𝔽, set integer to -integer.
+            let integer = number.abs().floor().copysign(number) as i64;
 
-        // 4. Let integer be the Number value that is the same sign as number and whose magnitude is floor(abs(number)).
-        // 5. If integer is -0, return +0.
-        // 6. Return integer.
-        Ok(number.trunc() + 0.0) // We add 0.0 to convert -0.0 to +0.0
+            // 7. Return integer.
+            Ok(IntegerOrInfinity::Integer(integer))
+        }
     }
 
     /// Converts a value to a double precision floating point.
@@ -918,37 +926,6 @@ impl JsValue {
             .and_then(|obj| obj.to_property_descriptor(context))
     }
 
-    /// Converts argument to an integer, +∞, or -∞.
-    ///
-    /// See: <https://tc39.es/ecma262/#sec-tointegerorinfinity>
-    pub fn to_integer_or_infinity(&self, context: &mut Context) -> JsResult<IntegerOrInfinity> {
-        // 1. Let number be ? ToNumber(argument).
-        let number = self.to_number(context)?;
-
-        // 2. If number is NaN, +0𝔽, or -0𝔽, return 0.
-        if number.is_nan() || number == 0.0 || number == -0.0 {
-            Ok(IntegerOrInfinity::Integer(0))
-        } else if number.is_infinite() && number.is_sign_positive() {
-            // 3. If number is +∞𝔽, return +∞.
-            Ok(IntegerOrInfinity::PositiveInfinity)
-        } else if number.is_infinite() && number.is_sign_negative() {
-            // 4. If number is -∞𝔽, return -∞.
-            Ok(IntegerOrInfinity::NegativeInfinity)
-        } else {
-            // 5. Let integer be floor(abs(ℝ(number))).
-            let integer = number.abs().floor();
-            let integer = integer.min(Number::MAX_SAFE_INTEGER) as i64;
-
-            // 6. If number < +0𝔽, set integer to -integer.
-            // 7. Return integer.
-            if number < 0.0 {
-                Ok(IntegerOrInfinity::Integer(-integer))
-            } else {
-                Ok(IntegerOrInfinity::Integer(integer))
-            }
-        }
-    }
-
     /// `typeof` operator. Returns a string representing the type of the
     /// given ECMA Value.
     ///