From 6eb3e3cd943dde716a9ed6c74a80696d1078b472 Mon Sep 17 00:00:00 2001 From: Jeremy Smart Date: Mon, 14 Apr 2025 07:34:01 -0400 Subject: [PATCH] parse_time: support hex durations, update sleep/timeout accordingly --- src/uu/sleep/Cargo.toml | 2 +- src/uu/sleep/src/sleep.rs | 31 ++----- .../src/lib/features/parser/num_parser.rs | 81 +++++++++++++------ .../src/lib/features/parser/parse_time.rs | 32 +++----- 4 files changed, 72 insertions(+), 74 deletions(-) diff --git a/src/uu/sleep/Cargo.toml b/src/uu/sleep/Cargo.toml index 5215606b8..141447cd3 100644 --- a/src/uu/sleep/Cargo.toml +++ b/src/uu/sleep/Cargo.toml @@ -20,7 +20,7 @@ path = "src/sleep.rs" [dependencies] clap = { workspace = true } fundu = { workspace = true } -uucore = { workspace = true } +uucore = { workspace = true, features = ["parser"] } [[bin]] name = "sleep" diff --git a/src/uu/sleep/src/sleep.rs b/src/uu/sleep/src/sleep.rs index 0f71c8e55..e377b375f 100644 --- a/src/uu/sleep/src/sleep.rs +++ b/src/uu/sleep/src/sleep.rs @@ -8,11 +8,12 @@ use std::time::Duration; use uucore::{ error::{UResult, USimpleError, UUsageError}, - format_usage, help_about, help_section, help_usage, show_error, + format_usage, help_about, help_section, help_usage, + parser::parse_time, + show_error, }; use clap::{Arg, ArgAction, Command}; -use fundu::{DurationParser, ParseError, SaturatingInto}; static ABOUT: &str = help_about!("sleep.md"); const USAGE: &str = help_usage!("sleep.md"); @@ -61,37 +62,17 @@ pub fn uu_app() -> Command { fn sleep(args: &[&str]) -> UResult<()> { let mut arg_error = false; - use fundu::TimeUnit::{Day, Hour, Minute, Second}; - let parser = DurationParser::with_time_units(&[Second, Minute, Hour, Day]); - let sleep_dur = args .iter() - .filter_map(|input| match parser.parse(input.trim()) { + .filter_map(|input| match parse_time::from_str(input) { Ok(duration) => Some(duration), Err(error) => { arg_error = true; - - let reason = match error { - ParseError::Empty if input.is_empty() => "Input was empty".to_string(), - ParseError::Empty => "Found only whitespace in input".to_string(), - ParseError::Syntax(pos, description) - | ParseError::TimeUnit(pos, description) => { - format!("{description} at position {}", pos.saturating_add(1)) - } - ParseError::NegativeExponentOverflow | ParseError::PositiveExponentOverflow => { - "Exponent was out of bounds".to_string() - } - ParseError::NegativeNumber => "Number was negative".to_string(), - error => error.to_string(), - }; - show_error!("invalid time interval '{input}': {reason}"); - + show_error!("{error}"); None } }) - .fold(Duration::ZERO, |acc, n| { - acc.saturating_add(SaturatingInto::::saturating_into(n)) - }); + .fold(Duration::ZERO, |acc, n| acc.saturating_add(n)); if arg_error { return Err(UUsageError::new(1, "")); diff --git a/src/uucore/src/lib/features/parser/num_parser.rs b/src/uucore/src/lib/features/parser/num_parser.rs index f21aa0114..8e0968509 100644 --- a/src/uucore/src/lib/features/parser/num_parser.rs +++ b/src/uucore/src/lib/features/parser/num_parser.rs @@ -150,7 +150,7 @@ impl ExtendedParser for i64 { } } - match parse(input, true) { + match parse(input, ParseTarget::Integral, &[]) { Ok(v) => into_i64(v), Err(e) => Err(e.map(into_i64)), } @@ -187,7 +187,7 @@ impl ExtendedParser for u64 { } } - match parse(input, true) { + match parse(input, ParseTarget::Integral, &[]) { Ok(v) => into_u64(v), Err(e) => Err(e.map(into_u64)), } @@ -219,7 +219,7 @@ impl ExtendedParser for f64 { Ok(v) } - match parse(input, false) { + match parse(input, ParseTarget::Decimal, &[]) { Ok(v) => into_f64(v), Err(e) => Err(e.map(into_f64)), } @@ -231,14 +231,15 @@ impl ExtendedParser for ExtendedBigDecimal { fn extended_parse( input: &str, ) -> Result> { - parse(input, false) + parse(input, ParseTarget::Decimal, &[]) } } -fn parse_special_value( - input: &str, +fn parse_special_value<'a>( + input: &'a str, negative: bool, -) -> Result> { + allowed_suffixes: &'a [(char, u32)], +) -> Result> { let input_lc = input.to_ascii_lowercase(); // Array of ("String to match", return value when sign positive, when sign negative) @@ -254,7 +255,14 @@ fn parse_special_value( if negative { special = -special; } - let match_len = str.len(); + let mut match_len = str.len(); + if let Some(ch) = input.chars().nth(str.chars().count()) { + if allowed_suffixes.iter().any(|(c, _)| ch == *c) { + // multiplying is unnecessary for these special values, but we have to note that + // we processed the character to avoid a partial match error + match_len += 1; + } + } return if input.len() == match_len { Ok(special) } else { @@ -381,24 +389,34 @@ fn construct_extended_big_decimal<'a>( Ok(ExtendedBigDecimal::BigDecimal(bd)) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ParseTarget { + Decimal, + Integral, + Duration, +} + // TODO: As highlighted by clippy, this function _is_ high cognitive complexity, jumps // around between integer and float parsing, and should be split in multiple parts. #[allow(clippy::cognitive_complexity)] -fn parse( - input: &str, - integral_only: bool, -) -> Result> { +pub(crate) fn parse<'a>( + input: &'a str, + target: ParseTarget, + allowed_suffixes: &'a [(char, u32)], +) -> Result> { // Parse the " and ' prefixes separately - if let Some(rest) = input.strip_prefix(['\'', '"']) { - let mut chars = rest.char_indices().fuse(); - let v = chars - .next() - .map(|(_, c)| ExtendedBigDecimal::BigDecimal(u32::from(c).into())); - return match (v, chars.next()) { - (Some(v), None) => Ok(v), - (Some(v), Some((i, _))) => Err(ExtendedParserError::PartialMatch(v, &rest[i..])), - (None, _) => Err(ExtendedParserError::NotNumeric), - }; + if target != ParseTarget::Duration { + if let Some(rest) = input.strip_prefix(['\'', '"']) { + let mut chars = rest.char_indices().fuse(); + let v = chars + .next() + .map(|(_, c)| ExtendedBigDecimal::BigDecimal(u32::from(c).into())); + return match (v, chars.next()) { + (Some(v), None) => Ok(v), + (Some(v), Some((i, _))) => Err(ExtendedParserError::PartialMatch(v, &rest[i..])), + (None, _) => Err(ExtendedParserError::NotNumeric), + }; + } } let trimmed_input = input.trim_ascii_start(); @@ -419,7 +437,7 @@ fn parse( let (base, rest) = if let Some(rest) = unsigned.strip_prefix('0') { if let Some(rest) = rest.strip_prefix(['x', 'X']) { (Base::Hexadecimal, rest) - } else if integral_only { + } else if target == ParseTarget::Integral { // Binary/Octal only allowed for integer parsing. if let Some(rest) = rest.strip_prefix(['b', 'B']) { (Base::Binary, rest) @@ -447,7 +465,7 @@ fn parse( } // Parse fractional/exponent part of the number for supported bases. - if matches!(base, Base::Decimal | Base::Hexadecimal) && !integral_only { + if matches!(base, Base::Decimal | Base::Hexadecimal) && target != ParseTarget::Integral { // Parse the fractional part of the number if there can be one and the input contains // a '.' decimal separator. if matches!(chars.peek(), Some(&(_, '.'))) { @@ -493,13 +511,24 @@ fn parse( // If nothing has been parsed, check if this is a special value, or declare the parsing unsuccessful if let Some((0, _)) = chars.peek() { - return if integral_only { + return if target == ParseTarget::Integral { Err(ExtendedParserError::NotNumeric) } else { - parse_special_value(unsigned, negative) + parse_special_value(unsigned, negative, allowed_suffixes) }; } + if let Some((_, ch)) = chars.peek() { + if let Some(times) = allowed_suffixes + .iter() + .find(|(c, _)| ch == c) + .map(|&(_, t)| t) + { + chars.next(); + digits *= times; + } + } + let ebd_result = construct_extended_big_decimal(digits, negative, base, scale, exponent); // Return what has been parsed so far. If there are extra characters, mark the diff --git a/src/uucore/src/lib/features/parser/parse_time.rs b/src/uucore/src/lib/features/parser/parse_time.rs index 2fd7854d5..ef8407add 100644 --- a/src/uucore/src/lib/features/parser/parse_time.rs +++ b/src/uucore/src/lib/features/parser/parse_time.rs @@ -11,9 +11,8 @@ use crate::{ display::Quotable, extendedbigdecimal::ExtendedBigDecimal, - parser::num_parser::{ExtendedParser, ExtendedParserError}, + parser::num_parser::{self, ExtendedParserError, ParseTarget}, }; -use bigdecimal::BigDecimal; use num_traits::Signed; use num_traits::ToPrimitive; use num_traits::Zero; @@ -59,26 +58,18 @@ pub fn from_str(string: &str) -> Result { let len = string.len(); if len == 0 { - return Err("empty string".to_owned()); - } - let Some(slice) = string.get(..len - 1) else { return Err(format!("invalid time interval {}", string.quote())); - }; - let (numstr, times) = match string.chars().next_back().unwrap() { - 's' => (slice, 1), - 'm' => (slice, 60), - 'h' => (slice, 60 * 60), - 'd' => (slice, 60 * 60 * 24), - val if !val.is_alphabetic() => (string, 1), - _ => match string.to_ascii_lowercase().as_str() { - "inf" | "infinity" => ("inf", 1), - _ => return Err(format!("invalid time interval {}", string.quote())), - }, - }; - let num = match ExtendedBigDecimal::extended_parse(numstr) { + } + let num = match num_parser::parse( + string, + ParseTarget::Duration, + &[('s', 1), ('m', 60), ('h', 60 * 60), ('d', 60 * 60 * 24)], + ) { Ok(ebd) | Err(ExtendedParserError::Overflow(ebd)) => ebd, Err(ExtendedParserError::Underflow(_)) => return Ok(NANOSECOND_DURATION), - _ => return Err(format!("invalid time interval {}", string.quote())), + _ => { + return Err(format!("invalid time interval {}", string.quote())); + } }; // Allow non-negative durations (-0 is fine), and infinity. @@ -89,9 +80,6 @@ pub fn from_str(string: &str) -> Result { _ => return Err(format!("invalid time interval {}", string.quote())), }; - // Pre-multiply times to avoid precision loss - let num: BigDecimal = num * times; - // Transform to nanoseconds (9 digits after decimal point) let (nanos_bi, _) = num.with_scale(9).into_bigint_and_scale();