From d40db78cae9cc315da78a38d6de4e52ca0b44fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Tue, 18 Feb 2025 20:42:53 +0000 Subject: [PATCH 1/7] migrate remaining errors to new model --- src/assign.rs | 15 ++------ src/index.rs | 97 ++++++++++++++++++++++++++++++++++++++++----------- src/token.rs | 72 +++++++++++++++++++++++++++++++------- 3 files changed, 139 insertions(+), 45 deletions(-) diff --git a/src/assign.rs b/src/assign.rs index 8b4e3bf..175f522 100644 --- a/src/assign.rs +++ b/src/assign.rs @@ -798,10 +798,7 @@ mod tests { expected: Err(Error::FailedToParseIndex { position: 0, offset: 0, - source: ParseIndexError::InvalidCharacter(InvalidCharacterError { - source: "12a".into(), - offset: 2, - }), + source: ParseIndexError::InvalidCharacter(InvalidCharacterError { offset: 2 }), }), expected_data: json!([]), }, @@ -823,10 +820,7 @@ mod tests { expected: Err(Error::FailedToParseIndex { position: 0, offset: 0, - source: ParseIndexError::InvalidCharacter(InvalidCharacterError { - source: "+23".into(), - offset: 0, - }), + source: ParseIndexError::InvalidCharacter(InvalidCharacterError { offset: 0 }), }), expected_data: json!([]), }, @@ -976,10 +970,7 @@ mod tests { expected: Err(Error::FailedToParseIndex { position: 0, offset: 0, - source: ParseIndexError::InvalidCharacter(InvalidCharacterError { - source: "a".into(), - offset: 0, - }), + source: ParseIndexError::InvalidCharacter(InvalidCharacterError { offset: 0 }), }), expected_data: Value::Array(vec![]), }, diff --git a/src/index.rs b/src/index.rs index bad40bc..b61226b 100644 --- a/src/index.rs +++ b/src/index.rs @@ -35,9 +35,12 @@ //! assert_eq!(Index::Next.for_len_unchecked(30), 30); //! ``` -use crate::Token; +use crate::{ + diagnostic::{diagnostic_url, IntoReport, Label}, + Token, +}; use alloc::string::String; -use core::{fmt, num::ParseIntError, str::FromStr}; +use core::{fmt, iter::once, num::ParseIntError, str::FromStr}; /// Represents an abstract index into an array. /// @@ -177,7 +180,6 @@ impl FromStr for Index { // representing a `usize` but not allowed in RFC 6901 array // indices Err(ParseIndexError::InvalidCharacter(InvalidCharacterError { - source: String::from(s), offset, })) }, @@ -309,6 +311,75 @@ impl fmt::Display for ParseIndexError { } } +// shouldn't be used directly, but is part of a public interface +#[doc(hidden)] +#[derive(Debug)] +pub enum StringOrToken { + String(String), + Token(Token<'static>), +} + +impl From for StringOrToken { + fn from(value: String) -> Self { + Self::String(value) + } +} + +impl From> for StringOrToken { + fn from(value: Token<'static>) -> Self { + Self::Token(value) + } +} + +impl core::ops::Deref for StringOrToken { + type Target = str; + + fn deref(&self) -> &Self::Target { + match self { + StringOrToken::String(s) => s.as_str(), + StringOrToken::Token(t) => t.encoded(), + } + } +} + +impl IntoReport for ParseIndexError { + type Subject = StringOrToken; + + fn url() -> &'static str { + diagnostic_url!(enum ParseIndexError) + } + + fn labels( + &self, + subject: &Self::Subject, + ) -> Option>> { + let subject = &**subject; + match self { + ParseIndexError::InvalidInteger(_) => None, + ParseIndexError::LeadingZeros => { + let len = subject + .chars() + .position(|c| c != '0') + .expect("starts with zeros"); + let text = String::from("leading zeros"); + Some(Box::new(once(Label::new(text, 0, len)))) + } + ParseIndexError::InvalidCharacter(err) => { + let len = subject + .chars() + .skip(err.offset) + .position(|c| !c.is_ascii_digit()) + .expect("at least one non-digit char"); + let text = String::from("invalid character(s)"); + Some(Box::new(once(Label::new(text, err.offset, len)))) + } + } + } +} + +#[cfg(feature = "miette")] +impl miette::Diagnostic for ParseIndexError {} + #[cfg(feature = "std")] impl std::error::Error for ParseIndexError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { @@ -323,7 +394,6 @@ impl std::error::Error for ParseIndexError { /// Indicates that a non-digit character was found when parsing the RFC 6901 array index. #[derive(Debug, Clone, PartialEq, Eq)] pub struct InvalidCharacterError { - pub(crate) source: String, pub(crate) offset: usize, } @@ -334,29 +404,14 @@ impl InvalidCharacterError { 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() + "token contains a non-digit character, \ + which is disallowed by RFC 6901", ) } } diff --git a/src/token.rs b/src/token.rs index fe574a2..17c9415 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1,6 +1,9 @@ -use core::str::Split; +use core::{iter::once, str::Split}; -use crate::index::{Index, ParseIndexError}; +use crate::{ + diagnostic::{diagnostic_url, IntoReport, Label}, + index::{Index, ParseIndexError}, +}; use alloc::{ borrow::Cow, fmt, @@ -99,7 +102,7 @@ impl<'a> Token<'a> { if escaped { return Err(EncodingError { offset: s.len(), - source: InvalidEncoding::Slash, + source: InvalidEncoding::Tilde, }); } Ok(Self { inner: s.into() }) @@ -381,13 +384,6 @@ pub struct EncodingError { pub source: InvalidEncoding, } -#[cfg(feature = "std")] -impl std::error::Error for EncodingError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - Some(&self.source) - } -} - impl fmt::Display for EncodingError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( @@ -398,6 +394,38 @@ impl fmt::Display for EncodingError { } } +impl IntoReport for EncodingError { + type Subject = String; + + fn url() -> &'static str { + diagnostic_url!(struct EncodingError) + } + + fn labels(&self, subject: &Self::Subject) -> Option>> { + let (text, offset) = match self.source { + InvalidEncoding::Tilde => { + if self.offset == subject.len() { + ("incomplete escape sequence", self.offset - 1) + } else { + ("must be 0 or 1", self.offset) + } + } + InvalidEncoding::Slash => ("invalid character", self.offset), + }; + Some(Box::new(once(Label::new(text.to_string(), offset, 1)))) + } +} + +#[cfg(feature = "miette")] +impl miette::Diagnostic for EncodingError {} + +#[cfg(feature = "std")] +impl std::error::Error for EncodingError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.source) + } +} + /* ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ ╔══════════════════════════════════════════════════════════════════════════════╗ @@ -425,6 +453,7 @@ impl fmt::Display for InvalidEncoding { } } } + #[cfg(feature = "std")] impl std::error::Error for InvalidEncoding {} @@ -476,8 +505,27 @@ mod tests { assert_eq!(Token::from_encoded("~0~1").unwrap().encoded(), "~0~1"); let t = Token::from_encoded("a~1b").unwrap(); assert_eq!(t.decoded(), "a/b"); - assert!(Token::from_encoded("a/b").is_err()); - assert!(Token::from_encoded("a~a").is_err()); + assert_eq!( + Token::from_encoded("a/b"), + Err(EncodingError { + offset: 1, + source: InvalidEncoding::Slash + }) + ); + assert_eq!( + Token::from_encoded("a~a"), + Err(EncodingError { + offset: 2, + source: InvalidEncoding::Tilde + }) + ); + assert_eq!( + Token::from_encoded("a~"), + Err(EncodingError { + offset: 2, + source: InvalidEncoding::Tilde + }) + ); } #[test] From 7c9e8ed6b4030937626929de1f6126161d927bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Sun, 23 Feb 2025 20:50:32 +0000 Subject: [PATCH 2/7] fix incomplete rebase --- src/index.rs | 4 ++-- src/token.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/index.rs b/src/index.rs index b61226b..f92718b 100644 --- a/src/index.rs +++ b/src/index.rs @@ -36,7 +36,7 @@ //! ``` use crate::{ - diagnostic::{diagnostic_url, IntoReport, Label}, + diagnostic::{diagnostic_url, Diagnostic, Label}, Token, }; use alloc::string::String; @@ -342,7 +342,7 @@ impl core::ops::Deref for StringOrToken { } } -impl IntoReport for ParseIndexError { +impl Diagnostic for ParseIndexError { type Subject = StringOrToken; fn url() -> &'static str { diff --git a/src/token.rs b/src/token.rs index 17c9415..64d5469 100644 --- a/src/token.rs +++ b/src/token.rs @@ -1,7 +1,7 @@ use core::{iter::once, str::Split}; use crate::{ - diagnostic::{diagnostic_url, IntoReport, Label}, + diagnostic::{diagnostic_url, Diagnostic, Label}, index::{Index, ParseIndexError}, }; use alloc::{ @@ -394,7 +394,7 @@ impl fmt::Display for EncodingError { } } -impl IntoReport for EncodingError { +impl Diagnostic for EncodingError { type Subject = String; fn url() -> &'static str { From 3a74fc038c947d23758723f01f65e5f9535feabe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Sun, 23 Feb 2025 20:56:10 +0000 Subject: [PATCH 3/7] update changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15dd2c6..7931ca5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,23 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- `EncodingError` and `ParseIndexError` now implement `Diagnostic`, which + unifies the API for errors originating from parse-like operations. +- Fixes returning an incorrect error type when parsing a `Token` that + terminates with `~`. This now correctly classifies the error as a `Tilde` + error. + +### Removed + +- Some methods were removed from `InvalidCharacterError`, as that type no + longer holds a copy of the input string internally. This is a breaking + change. To access equivalent functionality, use the `Diagnostic` API + integration. + ## [0.7.1] 2025-02-16 ### Changed From 1f1d744b8478682872bc3b5a53a10ba7238f7a35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Sun, 23 Feb 2025 21:00:12 +0000 Subject: [PATCH 4/7] box imports --- src/index.rs | 2 +- src/token.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/index.rs b/src/index.rs index f92718b..ff4680a 100644 --- a/src/index.rs +++ b/src/index.rs @@ -39,7 +39,7 @@ use crate::{ diagnostic::{diagnostic_url, Diagnostic, Label}, Token, }; -use alloc::string::String; +use alloc::{boxed::Box, string::String}; use core::{fmt, iter::once, num::ParseIntError, str::FromStr}; /// Represents an abstract index into an array. diff --git a/src/token.rs b/src/token.rs index 64d5469..1bb0806 100644 --- a/src/token.rs +++ b/src/token.rs @@ -6,6 +6,7 @@ use crate::{ }; use alloc::{ borrow::Cow, + boxed::Box, fmt, string::{String, ToString}, vec::Vec, From 61582835037e9eb68211e32be887d7fc16b63257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Sun, 23 Feb 2025 21:25:12 +0000 Subject: [PATCH 5/7] tests for coverage, fix, and clippy on default features --- src/assign.rs | 2 +- src/diagnostic.rs | 6 ++-- src/index.rs | 81 +++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/src/assign.rs b/src/assign.rs index 175f522..870e4b2 100644 --- a/src/assign.rs +++ b/src/assign.rs @@ -608,7 +608,6 @@ mod tests { index::{InvalidCharacterError, OutOfBoundsError, ParseIndexError}, Pointer, }; - use alloc::vec; use core::fmt::{Debug, Display}; #[derive(Debug)] @@ -833,6 +832,7 @@ mod tests { #[test] #[cfg(feature = "toml")] fn assign_toml() { + use alloc::vec; use toml::{toml, Table, Value}; [ Test { diff --git a/src/diagnostic.rs b/src/diagnostic.rs index 71d4643..b87fb84 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -226,8 +226,6 @@ where #[cfg(test)] mod tests { - use super::*; - use crate::{Pointer, PointerBuf}; #[test] #[cfg(all( feature = "assign", @@ -236,6 +234,8 @@ mod tests { feature = "json" ))] fn assign_error() { + use crate::{diagnostic::Diagnose, PointerBuf}; + let mut v = serde_json::json!({"foo": {"bar": ["0"]}}); let ptr = PointerBuf::parse("/foo/bar/invalid/cannot/reach").unwrap(); let report = ptr.assign(&mut v, "qux").diagnose(ptr).unwrap_err(); @@ -254,6 +254,7 @@ mod tests { feature = "json" ))] fn resolve_error() { + use crate::{diagnostic::Diagnose, PointerBuf}; let v = serde_json::json!({"foo": {"bar": ["0"]}}); let ptr = PointerBuf::parse("/foo/bar/invalid/cannot/reach").unwrap(); let report = ptr.resolve(&v).diagnose(ptr).unwrap_err(); @@ -267,6 +268,7 @@ mod tests { #[test] #[cfg(feature = "miette")] fn parse_error() { + use crate::{diagnostic::Diagnose, Pointer, PointerBuf}; let invalid = "/foo/bar/invalid~3~encoding/cannot/reach"; let report = Pointer::parse(invalid).diagnose(invalid).unwrap_err(); diff --git a/src/index.rs b/src/index.rs index ff4680a..f17094b 100644 --- a/src/index.rs +++ b/src/index.rs @@ -342,6 +342,19 @@ impl core::ops::Deref for StringOrToken { } } +#[cfg(feature = "miette")] +impl miette::SourceCode for StringOrToken { + fn read_span<'a>( + &'a self, + span: &miette::SourceSpan, + context_lines_before: usize, + context_lines_after: usize, + ) -> Result + 'a>, miette::MietteError> { + let s: &str = &**self; + s.read_span(span, context_lines_before, context_lines_after) + } +} + impl Diagnostic for ParseIndexError { type Subject = StringOrToken; @@ -368,8 +381,8 @@ impl Diagnostic for ParseIndexError { let len = subject .chars() .skip(err.offset) - .position(|c| !c.is_ascii_digit()) - .expect("at least one non-digit char"); + .position(|c| c.is_ascii_digit()) + .unwrap_or(subject.len()); let text = String::from("invalid character(s)"); Some(Box::new(once(Label::new(text, err.offset, len)))) } @@ -432,7 +445,7 @@ impl std::error::Error for InvalidCharacterError {} #[cfg(test)] mod tests { use super::*; - use crate::Token; + use crate::{Diagnose, Token}; #[test] fn index_from_usize() { @@ -522,4 +535,66 @@ mod tests { let index = Index::try_from(&token).unwrap(); assert_eq!(index, Index::Next); } + + #[test] + fn diagnose_works_with_token_or_string() { + let token = Token::new("foo"); + // despite the clone, this is cheap because `token` is borrowed + Index::try_from(token.clone()).diagnose(token).unwrap_err(); + let s = String::from("bar"); + Index::try_from(&s).diagnose(s).unwrap_err(); + } + + #[test] + fn error_from_invalid_chars() { + let s = String::from("bar"); + let err = Index::try_from(&s).diagnose(s).unwrap_err(); + + #[cfg(feature = "miette")] + { + let labels: Vec<_> = miette::Diagnostic::labels(&err) + .unwrap() + .into_iter() + .collect(); + assert_eq!( + labels, + vec![miette::LabeledSpan::new( + Some("invalid character(s)".into()), + 0, + 3 + )] + ); + } + + let (src, sub) = err.decompose(); + let labels: Vec<_> = src.labels(&sub).unwrap().into_iter().collect(); + + assert_eq!( + labels, + vec![Label::new("invalid character(s)".into(), 0, 3)] + ); + } + + #[test] + fn error_from_leading_zeros() { + let s = String::from("000001"); + let err = Index::try_from(&s).diagnose(s).unwrap_err(); + + #[cfg(feature = "miette")] + { + let labels: Vec<_> = miette::Diagnostic::labels(&err) + .unwrap() + .into_iter() + .collect(); + assert_eq!( + labels, + vec![miette::LabeledSpan::new(Some("leading zeros".into()), 0, 5)] + ); + } + + let (src, sub) = err.decompose(); + let labels: Vec<_> = src.labels(&sub).unwrap().into_iter().collect(); + + assert_eq!(labels, vec![Label::new("leading zeros".into(), 0, 5)]); + } } From 9202b37925e04b4e8fff8fae064f549e1c3c1967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Sun, 23 Feb 2025 21:35:09 +0000 Subject: [PATCH 6/7] more coverage --- src/index.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/index.rs b/src/index.rs index f17094b..c1ebf4c 100644 --- a/src/index.rs +++ b/src/index.rs @@ -597,4 +597,18 @@ mod tests { assert_eq!(labels, vec![Label::new("leading zeros".into(), 0, 5)]); } + + #[test] + fn error_from_empty_string() { + let s = String::from(""); + let err = Index::try_from(&s).diagnose(s).unwrap_err(); + + #[cfg(feature = "miette")] + { + assert!(miette::Diagnostic::labels(&err).is_none()); + } + + let (src, sub) = err.decompose(); + assert!(src.labels(&sub).is_none()); + } } From df39beb6b9d322fd70beb3935773862f40ac2a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20S=C3=A1=20de=20Mello?= Date: Sun, 23 Feb 2025 21:50:23 +0000 Subject: [PATCH 7/7] more coverage --- src/token.rs | 86 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 77 insertions(+), 9 deletions(-) diff --git a/src/token.rs b/src/token.rs index 1bb0806..25119c1 100644 --- a/src/token.rs +++ b/src/token.rs @@ -506,26 +506,94 @@ mod tests { assert_eq!(Token::from_encoded("~0~1").unwrap().encoded(), "~0~1"); let t = Token::from_encoded("a~1b").unwrap(); assert_eq!(t.decoded(), "a/b"); + + let sub = String::from("a/b"); + let err = Token::from_encoded(&sub).unwrap_err(); + let labels: Vec<_> = err.labels(&sub).unwrap().into_iter().collect(); + assert_eq!(labels, vec![Label::new("invalid character".into(), 1, 1)]); + let err = err.into_report(sub); + #[cfg(feature = "miette")] + { + let labels: Vec<_> = miette::Diagnostic::labels(&err) + .unwrap() + .into_iter() + .collect(); + assert_eq!( + labels, + vec![miette::LabeledSpan::new( + Some("invalid character".into()), + 1, + 1 + )] + ); + } + let (err, _) = err.decompose(); assert_eq!( - Token::from_encoded("a/b"), - Err(EncodingError { + err, + EncodingError { offset: 1, source: InvalidEncoding::Slash - }) + } ); + + let sub = String::from("a~a"); + let err = Token::from_encoded(&sub).unwrap_err(); + let labels: Vec<_> = err.labels(&sub).unwrap().into_iter().collect(); + assert_eq!(labels, vec![Label::new("must be 0 or 1".into(), 2, 1)]); + let err = err.into_report(sub); + #[cfg(feature = "miette")] + { + let labels: Vec<_> = miette::Diagnostic::labels(&err) + .unwrap() + .into_iter() + .collect(); + assert_eq!( + labels, + vec![miette::LabeledSpan::new( + Some("must be 0 or 1".into()), + 2, + 1 + )] + ); + } + let (err, _) = err.decompose(); assert_eq!( - Token::from_encoded("a~a"), - Err(EncodingError { + err, + EncodingError { offset: 2, source: InvalidEncoding::Tilde - }) + } ); + let sub = String::from("a~"); + let err = Token::from_encoded(&sub).unwrap_err(); + let labels: Vec<_> = err.labels(&sub).unwrap().into_iter().collect(); assert_eq!( - Token::from_encoded("a~"), - Err(EncodingError { + labels, + vec![Label::new("incomplete escape sequence".into(), 1, 1)] + ); + let err = err.into_report(sub); + #[cfg(feature = "miette")] + { + let labels: Vec<_> = miette::Diagnostic::labels(&err) + .unwrap() + .into_iter() + .collect(); + assert_eq!( + labels, + vec![miette::LabeledSpan::new( + Some("incomplete escape sequence".into()), + 1, + 1 + )] + ); + } + let (err, _) = err.decompose(); + assert_eq!( + err, + EncodingError { offset: 2, source: InvalidEncoding::Tilde - }) + } ); }