Skip to content

Commit

Permalink
Remove toInteger and document the string builtin (#1884)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jedel1043 committed Mar 2, 2022
1 parent fd889fd commit 00a1900
Show file tree
Hide file tree
Showing 11 changed files with 744 additions and 479 deletions.
19 changes: 7 additions & 12 deletions boa_engine/src/builtins/console/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
68 changes: 36 additions & 32 deletions boa_engine/src/builtins/number/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,23 +192,25 @@ 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() {
return Ok(JsValue::new(Self::to_native_string(this_num)));
}
// 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))
}
Expand All @@ -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,
// 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,
};

// 2. Let f be ? ToIntegerOrInfinity(fractionDigits).
// 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.
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)))
Expand Down Expand Up @@ -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)?
};

// 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;
// 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.
.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 {
Expand Down
3 changes: 1 addition & 2 deletions boa_engine/src/builtins/regexp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading

0 comments on commit 00a1900

Please sign in to comment.