From d2ee02748aaeaf349dedffa97e643a6faae42741 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Mon, 21 Oct 2024 19:09:19 +0100 Subject: [PATCH 1/5] simplify error variant --- src/assign.rs | 2 +- src/index.rs | 28 +++++++++++----------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/assign.rs b/src/assign.rs index 3381b27..aab49be 100644 --- a/src/assign.rs +++ b/src/assign.rs @@ -773,7 +773,7 @@ mod tests { assign: json!("foo"), expected: Err(AssignError::FailedToParseIndex { offset: 0, - source: ParseIndexError::InvalidCharacters("+".into()), + source: ParseIndexError::InvalidCharacter, }), expected_data: json!([]), }, diff --git a/src/index.rs b/src/index.rs index 6687448..5a67d88 100644 --- a/src/index.rs +++ b/src/index.rs @@ -36,7 +36,7 @@ //! ``` use crate::Token; -use alloc::{string::String, vec::Vec}; +use alloc::string::String; use core::{fmt, num::ParseIntError, str::FromStr}; /// Represents an abstract index into an array. @@ -173,13 +173,7 @@ impl FromStr for Index { // this comes up with the `+` sign which is valid for // representing a `usize` but not allowed in RFC 6901 array // indices - let mut invalid: Vec<_> = s.chars().filter(|c| !c.is_ascii_digit()).collect(); - // remove duplicate characters - invalid.sort_unstable(); - invalid.dedup(); - Err(ParseIndexError::InvalidCharacters( - invalid.into_iter().collect(), - )) + Err(ParseIndexError::InvalidCharacter) } } } @@ -281,8 +275,8 @@ pub enum ParseIndexError { InvalidInteger(ParseIntError), /// The Token contains leading zeros. LeadingZeros, - /// The Token contains non-digit characters. - InvalidCharacters(String), + /// The Token contains a non-digit character. + InvalidCharacter, } impl From for ParseIndexError { @@ -301,10 +295,10 @@ impl fmt::Display for ParseIndexError { f, "token contained leading zeros, which are disallowed by RFC 6901" ), - ParseIndexError::InvalidCharacters(chars) => write!( + ParseIndexError::InvalidCharacter => write!( f, - "token contains non-digit character(s) '{chars}', \ - which are disallowed by RFC 6901", + "token contains the non-digit character '+', \ + which is disallowed by RFC 6901", ), } } @@ -315,7 +309,7 @@ impl std::error::Error for ParseIndexError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { ParseIndexError::InvalidInteger(source) => Some(source), - ParseIndexError::LeadingZeros | ParseIndexError::InvalidCharacters(_) => None, + ParseIndexError::LeadingZeros | ParseIndexError::InvalidCharacter => None, } } } @@ -435,9 +429,9 @@ mod tests { "token contained leading zeros, which are disallowed by RFC 6901" ); assert_eq!( - ParseIndexError::InvalidCharacters("+@".into()).to_string(), - "token contains non-digit character(s) '+@', \ - which are disallowed by RFC 6901" + ParseIndexError::InvalidCharacter.to_string(), + "token contains the non-digit character '+', \ + which is disallowed by RFC 6901" ); } From 98431e7f8564554b60fc60cbf0e4c07770d77f32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Mon, 21 Oct 2024 22:06:15 +0100 Subject: [PATCH 2/5] rework error type --- src/assign.rs | 22 ++++++++++--- src/index.rs | 88 ++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 87 insertions(+), 23 deletions(-) diff --git a/src/assign.rs b/src/assign.rs index aab49be..b99d0a1 100644 --- a/src/assign.rs +++ b/src/assign.rs @@ -555,7 +555,6 @@ mod tests { index::{OutOfBoundsError, ParseIndexError}, Pointer, }; - use alloc::str::FromStr; use core::fmt::{Debug, Display}; #[derive(Debug)] @@ -607,6 +606,8 @@ mod tests { fn assign_json() { use alloc::vec; use serde_json::json; + + use crate::index::InvalidCharacterError; Test::all([ Test { ptr: "/foo", @@ -748,12 +749,15 @@ mod tests { expected_data: json!(["bar"]), }, Test { - ptr: "/a", + ptr: "/12a", data: json!([]), assign: json!("foo"), expected: Err(AssignError::FailedToParseIndex { offset: 0, - source: ParseIndexError::InvalidInteger(usize::from_str("foo").unwrap_err()), + source: ParseIndexError::InvalidCharacter(InvalidCharacterError { + source: "12a".into(), + offset: 2, + }), }), expected_data: json!([]), }, @@ -773,7 +777,10 @@ mod tests { assign: json!("foo"), expected: Err(AssignError::FailedToParseIndex { offset: 0, - source: ParseIndexError::InvalidCharacter, + source: ParseIndexError::InvalidCharacter(InvalidCharacterError { + source: "+23".into(), + offset: 0, + }), }), expected_data: json!([]), }, @@ -791,6 +798,8 @@ mod tests { fn assign_toml() { use alloc::vec; use toml::{toml, Table, Value}; + + use crate::index::InvalidCharacterError; Test::all([ Test { data: Value::Table(toml::Table::new()), @@ -925,7 +934,10 @@ mod tests { assign: "foo".into(), expected: Err(AssignError::FailedToParseIndex { offset: 0, - source: ParseIndexError::InvalidInteger(usize::from_str("foo").unwrap_err()), + source: ParseIndexError::InvalidCharacter(InvalidCharacterError { + source: "a".into(), + offset: 0, + }), }), expected_data: Value::Array(vec![]), }, diff --git a/src/index.rs b/src/index.rs index 5a67d88..c25d43c 100644 --- a/src/index.rs +++ b/src/index.rs @@ -166,15 +166,22 @@ impl FromStr for Index { } else if s.starts_with('0') && s != "0" { Err(ParseIndexError::LeadingZeros) } else { - let idx = s.parse::().map(Index::Num)?; - if s.chars().all(|c| c.is_ascii_digit()) { - Ok(idx) - } else { - // this comes up with the `+` sign which is valid for - // representing a `usize` but not allowed in RFC 6901 array - // indices - Err(ParseIndexError::InvalidCharacter) - } + s.chars().position(|c| !c.is_ascii_digit()).map_or_else( + || { + s.parse::() + .map(Index::Num) + .map_err(ParseIndexError::from) + }, + |offset| { + // this comes up with the `+` sign which is valid for + // representing a `usize` but not allowed in RFC 6901 array + // indices + Err(ParseIndexError::InvalidCharacter(InvalidCharacterError { + source: s.to_owned(), + offset, + })) + }, + ) } } } @@ -268,7 +275,7 @@ impl std::error::Error for OutOfBoundsError {} ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ */ -/// Indicates that the `Token` could not be parsed as valid RFC 6901 index. +/// Indicates that the `Token` could not be parsed as valid RFC 6901 array index. #[derive(Debug, PartialEq, Eq)] pub enum ParseIndexError { /// The Token does not represent a valid integer. @@ -276,7 +283,7 @@ pub enum ParseIndexError { /// The Token contains leading zeros. LeadingZeros, /// The Token contains a non-digit character. - InvalidCharacter, + InvalidCharacter(InvalidCharacterError), } impl From for ParseIndexError { @@ -295,11 +302,7 @@ impl fmt::Display for ParseIndexError { f, "token contained leading zeros, which are disallowed by RFC 6901" ), - ParseIndexError::InvalidCharacter => write!( - f, - "token contains the non-digit character '+', \ - which is disallowed by RFC 6901", - ), + ParseIndexError::InvalidCharacter(err) => err.fmt(f), } } } @@ -309,11 +312,56 @@ impl std::error::Error for ParseIndexError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { ParseIndexError::InvalidInteger(source) => Some(source), - ParseIndexError::LeadingZeros | ParseIndexError::InvalidCharacter => None, + ParseIndexError::InvalidCharacter(source) => Some(source), + ParseIndexError::LeadingZeros => None, } } } +/// Indicates that a non-digit character was found when parsing the RFC 6901 array index. +#[derive(Debug, PartialEq, Eq)] +pub struct InvalidCharacterError { + pub(crate) source: String, + pub(crate) offset: usize, +} + +impl InvalidCharacterError { + /// Returns the offset of the character in the string. + /// + /// This offset is given in characters, not in bytes. + pub fn offset(&self) -> usize { + self.offset + } + + /// Returns the source string. + pub fn source(&self) -> &str { + &self.source + } + + /// Returns the offending character. + #[allow(clippy::missing_panics_doc)] + pub fn char(&self) -> char { + self.source + .chars() + .nth(self.offset) + .expect("char was found at offset") + } +} + +impl fmt::Display for InvalidCharacterError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "token contains the non-digit character '{}', \ + which is disallowed by RFC 6901", + self.char() + ) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for InvalidCharacterError {} + /* ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ╔══════════════════════════════════════════════════════════════════════════════╗ @@ -429,7 +477,11 @@ mod tests { "token contained leading zeros, which are disallowed by RFC 6901" ); assert_eq!( - ParseIndexError::InvalidCharacter.to_string(), + ParseIndexError::InvalidCharacter(InvalidCharacterError { + source: "+10".into(), + offset: 0 + }) + .to_string(), "token contains the non-digit character '+', \ which is disallowed by RFC 6901" ); From bf9e72dcbb99e31cfc41dc96a0f8b7b261df8b97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Mon, 21 Oct 2024 22:07:42 +0100 Subject: [PATCH 3/5] tweak imports --- src/assign.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/assign.rs b/src/assign.rs index b99d0a1..c5b3f8a 100644 --- a/src/assign.rs +++ b/src/assign.rs @@ -552,10 +552,12 @@ mod toml { mod tests { use super::{Assign, AssignError}; use crate::{ - index::{OutOfBoundsError, ParseIndexError}, + index::{InvalidCharacterError, OutOfBoundsError, ParseIndexError}, Pointer, }; + use alloc::vec; use core::fmt::{Debug, Display}; + use serde_json::json; #[derive(Debug)] struct Test { @@ -604,10 +606,6 @@ mod tests { #[test] #[cfg(feature = "json")] fn assign_json() { - use alloc::vec; - use serde_json::json; - - use crate::index::InvalidCharacterError; Test::all([ Test { ptr: "/foo", From d7857c244991e34a1f2bfb74bac9418b1dfa4c68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Mon, 21 Oct 2024 22:15:04 +0100 Subject: [PATCH 4/5] tweak imports 2 --- src/assign.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/assign.rs b/src/assign.rs index c5b3f8a..a48bbb2 100644 --- a/src/assign.rs +++ b/src/assign.rs @@ -557,7 +557,6 @@ mod tests { }; use alloc::vec; use core::fmt::{Debug, Display}; - use serde_json::json; #[derive(Debug)] struct Test { @@ -606,6 +605,8 @@ mod tests { #[test] #[cfg(feature = "json")] fn assign_json() { + use serde_json::json; + Test::all([ Test { ptr: "/foo", @@ -794,10 +795,8 @@ mod tests { #[test] #[cfg(feature = "toml")] fn assign_toml() { - use alloc::vec; use toml::{toml, Table, Value}; - use crate::index::InvalidCharacterError; Test::all([ Test { data: Value::Table(toml::Table::new()), From 52f435af01a91dd5793701113ad3a642fc14edc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Mon, 21 Oct 2024 22:17:31 +0100 Subject: [PATCH 5/5] fix std --- src/index.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.rs b/src/index.rs index c25d43c..0acbb4b 100644 --- a/src/index.rs +++ b/src/index.rs @@ -177,7 +177,7 @@ impl FromStr for Index { // representing a `usize` but not allowed in RFC 6901 array // indices Err(ParseIndexError::InvalidCharacter(InvalidCharacterError { - source: s.to_owned(), + source: String::from(s), offset, })) },