From 5108d49c922d059dc6be3f3844ab2c9ca2c8155e Mon Sep 17 00:00:00 2001 From: Sebastian Bentmar Holgersson Date: Sun, 18 Jan 2026 18:59:23 +0100 Subject: [PATCH] numfmt: align error messages for suffixes (#9887) --- src/uu/numfmt/locales/en-US.ftl | 1 + src/uu/numfmt/locales/fr-FR.ftl | 1 + src/uu/numfmt/src/format.rs | 207 ++++++++++++++++++++++++++++---- src/uu/numfmt/src/units.rs | 20 +++ tests/by-util/test_numfmt.rs | 42 ++++++- 5 files changed, 244 insertions(+), 27 deletions(-) diff --git a/src/uu/numfmt/locales/en-US.ftl b/src/uu/numfmt/locales/en-US.ftl index a2ad787bd..d718e368a 100644 --- a/src/uu/numfmt/locales/en-US.ftl +++ b/src/uu/numfmt/locales/en-US.ftl @@ -59,6 +59,7 @@ numfmt-error-invalid-header = invalid header value { $value } numfmt-error-grouping-cannot-be-combined-with-to = grouping cannot be combined with --to numfmt-error-delimiter-must-be-single-character = the delimiter must be a single character numfmt-error-invalid-number-empty = invalid number: '' +numfmt-error-invalid-specific-suffix = invalid suffix in input { $input }: { $suffix } numfmt-error-invalid-suffix = invalid suffix in input: { $input } numfmt-error-invalid-number = invalid number: { $input } numfmt-error-missing-i-suffix = missing 'i' suffix in input: '{ $number }{ $suffix }' (e.g Ki/Mi/Gi) diff --git a/src/uu/numfmt/locales/fr-FR.ftl b/src/uu/numfmt/locales/fr-FR.ftl index 1a6294184..20bd91db9 100644 --- a/src/uu/numfmt/locales/fr-FR.ftl +++ b/src/uu/numfmt/locales/fr-FR.ftl @@ -59,6 +59,7 @@ numfmt-error-grouping-cannot-be-combined-with-to = le groupement ne peut pas êt numfmt-error-delimiter-must-be-single-character = le délimiteur doit être un seul caractère numfmt-error-invalid-number-empty = nombre invalide : '' numfmt-error-invalid-suffix = suffixe invalide dans l'entrée : { $input } +numfmt-error-invalid-specific-suffix = suffixe invalide dans l'entrée { $input } : { $suffix } numfmt-error-invalid-number = nombre invalide : { $input } numfmt-error-missing-i-suffix = suffixe 'i' manquant dans l'entrée : '{ $number }{ $suffix }' (par ex. Ki/Mi/Gi) numfmt-error-rejecting-suffix = rejet du suffixe dans l'entrée : '{ $number }{ $suffix }' (considérez utiliser --from) diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index f27926d87..3b1f41aa9 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -62,12 +62,97 @@ impl<'a> Iterator for WhitespaceSplitter<'a> { } } -fn parse_suffix(s: &str) -> Result<(f64, Option)> { +fn find_numeric_beginning(s: &str) -> Option<&str> { + let mut decimal_point_seen = false; + if s.is_empty() { + return None; + } + + for (idx, c) in s.char_indices() { + if c == '-' && idx == 0 { + continue; + } + if c.is_ascii_digit() { + continue; + } + if c == '.' && !decimal_point_seen { + decimal_point_seen = true; + continue; + } + if s[..idx].parse::().is_err() { + return None; + } + return Some(&s[..idx]); + } + + Some(s) +} + +// finds the valid beginning part of an input string, or None. +fn find_valid_number_with_suffix<'a>(s: &'a str, unit: &Unit) -> Option<&'a str> { + let numeric_part = find_numeric_beginning(s)?; + + let accepts_suffix = unit != &Unit::None; + let accepts_i = [Unit::Auto, Unit::Iec(true)].contains(unit); + + let mut characters = s.chars().skip(numeric_part.len()); + let potential_suffix = characters.next(); + let potential_i = characters.next(); + + if !accepts_suffix { + return Some(numeric_part); + } + + match (potential_suffix, potential_i) { + (Some(suffix), None) if RawSuffix::try_from(&suffix).is_ok() => { + Some(&s[..=numeric_part.len()]) + } + (Some(suffix), Some('i')) if accepts_i && RawSuffix::try_from(&suffix).is_ok() => { + Some(&s[..numeric_part.len() + 2]) + } + (Some(suffix), Some(_)) if RawSuffix::try_from(&suffix).is_ok() => { + Some(&s[..=numeric_part.len()]) + } + _ => Some(numeric_part), + } +} + +fn detailed_error_message(s: &str, unit: &Unit) -> Option { + if s.is_empty() { + return Some(translate!("numfmt-error-invalid-number-empty")); + } + + let valid_part = find_valid_number_with_suffix(s, unit) + .ok_or(translate!("numfmt-error-invalid-number", "input" => s.quote())) + .ok()?; + + if valid_part != s && valid_part.parse::().is_ok() { + return match s.chars().nth(valid_part.len()) { + Some(v) if RawSuffix::try_from(&v).is_ok() => Some( + translate!("numfmt-error-rejecting-suffix", "number" => valid_part, "suffix" => s[valid_part.len()..]), + ), + + _ => Some(translate!("numfmt-error-invalid-suffix", "input" => s.quote())), + }; + } + + if valid_part != s && valid_part.parse::().is_err() { + return Some( + translate!("numfmt-error-invalid-specific-suffix", "input" => s.quote(), "suffix" => s[valid_part.len()..].quote()), + ); + } + None +} + +fn parse_suffix(s: &str, unit: &Unit) -> Result<(f64, Option)> { if s.is_empty() { return Err(translate!("numfmt-error-invalid-number-empty")); } let with_i = s.ends_with('i'); + if with_i && ![Unit::Auto, Unit::Iec(true)].contains(unit) { + return Err(translate!("numfmt-error-invalid-suffix", "input" => s.quote())); + } let mut iter = s.chars(); if with_i { iter.next_back(); @@ -86,17 +171,7 @@ fn parse_suffix(s: &str) -> Result<(f64, Option)> { Some('Q') => Some((RawSuffix::Q, with_i)), Some('0'..='9') if !with_i => None, _ => { - // If with_i is true, the string ends with 'i' but there's no valid suffix letter - // This is always an invalid suffix (e.g., "1i", "2Ai") - if with_i { - return Err(translate!("numfmt-error-invalid-suffix", "input" => s.quote())); - } - // For other cases, check if the number part (without the last character) is valid - let number_part = &s[..s.len() - 1]; - if number_part.is_empty() || number_part.parse::().is_err() { - return Err(translate!("numfmt-error-invalid-number", "input" => s.quote())); - } - return Err(translate!("numfmt-error-invalid-suffix", "input" => s.quote())); + return Err(translate!("numfmt-error-invalid-number", "input" => s.quote())); } }; @@ -164,7 +239,8 @@ fn remove_suffix(i: f64, s: Option, u: &Unit) -> Result { } fn transform_from(s: &str, opts: &TransformOptions) -> Result { - let (i, suffix) = parse_suffix(s)?; + let (i, suffix) = parse_suffix(s, &opts.from) + .map_err(|original| detailed_error_message(s, &opts.from).unwrap_or(original))?; let i = i * (opts.from_unit as f64); remove_suffix(i, suffix, &opts.from).map(|n| { @@ -491,7 +567,7 @@ mod tests { #[test] fn test_parse_suffix_q_r_k() { - let result = parse_suffix("1Q"); + let result = parse_suffix("1Q", &Unit::Auto); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 1.0); @@ -500,7 +576,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::Q as i32); assert!(!with_i); - let result = parse_suffix("2R"); + let result = parse_suffix("2R", &Unit::Auto); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 2.0); @@ -509,7 +585,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::R as i32); assert!(!with_i); - let result = parse_suffix("3k"); + let result = parse_suffix("3k", &Unit::Auto); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 3.0); @@ -518,7 +594,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::K as i32); assert!(!with_i); - let result = parse_suffix("4Qi"); + let result = parse_suffix("4Qi", &Unit::Auto); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 4.0); @@ -527,7 +603,7 @@ mod tests { assert_eq!(raw_suffix as i32, RawSuffix::Q as i32); assert!(with_i); - let result = parse_suffix("5Ri"); + let result = parse_suffix("5Ri", &Unit::Auto); assert!(result.is_ok()); let (number, suffix) = result.unwrap(); assert_eq!(number, 5.0); @@ -539,22 +615,41 @@ mod tests { #[test] fn test_parse_suffix_error_messages() { - let result = parse_suffix("foo"); + let result = parse_suffix("foo", &Unit::Auto); assert!(result.is_err()); let error = result.unwrap_err(); assert!(error.contains("numfmt-error-invalid-number") || error.contains("invalid number")); assert!(!error.contains("invalid suffix")); - let result = parse_suffix("World"); + let result = parse_suffix("World", &Unit::Auto); assert!(result.is_err()); let error = result.unwrap_err(); assert!(error.contains("numfmt-error-invalid-number") || error.contains("invalid number")); assert!(!error.contains("invalid suffix")); + } - let result = parse_suffix("123i"); - assert!(result.is_err()); - let error = result.unwrap_err(); + #[test] + fn test_detailed_error_message() { + let result = detailed_error_message("123i", &Unit::Auto); + assert!(result.is_some()); + let error = result.unwrap(); assert!(error.contains("numfmt-error-invalid-suffix") || error.contains("invalid suffix")); + + let result = detailed_error_message("5MF", &Unit::Auto); + assert!(result.is_some()); + let error = result.unwrap(); + assert!( + error.contains("numfmt-error-invalid-specific-suffix") + || error.contains("invalid suffix") + ); + + let result = detailed_error_message("5KM", &Unit::Auto); + assert!(result.is_some()); + let error = result.unwrap(); + assert!( + error.contains("numfmt-error-invalid-specific-suffix") + || error.contains("invalid suffix") + ); } #[test] @@ -578,6 +673,72 @@ mod tests { assert_eq!(result.unwrap(), IEC_BASES[9]); } + #[test] + fn test_find_valid_part() { + assert_eq!( + find_valid_number_with_suffix("12345KL", &Unit::Auto), + Some("12345K") + ); + assert_eq!( + find_valid_number_with_suffix("12345K", &Unit::Auto), + Some("12345K") + ); + assert_eq!( + find_valid_number_with_suffix("12345", &Unit::Auto), + Some("12345") + ); + assert_eq!( + find_valid_number_with_suffix("asd12345KL", &Unit::Auto), + None + ); + assert_eq!( + find_valid_number_with_suffix("8asdf", &Unit::Auto), + Some("8") + ); + assert_eq!(find_valid_number_with_suffix("5i", &Unit::Si), Some("5")); + assert_eq!( + find_valid_number_with_suffix("5i", &Unit::Iec(true)), + Some("5") + ); + assert_eq!( + find_valid_number_with_suffix("0.1KL", &Unit::Auto), + Some("0.1K") + ); + assert_eq!( + find_valid_number_with_suffix("0.1", &Unit::Auto), + Some("0.1") + ); + assert_eq!( + find_valid_number_with_suffix("-0.1MT", &Unit::Auto), + Some("-0.1M") + ); + assert_eq!( + find_valid_number_with_suffix("-0.1PT", &Unit::Auto), + Some("-0.1P") + ); + assert_eq!( + find_valid_number_with_suffix("-0.1PT", &Unit::Auto), + Some("-0.1P") + ); + assert_eq!( + find_valid_number_with_suffix("123.4.5", &Unit::Auto), + Some("123.4") + ); + assert_eq!( + find_valid_number_with_suffix("0.55KiJ", &Unit::Iec(true)), + Some("0.55Ki") + ); + assert_eq!( + find_valid_number_with_suffix("0.55KiJ", &Unit::Iec(false)), + Some("0.55K") + ); + assert_eq!( + find_valid_number_with_suffix("123KICK", &Unit::Auto), + Some("123K") + ); + assert_eq!(find_valid_number_with_suffix("", &Unit::Auto), None); + } + #[test] fn test_consider_suffix_q_r() { use crate::options::RoundMethod; diff --git a/src/uu/numfmt/src/units.rs b/src/uu/numfmt/src/units.rs index bc5d480be..4343175f3 100644 --- a/src/uu/numfmt/src/units.rs +++ b/src/uu/numfmt/src/units.rs @@ -46,6 +46,26 @@ pub enum RawSuffix { Q, } +impl TryFrom<&char> for RawSuffix { + type Error = String; + + fn try_from(value: &char) -> Result { + match value { + 'K' | 'k' => Ok(Self::K), + 'M' => Ok(Self::M), + 'G' => Ok(Self::G), + 'T' => Ok(Self::T), + 'P' => Ok(Self::P), + 'E' => Ok(Self::E), + 'Z' => Ok(Self::Z), + 'Y' => Ok(Self::Y), + 'R' => Ok(Self::R), + 'Q' => Ok(Self::Q), + _ => Err(format!("Invalid suffix: {value}")), + } + } +} + pub type Suffix = (RawSuffix, WithI); pub struct DisplayableSuffix(pub Suffix, pub Unit); diff --git a/tests/by-util/test_numfmt.rs b/tests/by-util/test_numfmt.rs index 241286d07..f79c76006 100644 --- a/tests/by-util/test_numfmt.rs +++ b/tests/by-util/test_numfmt.rs @@ -62,6 +62,14 @@ fn test_from_iec_i_requires_suffix() { .stderr_is("numfmt: missing 'i' suffix in input: '10M' (e.g Ki/Mi/Gi)\n"); } +#[test] +fn test_from_iec_fails_if_i_suffix() { + new_ucmd!() + .args(&["--from=iec", "10Mi"]) + .fails_with_code(2) + .stderr_is("numfmt: invalid suffix in input '10Mi': 'i'\n"); +} + #[test] fn test_from_iec_i_without_suffix_are_bytes() { new_ucmd!() @@ -261,6 +269,34 @@ fn test_suffixes() { } } +#[test] +fn test_invalid_following_valid_suffix() { + let valid_suffixes = ['K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y', 'R', 'Q', 'k']; + + for valid_suffix in valid_suffixes { + for c in ('A'..='Z').chain('a'..='z') { + let args = ["--from=si", "--to=si", &format!("1{valid_suffix}{c}")]; + + new_ucmd!() + .args(&args) + .fails_with_code(2) + .stderr_only(format!( + "numfmt: invalid suffix in input '1{valid_suffix}{c}': '{c}'\n" + )); + } + } +} + +#[test] +fn test_long_invalid_suffix() { + let args = ["--from=si", "--to=si", "1500VVVVVVVV"]; + + new_ucmd!() + .args(&args) + .fails_with_code(2) + .stderr_only("numfmt: invalid suffix in input: '1500VVVVVVVV'\n"); +} + #[test] fn test_should_report_invalid_suffix_on_nan() { // GNU numfmt reports this one as "invalid number" @@ -273,12 +309,11 @@ fn test_should_report_invalid_suffix_on_nan() { #[test] fn test_should_report_invalid_number_with_interior_junk() { - // GNU numfmt reports this as “invalid suffix” new_ucmd!() .args(&["--from=auto"]) .pipe_in("1x0K") .fails() - .stderr_is("numfmt: invalid number: '1x0K'\n"); + .stderr_is("numfmt: invalid suffix in input: '1x0K'\n"); } #[test] @@ -535,12 +570,11 @@ fn test_delimiter_from_si() { #[test] fn test_delimiter_overrides_whitespace_separator() { - // GNU numfmt reports this as “invalid suffix” new_ucmd!() .args(&["-d,"]) .pipe_in("1 234,56") .fails() - .stderr_is("numfmt: invalid number: '1 234'\n"); + .stderr_is("numfmt: invalid suffix in input: '1 234'\n"); } #[test]