From 2e5de522b9250a9d2cb0f0c9dd3963fb4846eb36 Mon Sep 17 00:00:00 2001 From: Matilde Morrone Date: Mon, 3 Nov 2025 14:18:32 +0100 Subject: [PATCH] Refactor error handling --- src/lib.rs | 178 ++++++++++++++++++++--------------------------------- 1 file changed, 66 insertions(+), 112 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1d3e424..348cb71 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -113,68 +113,42 @@ pub enum Argument<'a> { Stdio, } -impl<'a> Argument<'a> { - /// Converts this argument into a [`ParsingError::UnexpectedArg`] error. +impl Argument<'_> { + /// Converts this argument into a [`ParsingError::Unexpected`] error. /// /// This is a convenience method for creating contextual error messages when an argument /// is encountered but not expected by the application. The resulting error message /// includes appropriate formatting based on the argument type. /// - /// # Parameters - /// - /// * `value` - Optional value associated with the argument (primarily used with options) - /// /// # Examples /// /// ```rust /// use sap::Argument; /// - /// // Long option with value + /// // Long option /// let arg = Argument::Long("unknown"); - /// let error = arg.into_error(Some("value")); - /// assert_eq!(error.to_string(), "unexpected argument: --unknown=value"); + /// let error = arg.unexpected(); + /// assert_eq!(error.to_string(), "unexpected argument: --unknown"); /// - /// // Short option without value + /// // Short option /// let arg = Argument::Short('x'); - /// let error = arg.into_error(None::<&str>); + /// let error = arg.unexpected(); /// assert_eq!(error.to_string(), "unexpected argument: -x"); /// + /// // Positional value + /// let arg = Argument::Value("file".into()); + /// let error = arg.unexpected(); + /// assert_eq!(error.to_string(), "unexpected argument: file"); + /// /// // Stdio argument /// let arg = Argument::Stdio; - /// let error = arg.into_error(None::<&str>); + /// let error = arg.unexpected(); /// assert_eq!(error.to_string(), "unexpected argument: -"); /// ``` - pub fn into_error(self, value: A) -> ParsingError - where - A: Into>, - { - use Argument::{Long, Short, Stdio, Value}; - - match self { - Long(arg) => ParsingError::UnexpectedArg { - offender: arg.to_string(), - value: value.into().map(String::from), - format: "=", - prefix: "--", - }, - Short(arg) => ParsingError::UnexpectedArg { - offender: arg.to_string(), - value: value.into().map(String::from), - format: " ", - prefix: "-", - }, - Value(arg) => ParsingError::UnexpectedArg { - offender: arg.to_string(), - value: None, - format: "", - prefix: "", - }, - Stdio => ParsingError::UnexpectedArg { - offender: "-".to_string(), - value: None, - format: "", - prefix: "", - }, + #[must_use] + pub fn unexpected(&self) -> ParsingError { + ParsingError::Unexpected { + argument: self.to_string(), } } } @@ -380,9 +354,8 @@ where if char == '=' { self.state = State::Poisoned; - return Err(ParsingError::InvalidOption { + return Err(ParsingError::InvalidSyntax { reason: "Short options do not support values", - offender: None, }); } @@ -623,8 +596,14 @@ where /// /// All parsing operations return a `Result` with this error type. Each variant /// provides specific context about what went wrong during parsing. -#[derive(Debug)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum ParsingError { + /// The argument iterator was empty (contained no program name). + /// + /// This should not occur during normal program execution but may happen + /// when creating parsers from empty custom iterators. + Empty, + /// Invalid option syntax or format was encountered. /// /// This typically occurs when: @@ -634,11 +613,7 @@ pub enum ParsingError { /// # Fields /// /// * `reason` - Human-readable description of what was invalid - /// * `offender` - The specific argument that caused the error (if available) - InvalidOption { - reason: &'static str, - offender: Option, - }, + InvalidSyntax { reason: &'static str }, /// An option value was not consumed after being parsed. /// @@ -651,63 +626,33 @@ pub enum ParsingError { /// * `value` - The unconsumed value that was attached to the option UnconsumedValue { value: String }, - /// The argument iterator was empty (contained no program name). - /// - /// This should not occur during normal program execution but may happen - /// when creating parsers from empty custom iterators. - Empty, - /// An unexpected or unrecognized argument was encountered. /// - /// This error is typically created by calling [`Argument::into_error`] when + /// This error is typically created by calling [`Argument::unexpected`] when /// the application encounters an argument it doesn't know how to handle. /// /// # Fields /// - /// * `offender` - The argument name that was unexpected - /// * `value` - Associated value (if any) - /// * `format` - How the value is formatted in error messages (e.g., "=" or " ") - /// * `prefix` - The argument prefix (e.g., "--" for long options, "-" for short) - UnexpectedArg { - offender: String, - value: Option, - format: &'static str, - prefix: &'static str, - }, + /// * `argument` - The formatted argument string (e.g., "--unknown", "-x value", "file.txt") + /// + /// # Examples + /// + /// ```rust + /// use sap::{Argument, ParsingError}; + /// + /// let error = Argument::Long("unknown").unexpected(); + /// assert_eq!(error.to_string(), "unexpected argument: --unknown"); + /// ``` + Unexpected { argument: String }, } impl Display for ParsingError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { - Self::InvalidOption { reason, offender } => { - write!(f, "reason: {reason}")?; - if let Some(sentence) = offender { - write!(f, " at: {sentence}")?; - } - - Ok(()) - } - - Self::UnconsumedValue { value } => { - write!(f, "leftover value: {value}",) - } - - Self::UnexpectedArg { - offender, - value, - format, - prefix, - } => match value { - Some(val) => { - write!(f, "unexpected argument: {prefix}{offender}{format}{val}") - } - - None => { - write!(f, "unexpected argument: {prefix}{offender}") - } - }, - - Self::Empty => write!(f, "env variables were empty"), + Self::Empty => write!(f, "argument list is empty"), + Self::InvalidSyntax { reason } => write!(f, "invalid syntax: {reason}"), + Self::UnconsumedValue { value } => write!(f, "unconsumed value: {value}"), + Self::Unexpected { argument } => write!(f, "unexpected argument: {argument}"), } } } @@ -950,30 +895,39 @@ mod tests { } #[test] - fn argument_into_error() { - assert_eq!( - Long("test").into_error("value").to_string(), - "unexpected argument: --test=value" - ); + fn argument_unexpected() { + use crate::ParsingError; + + // Test unexpected() without values assert_eq!( - Long("test").into_error(None::<&str>).to_string(), - "unexpected argument: --test" + Long("test").unexpected(), + ParsingError::Unexpected { + argument: "--test".to_string() + } ); assert_eq!( - Short('x').into_error("val").to_string(), - "unexpected argument: -x val" + Short('x').unexpected(), + ParsingError::Unexpected { + argument: "-x".to_string() + } ); assert_eq!( - Short('x').into_error(None::<&str>).to_string(), - "unexpected argument: -x" + Value("file".into()).unexpected(), + ParsingError::Unexpected { + argument: "file".to_string() + } ); assert_eq!( - Value("file".into()).into_error(None::<&str>).to_string(), - "unexpected argument: file" + Stdio.unexpected(), + ParsingError::Unexpected { + argument: "-".to_string() + } ); + + // Verify Display implementation still works correctly assert_eq!( - Stdio.into_error(None::<&str>).to_string(), - "unexpected argument: -" + Long("test").unexpected().to_string(), + "unexpected argument: --test" ); }