Skip to content

Commit

Permalink
Unwrap removal (#1842)
Browse files Browse the repository at this point in the history
This removes all the calls to  `unwrap()` in the codebase, which made me found a couple of places where it wasn't needed, and could be improved. I also noticed we don't have dependabot updates for the test262 submodule and the interner dependencies, so I added those.

I added lints so that no new unwraps are added.
  • Loading branch information
Razican committed Feb 17, 2022
1 parent 3d9c8b2 commit b8f8530
Show file tree
Hide file tree
Showing 30 changed files with 444 additions and 315 deletions.
50 changes: 29 additions & 21 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
@@ -1,30 +1,38 @@
version: 2
updates:
- package-ecosystem: "npm"
directory: "/"
- package-ecosystem: npm
directory: /
schedule:
interval: "daily"
- package-ecosystem: "github-actions"
directory: "/"
interval: daily
- package-ecosystem: github-actions
directory: /
schedule:
interval: "daily"
- package-ecosystem: "cargo"
directory: "/"
interval: daily
- package-ecosystem: cargo
directory: /
schedule:
interval: "daily"
- package-ecosystem: "cargo"
directory: "/boa/"
interval: daily
- package-ecosystem: cargo
directory: /boa/
schedule:
interval: "daily"
- package-ecosystem: "cargo"
directory: "/boa_cli/"
interval: daily
- package-ecosystem: cargo
directory: /boa_cli/
schedule:
interval: "daily"
- package-ecosystem: "cargo"
directory: "/boa_wasm/"
interval: daily
- package-ecosystem: cargo
directory: /boa_wasm/
schedule:
interval: "daily"
- package-ecosystem: "cargo"
directory: "/boa_tester/"
interval: daily
- package-ecosystem: cargo
directory: /boa_tester/
schedule:
interval: "daily"
interval: daily
- package-ecosystem: cargo
directory: /boa_interner/
schedule:
interval: daily
- package-ecosystem: gitsubmodule
directory: /test262/
schedule:
interval: weekly
25 changes: 15 additions & 10 deletions boa/src/builtins/array/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,35 +143,40 @@ impl Array {
if number_of_args == 0 {
// 4.a. Return ! ArrayCreate(0, proto).
Ok(Self::array_create(0, Some(prototype), context)
.unwrap()
.expect("this ArrayCreate call must not fail")
.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 = Self::array_create(0, Some(prototype), context).unwrap();
let array = Self::array_create(0, Some(prototype), context)
.expect("this ArrayCreate call must not fail");
// 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
.create_data_property_or_throw(0, len, context)
.unwrap();
.expect("this CreateDataPropertyOrThrow call must not fail");
// ii. Let intLen be 1𝔽.
1
// d. Else,
} else {
// i. Let intLen be ! ToUint32(len).
let int_len = len.to_u32(context).unwrap();
let int_len = len
.to_u32(context)
.expect("this ToUint32 call must not fail");
// ii. If SameValueZero(intLen, len) is false, throw a RangeError exception.
if !JsValue::same_value_zero(&int_len.into(), len) {
return context.throw_range_error("invalid array length");
}
int_len
};
// e. Perform ! Set(array, "length", intLen, true).
array.set("length", int_len, true, context).unwrap();
array
.set("length", int_len, true, context)
.expect("this Set call must not fail");
// f. Return array.
Ok(array.into())
// 6. Else,
Expand All @@ -189,7 +194,7 @@ impl Array {
// iii. Perform ! CreateDataPropertyOrThrow(array, Pk, itemK).
array
.create_data_property_or_throw(i, item, context)
.unwrap();
.expect("this CreateDataPropertyOrThrow must not fail");
// iv. Set k to k + 1.
}
// e. Assert: The mathematical value of array's "length" property is numberOfArgs.
Expand Down Expand Up @@ -360,8 +365,8 @@ impl Array {
Ok(
c.construct(&[JsValue::new(length)], &c.clone().into(), context)?
.as_object()
.cloned()
.unwrap(),
.expect("constructing an object should always return an object")
.clone(),
)
} else {
context.throw_type_error("Symbol.species must be a constructor")
Expand Down Expand Up @@ -537,7 +542,7 @@ impl Array {
if spreadable {
// item is guaranteed to be an object since is_concat_spreadable checks it,
// so we can call `.unwrap()`
let item = item.as_object().unwrap();
let item = item.as_object().expect("guaranteed to be an object");
// i. Let k be 0.
// ii. Let len be ? LengthOfArrayLike(E).
let len = item.length_of_array_like(context)?;
Expand Down Expand Up @@ -1639,7 +1644,7 @@ impl Array {
// v. If shouldFlatten is true
if should_flatten {
// For `should_flatten` to be true, element must be an object.
let element = element.as_object().unwrap();
let element = element.as_object().expect("must be an object");

// 1. If depth is +Infinity let newDepth be +Infinity
let new_depth = if depth == u64::MAX {
Expand Down
4 changes: 2 additions & 2 deletions boa/src/builtins/iterable/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ pub fn create_iter_result_object(value: JsValue, done: bool, context: &mut Conte

// 3. Perform ! CreateDataPropertyOrThrow(obj, "value", value).
obj.create_data_property_or_throw("value", value, context)
.unwrap();
.expect("this CreateDataPropertyOrThrow call must not fail");
// 4. Perform ! CreateDataPropertyOrThrow(obj, "done", done).
obj.create_data_property_or_throw("done", done, context)
.unwrap();
.expect("this CreateDataPropertyOrThrow call must not fail");
// 5. Return obj.
obj.into()
}
Expand Down
11 changes: 9 additions & 2 deletions boa/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ impl Json {
// ii. For each String P of keys, do
for p in keys {
// This is safe, because EnumerableOwnPropertyNames with 'key' type only returns strings.
let p = p.as_string().unwrap();
let p = p
.as_string()
.expect("EnumerableOwnPropertyNames only returns strings");

// 1. Let newElement be ? InternalizeJSONProperty(val, P, reviver).
let new_element =
Expand Down Expand Up @@ -553,7 +555,12 @@ impl Json {
// a. Let K be ? EnumerableOwnPropertyNames(value, key).
let keys = value.enumerable_own_property_names(PropertyNameKind::Key, context)?;
// Unwrap is safe, because EnumerableOwnPropertyNames with kind "key" only returns string values.
keys.iter().map(|v| v.to_string(context).unwrap()).collect()
keys.iter()
.map(|v| {
v.to_string(context)
.expect("EnumerableOwnPropertyNames only returns strings")
})
.collect()
};

// 7. Let partial be a new empty List.
Expand Down
31 changes: 20 additions & 11 deletions boa/src/builtins/number/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,10 @@ impl Number {
fn round_to_precision(digits: &mut String, precision: usize) -> bool {
if digits.len() > precision {
let to_round = digits.split_off(precision);
let mut digit = digits.pop().unwrap() as u8;
let mut digit = digits
.pop()
.expect("already checked that lenght is bigger than precision")
as u8;
if let Some(first) = to_round.chars().next() {
if first > '4' {
digit += 1;
Expand Down Expand Up @@ -563,8 +566,9 @@ impl Number {
delta *= f64::from(radix);
// Write digit.
let digit = fraction as u32;
frac_buf[fraction_cursor] =
std::char::from_digit(digit, u32::from(radix)).unwrap() as u8;
frac_buf[fraction_cursor] = std::char::from_digit(digit, u32::from(radix))
.expect("radix already checked")
as u8;
fraction_cursor += 1;
// Calculate remainder.
fraction -= f64::from(digit);
Expand All @@ -582,12 +586,16 @@ impl Number {
} else {
let c: u8 = frac_buf[fraction_cursor];
// Reconstruct digit.
let digit_0 = (c as char).to_digit(10).unwrap();
let digit_0 = (c as char)
.to_digit(10)
.expect("charactre was not a valid digit");
if digit_0 + 1 >= u32::from(radix) {
continue;
}
frac_buf[fraction_cursor] =
std::char::from_digit(digit_0 + 1, u32::from(radix)).unwrap() as u8;
std::char::from_digit(digit_0 + 1, u32::from(radix))
.expect("digit was not a valid number in the given radix")
as u8;
fraction_cursor += 1;
}
break;
Expand All @@ -601,28 +609,29 @@ impl Number {
}

// Compute integer digits. Fill unrepresented digits with zero.
let mut int_iter = int_buf.iter_mut().enumerate().rev(); //.rev();
let mut int_iter = int_buf.iter_mut().enumerate().rev();
while FloatCore::integer_decode(integer / f64::from(radix)).1 > 0 {
integer /= f64::from(radix);
*int_iter.next().unwrap().1 = b'0';
*int_iter.next().expect("integer buffer exhausted").1 = b'0';
}

loop {
let remainder = integer % f64::from(radix);
*int_iter.next().unwrap().1 =
std::char::from_digit(remainder as u32, u32::from(radix)).unwrap() as u8;
*int_iter.next().expect("integer buffer exhausted").1 =
std::char::from_digit(remainder as u32, u32::from(radix))
.expect("remainder not a digit in the given number") as u8;
integer = (integer - remainder) / f64::from(radix);
if integer <= 0f64 {
break;
}
}
// Add sign and terminate string.
if negative {
*int_iter.next().unwrap().1 = b'-';
*int_iter.next().expect("integer buffer exhausted").1 = b'-';
}
assert!(fraction_cursor < BUF_SIZE);

let integer_cursor = int_iter.next().unwrap().0 + 1;
let integer_cursor = int_iter.next().expect("integer buffer exhausted").0 + 1;
let fraction_cursor = fraction_cursor + BUF_SIZE / 2;
String::from_utf8_lossy(&buffer[integer_cursor..fraction_cursor]).into()
}
Expand Down
6 changes: 4 additions & 2 deletions boa/src/builtins/object/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,9 @@ impl Object {
// 3.a. If nextSource is neither undefined nor null, then
if !source.is_null_or_undefined() {
// 3.a.i. Let from be ! ToObject(nextSource).
let from = source.to_object(context).unwrap();
let from = source
.to_object(context)
.expect("this ToObject call must not fail");
// 3.a.ii. Let keys be ? from.[[OwnPropertyKeys]]().
let keys = from.__own_property_keys__(context)?;
// 3.a.iii. For each element nextKey of keys, do
Expand Down Expand Up @@ -925,7 +927,7 @@ impl Object {

// 4. Let closure be a new Abstract Closure with parameters (key, value) that captures
// obj and performs the following steps when called:
let mut closure = FunctionBuilder::closure_with_captures(
let closure = FunctionBuilder::closure_with_captures(
context,
|_, args, obj, context| {
let key = args.get_or_undefined(0);
Expand Down
Loading

0 comments on commit b8f8530

Please sign in to comment.