From 75e994b7031011da4a5a83561f16122082ebbfb2 Mon Sep 17 00:00:00 2001 From: Norbert Garfield Date: Sun, 8 May 2022 11:10:42 +0000 Subject: [PATCH] Set more strict types to parameters --- .../src/builtins/intl/date_time_format.rs | 80 ++++---- boa_engine/src/builtins/intl/mod.rs | 100 ++++----- boa_engine/src/builtins/intl/tests.rs | 191 +++++++++--------- 3 files changed, 165 insertions(+), 206 deletions(-) diff --git a/boa_engine/src/builtins/intl/date_time_format.rs b/boa_engine/src/builtins/intl/date_time_format.rs index da285aceeb4..837203b0978 100644 --- a/boa_engine/src/builtins/intl/date_time_format.rs +++ b/boa_engine/src/builtins/intl/date_time_format.rs @@ -134,52 +134,50 @@ pub(crate) fn to_date_time_options( ) -> JsResult { // 1. If options is undefined, let options be null; // otherwise let options be ? ToObject(options). - let maybe_options = if options.is_undefined() { - Ok(JsObject::empty()) + // 2. Let options be ! OrdinaryObjectCreate(options). + let options = if options.is_undefined() { + JsObject::from_proto_and_data(None, ObjectData::ordinary()) } else { - options.to_object(context) + let opt = options.to_object(context)?; + JsObject::from_proto_and_data(opt, ObjectData::ordinary()) }; - let options = maybe_options.unwrap_or_else(|_| JsObject::empty()); - - // 2. Let options be ! OrdinaryObjectCreate(options). - let options = JsObject::from_proto_and_data(options, ObjectData::ordinary()); // 3. Let needDefaults be true. let mut need_defaults = true; // 4. If required is "date" or "any", then - if required.eq("date") || required.eq("any") { + if ["date", "any"].contains(&required) { // a. For each property name prop of « "weekday", "year", "month", "day" », do - let property_names = vec!["weekday", "year", "month", "day"]; - // i. Let value be ? Get(options, prop). - // ii. If value is not undefined, let needDefaults be false. - need_defaults = property_names.iter().all(|prop_name| { - options - .get(*prop_name, context) - .unwrap_or_else(|_| JsValue::undefined()) - .is_undefined() - }); + for property in ["weekday", "year", "month", "day"] { + // i. Let value be ? Get(options, prop). + let value = options.get(property, context)?; + + // ii. If value is not undefined, let needDefaults be false. + if !value.is_undefined() { + need_defaults = false; + } + } } // 5. If required is "time" or "any", then - if required.eq("time") || required.eq("any") { + if ["time", "any"].contains(&required) { // a. For each property name prop of « "dayPeriod", "hour", "minute", "second", // "fractionalSecondDigits" », do - let property_names = vec![ + for property in [ "dayPeriod", "hour", "minute", "second", "fractionalSecondDigits", - ]; - // i. Let value be ? Get(options, prop). - // ii. If value is not undefined, let needDefaults be false. - need_defaults = property_names.iter().all(|prop_name| { - options - .get(*prop_name, context) - .unwrap_or_else(|_| JsValue::undefined()) - .is_undefined() - }); + ] { + // i. Let value be ? Get(options, prop). + let value = options.get(property, context)?; + + // ii. If value is not undefined, let needDefaults be false. + if !value.is_undefined() { + need_defaults = false; + } + } } // 6. Let dateStyle be ? Get(options, "dateStyle"). @@ -188,9 +186,7 @@ pub(crate) fn to_date_time_options( .unwrap_or_else(|_| JsValue::undefined()); // 7. Let timeStyle be ? Get(options, "timeStyle"). - let time_style = options - .get("timeStyle", context) - .unwrap_or_else(|_| JsValue::undefined()); + let time_style = options.get("timeStyle", context)?; // 8. If dateStyle is not undefined or timeStyle is not undefined, let needDefaults be false. if !date_style.is_undefined() || !time_style.is_undefined() { @@ -210,26 +206,20 @@ pub(crate) fn to_date_time_options( } // 11. If needDefaults is true and defaults is either "date" or "all", then - if need_defaults && (defaults.eq("date") || defaults.eq("all")) { + if need_defaults && ["date", "all"].contains(&defaults) { // a. For each property name prop of « "year", "month", "day" », do - let property_names = vec!["year", "month", "day"]; - // i. Perform ? CreateDataPropertyOrThrow(options, prop, "numeric"). - for prop_name in property_names { - options - .create_data_property_or_throw(prop_name, "numeric", context) - .expect("CreateDataPropertyOrThrow must not fail"); + for property in ["year", "month", "day"] { + // i. Perform ? CreateDataPropertyOrThrow(options, prop, "numeric"). + options.create_data_property_or_throw(property, "numeric", context)?; } } // 12. If needDefaults is true and defaults is either "time" or "all", then - if need_defaults && (defaults.eq("time") || defaults.eq("all")) { + if need_defaults && ["time", "all"].contains(&defaults) { // a. For each property name prop of « "hour", "minute", "second" », do - let property_names = vec!["hour", "minute", "second"]; - // i. Perform ? CreateDataPropertyOrThrow(options, prop, "numeric"). - for prop_name in property_names { - options - .create_data_property_or_throw(prop_name, "numeric", context) - .expect("CreateDataPropertyOrThrow must not fail"); + for property in ["hour", "minute", "second"] { + // i. Perform ? CreateDataPropertyOrThrow(options, prop, "numeric"). + options.create_data_property_or_throw(property, "numeric", context)?; } } diff --git a/boa_engine/src/builtins/intl/mod.rs b/boa_engine/src/builtins/intl/mod.rs index 977b7e7fed4..d4723131bec 100644 --- a/boa_engine/src/builtins/intl/mod.rs +++ b/boa_engine/src/builtins/intl/mod.rs @@ -16,6 +16,9 @@ use crate::{ Context, JsResult, JsString, JsValue, }; +#[cfg(test)] +use crate::object::JsObject; + pub mod date_time_format; #[cfg(test)] mod tests; @@ -654,6 +657,12 @@ fn resolve_locale( result } +#[cfg(test)] +pub(crate) enum GetOptionType { + String, + Boolean, +} + /// The abstract operation `GetOption` extracts the value of the property named `property` from the /// provided `options` object, converts it to the required `type`, checks whether it is one of a /// `List` of allowed `values`, and fills in a `fallback` value if necessary. If `values` is @@ -665,26 +674,16 @@ fn resolve_locale( /// [spec]: https://tc39.es/ecma402/#sec-getoption #[cfg(test)] pub(crate) fn get_option( - options: &JsValue, + options: &JsObject, property: &str, - r#type: &str, - values: &[JsValue], + r#type: &GetOptionType, + values: &[JsString], fallback: &JsValue, context: &mut Context, ) -> JsResult { // 1. Assert: Type(options) is Object. - if !options.is_object() { - return context.throw_type_error("GetOption: options should be an Object"); - } - - let options_obj = options - .to_object(context) - .expect("GetOption: options should be an Object"); - // 2. Let value be ? Get(options, property). - let mut value = options_obj - .get(property, context) - .unwrap_or_else(|_| JsValue::undefined()); + let mut value = options.get(property, context)?; // 3. If value is undefined, return fallback. if value.is_undefined() { @@ -692,31 +691,22 @@ pub(crate) fn get_option( } // 4. Assert: type is "boolean" or "string". - if r#type.ne("boolean") && r#type.ne("string") { - return context.throw_type_error("GetOption: type should be either 'boolean' or 'string'"); - } - // 5. If type is "boolean", then - if r#type.eq("boolean") { - // a. Set value to ! ToBoolean(value). - value = JsValue::Boolean(value.to_boolean()); - } - + // a. Set value to ! ToBoolean(value). // 6. If type is "string", then - if r#type.eq("string") { - // a. Set value to ? ToString(value). - value = JsValue::String( - value - .to_string(context) - .expect("GetOption: failed to convert value to string"), - ); - } - + // a. Set value to ? ToString(value). // 7. If values is not undefined and values does not contain an element equal to value, // throw a RangeError exception. - if !values.is_empty() && !values.contains(&value) { - return context.throw_range_error("GetOption: values array does not contain value"); - } + value = match r#type { + GetOptionType::Boolean => JsValue::Boolean(value.to_boolean()), + GetOptionType::String => { + let string_value = value.to_string(context)?; + if !values.is_empty() && !values.contains(&string_value) { + return context.throw_range_error("GetOption: values array does not contain value"); + } + JsValue::String(string_value) + } + }; // 8. Return value. Ok(value) @@ -732,27 +722,16 @@ pub(crate) fn get_option( /// [spec]: https://tc39.es/ecma402/#sec-getnumberoption #[cfg(test)] pub(crate) fn get_number_option( - options: &JsValue, + options: &JsObject, property: &str, - minimum: &JsValue, - maximum: &JsValue, - fallback: &JsValue, + minimum: f64, + maximum: f64, + fallback: Option, context: &mut Context, -) -> JsResult { - // FIXME untested +) -> JsResult { // 1. Assert: Type(options) is Object. - if !options.is_object() { - return context.throw_type_error("GetOption: options should be an Object"); - } - - let options_obj = options - .to_object(context) - .expect("GetOption: options should be an Object"); - // 2. Let value be ? Get(options, property). - let value = options_obj - .get(property, context) - .unwrap_or_else(|_| JsValue::undefined()); + let value = options.get(property, context)?; // 3. Return ? DefaultNumberOption(value, minimum, maximum, fallback). default_number_option(&value, minimum, maximum, fallback, context) @@ -768,20 +747,21 @@ pub(crate) fn get_number_option( #[cfg(test)] pub(crate) fn default_number_option( value: &JsValue, - minimum: &JsValue, - maximum: &JsValue, - fallback: &JsValue, + minimum: f64, + maximum: f64, + fallback: Option, context: &mut Context, -) -> JsResult { +) -> JsResult { // 1. If value is undefined, return fallback. if value.is_undefined() { - return Ok(fallback.clone()); + match fallback { + Some(val_f64) => return Ok(val_f64), + None => return context.throw_type_error("DefaultNumberOption: no fallback provided"), + }; } // 2. Set value to ? ToNumber(value). let value = value.to_number(context)?; - let minimum = minimum.to_number(context).unwrap_or(f64::NEG_INFINITY); - let maximum = maximum.to_number(context).unwrap_or(f64::INFINITY); // 3. If value is NaN or less than minimum or greater than maximum, throw a RangeError exception. if value.is_nan() || value.lt(&minimum) || value.gt(&maximum) { @@ -789,5 +769,5 @@ pub(crate) fn default_number_option( } // 4. Return floor(value). - Ok(JsValue::new(value.floor())) + Ok(value.floor()) } diff --git a/boa_engine/src/builtins/intl/tests.rs b/boa_engine/src/builtins/intl/tests.rs index 565a567de8c..7f16ecd663c 100644 --- a/boa_engine/src/builtins/intl/tests.rs +++ b/boa_engine/src/builtins/intl/tests.rs @@ -249,91 +249,92 @@ fn locale_resolution() { fn get_opt() { let mut context = Context::default(); - let values = Vec::::new(); - let options_obj = JsValue::undefined(); + let values = Vec::::new(); + let fallback = JsValue::String(JsString::new("fallback")); + let options_obj = JsObject::empty(); + let option_type = crate::builtins::intl::GetOptionType::String; let get_option_result = crate::builtins::intl::get_option( &options_obj, "", - "", + &option_type, &values, - &JsValue::String(JsString::empty()), + &fallback, &mut context, - ); - assert_eq!(get_option_result.is_err(), true); - - let values = Vec::::new(); - let fallback = JsValue::String(JsString::new("fallback")); - let options_obj = JsValue::new(JsObject::empty()); - let get_option_result = - crate::builtins::intl::get_option(&options_obj, "", "", &values, &fallback, &mut context) - .expect("GetOption should not fail on fallback test"); + ) + .expect("GetOption should not fail on fallback test"); assert_eq!(get_option_result, fallback); - let values = Vec::::new(); + let values = Vec::::new(); let fallback = JsValue::String(JsString::new("fallback")); let options_obj = JsObject::empty(); let locale_value = JsValue::String(JsString::new("en-US")); options_obj .set("Locale", locale_value.clone(), true, &mut context) .expect("Setting a property should not fail"); + let option_type = crate::builtins::intl::GetOptionType::String; let get_option_result = crate::builtins::intl::get_option( - &JsValue::new(options_obj), + &options_obj, "Locale", - "number", + &option_type, &values, &fallback, &mut context, - ); - assert_eq!(get_option_result.is_err(), true); + ) + .expect("GetOption should not fail on string test"); + assert_eq!(get_option_result, locale_value); - let values = Vec::::new(); let fallback = JsValue::String(JsString::new("fallback")); let options_obj = JsObject::empty(); - let locale_value = JsValue::String(JsString::new("en-US")); + let locale_string = JsString::new("en-US"); + let locale_value = JsValue::String(locale_string.clone()); + let values = vec![locale_string]; options_obj .set("Locale", locale_value.clone(), true, &mut context) .expect("Setting a property should not fail"); + let option_type = crate::builtins::intl::GetOptionType::String; let get_option_result = crate::builtins::intl::get_option( - &JsValue::new(options_obj), + &options_obj, "Locale", - "string", + &option_type, &values, &fallback, &mut context, ) - .expect("GetOption should not fail on string test"); + .expect("GetOption should not fail on values test"); assert_eq!(get_option_result, locale_value); - let fallback = JsValue::String(JsString::new("fallback")); + let fallback = JsValue::new(false); let options_obj = JsObject::empty(); - let locale_value = JsValue::String(JsString::new("en-US")); - let values = vec![locale_value.clone()]; + let boolean_value = JsValue::new(true); + let values = Vec::::new(); options_obj - .set("Locale", locale_value.clone(), true, &mut context) + .set("boolean_val", boolean_value.clone(), true, &mut context) .expect("Setting a property should not fail"); + let option_type = crate::builtins::intl::GetOptionType::Boolean; let get_option_result = crate::builtins::intl::get_option( - &JsValue::new(options_obj), - "Locale", - "string", + &options_obj, + "boolean_val", + &option_type, &values, &fallback, &mut context, ) - .expect("GetOption should not fail on values test"); - assert_eq!(get_option_result, locale_value); + .expect("GetOption should not fail on boolean test"); + assert_eq!(get_option_result, boolean_value); let fallback = JsValue::String(JsString::new("fallback")); let options_obj = JsObject::empty(); let locale_value = JsValue::String(JsString::new("en-US")); - let other_locale_value = JsValue::String(JsString::new("de-DE")); - let values = vec![other_locale_value]; + let other_locale_str = JsString::new("de-DE"); + let values = vec![other_locale_str]; options_obj .set("Locale", locale_value.clone(), true, &mut context) .expect("Setting a property should not fail"); + let option_type = crate::builtins::intl::GetOptionType::String; let get_option_result = crate::builtins::intl::get_option( - &JsValue::new(options_obj), + &options_obj, "Locale", - "string", + &option_type, &values, &fallback, &mut context, @@ -341,119 +342,107 @@ fn get_opt() { assert_eq!(get_option_result.is_err(), true); let value = JsValue::undefined(); - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); + let minimum = 1.0; + let maximum = 10.0; + let fallback_val = 5.0; + let fallback = Some(fallback_val); let get_option_result = crate::builtins::intl::default_number_option( &value, - &minimum, - &maximum, - &fallback, + minimum, + maximum, + fallback, &mut context, ); - assert_eq!(get_option_result, Ok(fallback)); + assert_eq!(get_option_result, Ok(fallback_val)); let value = JsValue::nan(); - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); + let minimum = 1.0; + let maximum = 10.0; + let fallback = Some(5.0); let get_option_result = crate::builtins::intl::default_number_option( &value, - &minimum, - &maximum, - &fallback, + minimum, + maximum, + fallback, &mut context, ); assert_eq!(get_option_result.is_err(), true); let value = JsValue::new(0); - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); + let minimum = 1.0; + let maximum = 10.0; + let fallback = Some(5.0); let get_option_result = crate::builtins::intl::default_number_option( &value, - &minimum, - &maximum, - &fallback, + minimum, + maximum, + fallback, &mut context, ); assert_eq!(get_option_result.is_err(), true); let value = JsValue::new(11); - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); + let minimum = 1.0; + let maximum = 10.0; + let fallback = Some(5.0); let get_option_result = crate::builtins::intl::default_number_option( &value, - &minimum, - &maximum, - &fallback, + minimum, + maximum, + fallback, &mut context, ); assert_eq!(get_option_result.is_err(), true); - let value = JsValue::new(7); - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); + let value_f64 = 7.0; + let value = JsValue::new(value_f64); + let minimum = 1.0; + let maximum = 10.0; + let fallback = Some(5.0); let get_option_result = crate::builtins::intl::default_number_option( &value, - &minimum, - &maximum, - &fallback, + minimum, + maximum, + fallback, &mut context, ); - assert_eq!(get_option_result, Ok(value)); + assert_eq!(get_option_result, Ok(value_f64)); - let options = JsValue::undefined(); - let property = "fractionalSecondDigits"; - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); - let get_option_result = crate::builtins::intl::get_number_option( - &options, - &property, - &minimum, - &maximum, - &fallback, - &mut context, - ); - assert_eq!(get_option_result.is_err(), true); - - let options = JsValue::Object(JsObject::empty()); + let options = JsObject::empty(); let property = "fractionalSecondDigits"; - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); + let minimum = 1.0; + let maximum = 10.0; + let fallback_val = 5.0; + let fallback = Some(fallback_val); let get_option_result = crate::builtins::intl::get_number_option( &options, &property, - &minimum, - &maximum, - &fallback, + minimum, + maximum, + fallback, &mut context, ); - assert_eq!(get_option_result, Ok(fallback)); + assert_eq!(get_option_result, Ok(fallback_val)); let options = JsObject::empty(); - let value = JsValue::new(8); + let value_f64 = 8.0; + let value = JsValue::new(value_f64); let property = "fractionalSecondDigits"; options .set(property, value.clone(), true, &mut context) .expect("Setting a property should not fail"); - let options = JsValue::Object(options); - let minimum = JsValue::new(1); - let maximum = JsValue::new(10); - let fallback = JsValue::new(5); + let minimum = 1.0; + let maximum = 10.0; + let fallback = Some(5.0); let get_option_result = crate::builtins::intl::get_number_option( &options, &property, - &minimum, - &maximum, - &fallback, + minimum, + maximum, + fallback, &mut context, ); - assert_eq!(get_option_result, Ok(value)); + assert_eq!(get_option_result, Ok(value_f64)); } #[test]