From 396ab96ce386addc785c6ae9b06c09ce82aec299 Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Sat, 2 Jul 2022 19:30:25 -0700 Subject: [PATCH 1/2] Make number coercion behave more like Ruby --- crates/core/src/model/scalar/mod.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/core/src/model/scalar/mod.rs b/crates/core/src/model/scalar/mod.rs index 47e015d67..03da3c215 100644 --- a/crates/core/src/model/scalar/mod.rs +++ b/crates/core/src/model/scalar/mod.rs @@ -99,7 +99,12 @@ impl<'s> ScalarCow<'s> { pub fn to_integer(&self) -> Option { match self.0 { ScalarCowEnum::Integer(ref x) => Some(*x), - ScalarCowEnum::Str(ref x) => x.parse::().ok(), + ScalarCowEnum::Float(ref x) => Some(x.trunc() as i64), + ScalarCowEnum::Str(ref x) => Some( + x.parse::() + .map(|parsed| parsed.trunc() as i64) + .unwrap_or(0), + ), _ => None, } } @@ -109,7 +114,7 @@ impl<'s> ScalarCow<'s> { match self.0 { ScalarCowEnum::Integer(ref x) => Some(*x as f64), ScalarCowEnum::Float(ref x) => Some(*x), - ScalarCowEnum::Str(ref x) => x.parse::().ok(), + ScalarCowEnum::Str(ref x) => Some(x.parse::().unwrap_or(0.0)), _ => None, } } @@ -915,19 +920,19 @@ mod test { #[test] fn test_to_integer_float() { let val: ScalarCow<'_> = 42f64.into(); - assert_eq!(val.to_integer(), None); + assert_eq!(val.to_integer(), Some(42)); let val: ScalarCow<'_> = 42.34.into(); - assert_eq!(val.to_integer(), None); + assert_eq!(val.to_integer(), Some(42)); } #[test] fn test_to_integer_str() { let val: ScalarCow<'_> = "foobar".into(); - assert_eq!(val.to_integer(), None); + assert_eq!(val.to_integer(), Some(0)); let val: ScalarCow<'_> = "42.34".into(); - assert_eq!(val.to_integer(), None); + assert_eq!(val.to_integer(), Some(42)); let val: ScalarCow<'_> = "42".into(); assert_eq!(val.to_integer(), Some(42)); @@ -956,7 +961,7 @@ mod test { #[test] fn test_to_float_str() { let val: ScalarCow<'_> = "foobar".into(); - assert_eq!(val.to_float(), None); + assert_eq!(val.to_float(), Some(0.0)); let val: ScalarCow<'_> = "42.34".into(); assert_eq!(val.to_float(), Some(42.34)); From 621feb4f7b899e538ede0c8948acd270b095f110 Mon Sep 17 00:00:00 2001 From: Nat Budin Date: Mon, 11 Jul 2022 16:08:32 -0700 Subject: [PATCH 2/2] Split out to_integer_strict and to_float_strict methods to get the previous behavior and use them where we're assuming the strict behavior --- crates/core/src/model/find.rs | 2 +- crates/core/src/model/scalar/mod.rs | 23 +++++++++++++-- crates/derive/src/filter_parameters.rs | 4 +-- crates/lib/src/stdlib/blocks/for_block.rs | 4 +-- crates/lib/src/stdlib/filters/math.rs | 34 +++++++++++------------ 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/crates/core/src/model/find.rs b/crates/core/src/model/find.rs index 9a81d8a8f..f197e5263 100644 --- a/crates/core/src/model/find.rs +++ b/crates/core/src/model/find.rs @@ -160,7 +160,7 @@ fn try_find_owned<'o, 'i>( fn augmented_get<'o>(value: &'o dyn ValueView, index: &ScalarCow<'_>) -> Option> { if let Some(arr) = value.as_array() { - if let Some(index) = index.to_integer() { + if let Some(index) = index.to_integer_strict() { arr.get(index).map(ValueCow::Borrowed) } else { match &*index.to_kstr() { diff --git a/crates/core/src/model/scalar/mod.rs b/crates/core/src/model/scalar/mod.rs index 03da3c215..b00a3eb25 100644 --- a/crates/core/src/model/scalar/mod.rs +++ b/crates/core/src/model/scalar/mod.rs @@ -95,7 +95,7 @@ impl<'s> ScalarCow<'s> { } } - /// Interpret as an integer, if possible + /// Interpret as an integer, if possible. Returns 0 if given a non-parseable string, and truncates floats. pub fn to_integer(&self) -> Option { match self.0 { ScalarCowEnum::Integer(ref x) => Some(*x), @@ -109,7 +109,16 @@ impl<'s> ScalarCow<'s> { } } - /// Interpret as a float, if possible + /// Interpret as an integer, if possible. Return None if given a float or non-parseable string. + pub fn to_integer_strict(&self) -> Option { + match self.0 { + ScalarCowEnum::Integer(ref x) => Some(*x), + ScalarCowEnum::Str(ref x) => x.parse::().ok(), + _ => None, + } + } + + /// Interpret as a float, if possible. Return 0.0 if given a non-parseable string. pub fn to_float(&self) -> Option { match self.0 { ScalarCowEnum::Integer(ref x) => Some(*x as f64), @@ -119,6 +128,16 @@ impl<'s> ScalarCow<'s> { } } + /// Interpret as a float, if possible. Return None if given a non-parseable string. + pub fn to_float_strict(&self) -> Option { + match self.0 { + ScalarCowEnum::Integer(ref x) => Some(*x as f64), + ScalarCowEnum::Float(ref x) => Some(*x), + ScalarCowEnum::Str(ref x) => x.parse::().ok(), + _ => None, + } + } + /// Interpret as a bool, if possible pub fn to_bool(&self) -> Option { match self.0 { diff --git a/crates/derive/src/filter_parameters.rs b/crates/derive/src/filter_parameters.rs index c250a8fda..594c77668 100644 --- a/crates/derive/src/filter_parameters.rs +++ b/crates/derive/src/filter_parameters.rs @@ -499,7 +499,7 @@ fn generate_evaluate_field(field: &FilterParameter<'_>) -> TokenStream { }, FilterParameterType::Integer => quote! { #name.as_scalar() - .and_then(|s| s.to_integer()) + .and_then(|s| s.to_integer_strict()) .ok_or_else(|| ::liquid_core::error::Error::with_msg("Invalid argument") .context("argument", #liquid_name) @@ -508,7 +508,7 @@ fn generate_evaluate_field(field: &FilterParameter<'_>) -> TokenStream { }, FilterParameterType::Float => quote! { #name.as_scalar() - .and_then(|s| s.to_float()) + .and_then(|s| s.to_float_strict()) .ok_or_else(|| ::liquid_core::error::Error::with_msg("Invalid argument") .context("argument", #liquid_name) diff --git a/crates/lib/src/stdlib/blocks/for_block.rs b/crates/lib/src/stdlib/blocks/for_block.rs index bb6650845..6475ab9b5 100644 --- a/crates/lib/src/stdlib/blocks/for_block.rs +++ b/crates/lib/src/stdlib/blocks/for_block.rs @@ -510,7 +510,7 @@ fn evaluate_attr(attr: &Option, runtime: &dyn Runtime) -> Result Resu let value = value .as_scalar() - .and_then(|v| v.to_integer()) + .and_then(|v| v.to_integer_strict()) .ok_or_else(|| unexpected_value_error("whole number", Some(value.type_name()))) .context_key_with(|| arg_name.to_owned().into()) .value_with(|| value.to_kstr().into_owned())?; diff --git a/crates/lib/src/stdlib/filters/math.rs b/crates/lib/src/stdlib/filters/math.rs index 5abdc2ca7..20485c213 100644 --- a/crates/lib/src/stdlib/filters/math.rs +++ b/crates/lib/src/stdlib/filters/math.rs @@ -28,7 +28,7 @@ impl Filter for AbsFilter { .as_scalar() .ok_or_else(|| invalid_input("Number expected"))?; input - .to_integer() + .to_integer_strict() .map(|i| Value::scalar(i.abs())) .or_else(|| input.to_float().map(|i| Value::scalar(i.abs()))) .ok_or_else(|| invalid_input("Number expected")) @@ -71,8 +71,8 @@ impl Filter for AtLeastFilter { .ok_or_else(|| invalid_argument("operand", "Number expected"))?; let result = input - .to_integer() - .and_then(|i| min.to_integer().map(|min| Value::scalar(i.max(min)))) + .to_integer_strict() + .and_then(|i| min.to_integer_strict().map(|min| Value::scalar(i.max(min)))) .or_else(|| { input .to_float() @@ -120,8 +120,8 @@ impl Filter for AtMostFilter { .ok_or_else(|| invalid_argument("operand", "Number expected"))?; let result = input - .to_integer() - .and_then(|i| max.to_integer().map(|max| Value::scalar(i.min(max)))) + .to_integer_strict() + .and_then(|i| max.to_integer_strict().map(|max| Value::scalar(i.min(max)))) .or_else(|| { input .to_float() @@ -169,8 +169,8 @@ impl Filter for PlusFilter { .ok_or_else(|| invalid_argument("operand", "Number expected"))?; let result = input - .to_integer() - .and_then(|i| operand.to_integer().map(|o| Value::scalar(i + o))) + .to_integer_strict() + .and_then(|i| operand.to_integer_strict().map(|o| Value::scalar(i + o))) .or_else(|| { input .to_float() @@ -218,8 +218,8 @@ impl Filter for MinusFilter { .ok_or_else(|| invalid_argument("operand", "Number expected"))?; let result = input - .to_integer() - .and_then(|i| operand.to_integer().map(|o| Value::scalar(i - o))) + .to_integer_strict() + .and_then(|i| operand.to_integer_strict().map(|o| Value::scalar(i - o))) .or_else(|| { input .to_float() @@ -267,8 +267,8 @@ impl Filter for TimesFilter { .ok_or_else(|| invalid_argument("operand", "Number expected"))?; let result = input - .to_integer() - .and_then(|i| operand.to_integer().map(|o| Value::scalar(i * o))) + .to_integer_strict() + .and_then(|i| operand.to_integer_strict().map(|o| Value::scalar(i * o))) .or_else(|| { input .to_float() @@ -315,7 +315,7 @@ impl Filter for DividedByFilter { .as_scalar() .ok_or_else(|| invalid_argument("operand", "Number expected"))?; - if let Some(o) = operand.to_integer() { + if let Some(o) = operand.to_integer_strict() { if o == 0 { return Err(invalid_argument("operand", "Can't divide by zero")); } @@ -326,8 +326,8 @@ impl Filter for DividedByFilter { } let result = input - .to_integer() - .and_then(|i| operand.to_integer().map(|o| Value::scalar(i / o))) + .to_integer_strict() + .and_then(|i| operand.to_integer_strict().map(|o| Value::scalar(i / o))) .or_else(|| { input .to_float() @@ -374,7 +374,7 @@ impl Filter for ModuloFilter { .as_scalar() .ok_or_else(|| invalid_argument("operand", "Number expected"))?; - if let Some(o) = operand.to_integer() { + if let Some(o) = operand.to_integer_strict() { if o == 0 { return Err(invalid_argument("operand", "Can't divide by zero")); } @@ -385,8 +385,8 @@ impl Filter for ModuloFilter { } let result = input - .to_integer() - .and_then(|i| operand.to_integer().map(|o| Value::scalar(i % o))) + .to_integer_strict() + .and_then(|i| operand.to_integer_strict().map(|o| Value::scalar(i % o))) .or_else(|| { input .to_float()