diff --git a/oof/src/error.rs b/oof/src/error.rs index 306f17629..3da041534 100644 --- a/oof/src/error.rs +++ b/oof/src/error.rs @@ -3,7 +3,7 @@ use std::{error, ffi::OsString, fmt}; use crate::Flag; #[derive(Debug)] -pub enum OofError<'t> { +pub enum OofError { FlagValueConflict { flag: Flag, previous_value: OsString, @@ -15,17 +15,17 @@ pub enum OofError<'t> { UnknownShortFlag(char), UnknownLongFlag(String), MisplacedShortArgFlagError(char), - MissingValueToFlag(&'t Flag), - DuplicatedFlag(&'t Flag), + MissingValueToFlag(Flag), + DuplicatedFlag(Flag), } -impl<'t> error::Error for OofError<'t> { +impl error::Error for OofError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { None } } -impl<'t> fmt::Display for OofError<'t> { +impl fmt::Display for OofError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // TODO: implement proper debug messages match self { diff --git a/oof/src/lib.rs b/oof/src/lib.rs index 662186791..78632f950 100644 --- a/oof/src/lib.rs +++ b/oof/src/lib.rs @@ -131,7 +131,7 @@ pub fn filter_flags( let is_last_letter = i == letters.len() - 1; let flag_info = - short_flags_info.get(&letter).ok_or(OofError::UnknownShortFlag(letter))?; + *short_flags_info.get(&letter).ok_or(OofError::UnknownShortFlag(letter))?; if !is_last_letter && flag_info.takes_value { return Err(OofError::MisplacedShortArgFlagError(letter)); @@ -145,19 +145,20 @@ pub fn filter_flags( if flag_info.takes_value { // If it was already inserted if result_flags.argument_flags.contains_key(flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } // pop the next one - let flag_argument = - iter.next().ok_or(OofError::MissingValueToFlag(flag_info))?; + let flag_argument = iter + .next() + .ok_or_else(|| OofError::MissingValueToFlag(flag_info.clone()))?; // Otherwise, insert it. result_flags.argument_flags.insert(flag_name, flag_argument); } else { // If it was already inserted if result_flags.boolean_flags.contains(flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } // Otherwise, insert it result_flags.boolean_flags.insert(flag_name); @@ -168,7 +169,7 @@ pub fn filter_flags( if let FlagType::Long = flag_type { let flag = trim_double_hyphen(flag); - let flag_info = long_flags_info + let flag_info = *long_flags_info .get(flag) .ok_or_else(|| OofError::UnknownLongFlag(String::from(flag)))?; @@ -177,15 +178,16 @@ pub fn filter_flags( if flag_info.takes_value { // If it was already inserted if result_flags.argument_flags.contains_key(&flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } - let flag_argument = iter.next().ok_or(OofError::MissingValueToFlag(flag_info))?; + let flag_argument = + iter.next().ok_or_else(|| OofError::MissingValueToFlag(flag_info.clone()))?; result_flags.argument_flags.insert(flag_name, flag_argument); } else { // If it was already inserted if result_flags.boolean_flags.contains(&flag_name) { - return Err(OofError::DuplicatedFlag(flag_info)); + return Err(OofError::DuplicatedFlag(flag_info.clone())); } // Otherwise, insert it result_flags.boolean_flags.insert(&flag_name); @@ -213,6 +215,8 @@ where #[cfg(test)] mod tests { + use std::os::unix::prelude::OsStringExt; + use crate::*; fn gen_args(text: &str) -> Vec { @@ -220,6 +224,77 @@ mod tests { args.map(OsString::from).collect() } + fn setup_args_scenario(arg_str: &str) -> Result<(Vec, Flags), OofError> { + let flags_info = [ + ArgFlag::long("output_file").short('o'), + Flag::long("verbose").short('v'), + Flag::long("help").short('h'), + ]; + + let args = gen_args(arg_str); + filter_flags(args, &flags_info) + } + + #[test] + fn test_unknown_flags() { + let result = setup_args_scenario("ouch a.zip -s b.tar.gz c.tar").unwrap_err(); + assert!(matches!(result, OofError::UnknownShortFlag('s'))); + + let unknown_long_flag = "foobar".to_string(); + let result = setup_args_scenario("ouch a.zip --foobar b.tar.gz c.tar").unwrap_err(); + + assert!(match result { + OofError::UnknownLongFlag(flag) if flag == unknown_long_flag => true, + _ => false, + }) + } + + #[test] + fn test_incomplete_flags() { + let incomplete_flag = ArgFlag::long("output_file").short('o'); + let result = setup_args_scenario("ouch a.zip b.tar.gz c.tar -o").unwrap_err(); + + assert!(match result { + OofError::MissingValueToFlag(flag) if flag == incomplete_flag => true, + _ => false, + }) + } + + #[test] + fn test_duplicated_flags() { + let duplicated_flag = ArgFlag::long("output_file").short('o'); + let result = setup_args_scenario("ouch a.zip b.tar.gz c.tar -o -o -o").unwrap_err(); + + assert!(match result { + OofError::DuplicatedFlag(flag) if flag == duplicated_flag => true, + _ => false, + }) + } + + #[test] + fn test_misplaced_flag() { + let misplaced_flag = ArgFlag::long("output_file").short('o'); + let result = setup_args_scenario("ouch -ov a.zip b.tar.gz c.tar").unwrap_err(); + + assert!(match result { + OofError::MisplacedShortArgFlagError(flag) if flag == misplaced_flag.short.unwrap() => + true, + _ => false, + }) + } + + #[test] + fn test_invalid_unicode_flag() { + // `invalid_unicode_flag` has to contain a leading hyphen to be considered a flag. + let invalid_unicode_flag = OsString::from_vec(vec![45, 0, 0, 0, 255, 255, 255, 255]); + let result = filter_flags(vec![invalid_unicode_flag.clone()], &[]).unwrap_err(); + + assert!(match result { + OofError::InvalidUnicode(flag) if flag == invalid_unicode_flag => true, + _ => false, + }) + } + // asdasdsa #[test] fn test_filter_flags() { diff --git a/src/error.rs b/src/error.rs index 83657e1b5..a64ff91cb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -120,7 +120,7 @@ impl From for Error { } } -impl<'t> From> for Error { +impl From for Error { fn from(err: oof::OofError) -> Self { // To avoid entering a lifetime hell, we'll just print the Oof error here // and skip saving it into a variant of Self