From a85a792c886310688373a091374bd33ebaff82f4 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Mon, 8 Jan 2024 15:04:34 +0100 Subject: [PATCH] format: use the new number parser and fix the error messages The error messages are more compliant with GNU coreutils. Also, floating hexadecimal numbers are now supported in `printf`. --- .../src/lib/features/format/argument.rs | 111 ++++++++---------- tests/by-util/test_printf.rs | 42 ++++++- 2 files changed, 88 insertions(+), 65 deletions(-) diff --git a/src/uucore/src/lib/features/format/argument.rs b/src/uucore/src/lib/features/format/argument.rs index 6f66230cb..92d6c1603 100644 --- a/src/uucore/src/lib/features/format/argument.rs +++ b/src/uucore/src/lib/features/format/argument.rs @@ -3,9 +3,14 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. +use crate::{ + error::set_exit_code, + features::format::num_parser::{ParseError, ParsedNumber}, + quoting_style::{escape_name, Quotes, QuotingStyle}, + show_error, show_warning, +}; use os_display::Quotable; - -use crate::{error::set_exit_code, show_warning}; +use std::ffi::OsStr; /// An argument for formatting /// @@ -40,9 +45,7 @@ impl<'a, T: Iterator> ArgumentIter<'a> for T { }; match next { FormatArgument::Char(c) => *c, - FormatArgument::Unparsed(s) => { - s.chars().next().unwrap_or('\0') - } + FormatArgument::Unparsed(s) => s.chars().next().unwrap_or('\0'), _ => '\0', } } @@ -53,25 +56,7 @@ impl<'a, T: Iterator> ArgumentIter<'a> for T { }; match next { FormatArgument::UnsignedInt(n) => *n, - FormatArgument::Unparsed(s) => { - let opt = if let Some(s) = s.strip_prefix("0x") { - u64::from_str_radix(s, 16).ok() - } else if let Some(s) = s.strip_prefix('0') { - u64::from_str_radix(s, 8).ok() - } else if let Some(s) = s.strip_prefix('\'') { - s.chars().next().map(|c| c as u64) - } else { - s.parse().ok() - }; - match opt { - Some(n) => n, - None => { - show_warning!("{}: expected a numeric value", s.quote()); - set_exit_code(1); - 0 - } - } - } + FormatArgument::Unparsed(s) => extract_value(ParsedNumber::parse_u64(s), s), _ => 0, } } @@ -82,29 +67,7 @@ impl<'a, T: Iterator> ArgumentIter<'a> for T { }; match next { FormatArgument::SignedInt(n) => *n, - FormatArgument::Unparsed(s) => { - // For hex, we parse `u64` because we do not allow another - // minus sign. We might need to do more precise parsing here. - let opt = if let Some(s) = s.strip_prefix("-0x") { - u64::from_str_radix(s, 16).ok().map(|x| -(x as i64)) - } else if let Some(s) = s.strip_prefix("0x") { - u64::from_str_radix(s, 16).ok().map(|x| x as i64) - } else if s.starts_with("-0") || s.starts_with('0') { - i64::from_str_radix(s, 8).ok() - } else if let Some(s) = s.strip_prefix('\'') { - s.chars().next().map(|x| x as i64) - } else { - s.parse().ok() - }; - match opt { - Some(n) => n, - None => { - show_warning!("{}: expected a numeric value", s.quote()); - set_exit_code(1); - 0 - } - } - } + FormatArgument::Unparsed(s) => extract_value(ParsedNumber::parse_i64(s), s), _ => 0, } } @@ -115,23 +78,7 @@ impl<'a, T: Iterator> ArgumentIter<'a> for T { }; match next { FormatArgument::Float(n) => *n, - FormatArgument::Unparsed(s) => { - let opt = if s.starts_with("0x") || s.starts_with("-0x") { - unimplemented!("Hexadecimal floats are unimplemented!") - } else if let Some(s) = s.strip_prefix('\'') { - s.chars().next().map(|x| x as u64 as f64) - } else { - s.parse().ok() - }; - match opt { - Some(n) => n, - None => { - show_warning!("{}: expected a numeric value", s.quote()); - set_exit_code(1); - 0.0 - } - } - } + FormatArgument::Unparsed(s) => extract_value(ParsedNumber::parse_f64(s), s), _ => 0.0, } } @@ -143,3 +90,39 @@ impl<'a, T: Iterator> ArgumentIter<'a> for T { } } } + +fn extract_value(p: Result>, input: &str) -> T { + match p { + Ok(v) => v, + Err(e) => { + set_exit_code(1); + let input = escape_name( + OsStr::new(input), + &QuotingStyle::C { + quotes: Quotes::None, + }, + ); + match e { + ParseError::Overflow => { + show_error!("{}: Numerical result out of range", input.quote()); + Default::default() + } + ParseError::NotNumeric => { + show_error!("{}: expected a numeric value", input.quote()); + Default::default() + } + ParseError::PartialMatch(v, rest) => { + if input.starts_with('\'') { + show_warning!( + "{}: character(s) following character constant have been ignored", + &rest, + ); + } else { + show_error!("{}: value not completely converted", input.quote()); + } + v + } + } + } + } +} diff --git a/tests/by-util/test_printf.rs b/tests/by-util/test_printf.rs index c106e5512..48fc1e6ac 100644 --- a/tests/by-util/test_printf.rs +++ b/tests/by-util/test_printf.rs @@ -435,7 +435,6 @@ fn sub_float_dec_places() { } #[test] -#[ignore = "hexadecimal floats are unimplemented"] fn sub_float_hex_in() { new_ucmd!() .args(&["%f", "0xF1.1F"]) @@ -599,3 +598,44 @@ fn sub_general_round_float_leading_zeroes() { .succeeds() .stdout_only("1.00001"); } + +#[test] +fn partial_float() { + new_ucmd!() + .args(&["%.2f is %s", "42.03x", "a lot"]) + .fails() + .code_is(1) + .stdout_is("42.03 is a lot") + .stderr_is("printf: '42.03x': value not completely converted\n"); +} + +#[test] +fn partial_integer() { + new_ucmd!() + .args(&["%d is %s", "42x23", "a lot"]) + .fails() + .code_is(1) + .stdout_is("42 is a lot") + .stderr_is("printf: '42x23': value not completely converted\n"); +} + +#[test] +fn test_overflow() { + new_ucmd!() + .args(&["%d", "36893488147419103232"]) + .fails() + .code_is(1) + .stderr_is("printf: '36893488147419103232': Numerical result out of range\n"); +} + +#[test] +fn partial_char() { + new_ucmd!() + .args(&["%d", "'abc"]) + .fails() + .code_is(1) + .stdout_is("97") + .stderr_is( + "printf: warning: bc: character(s) following character constant have been ignored\n", + ); +}