From 75fa1efda155ff78e163d542a865991d1cf0dfad Mon Sep 17 00:00:00 2001 From: Trevor Hilton Date: Sun, 4 Aug 2024 09:20:57 -0700 Subject: [PATCH] refactor: use an unchecked initializer for integers Integer initialization is done for small safe values, e.g., 0, 1, -1, so this uses a panicing version of the from_i64_opt function to remove the need for unwrap everywhere. --- serde_json_path/src/parser/primitive/int.rs | 17 ++------- serde_json_path/src/parser/segment.rs | 2 +- serde_json_path/src/parser/selector/mod.rs | 7 +--- serde_json_path_core/src/spec/integer.rs | 15 ++++++-- .../src/spec/selector/slice.rs | 38 +++++++++---------- 5 files changed, 38 insertions(+), 41 deletions(-) diff --git a/serde_json_path/src/parser/primitive/int.rs b/serde_json_path/src/parser/primitive/int.rs index cd2bbd5..8338cbb 100644 --- a/serde_json_path/src/parser/primitive/int.rs +++ b/serde_json_path/src/parser/primitive/int.rs @@ -50,18 +50,9 @@ mod tests { #[test] fn parse_integers() { - assert_eq!(parse_int("0"), Ok(("", Integer::from_i64_opt(0).unwrap()))); - assert_eq!( - parse_int("10"), - Ok(("", Integer::from_i64_opt(10).unwrap())) - ); - assert_eq!( - parse_int("-10"), - Ok(("", Integer::from_i64_opt(-10).unwrap())) - ); - assert_eq!( - parse_int("010"), - Ok(("10", Integer::from_i64_opt(0).unwrap())) - ); + assert_eq!(parse_int("0"), Ok(("", Integer::from_i64_unchecked(0)))); + assert_eq!(parse_int("10"), Ok(("", Integer::from_i64_unchecked(10)))); + assert_eq!(parse_int("-10"), Ok(("", Integer::from_i64_unchecked(-10)))); + assert_eq!(parse_int("010"), Ok(("10", Integer::from_i64_unchecked(0)))); } } diff --git a/serde_json_path/src/parser/segment.rs b/serde_json_path/src/parser/segment.rs index a6365cb..e48b595 100644 --- a/serde_json_path/src/parser/segment.rs +++ b/serde_json_path/src/parser/segment.rs @@ -199,7 +199,7 @@ mod tests { assert_eq!(s[0], Selector::Name(Name::from("name"))); assert_eq!( s[1], - Selector::Index(Index(Integer::from_i64_opt(10).unwrap())) + Selector::Index(Index(Integer::from_i64_unchecked(10))) ); assert_eq!( s[2], diff --git a/serde_json_path/src/parser/selector/mod.rs b/serde_json_path/src/parser/selector/mod.rs index e66bf58..54ba2f5 100644 --- a/serde_json_path/src/parser/selector/mod.rs +++ b/serde_json_path/src/parser/selector/mod.rs @@ -87,14 +87,11 @@ mod tests { fn all_selectors() { { let (_, s) = parse_selector("0").unwrap(); - assert_eq!(s, Selector::Index(Index(Integer::from_i64_opt(0).unwrap()))); + assert_eq!(s, Selector::Index(Index(Integer::from_i64_unchecked(0)))); } { let (_, s) = parse_selector("10").unwrap(); - assert_eq!( - s, - Selector::Index(Index(Integer::from_i64_opt(10).unwrap())) - ); + assert_eq!(s, Selector::Index(Index(Integer::from_i64_unchecked(10)))); } { let (_, s) = parse_selector("'name'").unwrap(); diff --git a/serde_json_path_core/src/spec/integer.rs b/serde_json_path_core/src/spec/integer.rs index 6c86863..2dd4535 100644 --- a/serde_json_path_core/src/spec/integer.rs +++ b/serde_json_path_core/src/spec/integer.rs @@ -35,12 +35,21 @@ impl Integer { (MIN..=MAX).contains(&self.0) } + /// Produce an [`Integer`] with the value 0 + pub fn zero() -> Self { + Self(0) + } + /// Get an [`Integer`] from an `i64` /// - /// This will produce `None` if the inputted value is out of the valid range + /// This is intended for initializing an integer with small, non-zero numbers. + /// + /// # Panics + /// + /// This will panic if the inputted value is out of the valid range /// [-(253)+1, (253)-1]). - pub fn from_i64_opt(value: i64) -> Option { - Self::try_new(value).ok() + pub fn from_i64_unchecked(value: i64) -> Self { + Self::try_new(value).expect("value is out of the valid range") } /// Take the absolute value, producing `None` if the resulting value is outside diff --git a/serde_json_path_core/src/spec/selector/slice.rs b/serde_json_path_core/src/spec/selector/slice.rs index a829abe..3eb7750 100644 --- a/serde_json_path_core/src/spec/selector/slice.rs +++ b/serde_json_path_core/src/spec/selector/slice.rs @@ -55,7 +55,7 @@ impl Slice { /// /// This will panic if the provided value is outside the range [-(253) + 1, (253) - 1]. pub fn with_start(mut self, start: i64) -> Self { - self.start = Some(Integer::from_i64_opt(start).expect("valid start")); + self.start = Some(Integer::from_i64_unchecked(start)); self } @@ -65,7 +65,7 @@ impl Slice { /// /// This will panic if the provided value is outside the range [-(253) + 1, (253) - 1]. pub fn with_end(mut self, end: i64) -> Self { - self.end = Some(Integer::from_i64_opt(end).expect("valid end")); + self.end = Some(Integer::from_i64_unchecked(end)); self } @@ -75,20 +75,20 @@ impl Slice { /// /// This will panic if the provided value is outside the range [-(253) + 1, (253) - 1]. pub fn with_step(mut self, step: i64) -> Self { - self.step = Some(Integer::from_i64_opt(step).expect("valid step")); + self.step = Some(Integer::from_i64_unchecked(step)); self } #[inline] fn bounds_on_forward_slice(&self, len: Integer) -> (Integer, Integer) { - let start_default = self.start.unwrap_or_default(); + let start_default = self.start.unwrap_or(Integer::zero()); let end_default = self.end.unwrap_or(len); let start = normalize_slice_index(start_default, len) - .unwrap_or_default() - .max(Integer::default()); + .unwrap_or(Integer::zero()) + .max(Integer::zero()); let end = normalize_slice_index(end_default, len) - .unwrap_or_default() - .max(Integer::default()); + .unwrap_or(Integer::zero()) + .max(Integer::zero()); let lower = start.min(len); let upper = end.min(len); (lower, upper) @@ -98,23 +98,23 @@ impl Slice { fn bounds_on_reverse_slice(&self, len: Integer) -> Option<(Integer, Integer)> { let start_default = self .start - .or_else(|| Integer::from_i64_opt(1).and_then(|i| len.checked_sub(i)))?; + .or_else(|| len.checked_sub(Integer::from_i64_unchecked(1)))?; let end_default = self.end.or_else(|| { - let l = len.checked_mul(Integer::from_i64_opt(-1).unwrap())?; - l.checked_sub(Integer::from_i64_opt(1).unwrap()) + let l = len.checked_mul(Integer::from_i64_unchecked(-1))?; + l.checked_sub(Integer::from_i64_unchecked(1)) })?; let start = normalize_slice_index(start_default, len) - .unwrap_or_default() - .max(Integer::from_i64_opt(-1).unwrap()); + .unwrap_or(Integer::zero()) + .max(Integer::from_i64_unchecked(-1)); let end = normalize_slice_index(end_default, len) - .unwrap_or_default() - .max(Integer::from_i64_opt(-1).unwrap()); + .unwrap_or(Integer::zero()) + .max(Integer::from_i64_unchecked(-1)); let lower = end.min( - len.checked_sub(Integer::from_i64_opt(1).unwrap()) + len.checked_sub(Integer::from_i64_unchecked(1)) .unwrap_or(len), ); let upper = start.min( - len.checked_sub(Integer::from_i64_opt(1).unwrap()) + len.checked_sub(Integer::from_i64_unchecked(1)) .unwrap_or(len), ); Some((lower, upper)) @@ -126,7 +126,7 @@ impl Queryable for Slice { fn query<'b>(&self, current: &'b Value, _root: &'b Value) -> Vec<&'b Value> { if let Some(list) = current.as_array() { let mut query = Vec::new(); - let step = self.step.unwrap_or(Integer::from_i64_opt(1).unwrap()); + let step = self.step.unwrap_or(Integer::from_i64_unchecked(1)); if step == 0 { return vec![]; } @@ -176,7 +176,7 @@ impl Queryable for Slice { ) -> Vec> { if let Some(list) = current.as_array() { let mut result = Vec::new(); - let step = self.step.unwrap_or(Integer::from_i64_opt(1).unwrap()); + let step = self.step.unwrap_or(Integer::from_i64_unchecked(1)); if step == 0 { return vec![]; }