From 5b6a617024517dd9e61fd22daa2eefe314fa08ac Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Fri, 11 Jul 2025 11:54:20 +0800 Subject: [PATCH] uucore: format: Have get_num return T directly (not result) And fix all the callers, e.g. all the next_ functions. --- .../src/lib/features/format/argument.rs | 185 ++++++------------ src/uucore/src/lib/features/format/spec.rs | 12 +- 2 files changed, 70 insertions(+), 127 deletions(-) diff --git a/src/uucore/src/lib/features/format/argument.rs b/src/uucore/src/lib/features/format/argument.rs index b3d002dd7..d02ffde04 100644 --- a/src/uucore/src/lib/features/format/argument.rs +++ b/src/uucore/src/lib/features/format/argument.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use super::{ExtendedBigDecimal, FormatError}; +use super::ExtendedBigDecimal; use crate::format::spec::ArgumentLocation; use crate::{ error::set_exit_code, @@ -73,14 +73,14 @@ impl<'a> FormatArguments<'a> { self.next_arg_position = self.current_offset; } - pub fn next_char(&mut self, position: &ArgumentLocation) -> Result { + pub fn next_char(&mut self, position: &ArgumentLocation) -> u8 { match self.next_arg(position) { - Some(FormatArgument::Char(c)) => Ok(*c as u8), - Some(FormatArgument::Unparsed(os)) => match os_str_as_bytes(os)?.first() { - Some(&byte) => Ok(byte), - None => Ok(b'\0'), + Some(FormatArgument::Char(c)) => *c as u8, + Some(FormatArgument::Unparsed(os)) => match os_str_as_bytes(os) { + Ok(bytes) => bytes.first().copied().unwrap_or(b'\0'), + Err(_) => b'\0', }, - _ => Ok(b'\0'), + _ => b'\0', } } @@ -91,38 +91,36 @@ impl<'a> FormatArguments<'a> { } } - pub fn next_i64(&mut self, position: &ArgumentLocation) -> Result { + pub fn next_i64(&mut self, position: &ArgumentLocation) -> i64 { match self.next_arg(position) { - Some(FormatArgument::SignedInt(n)) => Ok(*n), + Some(FormatArgument::SignedInt(n)) => *n, Some(FormatArgument::Unparsed(os)) => Self::get_num::(os), - _ => Ok(0), + _ => 0, } } - pub fn next_u64(&mut self, position: &ArgumentLocation) -> Result { + pub fn next_u64(&mut self, position: &ArgumentLocation) -> u64 { match self.next_arg(position) { - Some(FormatArgument::UnsignedInt(n)) => Ok(*n), + Some(FormatArgument::UnsignedInt(n)) => *n, Some(FormatArgument::Unparsed(os)) => Self::get_num::(os), - _ => Ok(0), + _ => 0, } } - pub fn next_extended_big_decimal( - &mut self, - position: &ArgumentLocation, - ) -> Result { + pub fn next_extended_big_decimal(&mut self, position: &ArgumentLocation) -> ExtendedBigDecimal { match self.next_arg(position) { - Some(FormatArgument::Float(n)) => Ok(n.clone()), + Some(FormatArgument::Float(n)) => n.clone(), Some(FormatArgument::Unparsed(os)) => Self::get_num::(os), - _ => Ok(ExtendedBigDecimal::zero()), + _ => ExtendedBigDecimal::zero(), } } - fn get_num(os: &OsStr) -> Result + fn get_num(os: &OsStr) -> T where T: ExtendedParser + From + From + Default, { - let s = os_str_as_bytes(os)?; + // FIXME: Remove unwrap + let s = os_str_as_bytes(os).unwrap(); // Check if the string begins with a quote, and is therefore a literal if let Some((&first, bytes)) = s.split_first() { if (first == b'"' || first == b'\'') && !bytes.is_empty() { @@ -148,11 +146,11 @@ impl<'a> FormatArguments<'a> { String::from_utf8_lossy(&bytes[len..]) ); } - return Ok(val); + return val; } } let s = os.to_string_lossy(); - Ok(extract_value(T::extended_parse(&s), &s)) + extract_value(T::extended_parse(&s), &s) } fn get_at_relative_position(&mut self, pos: NonZero) -> Option<&'a FormatArgument> { @@ -227,10 +225,7 @@ mod tests { assert!(!args.is_exhausted()); assert_eq!(Some(&FormatArgument::Char('a')), args.peek_arg()); assert!(!args.is_exhausted()); // Peek shouldn't consume - assert_eq!( - b'a', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); + assert_eq!(b'a', args.next_char(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(args.is_exhausted()); // After batch, exhausted with a single arg assert_eq!(None, args.peek_arg()); @@ -251,50 +246,26 @@ mod tests { ]); // First batch - two sequential calls - assert_eq!( - b'z', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); - assert_eq!( - b'y', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); + assert_eq!(b'z', args.next_char(&ArgumentLocation::NextArgument)); + assert_eq!(b'y', args.next_char(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same pattern - assert_eq!( - b'x', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); - assert_eq!( - b'w', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); + assert_eq!(b'x', args.next_char(&ArgumentLocation::NextArgument)); + assert_eq!(b'w', args.next_char(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(!args.is_exhausted()); // Third batch - same pattern - assert_eq!( - b'v', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); - assert_eq!( - b'u', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); + assert_eq!(b'v', args.next_char(&ArgumentLocation::NextArgument)); + assert_eq!(b'u', args.next_char(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(!args.is_exhausted()); // Fourth batch - same pattern (last batch) - assert_eq!( - b't', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); - assert_eq!( - b's', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); + assert_eq!(b't', args.next_char(&ArgumentLocation::NextArgument)); + assert_eq!(b's', args.next_char(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(args.is_exhausted()); } @@ -313,28 +284,19 @@ mod tests { let mut args = FormatArguments::new(&args); // First batch - next_char followed by next_string - assert_eq!( - b'a', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); + assert_eq!(b'a', args.next_char(&ArgumentLocation::NextArgument)); assert_eq!("hello", args.next_string(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same pattern - assert_eq!( - b'1', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); // First byte of 123 + assert_eq!(b'1', args.next_char(&ArgumentLocation::NextArgument)); // First byte of 123 assert_eq!("world", args.next_string(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(!args.is_exhausted()); // Third batch - same pattern (last batch) - assert_eq!( - b'z', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); + assert_eq!(b'z', args.next_char(&ArgumentLocation::NextArgument)); assert_eq!("test", args.next_string(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(args.is_exhausted()); @@ -360,23 +322,23 @@ mod tests { ]); // First batch - positional access - assert_eq!(b'b', args.next_char(&non_zero_pos(2)).unwrap()); // Position 2 - assert_eq!(b'a', args.next_char(&non_zero_pos(1)).unwrap()); // Position 1 - assert_eq!(b'c', args.next_char(&non_zero_pos(3)).unwrap()); // Position 3 + assert_eq!(b'b', args.next_char(&non_zero_pos(2))); // Position 2 + assert_eq!(b'a', args.next_char(&non_zero_pos(1))); // Position 1 + assert_eq!(b'c', args.next_char(&non_zero_pos(3))); // Position 3 args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same positional pattern - assert_eq!(b'e', args.next_char(&non_zero_pos(2)).unwrap()); // Position 2 - assert_eq!(b'd', args.next_char(&non_zero_pos(1)).unwrap()); // Position 1 - assert_eq!(b'f', args.next_char(&non_zero_pos(3)).unwrap()); // Position 3 + assert_eq!(b'e', args.next_char(&non_zero_pos(2))); // Position 2 + assert_eq!(b'd', args.next_char(&non_zero_pos(1))); // Position 1 + assert_eq!(b'f', args.next_char(&non_zero_pos(3))); // Position 3 args.start_next_batch(); assert!(!args.is_exhausted()); // Third batch - same positional pattern (last batch) - assert_eq!(b'h', args.next_char(&non_zero_pos(2)).unwrap()); // Position 2 - assert_eq!(b'g', args.next_char(&non_zero_pos(1)).unwrap()); // Position 1 - assert_eq!(b'i', args.next_char(&non_zero_pos(3)).unwrap()); // Position 3 + assert_eq!(b'h', args.next_char(&non_zero_pos(2))); // Position 2 + assert_eq!(b'g', args.next_char(&non_zero_pos(1))); // Position 1 + assert_eq!(b'i', args.next_char(&non_zero_pos(3))); // Position 3 args.start_next_batch(); assert!(args.is_exhausted()); } @@ -396,29 +358,20 @@ mod tests { ]); // First batch - mix of sequential and positional - assert_eq!( - b'a', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); // Sequential - assert_eq!(b'c', args.next_char(&non_zero_pos(3)).unwrap()); // Positional + assert_eq!(b'a', args.next_char(&ArgumentLocation::NextArgument)); // Sequential + assert_eq!(b'c', args.next_char(&non_zero_pos(3))); // Positional args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same mixed pattern - assert_eq!( - b'd', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); // Sequential - assert_eq!(b'f', args.next_char(&non_zero_pos(3)).unwrap()); // Positional + assert_eq!(b'd', args.next_char(&ArgumentLocation::NextArgument)); // Sequential + assert_eq!(b'f', args.next_char(&non_zero_pos(3))); // Positional args.start_next_batch(); assert!(!args.is_exhausted()); // Last batch - same mixed pattern - assert_eq!( - b'g', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); // Sequential - assert_eq!(b'\0', args.next_char(&non_zero_pos(3)).unwrap()); // Out of bounds + assert_eq!(b'g', args.next_char(&ArgumentLocation::NextArgument)); // Sequential + assert_eq!(b'\0', args.next_char(&non_zero_pos(3))); // Out of bounds args.start_next_batch(); assert!(args.is_exhausted()); } @@ -437,21 +390,17 @@ mod tests { let mut args = FormatArguments::new(&args); // First batch - i64, u64, decimal - assert_eq!(10, args.next_i64(&ArgumentLocation::NextArgument).unwrap()); - assert_eq!(20, args.next_u64(&ArgumentLocation::NextArgument).unwrap()); - let result = args - .next_extended_big_decimal(&ArgumentLocation::NextArgument) - .unwrap(); + assert_eq!(10, args.next_i64(&ArgumentLocation::NextArgument)); + assert_eq!(20, args.next_u64(&ArgumentLocation::NextArgument)); + let result = args.next_extended_big_decimal(&ArgumentLocation::NextArgument); assert_eq!(ExtendedBigDecimal::zero(), result); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same pattern - assert_eq!(30, args.next_i64(&ArgumentLocation::NextArgument).unwrap()); - assert_eq!(40, args.next_u64(&ArgumentLocation::NextArgument).unwrap()); - let result = args - .next_extended_big_decimal(&ArgumentLocation::NextArgument) - .unwrap(); + assert_eq!(30, args.next_i64(&ArgumentLocation::NextArgument)); + assert_eq!(40, args.next_u64(&ArgumentLocation::NextArgument)); + let result = args.next_extended_big_decimal(&ArgumentLocation::NextArgument); assert_eq!(ExtendedBigDecimal::zero(), result); args.start_next_batch(); assert!(args.is_exhausted()); @@ -470,13 +419,13 @@ mod tests { // First batch - string, number assert_eq!("hello", args.next_string(&ArgumentLocation::NextArgument)); - assert_eq!(123, args.next_i64(&ArgumentLocation::NextArgument).unwrap()); + assert_eq!(123, args.next_i64(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same pattern assert_eq!("hello", args.next_string(&ArgumentLocation::NextArgument)); - assert_eq!(456, args.next_i64(&ArgumentLocation::NextArgument).unwrap()); + assert_eq!(456, args.next_i64(&ArgumentLocation::NextArgument)); args.start_next_batch(); assert!(args.is_exhausted()); } @@ -495,16 +444,16 @@ mod tests { let mut args = FormatArguments::new(&args); // First batch - positional access of different types - assert_eq!(b'a', args.next_char(&non_zero_pos(1)).unwrap()); + assert_eq!(b'a', args.next_char(&non_zero_pos(1))); assert_eq!("test", args.next_string(&non_zero_pos(2))); - assert_eq!(42, args.next_u64(&non_zero_pos(3)).unwrap()); + assert_eq!(42, args.next_u64(&non_zero_pos(3))); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch - same pattern - assert_eq!(b'b', args.next_char(&non_zero_pos(1)).unwrap()); + assert_eq!(b'b', args.next_char(&non_zero_pos(1))); assert_eq!("more", args.next_string(&non_zero_pos(2))); - assert_eq!(99, args.next_u64(&non_zero_pos(3)).unwrap()); + assert_eq!(99, args.next_u64(&non_zero_pos(3))); args.start_next_batch(); assert!(args.is_exhausted()); } @@ -521,20 +470,14 @@ mod tests { ]); // First batch - assert_eq!( - b'a', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); - assert_eq!(b'c', args.next_char(&non_zero_pos(3)).unwrap()); + assert_eq!(b'a', args.next_char(&ArgumentLocation::NextArgument)); + assert_eq!(b'c', args.next_char(&non_zero_pos(3))); args.start_next_batch(); assert!(!args.is_exhausted()); // Second batch (partial) - assert_eq!( - b'd', - args.next_char(&ArgumentLocation::NextArgument).unwrap() - ); - assert_eq!(b'\0', args.next_char(&non_zero_pos(3)).unwrap()); // Out of bounds + assert_eq!(b'd', args.next_char(&ArgumentLocation::NextArgument)); + assert_eq!(b'\0', args.next_char(&non_zero_pos(3))); // Out of bounds args.start_next_batch(); assert!(args.is_exhausted()); } diff --git a/src/uucore/src/lib/features/format/spec.rs b/src/uucore/src/lib/features/format/spec.rs index d91180860..98d455994 100644 --- a/src/uucore/src/lib/features/format/spec.rs +++ b/src/uucore/src/lib/features/format/spec.rs @@ -357,7 +357,7 @@ impl Spec { let (width, neg_width) = resolve_asterisk_width(*width, args).unwrap_or_default(); write_padded( writer, - &[args.next_char(position)?], + &[args.next_char(position)], width, *align_left || neg_width, ) @@ -419,7 +419,7 @@ impl Spec { } => { let (width, neg_width) = resolve_asterisk_width(*width, args).unwrap_or((0, false)); let precision = resolve_asterisk_precision(*precision, args).unwrap_or_default(); - let i = args.next_i64(position)?; + let i = args.next_i64(position); if precision as u64 > i32::MAX as u64 { return Err(FormatError::InvalidPrecision(precision.to_string())); @@ -447,7 +447,7 @@ impl Spec { } => { let (width, neg_width) = resolve_asterisk_width(*width, args).unwrap_or((0, false)); let precision = resolve_asterisk_precision(*precision, args).unwrap_or_default(); - let i = args.next_u64(position)?; + let i = args.next_u64(position); if precision as u64 > i32::MAX as u64 { return Err(FormatError::InvalidPrecision(precision.to_string())); @@ -478,7 +478,7 @@ impl Spec { } => { let (width, neg_width) = resolve_asterisk_width(*width, args).unwrap_or((0, false)); let precision = resolve_asterisk_precision(*precision, args); - let f: ExtendedBigDecimal = args.next_extended_big_decimal(position)?; + let f: ExtendedBigDecimal = args.next_extended_big_decimal(position); if precision.is_some_and(|p| p as u64 > i32::MAX as u64) { return Err(FormatError::InvalidPrecision( @@ -515,7 +515,7 @@ fn resolve_asterisk_width( match option { None => None, Some(CanAsterisk::Asterisk(loc)) => { - let nb = args.next_i64(&loc).unwrap_or(0); + let nb = args.next_i64(&loc); if nb < 0 { Some((usize::try_from(-(nb as isize)).ok().unwrap_or(0), true)) } else { @@ -534,7 +534,7 @@ fn resolve_asterisk_precision( ) -> Option { match option { None => None, - Some(CanAsterisk::Asterisk(loc)) => match args.next_i64(&loc).unwrap_or(0) { + Some(CanAsterisk::Asterisk(loc)) => match args.next_i64(&loc) { v if v >= 0 => usize::try_from(v).ok(), v if v < 0 => Some(0usize), _ => None,