Skip to content

Commit

Permalink
refactor: use an unchecked initializer for integers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hiltontj committed Aug 4, 2024
1 parent c851bc1 commit 75fa1ef
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 41 deletions.
17 changes: 4 additions & 13 deletions serde_json_path/src/parser/primitive/int.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))));
}
}
2 changes: 1 addition & 1 deletion serde_json_path/src/parser/segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
7 changes: 2 additions & 5 deletions serde_json_path/src/parser/selector/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
15 changes: 12 additions & 3 deletions serde_json_path_core/src/spec/integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// [-(2<sup>53</sup>)+1, (2<sup>53</sup>)-1]).
pub fn from_i64_opt(value: i64) -> Option<Self> {
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
Expand Down
38 changes: 19 additions & 19 deletions serde_json_path_core/src/spec/selector/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl Slice {
///
/// This will panic if the provided value is outside the range [-(2<sup>53</sup>) + 1, (2<sup>53</sup>) - 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
}

Expand All @@ -65,7 +65,7 @@ impl Slice {
///
/// This will panic if the provided value is outside the range [-(2<sup>53</sup>) + 1, (2<sup>53</sup>) - 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
}

Expand All @@ -75,20 +75,20 @@ impl Slice {
///
/// This will panic if the provided value is outside the range [-(2<sup>53</sup>) + 1, (2<sup>53</sup>) - 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)
Expand All @@ -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))
Expand All @@ -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![];
}
Expand Down Expand Up @@ -176,7 +176,7 @@ impl Queryable for Slice {
) -> Vec<LocatedNode<'b>> {
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![];
}
Expand Down

0 comments on commit 75fa1ef

Please sign in to comment.