From afbab45350ccb0cc3ed117c53fd31f2d0889da8b Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Thu, 20 Mar 2025 14:35:05 +0100 Subject: [PATCH 1/2] uucore: format: Workaround BigDecimal printing bug with 0 This is a bigdecimal issue, see https://github.com/akubera/bigdecimal-rs/issues/144 . Also add a few tests, including a disabled one (our workaround is _before_ the call to format_float_decimal). --- .../src/lib/features/format/num_format.rs | 51 ++++++++++++++++++- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index d91f86fd1..256693b4a 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore bigdecimal prec +// spell-checker:ignore bigdecimal prec cppreference //! Utilities for formatting numbers in various formats use bigdecimal::BigDecimal; @@ -244,7 +244,13 @@ impl Formatter<&ExtendedBigDecimal> for Float { */ let (abs, negative) = match e { ExtendedBigDecimal::BigDecimal(bd) => { - (ExtendedBigDecimal::BigDecimal(bd.abs()), bd.is_negative()) + // Workaround printing bug in BigDecimal, force 0 to scale 0. + // TODO: Remove after https://github.com/akubera/bigdecimal-rs/issues/144 is fixed. + if bd.is_zero() { + (ExtendedBigDecimal::zero(), false) + } else { + (ExtendedBigDecimal::BigDecimal(bd.abs()), bd.is_negative()) + } } ExtendedBigDecimal::MinusZero => (ExtendedBigDecimal::zero(), true), ExtendedBigDecimal::Infinity => (ExtendedBigDecimal::Infinity, false), @@ -719,6 +725,21 @@ mod test { ); } + #[test] + #[ignore = "Need https://github.com/akubera/bigdecimal-rs/issues/144 to be fixed"] + fn decimal_float_zero() { + use super::format_float_decimal; + // We've had issues with "0e10"/"0e-10" formatting. + // TODO: Enable after https://github.com/akubera/bigdecimal-rs/issues/144 is fixed, + // as our workaround is in .fmt. + let f = |digits, scale| { + format_float_decimal(&BigDecimal::from_bigint(digits, scale), 6, ForceDecimal::No) + }; + assert_eq!(f(0.into(), 0), "0.000000"); + assert_eq!(f(0.into(), -10), "0.000000"); + assert_eq!(f(0.into(), 10), "0.000000"); + } + #[test] fn scientific_float() { use super::format_float_scientific; @@ -748,6 +769,19 @@ mod test { }; assert_eq!(f(0.0), "0.000000E+00"); assert_eq!(f(123_456.789), "1.234568E+05"); + + // Test "0e10"/"0e-10". From cppreference.com: "If the value is ​0​, the exponent is also ​0​." + let f = |digits, scale| { + format_float_scientific( + &BigDecimal::from_bigint(digits, scale), + 6, + Case::Lowercase, + ForceDecimal::No, + ) + }; + assert_eq!(f(0.into(), 0), "0.000000e+00"); + assert_eq!(f(0.into(), -10), "0.000000e+00"); + assert_eq!(f(0.into(), 10), "0.000000e+00"); } #[test] @@ -928,6 +962,19 @@ mod test { }; assert_eq!(f("0.00001"), "0xA.7C5AC4P-20"); assert_eq!(f("0.125"), "0x8.000000P-6"); + + // Test "0e10"/"0e-10". From cppreference.com: "If the value is ​0​, the exponent is also ​0​." + let f = |digits, scale| { + format_float_hexadecimal( + &BigDecimal::from_bigint(digits, scale), + 6, + Case::Lowercase, + ForceDecimal::No, + ) + }; + assert_eq!(f(0.into(), 0), "0x0.000000p+0"); + assert_eq!(f(0.into(), -10), "0x0.000000p+0"); + assert_eq!(f(0.into(), 10), "0x0.000000p+0"); } #[test] From 4cecad3e3505c32d9ba4505d7d5071bfe8e78a08 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Thu, 20 Mar 2025 18:14:16 +0100 Subject: [PATCH 2/2] uucore: format: num_format: add `fmt` function tests All the other tests directly called format_float_* functions, bypassing the additional logic in `fmt` (negative numbers, padding, etc.). This also tests the `parse` function in `mod.rs`, which calls back into `try_from_spec` here. This also makes it easier to test a lot of different format combinations without having to do end-to-end tests in `test_printf.rs`. Also add broken tests for the issues in #7509 and #7510. --- .../src/lib/features/format/num_format.rs | 168 +++++++++++++++++- 1 file changed, 166 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/format/num_format.rs b/src/uucore/src/lib/features/format/num_format.rs index 256693b4a..b1c9172d0 100644 --- a/src/uucore/src/lib/features/format/num_format.rs +++ b/src/uucore/src/lib/features/format/num_format.rs @@ -662,10 +662,12 @@ mod test { use std::str::FromStr; use crate::format::{ - ExtendedBigDecimal, - num_format::{Case, ForceDecimal}, + ExtendedBigDecimal, Format, + num_format::{Case, Float, ForceDecimal, UnsignedInt}, }; + use super::{Formatter, SignedInt}; + #[test] fn unsigned_octal() { use super::{Formatter, NumberAlignment, Prefix, UnsignedInt, UnsignedIntVariant}; @@ -1025,4 +1027,166 @@ mod test { assert_eq!(f(0.00001), "1e-05"); assert_eq!(f(0.000001), "1e-06"); } + + // Wrapper function to get a string out of Format.fmt() + fn fmt(format: &Format, n: T) -> String + where + U: Formatter, + { + let mut v = Vec::::new(); + format.fmt(&mut v, n as T).unwrap(); + String::from_utf8_lossy(&v).to_string() + } + + // Some end-to-end tests, `printf` will also test some of those but it's easier to add more + // tests here. We mostly focus on padding, negative numbers, and format specifiers that are not + // covered above. + #[test] + fn format_signed_int() { + let format = Format::::parse("%d").unwrap(); + assert_eq!(fmt(&format, 123i64), "123"); + assert_eq!(fmt(&format, -123i64), "-123"); + + let format = Format::::parse("%i").unwrap(); + assert_eq!(fmt(&format, 123i64), "123"); + assert_eq!(fmt(&format, -123i64), "-123"); + + let format = Format::::parse("%6d").unwrap(); + assert_eq!(fmt(&format, 123i64), " 123"); + assert_eq!(fmt(&format, -123i64), " -123"); + + let format = Format::::parse("%06d").unwrap(); + assert_eq!(fmt(&format, 123i64), "000123"); + assert_eq!(fmt(&format, -123i64), "-00123"); + + let format = Format::::parse("%+6d").unwrap(); + assert_eq!(fmt(&format, 123i64), " +123"); + assert_eq!(fmt(&format, -123i64), " -123"); + + let format = Format::::parse("% d").unwrap(); + assert_eq!(fmt(&format, 123i64), " 123"); + assert_eq!(fmt(&format, -123i64), "-123"); + } + + #[test] + #[ignore = "Need issue #7509 to be fixed"] + fn format_signed_int_precision_zero() { + let format = Format::::parse("%.0d").unwrap(); + assert_eq!(fmt(&format, 123i64), "123"); + // From cppreference.com: "If both the converted value and the precision are ​0​ the conversion results in no characters." + assert_eq!(fmt(&format, 0i64), ""); + } + + #[test] + fn format_unsigned_int() { + let f = |fmt_str: &str, n: u64| { + let format = Format::::parse(fmt_str).unwrap(); + fmt(&format, n) + }; + + assert_eq!(f("%u", 123u64), "123"); + assert_eq!(f("%o", 123u64), "173"); + assert_eq!(f("%#o", 123u64), "0173"); + assert_eq!(f("%6x", 123u64), " 7b"); + assert_eq!(f("%#6x", 123u64), " 0x7b"); + assert_eq!(f("%06X", 123u64), "00007B"); + assert_eq!(f("%+6u", 123u64), " 123"); // '+' is ignored for unsigned numbers. + assert_eq!(f("% u", 123u64), "123"); // ' ' is ignored for unsigned numbers. + assert_eq!(f("%#x", 0), "0"); // No prefix for 0 + } + + #[test] + #[ignore = "Need issues #7509 and #7510 to be fixed"] + fn format_unsigned_int_broken() { + // TODO: Merge this back into format_unsigned_int. + let f = |fmt_str: &str, n: u64| { + let format = Format::::parse(fmt_str).unwrap(); + fmt(&format, n) + }; + + // #7509 + assert_eq!(f("%.0o", 0), ""); + assert_eq!(f("%#0o", 0), "0"); // Already correct, but probably an accident. + assert_eq!(f("%.0x", 0), ""); + // #7510 + assert_eq!(f("%#06x", 123u64), "0x007b"); + } + + #[test] + fn format_float_decimal() { + let format = Format::::parse("%f").unwrap(); + assert_eq!(fmt(&format, &123.0.into()), "123.000000"); + assert_eq!(fmt(&format, &(-123.0).into()), "-123.000000"); + assert_eq!(fmt(&format, &123.15e-8.into()), "0.000001"); + assert_eq!(fmt(&format, &(-123.15e8).into()), "-12315000000.000000"); + let zero_exp = |exp| ExtendedBigDecimal::BigDecimal(BigDecimal::from_bigint(0.into(), exp)); + // We've had issues with "0e10"/"0e-10" formatting, and our current workaround is in Format.fmt function. + assert_eq!(fmt(&format, &zero_exp(0)), "0.000000"); + assert_eq!(fmt(&format, &zero_exp(10)), "0.000000"); + assert_eq!(fmt(&format, &zero_exp(-10)), "0.000000"); + + let format = Format::::parse("%12f").unwrap(); + assert_eq!(fmt(&format, &123.0.into()), " 123.000000"); + assert_eq!(fmt(&format, &(-123.0).into()), " -123.000000"); + assert_eq!(fmt(&format, &123.15e-8.into()), " 0.000001"); + assert_eq!(fmt(&format, &(-123.15e8).into()), "-12315000000.000000"); + assert_eq!( + fmt(&format, &(ExtendedBigDecimal::Infinity)), + " inf" + ); + assert_eq!( + fmt(&format, &(ExtendedBigDecimal::MinusInfinity)), + " -inf" + ); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::Nan)), " nan"); + assert_eq!( + fmt(&format, &(ExtendedBigDecimal::MinusNan)), + " -nan" + ); + + let format = Format::::parse("%+#.0f").unwrap(); + assert_eq!(fmt(&format, &123.0.into()), "+123."); + assert_eq!(fmt(&format, &(-123.0).into()), "-123."); + assert_eq!(fmt(&format, &123.15e-8.into()), "+0."); + assert_eq!(fmt(&format, &(-123.15e8).into()), "-12315000000."); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::Infinity)), "+inf"); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::Nan)), "+nan"); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::MinusZero)), "-0."); + + let format = Format::::parse("%#06.0f").unwrap(); + assert_eq!(fmt(&format, &123.0.into()), "00123."); + assert_eq!(fmt(&format, &(-123.0).into()), "-0123."); + assert_eq!(fmt(&format, &123.15e-8.into()), "00000."); + assert_eq!(fmt(&format, &(-123.15e8).into()), "-12315000000."); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::Infinity)), " inf"); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::MinusInfinity)), " -inf"); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::Nan)), " nan"); + assert_eq!(fmt(&format, &(ExtendedBigDecimal::MinusNan)), " -nan"); + } + + #[test] + fn format_float_others() { + let f = |fmt_str: &str, n: &ExtendedBigDecimal| { + let format = Format::::parse(fmt_str).unwrap(); + fmt(&format, n) + }; + + assert_eq!(f("%e", &(-123.0).into()), "-1.230000e+02"); + assert_eq!(f("%#09.e", &(-100.0).into()), "-001.e+02"); + assert_eq!(f("%# 9.E", &100.0.into()), " 1.E+02"); + assert_eq!(f("% 12.2A", &(-100.0).into()), " -0xC.80P+3"); + } + + #[test] + #[ignore = "Need issue #7510 to be fixed"] + fn format_float_others_broken() { + // TODO: Merge this back into format_float_others. + let f = |fmt_str: &str, n: &ExtendedBigDecimal| { + let format = Format::::parse(fmt_str).unwrap(); + fmt(&format, n) + }; + + // #7510 + assert_eq!(f("%012.2a", &(-100.0).into()), "-0x00c.80p+3"); + } }