diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bdad4d9..a31332e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -138,6 +138,10 @@ jobs: - name: "Generate Cargo Lockfile" run: "cargo generate-lockfile" + - name: "Run clippy" + shell: bash + run: "cargo clippy --all --tests -- -D warnings" + # Cache build dependencies - name: "Cache Build Fragments" id: "cache-build-fragments" diff --git a/Cargo.toml b/Cargo.toml index 6a392da..e4feed3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,10 +27,10 @@ required-features = ["cmdline"] cmdline = ["anyhow", "clap"] default = [] python = ["cpython"] -wasm = ["wasm-bindgen"] +wasm = ["wasm-bindgen", "gloo-utils"] [dependencies] -phf = {version = "~0.8.0", features = ["macros"]} +phf = { version = "~0.8.0", features = ["macros"] } serde_json = "~1.0.41" thiserror = "~1.0.11" @@ -39,6 +39,16 @@ features = ["serde-serialize"] optional = true version = "~0.2.62" +[dependencies.gloo-utils] +features = [] +optional = true +version = "~0.2.0" + + +[dependencies.serde] +features = ["derive"] +version = "1" + [dependencies.cpython] features = ["extension-module"] optional = true diff --git a/src/bin.rs b/src/bin.rs index 4b8accd..4eb787f 100644 --- a/src/bin.rs +++ b/src/bin.rs @@ -3,11 +3,8 @@ use std::io::Read; use anyhow::{Context, Result}; use clap::{App, Arg}; -use serde_json; use serde_json::Value; -use jsonlogic_rs; - fn configure_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> { app.version(env!("CARGO_PKG_VERSION")) .author("Matthew Planchard ") @@ -67,7 +64,7 @@ fn main() -> Result<()> { let result = jsonlogic_rs::apply(&json_logic, &json_data) .context("Could not execute logic")?; - println!("{}", result.to_string()); + println!("{}", result); Ok(()) } diff --git a/src/error.rs b/src/error.rs index 6b586b7..832e561 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,6 @@ //! Error handling //! use serde_json::Value; -use thiserror; use crate::op::NumParams; @@ -30,6 +29,7 @@ pub enum Error { #[error("Invalid variable mapping - {0} is not an object.")] InvalidVarMap(Value), + #[allow(clippy::enum_variant_names)] #[error("Encountered an unexpected error. Please raise an issue on GitHub and include the following error message: {0}")] UnexpectedError(String), diff --git a/src/js_op.rs b/src/js_op.rs index b015263..e560216 100644 --- a/src/js_op.rs +++ b/src/js_op.rs @@ -7,7 +7,7 @@ use std::str::FromStr; use crate::error::Error; // numeric characters according to parseFloat -const NUMERICS: &'static [char] = &[ +const NUMERICS: &[char] = &[ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '.', '-', '+', 'e', 'E', ]; @@ -58,7 +58,7 @@ fn to_primitive_number(value: &Value) -> Option { pub fn str_to_number>(string: S) -> Option { let s = string.as_ref(); - if s == "" { + if s.is_empty() { Some(0.0) } else { f64::from_str(s).ok() @@ -240,7 +240,7 @@ pub fn abstract_eq(first: &Value, second: &Value) -> bool { } // 6. If Type(x) is Boolean, return the result of the comparison ToNumber(x) == y. (Value::Bool(x), _) => match x { - true => Number::from_f64(1 as f64) + true => Number::from_f64(1_f64) .map(|num| { let value = Value::Number(num); abstract_eq(&value, second) @@ -255,7 +255,7 @@ pub fn abstract_eq(first: &Value, second: &Value) -> bool { }, // 7. If Type(y) is Boolean, return the result of the comparison x == ToNumber(y). (_, Value::Bool(y)) => match y { - true => Number::from_f64(1 as f64) + true => Number::from_f64(1_f64) .map(|num| { let value = Value::Number(num); abstract_eq(first, &value) @@ -430,18 +430,18 @@ pub fn abstract_gte(first: &Value, second: &Value) -> bool { } /// Get the max of an array of values, performing abstract type conversion -pub fn abstract_max(items: &Vec<&Value>) -> Result { +pub fn abstract_max(items: &[&Value]) -> Result { items - .into_iter() + .iter() .map(|v| { - to_number(v).ok_or(Error::InvalidArgument { + to_number(v).ok_or_else(|| Error::InvalidArgument { value: (*v).clone(), operation: "max".into(), reason: "Could not convert value to number".into(), }) }) - .fold(Ok(f64::NEG_INFINITY), |acc, cur| { - let max = acc?; + .try_fold(f64::NEG_INFINITY, |acc, cur| { + let max = acc; match cur { Ok(num) => { if num > max { @@ -456,18 +456,18 @@ pub fn abstract_max(items: &Vec<&Value>) -> Result { } /// Get the max of an array of values, performing abstract type conversion -pub fn abstract_min(items: &Vec<&Value>) -> Result { +pub fn abstract_min(items: &[&Value]) -> Result { items - .into_iter() + .iter() .map(|v| { - to_number(v).ok_or(Error::InvalidArgument { + to_number(v).ok_or_else(|| Error::InvalidArgument { value: (*v).clone(), operation: "max".into(), reason: "Could not convert value to number".into(), }) }) - .fold(Ok(f64::INFINITY), |acc, cur| { - let min = acc?; + .try_fold(f64::INFINITY, |acc, cur| { + let min = acc; match cur { Ok(num) => { if num < min { @@ -486,11 +486,8 @@ pub fn abstract_plus(first: &Value, second: &Value) -> Value { let first_num = to_primitive_number(first); let second_num = to_primitive_number(second); - match (first_num, second_num) { - (Some(f), Some(s)) => { - return Value::Number(Number::from_f64(f + s).unwrap()); - } - _ => {} + if let (Some(f), Some(s)) = (first_num, second_num) { + return Value::Number(Number::from_f64(f + s).unwrap()); }; let first_string = to_string(first); @@ -515,17 +512,17 @@ pub fn abstract_plus(first: &Value, second: &Value) -> Value { /// the behavior for non-numeric inputs is not specified in the spec, /// and returning errors seems like a more reasonable course of action /// than returning null. -pub fn parse_float_add(vals: &Vec<&Value>) -> Result { - vals.into_iter() +pub fn parse_float_add(vals: &[&Value]) -> Result { + vals.iter() .map(|&v| { - parse_float(v).ok_or(Error::InvalidArgument { + parse_float(v).ok_or_else(|| Error::InvalidArgument { value: v.clone(), operation: "+".into(), reason: "Argument could not be converted to a float".into(), }) }) - .fold(Ok(0.0), |acc, cur| { - let total = acc?; + .try_fold(0.0, |acc, cur| { + let total = acc; match cur { Ok(num) => Ok(total + num), _ => cur, @@ -538,17 +535,17 @@ pub fn parse_float_add(vals: &Vec<&Value>) -> Result { /// See notes for parse_float_add on how this differs from normal number /// conversion as is done for _other_ arithmetic operators in the reference /// implementation -pub fn parse_float_mul(vals: &Vec<&Value>) -> Result { - vals.into_iter() +pub fn parse_float_mul(vals: &[&Value]) -> Result { + vals.iter() .map(|&v| { - parse_float(v).ok_or(Error::InvalidArgument { + parse_float(v).ok_or_else(|| Error::InvalidArgument { value: v.clone(), operation: "*".into(), reason: "Argument could not be converted to a float".into(), }) }) - .fold(Ok(1.0), |acc, cur| { - let total = acc?; + .try_fold(1.0, |acc, cur| { + let total = acc; match cur { Ok(num) => Ok(total * num), _ => cur, @@ -561,14 +558,14 @@ pub fn abstract_minus(first: &Value, second: &Value) -> Result { let first_num = to_number(first); let second_num = to_number(second); - if let None = first_num { + if first_num.is_none() { return Err(Error::InvalidArgument { value: first.clone(), operation: "-".into(), reason: "Could not convert value to number.".into(), }); } - if let None = second_num { + if second_num.is_none() { return Err(Error::InvalidArgument { value: second.clone(), operation: "-".into(), @@ -584,14 +581,14 @@ pub fn abstract_div(first: &Value, second: &Value) -> Result { let first_num = to_number(first); let second_num = to_number(second); - if let None = first_num { + if first_num.is_none() { return Err(Error::InvalidArgument { value: first.clone(), operation: "/".into(), reason: "Could not convert value to number.".into(), }); } - if let None = second_num { + if second_num.is_none() { return Err(Error::InvalidArgument { value: second.clone(), operation: "/".into(), @@ -607,14 +604,14 @@ pub fn abstract_mod(first: &Value, second: &Value) -> Result { let first_num = to_number(first); let second_num = to_number(second); - if let None = first_num { + if first_num.is_none() { return Err(Error::InvalidArgument { value: first.clone(), operation: "%".into(), reason: "Could not convert value to number.".into(), }); } - if let None = second_num { + if second_num.is_none() { return Err(Error::InvalidArgument { value: second.clone(), operation: "%".into(), @@ -629,7 +626,7 @@ pub fn abstract_mod(first: &Value, second: &Value) -> Result { pub fn to_negative(val: &Value) -> Result { to_number(val) .map(|v| -1.0 * v) - .ok_or(Error::InvalidArgument { + .ok_or_else(|| Error::InvalidArgument { value: val.clone(), operation: "to_negative".into(), reason: "Could not convert value to a number".into(), @@ -643,7 +640,7 @@ pub fn to_negative(val: &Value) -> Result { /// quite follow the spec exactly: we don't deal with infinity /// and NaN. That is okay, because this is only used in a context dealing /// with JSON values, which can't be Infinity or NaN. -fn parse_float_string(val: &String) -> Option { +fn parse_float_string(val: &str) -> Option { let (mut leading_numerics, _, _) = val.trim().chars().fold( (Vec::new(), false, false), |(mut acc, broke, saw_decimal), c| { @@ -667,7 +664,7 @@ fn parse_float_string(val: &String) -> Option { }, ); // don't bother collecting into a string if we don't need to - if leading_numerics.len() == 0 { + if leading_numerics.is_empty() { return None; }; if let Some('e') | Some('E') = leading_numerics.last() { @@ -693,7 +690,7 @@ pub fn parse_float(val: &Value) -> Option { match val { Value::Number(num) => num.as_f64(), Value::String(string) => parse_float_string(string), - _ => parse_float(&Value::String(to_string(&val))), + _ => parse_float(&Value::String(to_string(val))), } } @@ -908,7 +905,7 @@ mod abstract_operations { fn test_abstract_eq() { equal_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert!(abstract_eq(&first, &second), true); + assert!(abstract_eq(first, second), "{}", true); }) } @@ -916,7 +913,7 @@ mod abstract_operations { fn test_abstract_ne() { ne_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_ne(&first, &second), true); + assert!(abstract_ne(first, second)); }) } @@ -924,7 +921,7 @@ mod abstract_operations { fn test_abstract_lt() { lt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_lt(&first, &second), true); + assert!(abstract_lt(first, second)); }) } @@ -932,7 +929,7 @@ mod abstract_operations { fn test_abstract_gt() { gt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_gt(&first, &second), true); + assert!(abstract_gt(first, second)); }) } @@ -940,7 +937,7 @@ mod abstract_operations { fn test_eq_values_are_not_lt() { equal_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_lt(&first, &second), false); + assert!(!abstract_lt(first, second)); }) } @@ -948,7 +945,7 @@ mod abstract_operations { fn test_eq_values_are_not_gt() { equal_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_gt(&first, &second), false); + assert!(!abstract_gt(first, second)); }) } @@ -956,7 +953,7 @@ mod abstract_operations { fn test_eq_values_are_not_ne() { equal_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_ne(&first, &second), false); + assert!(!abstract_ne(first, second)); }) } @@ -964,7 +961,7 @@ mod abstract_operations { fn test_lt_values_are_not_eq() { lt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_eq(&first, &second), false); + assert!(!abstract_eq(first, second)); }) } @@ -972,7 +969,7 @@ mod abstract_operations { fn test_lt_values_are_not_gt() { lt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_gt(&first, &second), false); + assert!(!abstract_gt(first, second)); }) } @@ -980,7 +977,7 @@ mod abstract_operations { fn test_lt_values_are_ne() { lt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_ne(&first, &second), true); + assert!(abstract_ne(first, second)); }) } @@ -988,7 +985,7 @@ mod abstract_operations { fn test_gt_values_are_not_eq() { gt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_eq(&first, &second), false); + assert!(!abstract_eq(first, second)); }) } @@ -996,7 +993,7 @@ mod abstract_operations { fn test_gt_values_are_not_lt() { gt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_lt(&first, &second), false); + assert!(!abstract_lt(first, second)); }) } @@ -1004,7 +1001,7 @@ mod abstract_operations { fn test_gt_values_are_ne() { gt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_ne(&first, &second), true); + assert!(abstract_ne(first, second)); }) } @@ -1012,9 +1009,9 @@ mod abstract_operations { fn test_incomparable() { not_gt_not_lt_not_eq().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_lt(&first, &second), false); - assert_eq!(abstract_gt(&first, &second), false); - assert_eq!(abstract_eq(&first, &second), false); + assert!(!abstract_lt(first, second)); + assert!(!abstract_gt(first, second)); + assert!(!abstract_eq(first, second)); }) } @@ -1024,7 +1021,7 @@ mod abstract_operations { fn test_lt_values_are_lte() { lt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_lte(&first, &second), true); + assert!(abstract_lte(first, second)); }) } @@ -1032,7 +1029,7 @@ mod abstract_operations { fn test_eq_values_are_lte() { equal_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_lte(&first, &second), true); + assert!(abstract_lte(first, second)); }) } @@ -1040,7 +1037,7 @@ mod abstract_operations { fn test_gt_values_are_not_lte() { gt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_lte(&first, &second), false); + assert!(!abstract_lte(first, second)); }) } @@ -1050,7 +1047,7 @@ mod abstract_operations { fn test_gt_values_are_gte() { gt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_gte(&first, &second), true); + assert!(abstract_gte(first, second)); }) } @@ -1058,7 +1055,7 @@ mod abstract_operations { fn test_eq_values_are_gte() { equal_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_gte(&first, &second), true); + assert!(abstract_gte(first, second)); }) } @@ -1066,7 +1063,7 @@ mod abstract_operations { fn test_lt_values_are_not_gte() { lt_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert_eq!(abstract_gte(&first, &second), false); + assert!(!abstract_gte(first, second)); }) } @@ -1074,17 +1071,17 @@ mod abstract_operations { fn test_abstract_plus() { plus_cases().iter().for_each(|(first, second, exp)| { println!("{:?}-{:?}", &first, &second); - let result = abstract_plus(&first, &second); + let result = abstract_plus(first, second); match result { Value::Number(ref i) => match exp { Value::Number(j) => assert_eq!(i, j), - _ => assert!(false), + _ => panic!("Both must be Numbers"), }, Value::String(ref i) => match exp { Value::String(j) => assert_eq!(i, j), - _ => assert!(false), + _ => panic!("Both must be Strings"), }, - _ => assert!(false), + _ => panic!("Only support two numbers and two strings"), } }) } @@ -1112,7 +1109,7 @@ mod test_abstract_max { fn test_abstract_max() { max_cases().into_iter().for_each(|(items, exp)| { println!("Max: {:?}", items); - let res = abstract_max(&items.iter().collect()); + let res = abstract_max(&items.iter().collect::>()); println!("Res: {:?}", res); match exp { Ok(exp) => assert_eq!(res.unwrap(), exp), @@ -1146,7 +1143,7 @@ mod test_abstract_min { fn test_abstract_min() { min_cases().into_iter().for_each(|(items, exp)| { println!("Min: {:?}", items); - let res = abstract_min(&items.iter().collect()); + let res = abstract_min(&items.iter().collect::>()); println!("Res: {:?}", res); match exp { Ok(exp) => assert_eq!(res.unwrap(), exp), @@ -1229,11 +1226,11 @@ mod test_strict { fn test_strict_eq() { eq_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert!(strict_eq(&first, &second)); + assert!(strict_eq(first, second)); }); ne_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert!(!strict_eq(&first, &second)); + assert!(!strict_eq(first, second)); }); } @@ -1247,11 +1244,11 @@ mod test_strict { fn test_strict_ne() { ne_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert!(strict_ne(&first, &second)); + assert!(strict_ne(first, second)); }); eq_values().iter().for_each(|(first, second)| { println!("{:?}-{:?}", &first, &second); - assert!(!strict_ne(&first, &second)); + assert!(!strict_ne(first, second)); }); } diff --git a/src/lib.rs b/src/lib.rs index 35103f9..4f2c8ae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,3 @@ -use serde_json; use serde_json::Value; mod error; @@ -19,6 +18,7 @@ trait Parser<'a>: Sized + Into { #[cfg(feature = "wasm")] pub mod javascript_iface { + use gloo_utils::format::JsValueSerdeExt; use serde_json::Value; use wasm_bindgen::prelude::*; @@ -34,8 +34,8 @@ pub mod javascript_iface { serde_json::from_str(&js_string).or(Ok(Value::String(js_string))) } else { // If we're passed anything else, convert it directly to a serde Value. - js_value - .into_serde::() + + JsValueSerdeExt::into_serde::(&js_value) .map_err(|err| format!("{}", err)) .map_err(JsValue::from) } @@ -50,7 +50,7 @@ pub mod javascript_iface { .map_err(|err| format!("{}", err)) .map_err(JsValue::from)?; - JsValue::from_serde(&res) + ::from_serde(&res) .map_err(|err| format!("{}", err)) .map_err(JsValue::from) } @@ -63,6 +63,8 @@ pub mod python_iface { py_module_initializer!(jsonlogic, initjsonlogic, PyInit_jsonlogic, |py, m| { m.add(py, "__doc__", "Python bindings for json-logic-rs")?; + // Manual strip is done in cpython's macro code + #[allow(clippy::manual_strip)] m.add(py, "apply", py_fn!(py, py_apply(value: &str, data: &str)))?; Ok(()) }); @@ -85,7 +87,7 @@ pub mod python_iface { /// Run JSONLogic for the given operation and data. /// pub fn apply(value: &Value, data: &Value) -> Result { - let parsed = Parsed::from_value(&value)?; + let parsed = Parsed::from_value(value)?; parsed.evaluate(data).map(Value::from) } @@ -1147,7 +1149,7 @@ mod jsonlogic_tests { ] } - fn assert_jsonlogic((op, data, exp): (Value, Value, Result)) -> () { + fn assert_jsonlogic((op, data, exp): (Value, Value, Result)) { println!("Running rule: {:?} with data: {:?}", op, data); let result = apply(&op, &data); println!("- Result: {:?}", result); diff --git a/src/op/array.rs b/src/op/array.rs index f170f6a..a1e3a44 100644 --- a/src/op/array.rs +++ b/src/op/array.rs @@ -10,7 +10,7 @@ use crate::op::logic; use crate::value::{Evaluated, Parsed}; /// Map an operation onto values -pub fn map(data: &Value, args: &Vec<&Value>) -> Result { +pub fn map(data: &Value, args: &[&Value]) -> Result { let (items, expression) = (args[0], args[1]); let _parsed = Parsed::from_value(items)?; @@ -45,7 +45,7 @@ pub fn map(data: &Value, args: &Vec<&Value>) -> Result { } /// Filter values by some predicate -pub fn filter(data: &Value, args: &Vec<&Value>) -> Result { +pub fn filter(data: &Value, args: &[&Value]) -> Result { let (items, expression) = (args[0], args[1]); let _parsed = Parsed::from_value(items)?; @@ -53,9 +53,7 @@ pub fn filter(data: &Value, args: &Vec<&Value>) -> Result { let values: Vec = match evaluated_items { Evaluated::New(Value::Array(vals)) => vals, - Evaluated::Raw(Value::Array(vals)) => { - vals.into_iter().map(|v| v.clone()).collect() - } + Evaluated::Raw(Value::Array(vals)) => vals.to_vec(), // null is treated as an empty array in the reference tests, // for whatever reason Evaluated::New(Value::Null) => vec![], @@ -77,8 +75,8 @@ pub fn filter(data: &Value, args: &Vec<&Value>) -> Result { let value_vec: Vec = Vec::with_capacity(values.len()); values .into_iter() - .fold(Ok(value_vec), |acc, cur| { - let mut filtered = acc?; + .try_fold(value_vec, |acc, cur| { + let mut filtered = acc; let predicate = parsed_expression.evaluate(&cur)?; match logic::truthy_from_evaluated(&predicate) { @@ -97,7 +95,7 @@ pub fn filter(data: &Value, args: &Vec<&Value>) -> Result { /// Note this differs from the reference implementation of jsonlogic /// (but not the spec), in that it evaluates the initializer as a /// jsonlogic expression rather than a raw value. -pub fn reduce(data: &Value, args: &Vec<&Value>) -> Result { +pub fn reduce(data: &Value, args: &[&Value]) -> Result { let (items, expression, initializer) = (args[0], args[1], args[2]); let _parsed_items = Parsed::from_value(items)?; @@ -108,7 +106,7 @@ pub fn reduce(data: &Value, args: &Vec<&Value>) -> Result { let values: Vec = match evaluated_items { Evaluated::New(Value::Array(vals)) => vals, - Evaluated::Raw(Value::Array(vals)) => vals.iter().map(|v| v.clone()).collect(), + Evaluated::Raw(Value::Array(vals)) => vals.to_vec(), // null is treated as an empty array in the reference tests, // for whatever reason Evaluated::New(Value::Null) => vec![], @@ -129,8 +127,8 @@ pub fn reduce(data: &Value, args: &Vec<&Value>) -> Result { values .into_iter() - .fold(Ok(Value::from(evaluated_initializer)), |acc, cur| { - let accumulator = acc?; + .try_fold(Value::from(evaluated_initializer), |acc, cur| { + let accumulator = acc; let mut data = Map::with_capacity(2); data.insert("current".into(), cur); data.insert("accumulator".into(), accumulator); @@ -146,7 +144,7 @@ pub fn reduce(data: &Value, args: &Vec<&Value>) -> Result { /// The predicate does not need to return true or false explicitly. Its /// return is evaluated using the "truthy" definition specified in the /// jsonlogic spec. -pub fn all(data: &Value, args: &Vec<&Value>) -> Result { +pub fn all(data: &Value, args: &[&Value]) -> Result { let (first_arg, second_arg) = (args[0], args[1]); // The first argument must be an array of values or a string of chars @@ -173,7 +171,6 @@ pub fn all(data: &Value, args: &Vec<&Value>) -> Result { Value::String(string) => { _new_arr = string .chars() - .into_iter() .map(|c| Value::String(c.to_string())) .collect(); &_new_arr @@ -196,7 +193,7 @@ pub fn all(data: &Value, args: &Vec<&Value>) -> Result { // Special-case the empty array, since it for some reason is specified // to return false. - if items.len() == 0 { + if items.is_empty() { return Ok(Value::Bool(false)); } @@ -205,19 +202,17 @@ pub fn all(data: &Value, args: &Vec<&Value>) -> Result { // returning 1 for each of the items and thus evaluating to true. let predicate = Parsed::from_value(second_arg)?; - let result = items.into_iter().fold(Ok(true), |acc, i| { - acc.and_then(|res| { - // "Short-circuit": return false if the previous eval was false - if !res { - return Ok(false); - }; - let _parsed_item = Parsed::from_value(i)?; - // Evaluate each item as we go, in case we can short-circuit - let evaluated_item = _parsed_item.evaluate(data)?; - Ok(logic::truthy_from_evaluated( - &predicate.evaluate(&evaluated_item.into())?, - )) - }) + let result = items.iter().try_fold(true, |acc, i| { + // "Short-circuit": return false if the previous eval was false + if !acc { + return Ok(false); + }; + let _parsed_item = Parsed::from_value(i)?; + // Evaluate each item as we go, in case we can short-circuit + let evaluated_item = _parsed_item.evaluate(data)?; + Ok(logic::truthy_from_evaluated( + &predicate.evaluate(&evaluated_item.into())?, + )) })?; Ok(Value::Bool(result)) @@ -228,7 +223,7 @@ pub fn all(data: &Value, args: &Vec<&Value>) -> Result { /// The predicate does not need to return true or false explicitly. Its /// return is evaluated using the "truthy" definition specified in the /// jsonlogic spec. -pub fn some(data: &Value, args: &Vec<&Value>) -> Result { +pub fn some(data: &Value, args: &[&Value]) -> Result { let (first_arg, second_arg) = (args[0], args[1]); // The first argument must be an array of values or a string of chars @@ -255,7 +250,6 @@ pub fn some(data: &Value, args: &Vec<&Value>) -> Result { Value::String(string) => { _new_arr = string .chars() - .into_iter() .map(|c| Value::String(c.to_string())) .collect(); &_new_arr @@ -278,7 +272,7 @@ pub fn some(data: &Value, args: &Vec<&Value>) -> Result { // Special-case the empty array, since it for some reason is specified // to return false. - if items.len() == 0 { + if items.is_empty() { return Ok(Value::Bool(false)); } @@ -287,19 +281,17 @@ pub fn some(data: &Value, args: &Vec<&Value>) -> Result { // returning 1 for each of the items and thus evaluating to true. let predicate = Parsed::from_value(second_arg)?; - let result = items.into_iter().fold(Ok(false), |acc, i| { - acc.and_then(|res| { - // "Short-circuit": return false if the previous eval was false - if res { - return Ok(true); - }; - let _parsed_item = Parsed::from_value(i)?; - // Evaluate each item as we go, in case we can short-circuit - let evaluated_item = _parsed_item.evaluate(data)?; - Ok(logic::truthy_from_evaluated( - &predicate.evaluate(&evaluated_item.into())?, - )) - }) + let result = items.iter().try_fold(false, |acc, i| { + // "Short-circuit": return false if the previous eval was false + if acc { + return Ok(true); + }; + let _parsed_item = Parsed::from_value(i)?; + // Evaluate each item as we go, in case we can short-circuit + let evaluated_item = _parsed_item.evaluate(data)?; + Ok(logic::truthy_from_evaluated( + &predicate.evaluate(&evaluated_item.into())?, + )) })?; Ok(Value::Bool(result)) @@ -310,7 +302,7 @@ pub fn some(data: &Value, args: &Vec<&Value>) -> Result { /// The predicate does not need to return true or false explicitly. Its /// return is evaluated using the "truthy" definition specified in the /// jsonlogic spec. -pub fn none(data: &Value, args: &Vec<&Value>) -> Result { +pub fn none(data: &Value, args: &[&Value]) -> Result { some(data, args).and_then(|had_some| match had_some { Value::Bool(res) => Ok(Value::Bool(!res)), _ => Err(Error::UnexpectedError( @@ -323,26 +315,23 @@ pub fn none(data: &Value, args: &Vec<&Value>) -> Result { /// /// Values that are not arrays are (effectively) converted to arrays /// before flattening. -pub fn merge(items: &Vec<&Value>) -> Result { +pub fn merge(items: &[&Value]) -> Result { let rv_vec: Vec = Vec::new(); - Ok(Value::Array(items.into_iter().fold( - rv_vec, - |mut acc, i| { - match i { - Value::Array(i_vals) => { - i_vals.into_iter().for_each(|val| acc.push((*val).clone())); - } - _ => acc.push((**i).clone()), - }; - acc - }, - ))) + Ok(Value::Array(items.iter().fold(rv_vec, |mut acc, i| { + match i { + Value::Array(i_vals) => { + i_vals.iter().for_each(|val| acc.push((*val).clone())); + } + _ => acc.push((**i).clone()), + }; + acc + }))) } /// Perform containment checks with "in" // TODO: make this a lazy operator, since we don't need to parse things // later on in the list if we find something that matches early. -pub fn in_(items: &Vec<&Value>) -> Result { +pub fn in_(items: &[&Value]) -> Result { let needle = items[0]; let haystack = items[1]; diff --git a/src/op/data.rs b/src/op/data.rs index be84972..27709dc 100644 --- a/src/op/data.rs +++ b/src/op/data.rs @@ -23,12 +23,12 @@ impl<'a> TryFrom for KeyType<'a> { match value { Value::Null => Ok(Self::Null), Value::String(s) => Ok(Self::String(Cow::from(s))), - Value::Number(n) => Ok(Self::Number(n.as_i64().ok_or( + Value::Number(n) => Ok(Self::Number(n.as_i64().ok_or_else(|| { Error::InvalidVariableKey { value: Value::Number(n), reason: "Numeric keys must be valid integers".into(), - }, - )?)), + } + })?)), _ => Err(Error::InvalidVariableKey { value: value.clone(), reason: "Variable keys must be strings, integers, or null".into(), @@ -43,12 +43,12 @@ impl<'a> TryFrom<&'a Value> for KeyType<'a> { match value { Value::Null => Ok(Self::Null), Value::String(s) => Ok(Self::String(Cow::from(s))), - Value::Number(n) => Ok(Self::Number(n.as_i64().ok_or( + Value::Number(n) => Ok(Self::Number(n.as_i64().ok_or_else(|| { Error::InvalidVariableKey { value: value.clone(), reason: "Numeric keys must be valid integers".into(), - }, - )?)), + } + })?)), _ => Err(Error::InvalidVariableKey { value: value.clone(), reason: "Variable keys must be strings, integers, or null".into(), @@ -85,7 +85,7 @@ fn get(slice: &[T], idx: i64) -> Option<&T> { /// /// Note that the reference implementation does not support negative /// indexing for numeric values, but we do. -pub fn var(data: &Value, args: &Vec<&Value>) -> Result { +pub fn var(data: &Value, args: &[&Value]) -> Result { let arg_count = args.len(); if arg_count == 0 { return Ok(data.clone()); @@ -98,12 +98,12 @@ pub fn var(data: &Value, args: &Vec<&Value>) -> Result { NULL } else { let _parsed_default = Parsed::from_value(args[1])?; - _parsed_default.evaluate(&data)?.into() + _parsed_default.evaluate(data)?.into() })) } /// Check for keys that are missing from the data -pub fn missing(data: &Value, args: &Vec<&Value>) -> Result { +pub fn missing(data: &Value, args: &[&Value]) -> Result { let mut missing_keys: Vec = Vec::new(); // This bit of insanity is because for some reason the reference @@ -111,7 +111,7 @@ pub fn missing(data: &Value, args: &Vec<&Value>) -> Result { // multiple args and the first arg is an array, _that_ array is // treated as the only argument. let inner_vec: Vec<&Value>; - let adjusted_args = if args.len() > 0 { + let adjusted_args = if !args.is_empty() { match args[0] { Value::Array(vals) => { inner_vec = vals.iter().collect(); @@ -123,8 +123,7 @@ pub fn missing(data: &Value, args: &Vec<&Value>) -> Result { args }; - adjusted_args.into_iter().fold(Ok(()), |had_error, arg| { - had_error?; + adjusted_args.iter().try_fold((), |_, arg| { let key: KeyType = (*arg).try_into()?; match key { KeyType::Null => Ok(()), @@ -148,14 +147,14 @@ pub fn missing(data: &Value, args: &Vec<&Value>) -> Result { /// to or more than the threshold value _present_ in the data, an empty /// array is returned. Otherwise, an array containing all missing keys /// is returned. -pub fn missing_some(data: &Value, args: &Vec<&Value>) -> Result { +pub fn missing_some(data: &Value, args: &[&Value]) -> Result { let (threshold_arg, keys_arg) = (args[0], args[1]); let threshold = match threshold_arg { Value::Number(n) => n.as_u64(), _ => None, } - .ok_or(Error::InvalidArgument { + .ok_or_else(|| Error::InvalidArgument { value: threshold_arg.clone(), operation: "missing_some".into(), reason: "missing_some threshold must be a valid, positive integer".into(), @@ -171,9 +170,9 @@ pub fn missing_some(data: &Value, args: &Vec<&Value>) -> Result { }?; let mut missing_keys: Vec = Vec::new(); - let present_count = keys.into_iter().fold(Ok(0 as u64), |last, key| { + let present_count = keys.iter().try_fold(0_u64, |last, key| { // Don't bother evaluating once we've met the threshold. - let prev_present_count = last?; + let prev_present_count = last; if prev_present_count >= threshold { return Ok(prev_present_count); }; @@ -213,11 +212,11 @@ fn get_key(data: &Value, key: KeyType) -> Option { match key { // If the key is null, we return the data, always, even if there // is a default parameter. - KeyType::Null => return Some(data.clone()), + KeyType::Null => Some(data.clone()), KeyType::String(k) => get_str_key(data, k), KeyType::Number(i) => match data { Value::Object(_) => get_str_key(data, i.to_string()), - Value::Array(arr) => get(arr, i).map(Value::clone), + Value::Array(arr) => get(arr, i).cloned(), Value::String(s) => { let s_vec: Vec = s.chars().collect(); get(&s_vec, i).map(|c| c.to_string()).map(Value::String) @@ -229,22 +228,20 @@ fn get_key(data: &Value, key: KeyType) -> Option { fn get_str_key>(data: &Value, key: K) -> Option { let k = key.as_ref(); - if k == "" { + if k.is_empty() { return Some(data.clone()); }; match data { Value::Object(_) | Value::Array(_) | Value::String(_) => { // Exterior ref in case we need to make a new value in the match. - k.split(".").fold(Some(data.clone()), |acc, i| match acc? { + k.split('.').try_fold(data.clone(), |acc, i| match acc { // If the current value is an object, try to get the value - Value::Object(map) => map.get(i).map(Value::clone), + Value::Object(map) => map.get(i).cloned(), // If the current value is an array, we need an integer // index. If integer conversion fails, return None. - Value::Array(arr) => i - .parse::() - .ok() - .and_then(|i| get(&arr, i)) - .map(Value::clone), + Value::Array(arr) => { + i.parse::().ok().and_then(|i| get(&arr, i)).cloned() + } // Same deal if it's a string. Value::String(s) => { let s_chars: Vec = s.chars().collect(); diff --git a/src/op/impure.rs b/src/op/impure.rs index a7456fa..06f4943 100644 --- a/src/op/impure.rs +++ b/src/op/impure.rs @@ -9,7 +9,7 @@ use crate::error::Error; /// The reference implementation ignores any arguments beyond the first, /// and the specification seems to indicate that the first argument is /// the only one considered, so we're doing the same. -pub fn log(items: &Vec<&Value>) -> Result { +pub fn log(items: &[&Value]) -> Result { println!("{}", items[0]); Ok(items[0].clone()) } diff --git a/src/op/logic.rs b/src/op/logic.rs index 68a2fc8..d681fc3 100644 --- a/src/op/logic.rs +++ b/src/op/logic.rs @@ -12,7 +12,7 @@ use crate::NULL; /// However, it can lso work like: /// [condition, true, condition2, true2, false2] /// for an if/elseif/else type of operation -pub fn if_(data: &Value, args: &Vec<&Value>) -> Result { +pub fn if_(data: &Value, args: &[&Value]) -> Result { // Special case incorrect arguments. These are not defined in the // specification, but they are defined in the test cases. match args.len() { @@ -25,26 +25,25 @@ pub fn if_(data: &Value, args: &Vec<&Value>) -> Result { // from the tests. 1 => { let parsed = Parsed::from_value(args[0])?; - let evaluated = parsed.evaluate(&data)?; + let evaluated = parsed.evaluate(data)?; return Ok(evaluated.into()); } _ => {} } - args.into_iter() + args.iter() .enumerate() // Our accumulator is: // - last conditional evaluation value, // - whether that evaluation is truthy, // - whether we know we should return without further evaluation - .fold(Ok((NULL, false, false)), |last_res, (i, val)| { - let (last_eval, was_truthy, should_return) = last_res?; + .try_fold((NULL, false, false), |last_res, (i, val)| { + let (last_eval, was_truthy, should_return) = last_res; // We hit a final value already if should_return { Ok((last_eval, was_truthy, should_return)) - } - // Potential false-value, initial evaluation, or else-if clause - else if i % 2 == 0 { + } else if i % 2 == 0 { + // Potential false-value, initial evaluation, or else-if clause let parsed = Parsed::from_value(val)?; let eval = parsed.evaluate(data)?; let is_truthy = match eval { @@ -74,32 +73,32 @@ pub fn if_(data: &Value, args: &Vec<&Value>) -> Result { } /// Perform short-circuiting or evaluation -pub fn or(data: &Value, args: &Vec<&Value>) -> Result { +pub fn or(data: &Value, args: &[&Value]) -> Result { enum OrResult { Uninitialized, Truthy(Value), Current(Value), } - let eval = - args.into_iter() - .fold(Ok(OrResult::Uninitialized), |last_res, current| { - let last_eval = last_res?; + let eval = args + .iter() + .try_fold(OrResult::Uninitialized, |last_res, current| { + let last_eval = last_res; - // if we've found a truthy value, don't evaluate anything else - if let OrResult::Truthy(_) = last_eval { - return Ok(last_eval); - } + // if we've found a truthy value, don't evaluate anything else + if let OrResult::Truthy(_) = last_eval { + return Ok(last_eval); + } - let parsed = Parsed::from_value(current)?; - let evaluated = parsed.evaluate(data)?; + let parsed = Parsed::from_value(current)?; + let evaluated = parsed.evaluate(data)?; - if truthy_from_evaluated(&evaluated) { - return Ok(OrResult::Truthy(evaluated.into())); - } + if truthy_from_evaluated(&evaluated) { + return Ok(OrResult::Truthy(evaluated.into())); + } - Ok(OrResult::Current(evaluated.into())) - })?; + Ok(OrResult::Current(evaluated.into())) + })?; match eval { OrResult::Truthy(v) => Ok(v), @@ -111,7 +110,7 @@ pub fn or(data: &Value, args: &Vec<&Value>) -> Result { } /// Perform short-circuiting and evaluation -pub fn and(data: &Value, args: &Vec<&Value>) -> Result { +pub fn and(data: &Value, args: &[&Value]) -> Result { enum AndResult { Uninitialized, Falsey(Value), @@ -119,9 +118,9 @@ pub fn and(data: &Value, args: &Vec<&Value>) -> Result { } let eval = - args.into_iter() - .fold(Ok(AndResult::Uninitialized), |last_res, current| { - let last_eval = last_res?; + args.iter() + .try_fold(AndResult::Uninitialized, |last_res, current| { + let last_eval = last_res; if let AndResult::Falsey(_) = last_eval { return Ok(last_eval); @@ -167,24 +166,9 @@ pub fn truthy(val: &Value) -> bool { match val { Value::Null => false, Value::Bool(v) => *v, - Value::Number(v) => v - .as_f64() - .map(|v_num| if v_num == 0.0 { false } else { true }) - .unwrap_or(false), - Value::String(v) => { - if v == "" { - false - } else { - true - } - } - Value::Array(v) => { - if v.len() == 0 { - false - } else { - true - } - } + Value::Number(v) => v.as_f64().map(|v_num| v_num != 0.0).unwrap_or(false), + Value::String(v) => !v.is_empty(), + Value::Array(v) => !v.is_empty(), Value::Object(_) => true, } } @@ -209,7 +193,7 @@ mod test_truthy { let falses = [json!(false), json!([]), json!(""), json!(0), json!(null)]; - trues.iter().for_each(|v| assert!(truthy(&v))); - falses.iter().for_each(|v| assert!(!truthy(&v))); + trues.iter().for_each(|v| assert!(truthy(v))); + falses.iter().for_each(|v| assert!(!truthy(v))); } } diff --git a/src/op/mod.rs b/src/op/mod.rs index efb074c..cade308 100644 --- a/src/op/mod.rs +++ b/src/op/mod.rs @@ -254,7 +254,7 @@ impl NumParams { true => Ok(len), false => Err(Error::WrongArgumentCount { expected: self.clone(), - actual: len.clone(), + actual: *len, }), } } @@ -280,7 +280,7 @@ pub struct Operator { num_params: NumParams, } impl Operator { - pub fn execute(&self, items: &Vec<&Value>) -> Result { + pub fn execute(&self, items: &[&Value]) -> Result { (self.operator)(items) } } @@ -304,7 +304,7 @@ pub struct LazyOperator { num_params: NumParams, } impl LazyOperator { - pub fn execute(&self, data: &Value, items: &Vec<&Value>) -> Result { + pub fn execute(&self, data: &Value, items: &[&Value]) -> Result { (self.operator)(data, items) } } @@ -333,7 +333,7 @@ pub struct DataOperator { num_params: NumParams, } impl DataOperator { - pub fn execute(&self, data: &Value, items: &Vec<&Value>) -> Result { + pub fn execute(&self, data: &Value, items: &[&Value]) -> Result { (self.operator)(data, items) } } @@ -351,9 +351,9 @@ impl fmt::Debug for DataOperator { } } -type OperatorFn = fn(&Vec<&Value>) -> Result; -type LazyOperatorFn = fn(&Value, &Vec<&Value>) -> Result; -type DataOperatorFn = fn(&Value, &Vec<&Value>) -> Result; +type OperatorFn = fn(&[&Value]) -> Result; +type LazyOperatorFn = fn(&Value, &[&Value]) -> Result; +type DataOperatorFn = fn(&Value, &[&Value]) -> Result; /// An operation that doesn't do any recursive parsing or evaluation. /// @@ -369,7 +369,7 @@ impl<'a> Parser<'a> for LazyOperation<'a> { opt.map(|op| { Ok(LazyOperation { operator: op.op, - arguments: op.args.into_iter().map(|v| v.clone()).collect(), + arguments: op.args.into_iter().cloned().collect(), }) }) .transpose() @@ -378,7 +378,7 @@ impl<'a> Parser<'a> for LazyOperation<'a> { fn evaluate(&self, data: &'a Value) -> Result { self.operator - .execute(data, &self.arguments.iter().collect()) + .execute(data, &self.arguments.iter().collect::>()) .map(Evaluated::New) } } @@ -420,7 +420,7 @@ impl<'a> Parser<'a> for Operation<'a> { .map(|value| value.evaluate(data).map(Value::from)) .collect::, Error>>()?; self.operator - .execute(&arguments.iter().collect()) + .execute(&arguments.iter().collect::>()) .map(Evaluated::New) } } @@ -464,7 +464,7 @@ impl<'a> Parser<'a> for DataOperation<'a> { .map(|value| value.evaluate(data).map(Value::from)) .collect::, Error>>()?; self.operator - .execute(data, &arguments.iter().collect()) + .execute(data, &arguments.iter().collect::>()) .map(Evaluated::New) } } @@ -501,14 +501,18 @@ fn op_from_map<'a, 'b, T: CommonOperator>( // We've already validated the length to be one, so any error // here is super unexpected. - let key = obj.keys().next().ok_or(Error::UnexpectedError(format!( - "could not get first key from len(1) object: {:?}", - obj - )))?; - let val = obj.get(key).ok_or(Error::UnexpectedError(format!( - "could not get value for key '{}' from len(1) object: {:?}", - key, obj - )))?; + let key = obj.keys().next().ok_or_else(|| { + Error::UnexpectedError(format!( + "could not get first key from len(1) object: {:?}", + obj + )) + })?; + let val = obj.get(key).ok_or_else(|| { + Error::UnexpectedError(format!( + "could not get value for key '{}' from len(1) object: {:?}", + key, obj + )) + })?; // See if the key is an operator. If it's not, return None. let op = match map.get(key.as_str()) { diff --git a/src/op/numeric.rs b/src/op/numeric.rs index 01a174f..7c03132 100644 --- a/src/op/numeric.rs +++ b/src/op/numeric.rs @@ -6,7 +6,7 @@ use crate::error::Error; use crate::js_op; use crate::value::to_number_value; -fn compare(func: F, items: &Vec<&Value>) -> Result +fn compare(func: F, items: &[&Value]) -> Result where F: Fn(&Value, &Value) -> bool, { @@ -20,27 +20,27 @@ where } /// Do < for either 2 or 3 values -pub fn lt(items: &Vec<&Value>) -> Result { +pub fn lt(items: &[&Value]) -> Result { compare(js_op::abstract_lt, items) } /// Do <= for either 2 or 3 values -pub fn lte(items: &Vec<&Value>) -> Result { +pub fn lte(items: &[&Value]) -> Result { compare(js_op::abstract_lte, items) } /// Do > for either 2 or 3 values -pub fn gt(items: &Vec<&Value>) -> Result { +pub fn gt(items: &[&Value]) -> Result { compare(js_op::abstract_gt, items) } /// Do >= for either 2 or 3 values -pub fn gte(items: &Vec<&Value>) -> Result { +pub fn gte(items: &[&Value]) -> Result { compare(js_op::abstract_gte, items) } /// Perform subtraction or convert a number to a negative -pub fn minus(items: &Vec<&Value>) -> Result { +pub fn minus(items: &[&Value]) -> Result { let value = if items.len() == 1 { js_op::to_negative(items[0])? } else { diff --git a/src/op/string.rs b/src/op/string.rs index 6e3b3e7..8825d91 100644 --- a/src/op/string.rs +++ b/src/op/string.rs @@ -15,16 +15,16 @@ use crate::NULL; /// evaluates to `"foo[object Object]". Here we explicitly require all /// arguments to be strings, because the specification explicitly defines /// `cat` as a string operation. -pub fn cat(items: &Vec<&Value>) -> Result { +pub fn cat(items: &[&Value]) -> Result { let mut rv = String::from(""); items - .into_iter() + .iter() .map(|i| match i { Value::String(i_string) => Ok(i_string.clone()), _ => Ok(js_op::to_string(i)), }) - .fold(Ok(&mut rv), |acc: Result<&mut String, Error>, i| { - let rv = acc?; + .try_fold(&mut rv, |acc: &mut String, i| { + let rv = acc; rv.push_str(&i?); Ok(rv) })?; @@ -36,15 +36,14 @@ pub fn cat(items: &Vec<&Value>) -> Result { /// Note: the reference implementation casts the first argument to a string, /// but since the specification explicitly defines this as a string operation, /// the argument types are enforced here to avoid unpredictable behavior. -pub fn substr(items: &Vec<&Value>) -> Result { +pub fn substr(items: &[&Value]) -> Result { // We can only have 2 or 3 arguments. Number of arguments is validated elsewhere. let (string_arg, idx_arg) = (items[0], items[1]); - let limit_opt: Option<&Value>; - if items.len() > 2 { - limit_opt = Some(items[2]); + let limit_opt = if items.len() > 2 { + Some(items[2]) } else { - limit_opt = None; - } + None + }; let string = match string_arg { Value::String(s) => s, @@ -85,7 +84,8 @@ pub fn substr(items: &Vec<&Value>) -> Result { Err(Error::InvalidArgument { value: limit_arg.clone(), operation: "substr".into(), - reason: "Optional third argument to substr must be an integer".into(), + reason: "Optional third argument to substr must be an integer" + .into(), }) } } @@ -111,7 +111,7 @@ pub fn substr(items: &Vec<&Value>) -> Result { // If the index is negative it means "number of characters prior to the // end of the string from which to start", and corresponds to the string // length minus the index. - idx if idx < 0 => string_len.checked_sub(idx_abs).unwrap_or(0), + idx if idx < 0 => string_len.saturating_sub(idx_abs), // A positive index is simply the starting point. Max starting point // is the length, which will yield an empty string. _ => cmp::min(string_len, idx_abs), @@ -120,19 +120,20 @@ pub fn substr(items: &Vec<&Value>) -> Result { let end_idx = match limit { None => string_len, Some(l) => { - let limit_abs: usize = l.abs().try_into().map_err(|e| Error::InvalidArgument { - value: limit_opt.or(Some(&NULL)).map(|v| v.clone()).unwrap(), - operation: "substr".into(), - reason: format!( - "The number {} is too large to index strings on this system", - e - ), - })?; + let limit_abs: usize = + l.abs().try_into().map_err(|e| Error::InvalidArgument { + value: limit_opt.or(Some(&NULL)).cloned().unwrap(), + operation: "substr".into(), + reason: format!( + "The number {} is too large to index strings on this system", + e + ), + })?; match l { // If the limit is negative, it means "characters before the end // at which to stop", corresponding to an index of either 0 or // the length of the string minus the limit. - l if l < 0 => string_len.checked_sub(limit_abs).unwrap_or(0), + l if l < 0 => string_len.saturating_sub(limit_abs), // A positive limit indicates the number of characters to take, // so it corresponds to an index of the start index plus the // limit (with a maximum value of the string length). @@ -144,7 +145,7 @@ pub fn substr(items: &Vec<&Value>) -> Result { } }; - let count_in_substr = end_idx.checked_sub(start_idx).unwrap_or(0); + let count_in_substr = end_idx.saturating_sub(start_idx); // Iter over our expected count rather than indexing directly to avoid // potential panics if any of our math is wrong. diff --git a/src/value.rs b/src/value.rs index 0669220..25b8e48 100644 --- a/src/value.rs +++ b/src/value.rs @@ -26,10 +26,9 @@ impl<'a> Parsed<'a> { .or(LazyOperation::from_value(value)?.map(Self::LazyOperation)) .or(DataOperation::from_value(value)?.map(Self::DataOperation)) .or(Raw::from_value(value)?.map(Self::Raw)) - .ok_or(Error::UnexpectedError(format!( - "Failed to parse Value {:?}", - value - ))) + .ok_or_else(|| { + Error::UnexpectedError(format!("Failed to parse Value {:?}", value)) + }) } pub fn from_values(values: Vec<&'a Value>) -> Result, Error> { @@ -106,10 +105,12 @@ pub fn to_number_value(number: f64) -> Result { Ok(Value::Number(Number::from(number as i64))) } else { Number::from_f64(number) - .ok_or(Error::UnexpectedError(format!( - "Could not make JSON number from result {:?}", - number - ))) + .ok_or_else(|| { + Error::UnexpectedError(format!( + "Could not make JSON number from result {:?}", + number + )) + }) .map(Value::Number) } } diff --git a/tests/test_lib.rs b/tests/test_lib.rs index 6b32377..9fc8509 100644 --- a/tests/test_lib.rs +++ b/tests/test_lib.rs @@ -4,11 +4,11 @@ use std::fs::File; use std::io::prelude::*; use std::path::Path; -use reqwest; -use serde_json; + + use serde_json::Value; -use jsonlogic_rs; + struct TestCase { logic: Value, @@ -16,7 +16,7 @@ struct TestCase { result: Value, } -const TEST_URL: &'static str = "http://jsonlogic.com/tests.json"; +const TEST_URL: &str = "http://jsonlogic.com/tests.json"; fn load_file_json() -> Value { let mut file = File::open(Path::join( @@ -57,7 +57,7 @@ fn check_test_file() { Ok(r) => r, Err(e) => { println!("Failed to get new version of test JSON: {:?}", e); - return (); + return ; } }; let http_json: Value = serde_json::from_str(&resp).unwrap();