Merge pull request #72 from hiltontj/69-cleanup-todos-nice

[#69] Remove `TODO`s from code
hiltontj · Jan 6, 2024 · ead2ed2 · ead2ed2
2 parents 5dd145d + 1292b47
commit ead2ed2
Show file tree

Hide file tree

Showing 8 changed files with 21 additions and 12 deletions.
diff --git 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/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
@@ -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
@@ -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/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_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/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)
 

diff --git 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<TokenStream> {
-    // 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()