From 1378f7e5f23a9b0085987fc5c8706c517d3a364c Mon Sep 17 00:00:00 2001 From: Trevor Hilton Date: Mon, 4 Dec 2023 18:48:38 -0500 Subject: [PATCH 1/2] remove TODOs from code, use some let else statements --- serde_json_path/src/parser/primitive/int.rs | 3 --- serde_json_path/src/parser/segment.rs | 6 +++++- serde_json_path/src/parser/selector/filter.rs | 1 - serde_json_path_core/src/spec/selector/slice.rs | 15 ++++++++++----- .../src/internal/common/extract.rs | 2 -- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/serde_json_path/src/parser/primitive/int.rs b/serde_json_path/src/parser/primitive/int.rs index f2a3dd7..bc2a267 100644 --- a/serde_json_path/src/parser/primitive/int.rs +++ b/serde_json_path/src/parser/primitive/int.rs @@ -50,9 +50,6 @@ mod tests { assert_eq!(parse_int("0"), Ok(("", 0))); assert_eq!(parse_int("10"), Ok(("", 10))); assert_eq!(parse_int("-10"), Ok(("", -10))); - // TODO - I don't know if this following test demonstrates the actual behaviour we want - // i.e., do we want this to fail instead? or do we rely on higher level sequenced parsers - // to fail, e.g., if a delimiter was expected after. assert_eq!(parse_int("010"), Ok(("10", 0))); } } diff --git a/serde_json_path/src/parser/segment.rs b/serde_json_path/src/parser/segment.rs index 0cd5cc3..0e8b75c 100644 --- a/serde_json_path/src/parser/segment.rs +++ b/serde_json_path/src/parser/segment.rs @@ -17,7 +17,11 @@ use super::selector::{parse_selector, parse_wildcard_selector}; use super::utils::cut_with; use super::PResult; -// TODO - I have no idea if this is correct, supposed to be %x80-10FFFF +// The specification requires that a non-ASCII character is in the range +// %x80-10FFFF. In Rust, the `char` type can not hold characters higher +// than %x10FFFF, so we only need to check the lower bound in this function. +// +// See: https://doc.rust-lang.org/std/primitive.char.html#validity fn is_non_ascii_unicode(chr: char) -> bool { chr >= '\u{0080}' } diff --git a/serde_json_path/src/parser/selector/filter.rs b/serde_json_path/src/parser/selector/filter.rs index bfc53b5..048629e 100644 --- a/serde_json_path/src/parser/selector/filter.rs +++ b/serde_json_path/src/parser/selector/filter.rs @@ -236,7 +236,6 @@ mod tests { #[test] fn comp_expr() { - // TODO - test more let (_, cxp) = parse_comp_expr("true != false").unwrap(); assert!(matches!(cxp.left, Comparable::Literal(Literal::Bool(true)))); assert!(matches!(cxp.op, ComparisonOperator::NotEqualTo)); diff --git a/serde_json_path_core/src/spec/selector/slice.rs b/serde_json_path_core/src/spec/selector/slice.rs index ed16fa8..ffe1140 100644 --- a/serde_json_path_core/src/spec/selector/slice.rs +++ b/serde_json_path_core/src/spec/selector/slice.rs @@ -70,9 +70,7 @@ impl Queryable for Slice { if step == 0 { return vec![]; } - let len = if let Ok(l) = isize::try_from(list.len()) { - l - } else { + let Ok(len) = isize::try_from(list.len()) else { return vec![]; }; if step > 0 { @@ -92,8 +90,15 @@ impl Queryable for Slice { i += step; } } else { - let start_default = self.start.unwrap_or(len - 1); // TODO - not checked sub - let end_default = self.end.unwrap_or(-len - 1); // TODO - not checked sub + let Some(start_default) = self.start.or_else(|| len.checked_sub(1)) else { + return vec![]; + }; + let Some(end_default) = self + .end + .or_else(|| len.checked_mul(-1).and_then(|l| l.checked_sub(1))) + else { + return vec![]; + }; let start = normalize_slice_index(start_default, len) .unwrap_or(0) .max(-1); diff --git a/serde_json_path_macros/src/internal/common/extract.rs b/serde_json_path_macros/src/internal/common/extract.rs index f37add5..1e6344f 100644 --- a/serde_json_path_macros/src/internal/common/extract.rs +++ b/serde_json_path_macros/src/internal/common/extract.rs @@ -43,8 +43,6 @@ fn extract_type_path(ty: &Type) -> Option<&Path> { } fn extract_json_path_type(p: &Path) -> Result { - // TODO - support full type path to ensure that correct type is being used? - // - i.e., instead of just looking at last path segment let p_seg = p .segments .last() From 1292b47f7f3e81c5724f5ca957c4e162b4ca8890 Mon Sep 17 00:00:00 2001 From: Trevor Hilton Date: Mon, 4 Dec 2023 18:57:58 -0500 Subject: [PATCH 2/2] update changelogs --- serde_json_path/CHANGELOG.md | 2 ++ serde_json_path_core/CHANGELOG.md | 2 ++ serde_json_path_macros/CHANGELOG.md | 2 ++ 3 files changed, 6 insertions(+) diff --git a/serde_json_path/CHANGELOG.md b/serde_json_path/CHANGELOG.md index 2b793c9..de56ed1 100644 --- a/serde_json_path/CHANGELOG.md +++ b/serde_json_path/CHANGELOG.md @@ -8,8 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 # Unreleased - **internal**: address new clippy lints in Rust 1.74 ([#70]) +- **internal**: code clean-up ([#72]) [#70]: https://github.com/hiltontj/serde_json_path/pull/70 +[#72]: https://github.com/hiltontj/serde_json_path/pull/72 # 0.6.4 (9 November 2023) diff --git a/serde_json_path_core/CHANGELOG.md b/serde_json_path_core/CHANGELOG.md index 0d9cfdf..bac6e99 100644 --- a/serde_json_path_core/CHANGELOG.md +++ b/serde_json_path_core/CHANGELOG.md @@ -8,8 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 # Unreleased - **internal**: address new clippy lints in Rust 1.74 and update some tracing instrumentation ([#70]) +- **internal**: code clean-up ([#72]) [#70]: https://github.com/hiltontj/serde_json_path/pull/70 +[#72]: https://github.com/hiltontj/serde_json_path/pull/72 # 0.1.3 (9 November 2023) diff --git a/serde_json_path_macros/CHANGELOG.md b/serde_json_path_macros/CHANGELOG.md index 72c1579..70748e8 100644 --- a/serde_json_path_macros/CHANGELOG.md +++ b/serde_json_path_macros/CHANGELOG.md @@ -8,8 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 # Unreleased - **internal**: address new clippy lints in Rust 1.74 ([#70]) +- **internal**: code clean-up ([#72]) [#70]: https://github.com/hiltontj/serde_json_path/pull/70 +[#72]: https://github.com/hiltontj/serde_json_path/pull/72 # 0.1.1 (9 November 2023)