From 9477c8e6a99d5f5dbe53c1b87016019b67884d3d Mon Sep 17 00:00:00 2001 From: Kevin Ness <46825870+nekevss@users.noreply.github.com> Date: Fri, 24 Jan 2025 22:44:11 -0600 Subject: [PATCH 1/7] Update fraction to handle unbounded fraction length --- utils/ixdtf/README.md | 12 +- utils/ixdtf/src/lib.rs | 12 +- utils/ixdtf/src/parsers/duration.rs | 25 ++- utils/ixdtf/src/parsers/mod.rs | 22 +-- utils/ixdtf/src/parsers/records.rs | 34 ++++- utils/ixdtf/src/parsers/tests.rs | 228 +++++++++++++++++++++++++--- utils/ixdtf/src/parsers/time.rs | 55 +++++-- utils/ixdtf/src/parsers/timezone.rs | 12 +- 8 files changed, 322 insertions(+), 78 deletions(-) diff --git a/utils/ixdtf/README.md b/utils/ixdtf/README.md index a0c752fcf3d..ad387e80078 100644 --- a/utils/ixdtf/README.md +++ b/utils/ixdtf/README.md @@ -20,7 +20,7 @@ RFC 9557 also updates the interpretation of `Z` from RFC 3339. ```rust use ixdtf::parsers::{ - records::{Sign, TimeZoneRecord}, + records::{Sign, TimeZoneRecord, Fraction}, IxdtfParser, }; @@ -42,7 +42,7 @@ assert_eq!(offset.sign, Sign::Negative); assert_eq!(offset.hour, 5); assert_eq!(offset.minute, 0); assert_eq!(offset.second, 0); -assert_eq!(offset.nanosecond, 0); +assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); assert!(!tz_annotation.critical); assert_eq!( tz_annotation.tz, @@ -74,7 +74,7 @@ RFC 9557 updates the interpretation of `Z` to align with `-00:00`. ```rust use ixdtf::parsers::{ - records::{Sign, TimeZoneRecord}, + records::{Sign, TimeZoneRecord, Fraction}, IxdtfParser, }; @@ -96,7 +96,7 @@ assert_eq!(offset.sign, Sign::Negative); assert_eq!(offset.hour, 0); assert_eq!(offset.minute, 0); assert_eq!(offset.second, 0); -assert_eq!(offset.nanosecond, 0); +assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); assert!(!tz_annotation.critical); assert_eq!( tz_annotation.tz, @@ -135,7 +135,7 @@ zone annotation if it is provided. ```rust use ixdtf::parsers::{ - records::{Sign, TimeZoneRecord}, + records::{Sign, TimeZoneRecord, Fraction}, IxdtfParser, }; @@ -152,7 +152,7 @@ assert_eq!(offset.sign, Sign::Negative); assert_eq!(offset.hour, 0); assert_eq!(offset.minute, 0); assert_eq!(offset.second, 0); -assert_eq!(offset.nanosecond, 0); +assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); assert!(tz_annotation.critical); assert_eq!( tz_annotation.tz, diff --git a/utils/ixdtf/src/lib.rs b/utils/ixdtf/src/lib.rs index 8e6b587244b..3e686684ad0 100644 --- a/utils/ixdtf/src/lib.rs +++ b/utils/ixdtf/src/lib.rs @@ -20,7 +20,7 @@ //! //! ``` //! use ixdtf::parsers::{ -//! records::{Sign, TimeZoneRecord}, +//! records::{Sign, TimeZoneRecord, Fraction}, //! IxdtfParser, //! }; //! @@ -42,7 +42,7 @@ //! assert_eq!(offset.hour, 5); //! assert_eq!(offset.minute, 0); //! assert_eq!(offset.second, 0); -//! assert_eq!(offset.nanosecond, 0); +//! assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); //! assert!(!tz_annotation.critical); //! assert_eq!( //! tz_annotation.tz, @@ -74,7 +74,7 @@ //! //! ```rust //! use ixdtf::parsers::{ -//! records::{Sign, TimeZoneRecord}, +//! records::{Sign, TimeZoneRecord, Fraction}, //! IxdtfParser, //! }; //! @@ -96,7 +96,7 @@ //! assert_eq!(offset.hour, 0); //! assert_eq!(offset.minute, 0); //! assert_eq!(offset.second, 0); -//! assert_eq!(offset.nanosecond, 0); +//! assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); //! assert!(!tz_annotation.critical); //! assert_eq!( //! tz_annotation.tz, @@ -135,7 +135,7 @@ //! //! ```rust //! use ixdtf::parsers::{ -//! records::{Sign, TimeZoneRecord}, +//! records::{Sign, TimeZoneRecord, Fraction}, //! IxdtfParser, //! }; //! @@ -152,7 +152,7 @@ //! assert_eq!(offset.hour, 0); //! assert_eq!(offset.minute, 0); //! assert_eq!(offset.second, 0); -//! assert_eq!(offset.nanosecond, 0); +//! assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); //! assert!(tz_annotation.critical); //! assert_eq!( //! tz_annotation.tz, diff --git a/utils/ixdtf/src/parsers/duration.rs b/utils/ixdtf/src/parsers/duration.rs index e87f2b501f6..cdadd4e55de 100644 --- a/utils/ixdtf/src/parsers/duration.rs +++ b/utils/ixdtf/src/parsers/duration.rs @@ -12,7 +12,7 @@ use crate::{ is_month_designator, is_second_designator, is_sign, is_time_designator, is_week_designator, is_year_designator, }, - records::{DateDurationRecord, DurationParseRecord, TimeDurationRecord}, + records::{DateDurationRecord, DurationParseRecord, Fraction, TimeDurationRecord}, time::parse_fraction, Cursor, }, @@ -134,7 +134,7 @@ pub(crate) fn parse_time_duration(cursor: &mut Cursor) -> ParserResult) = (0, 0, 0, None); + let mut time: (u64, u64, u64, Option) = (0, 0, 0, None); let mut previous_unit = TimeUnit::None; while cursor.check_or(false, |c| c.is_ascii_digit()) { let mut value: u64 = 0; @@ -194,20 +194,35 @@ pub(crate) fn parse_time_duration(cursor: &mut Cursor) -> ParserResult see test maximum_duration_fraction TimeUnit::Hour => Ok(Some(TimeDurationRecord::Hours { hours: time.0, - fraction: time.3.map(|f| 3600 * u64::from(f)).unwrap_or(0), + fraction: time + .3 + .map(|f| adjust_fraction_for_unit(f, 3600)) + .unwrap_or_default(), })), // Safety: Max fraction * 60 is within u64 -> see test maximum_duration_fraction TimeUnit::Minute => Ok(Some(TimeDurationRecord::Minutes { hours: time.0, minutes: time.1, - fraction: time.3.map(|f| 60 * u64::from(f)).unwrap_or(0), + fraction: time + .3 + .map(|f| adjust_fraction_for_unit(f, 60)) + .unwrap_or_default(), })), TimeUnit::Second => Ok(Some(TimeDurationRecord::Seconds { hours: time.0, minutes: time.1, seconds: time.2, - fraction: time.3.unwrap_or(0), + fraction: time.3.unwrap_or_default(), })), TimeUnit::None => Err(ParseError::abrupt_end("TimeDurationDesignator")), } } + +fn adjust_fraction_for_unit(fraction: Fraction, unit: u64) -> Fraction { + match fraction { + Fraction::Nanoseconds(d) => Fraction::Nanoseconds(unit * d), + Fraction::Picoseconds(d) => Fraction::Picoseconds(unit * d), + Fraction::Femtoseconds(d) => Fraction::Femtoseconds(unit * d), + Fraction::Truncated(d) => Fraction::Truncated(unit * d), + } +} diff --git a/utils/ixdtf/src/parsers/mod.rs b/utils/ixdtf/src/parsers/mod.rs index df21c1688b9..4fa4c088427 100644 --- a/utils/ixdtf/src/parsers/mod.rs +++ b/utils/ixdtf/src/parsers/mod.rs @@ -236,7 +236,7 @@ impl<'a> IxdtfParser<'a> { /// # Example /// /// ```rust -/// use ixdtf::parsers::{IsoDurationParser, records::{Sign, DurationParseRecord, TimeDurationRecord}}; +/// use ixdtf::parsers::{IsoDurationParser, records::{Sign, DurationParseRecord, Fraction, TimeDurationRecord}}; /// /// let duration_str = "P1Y2M1DT2H10M30S"; /// @@ -245,13 +245,13 @@ impl<'a> IxdtfParser<'a> { /// let date_duration = result.date.unwrap(); /// /// let (hours, minutes, seconds, fraction) = match result.time { -/// // Hours variant is defined as { hours: u32, fraction: u64 } +/// // Hours variant is defined as { hours: u32, fraction: Fraction } /// Some(TimeDurationRecord::Hours{ hours, fraction }) => (hours, 0, 0, fraction), -/// // Minutes variant is defined as { hours: u32, minutes: u32, fraction: u64 } +/// // Minutes variant is defined as { hours: u32, minutes: u32, fraction: Fraction } /// Some(TimeDurationRecord::Minutes{ hours, minutes, fraction }) => (hours, minutes, 0, fraction), -/// // Seconds variant is defined as { hours: u32, minutes: u32, seconds: u32, fraction: u32 } -/// Some(TimeDurationRecord::Seconds{ hours, minutes, seconds, fraction }) => (hours, minutes, seconds, fraction as u64), -/// None => (0,0,0,0), +/// // Seconds variant is defined as { hours: u32, minutes: u32, seconds: u32, fraction: Fraction } +/// Some(TimeDurationRecord::Seconds{ hours, minutes, seconds, fraction }) => (hours, minutes, seconds, fraction), +/// None => (0,0,0, Fraction::Nanoseconds(0)), /// }; /// /// assert_eq!(result.sign, Sign::Positive); @@ -262,7 +262,7 @@ impl<'a> IxdtfParser<'a> { /// assert_eq!(hours, 2); /// assert_eq!(minutes, 10); /// assert_eq!(seconds, 30); -/// assert_eq!(fraction, 0); +/// assert_eq!(fraction, Fraction::Nanoseconds(0)); /// ``` #[cfg(feature = "duration")] #[derive(Debug)] @@ -313,7 +313,7 @@ impl<'a> IsoDurationParser<'a> { /// ## Parsing a time duration /// /// ```rust - /// # use ixdtf::parsers::{IsoDurationParser, records::{DurationParseRecord, TimeDurationRecord }}; + /// # use ixdtf::parsers::{IsoDurationParser, records::{DurationParseRecord, Fraction, TimeDurationRecord }}; /// let time_duration = "PT2H10M30S"; /// /// let result = IsoDurationParser::from_str(time_duration).parse().unwrap(); @@ -324,14 +324,14 @@ impl<'a> IsoDurationParser<'a> { /// // Minutes variant is defined as { hours: u32, minutes: u32, fraction: u64 } /// Some(TimeDurationRecord::Minutes{ hours, minutes, fraction }) => (hours, minutes, 0, fraction), /// // Seconds variant is defined as { hours: u32, minutes: u32, seconds: u32, fraction: u32 } - /// Some(TimeDurationRecord::Seconds{ hours, minutes, seconds, fraction }) => (hours, minutes, seconds, fraction as u64), - /// None => (0,0,0,0), + /// Some(TimeDurationRecord::Seconds{ hours, minutes, seconds, fraction }) => (hours, minutes, seconds, fraction), + /// None => (0,0,0,Fraction::Nanoseconds(0)), /// }; /// assert!(result.date.is_none()); /// assert_eq!(hours, 2); /// assert_eq!(minutes, 10); /// assert_eq!(seconds, 30); - /// assert_eq!(fraction, 0); + /// assert_eq!(fraction, Fraction::Nanoseconds(0)); /// ``` pub fn parse(&mut self) -> ParserResult { duration::parse_duration(&mut self.cursor) diff --git a/utils/ixdtf/src/parsers/records.rs b/utils/ixdtf/src/parsers/records.rs index 8377b5f97c8..d59608b5c8a 100644 --- a/utils/ixdtf/src/parsers/records.rs +++ b/utils/ixdtf/src/parsers/records.rs @@ -55,7 +55,7 @@ pub struct TimeRecord { /// A second value. pub second: u8, /// A nanosecond value representing all sub-second components. - pub nanosecond: u32, + pub fraction: Fraction, } /// A `TimeZoneAnnotation` that represents a parsed `TimeZoneRecord` and its critical flag. @@ -111,7 +111,7 @@ pub struct UtcOffsetRecord { /// The second value of the `UtcOffsetRecord`. pub second: u8, /// Any nanosecond value of the `UTCOffsetRecord`. - pub nanosecond: u32, + pub fraction: Fraction, } impl UtcOffsetRecord { @@ -122,7 +122,7 @@ impl UtcOffsetRecord { hour: 0, minute: 0, second: 0, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), } } } @@ -144,7 +144,7 @@ impl UtcOffsetRecordOrZ { hour: 0, minute: 0, second: 0, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }, } } @@ -191,7 +191,7 @@ pub enum TimeDurationRecord { /// Hours value. hours: u64, /// The parsed fraction value in nanoseconds. - fraction: u64, + fraction: Fraction, }, // A Minutes Time duration record. Minutes { @@ -200,7 +200,7 @@ pub enum TimeDurationRecord { /// Minutes value. minutes: u64, /// The parsed fraction value in nanoseconds. - fraction: u64, + fraction: Fraction, }, // A Seconds Time duration record. Seconds { @@ -211,6 +211,26 @@ pub enum TimeDurationRecord { /// Seconds value. seconds: u64, /// The parsed fraction value in nanoseconds. - fraction: u32, + fraction: Fraction, }, } + +/// A fraction value in nanoseconds or lower value. +#[derive(Debug, Clone, Copy, PartialEq)] +#[allow(clippy::exhaustive_enums)] // Parsed fraction must be one of the four provided options. +pub enum Fraction { + /// Parsed nanoseconds value (A fraction value from 1-9 digits length) + Nanoseconds(u64), + /// Parsed picoseconds value (A fraction value from 10-12 digits length) + Picoseconds(u64), + /// Parsed femtoseconds value (A fraction value from 12-15 digits length) + Femtoseconds(u64), + /// A parsed value truncated to nanoseconds (A fraction value of 15+ digits length) + Truncated(u64), // An unbound fraction value truncated to nanoseconds +} + +impl Default for Fraction { + fn default() -> Self { + Self::Nanoseconds(0) + } +} diff --git a/utils/ixdtf/src/parsers/tests.rs b/utils/ixdtf/src/parsers/tests.rs index 6336e0a3f1f..11d26a56a67 100644 --- a/utils/ixdtf/src/parsers/tests.rs +++ b/utils/ixdtf/src/parsers/tests.rs @@ -8,7 +8,7 @@ use alloc::vec::Vec; use crate::{ parsers::{ records::{ - Annotation, DateRecord, IxdtfParseRecord, TimeRecord, TimeZoneAnnotation, + Annotation, DateRecord, Fraction, IxdtfParseRecord, TimeRecord, TimeZoneAnnotation, TimeZoneRecord, UtcOffsetRecordOrZ, }, IxdtfParser, @@ -46,7 +46,7 @@ fn temporal_date_time_max() { hour: 12, minute: 28, second: 32, - nanosecond: 329402834, + fraction: Fraction::Nanoseconds(329402834), }) ); } @@ -573,7 +573,7 @@ fn temporal_duration_parsing() { hours: 1, minutes: 1, seconds: 1, - fraction: 123456789 + fraction: Fraction::Nanoseconds(123456789) }) }, "Failing to parse a valid Duration string: \"{}\" should pass.", @@ -593,7 +593,7 @@ fn temporal_duration_parsing() { }), time: Some(TimeDurationRecord::Hours { hours: 0, - fraction: 1_800_000_000_000, + fraction: Fraction::Nanoseconds(1_800_000_000_000), }) } ); @@ -604,12 +604,7 @@ fn temporal_duration_parsing() { fn temporal_invalid_durations() { use crate::parsers::IsoDurationParser; - let invalids = [ - "P1Y1M1W0,5D", - "P1Y1M1W1DT1H1M1.123456789123S", - "+PT", - "P1Y1M1W1DT1H0.5M0.5S", - ]; + let invalids = ["P1Y1M1W0,5D", "+PT", "P1Y1M1W1DT1H0.5M0.5S"]; for test in invalids { let err = IsoDurationParser::from_str(test).parse(); @@ -638,6 +633,29 @@ fn maximum_duration_fraction() { assert!(result.is_ok()); } +#[test] +#[cfg(feature = "duration")] +fn duration_fraction_extended() { + use crate::parsers::{ + records::{DurationParseRecord, Sign, TimeDurationRecord}, + IsoDurationParser, + }; + let test = "PT1H1.123456789123M"; + let result = IsoDurationParser::from_str(test).parse(); + assert_eq!( + result, + Ok(DurationParseRecord { + sign: Sign::Positive, + date: None, + time: Some(TimeDurationRecord::Minutes { + hours: 1, + minutes: 1, + fraction: Fraction::Picoseconds(7_407_407_347_380) + }) + }) + ); +} + #[test] #[cfg(feature = "duration")] fn duration_exceeds_range() { @@ -781,7 +799,7 @@ fn test_correct_datetime() { hour: 4, minute: 0, second: 0, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }), offset: None, tz: None, @@ -803,7 +821,7 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 0, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }), offset: None, tz: None, @@ -825,7 +843,7 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 22, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }), offset: None, tz: None, @@ -847,7 +865,7 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 22, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }), offset: None, tz: None, @@ -869,7 +887,7 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 22, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }), offset: None, tz: None, @@ -974,7 +992,7 @@ fn test_zulu_offset() { hour: 14, minute: 0, second: 0, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }), offset: Some(crate::parsers::records::UtcOffsetRecordOrZ::Z), tz: Some(TimeZoneAnnotation { @@ -999,7 +1017,7 @@ fn test_zulu_offset() { hour: 14, minute: 0, second: 0, - nanosecond: 0, + fraction: Fraction::Nanoseconds(0), }), offset: Some(UtcOffsetRecordOrZ::Z), tz: None, @@ -1011,6 +1029,10 @@ fn test_zulu_offset() { // Examples referenced from #[test] fn subsecond_string_tests() { + let hanging_subsecond_start = "15:23:30."; + let err = IxdtfParser::from_str(hanging_subsecond_start).parse_time(); + assert!(err.is_err()); + let subsecond_time = "2025-01-15T15:23:30.1"; let result = IxdtfParser::from_str(subsecond_time).parse(); assert_eq!( @@ -1025,7 +1047,7 @@ fn subsecond_string_tests() { hour: 15, minute: 23, second: 30, - nanosecond: 100_000_000, + fraction: Fraction::Nanoseconds(100_000_000), }), offset: None, tz: None, @@ -1047,7 +1069,7 @@ fn subsecond_string_tests() { hour: 15, minute: 23, second: 30, - nanosecond: 123_456_780, + fraction: Fraction::Nanoseconds(123_456_780), }), offset: None, tz: None, @@ -1069,7 +1091,152 @@ fn subsecond_string_tests() { hour: 15, minute: 23, second: 30, - nanosecond: 123_456_789, + fraction: Fraction::Nanoseconds(123_456_789), + }), + offset: None, + tz: None, + calendar: None, + }) + ); +} + +#[test] +fn subseconds_parsing_extended_nanoseconds() { + // Test nanoseconds unbalanced + let subsecond_time = "15:23:30.1234567"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Nanoseconds(123_456_700), + }), + offset: None, + tz: None, + calendar: None, + }) + ); + + // Test nanoseconds + let subsecond_time = "15:23:30.123456789"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Nanoseconds(123_456_789), + }), + offset: None, + tz: None, + calendar: None, + }) + ); +} + +#[test] +fn subseconds_parsing_extended_picoseconds() { + // Test picoseconds unbalanced + let subsecond_time = "15:23:30.1234567890"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Picoseconds(123_456_789_000), + }), + offset: None, + tz: None, + calendar: None, + }) + ); + + // Test picoseconds + let subsecond_time = "15:23:30.123456789876"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Picoseconds(123_456_789_876), + }), + offset: None, + tz: None, + calendar: None, + }) + ); +} + +#[test] +fn subseconds_parsing_extended_femtoseconds() { + // Test femtoseconds unbalanced + let subsecond_time = "15:23:30.1234567898765"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Femtoseconds(123_456_789_876_500), + }), + offset: None, + tz: None, + calendar: None, + }) + ); + + // Test femtoseconds + let subsecond_time = "15:23:30.123456789876543"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Femtoseconds(123_456_789_876_543), + }), + offset: None, + tz: None, + calendar: None, + }) + ); +} + +#[test] +fn subseconds_parsing_extended_truncated() { + // Test truncated unbalanced + let subsecond_time = "15:23:30.1234567898765432"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Truncated(123_456_789), }), offset: None, tz: None, @@ -1077,7 +1244,22 @@ fn subsecond_string_tests() { }) ); - let subsecond_time = "1976-11-18T15:23:30.1234567890"; - let err = IxdtfParser::from_str(subsecond_time).parse(); - assert_eq!(err, Err(ParseError::FractionPart)); + // Test truncation + let subsecond_time = "15:23:30.1234567898765432101234567890987654321"; + let result = IxdtfParser::from_str(subsecond_time).parse_time(); + assert_eq!( + result, + Ok(IxdtfParseRecord { + date: None, + time: Some(TimeRecord { + hour: 15, + minute: 23, + second: 30, + fraction: Fraction::Truncated(123_456_789), + }), + offset: None, + tz: None, + calendar: None, + }) + ); } diff --git a/utils/ixdtf/src/parsers/time.rs b/utils/ixdtf/src/parsers/time.rs index bf6f537956b..3edb0502477 100644 --- a/utils/ixdtf/src/parsers/time.rs +++ b/utils/ixdtf/src/parsers/time.rs @@ -18,7 +18,10 @@ use crate::{ ParseError, ParserResult, }; -use super::{annotations, records::IxdtfParseRecord}; +use super::{ + annotations, + records::{Fraction, IxdtfParseRecord}, +}; /// Parse annotated time record is silently fallible returning None in the case that the /// value does not align @@ -75,7 +78,7 @@ pub(crate) fn parse_time_record(cursor: &mut Cursor) -> ParserResult hour, minute: 0, second: 0, - nanosecond: 0, + fraction: Fraction::default(), }); } @@ -89,7 +92,7 @@ pub(crate) fn parse_time_record(cursor: &mut Cursor) -> ParserResult hour, minute, second: 0, - nanosecond: 0, + fraction: Fraction::default(), }); } @@ -99,13 +102,13 @@ pub(crate) fn parse_time_record(cursor: &mut Cursor) -> ParserResult let second = parse_minute_second(cursor, true)?; - let nanosecond = parse_fraction(cursor)?.unwrap_or(0); + let fraction = parse_fraction(cursor)?.unwrap_or_default(); Ok(TimeRecord { hour, minute, second, - nanosecond, + fraction, }) } @@ -141,7 +144,7 @@ pub(crate) fn parse_minute_second(cursor: &mut Cursor, is_second: bool) -> Parse /// This is primarily used in ISO8601 to add percision past /// a second. #[inline] -pub(crate) fn parse_fraction(cursor: &mut Cursor) -> ParserResult> { +pub(crate) fn parse_fraction(cursor: &mut Cursor) -> ParserResult> { // Assert that the first char provided is a decimal separator. if !cursor.check_or(false, is_decimal_separator) { return Ok(None); @@ -151,18 +154,40 @@ pub(crate) fn parse_fraction(cursor: &mut Cursor) -> ParserResult> { let mut result = 0; let mut fraction_len = 0; while cursor.check_or(false, |ch| ch.is_ascii_digit()) { - if fraction_len >= 9 { - return Err(ParseError::FractionPart); + let next_value = u64::from(cursor.next_digit()?.ok_or(ParseError::ImplAssert)?); + if fraction_len < 15 { + result = result * 10 + next_value; } - result = result * 10 + u32::from(cursor.next_digit()?.ok_or(ParseError::FractionPart)?); fraction_len += 1; } - // Assert: 10^9-1 should always be a valid u32. - let result = result - * 10u32 - .checked_pow(9 - fraction_len) - .ok_or(ParseError::ImplAssert)?; + if fraction_len == 0 { + return Err(ParseError::FractionPart); + } + + let result = if fraction_len <= 9 { + // Assert: 10^9-1 should always be a valid u32. + let result = result + * 10u64 + .checked_pow(9 - fraction_len) + .ok_or(ParseError::ImplAssert)?; + Some(Fraction::Nanoseconds(result)) + } else if fraction_len <= 12 { + let result = result + * 10u64 + .checked_pow(12 - fraction_len) + .ok_or(ParseError::ImplAssert)?; + Some(Fraction::Picoseconds(result)) + } else if fraction_len <= 15 { + let result = result + * 10u64 + .checked_pow(15 - fraction_len) + .ok_or(ParseError::ImplAssert)?; + Some(Fraction::Femtoseconds(result)) + } else { + let result = result / 1_000_000; + Some(Fraction::Truncated(result)) + }; - Ok(Some(result)) + Ok(result) } diff --git a/utils/ixdtf/src/parsers/timezone.rs b/utils/ixdtf/src/parsers/timezone.rs index 308ff1c3c5e..f4d9ac43882 100644 --- a/utils/ixdtf/src/parsers/timezone.rs +++ b/utils/ixdtf/src/parsers/timezone.rs @@ -10,7 +10,9 @@ use super::{ is_annotation_key_value_separator, is_annotation_open, is_critical_flag, is_sign, is_time_separator, is_tz_char, is_tz_leading_char, is_tz_name_separator, is_utc_designator, }, - records::{Sign, TimeZoneAnnotation, TimeZoneRecord, UtcOffsetRecord, UtcOffsetRecordOrZ}, + records::{ + Fraction, Sign, TimeZoneAnnotation, TimeZoneRecord, UtcOffsetRecord, UtcOffsetRecordOrZ, + }, time::{parse_fraction, parse_hour, parse_minute_second}, Cursor, }; @@ -155,9 +157,9 @@ pub(crate) fn parse_date_time_utc(cursor: &mut Cursor) -> ParserResult Date: Sat, 25 Jan 2025 00:39:37 -0600 Subject: [PATCH 2/7] Add support for Fraction to icu_timezone --- components/timezone/src/ixdtf.rs | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/components/timezone/src/ixdtf.rs b/components/timezone/src/ixdtf.rs index afb2049b73f..deecc4e301e 100644 --- a/components/timezone/src/ixdtf.rs +++ b/components/timezone/src/ixdtf.rs @@ -15,7 +15,7 @@ use icu_provider::prelude::*; use ixdtf::{ parsers::{ records::{ - DateRecord, IxdtfParseRecord, TimeRecord, TimeZoneAnnotation, TimeZoneRecord, + DateRecord, Fraction, IxdtfParseRecord, TimeRecord, TimeZoneAnnotation, TimeZoneRecord, UtcOffsetRecord, UtcOffsetRecordOrZ, }, IxdtfParser, @@ -326,11 +326,15 @@ impl<'a> Intermediate<'a> { }; let time_zone_id = mapper.iana_bytes_to_bcp47(iana_identifier); let date = Date::::try_new_iso(self.date.year, self.date.month, self.date.day)?; + let nanosecond = match self.time.fraction { + Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), + _ => Err(IxdtfParseError::FractionPart), + }?; let time = Time::try_new( self.time.hour, self.time.minute, self.time.second, - self.time.nanosecond, + nanosecond, )?; let offset = match time_zone_id.as_str() { "utc" | "gmt" => Some(UtcOffset::zero()), @@ -367,11 +371,15 @@ impl<'a> Intermediate<'a> { }, }; let date = Date::::try_new_iso(self.date.year, self.date.month, self.date.day)?; + let nanosecond = match self.time.fraction { + Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), + _ => Err(IxdtfParseError::FractionPart), + }?; let time = Time::try_new( self.time.hour, self.time.minute, self.time.second, - self.time.nanosecond, + nanosecond, )?; Ok(time_zone_id.with_offset(offset).at_time((date, time))) } @@ -389,11 +397,15 @@ impl<'a> Intermediate<'a> { }; let time_zone_id = mapper.iana_bytes_to_bcp47(iana_identifier); let date = Date::try_new_iso(self.date.year, self.date.month, self.date.day)?; + let nanosecond = match self.time.fraction { + Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), + _ => Err(IxdtfParseError::FractionPart), + }?; let time = Time::try_new( self.time.hour, self.time.minute, self.time.second, - self.time.nanosecond, + nanosecond, )?; let offset = UtcOffset::try_from_utc_offset_record(offset)?; let zone_variant = match zone_offset_calculator @@ -792,11 +804,15 @@ impl Time { fn try_from_ixdtf_record(ixdtf_record: &IxdtfParseRecord) -> Result { let time_record = ixdtf_record.time.ok_or(ParseError::MissingFields)?; + let nanosecond = match time_record.fraction { + Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), + _ => Err(IxdtfParseError::FractionPart), + }?; let time = Self::try_new( time_record.hour, time_record.minute, time_record.second, - time_record.nanosecond, + nanosecond, )?; Ok(time) } From 6192c3ef6dbc81bd7841b6f669b65ef58185556e Mon Sep 17 00:00:00 2001 From: Kevin Ness <46825870+nekevss@users.noreply.github.com> Date: Mon, 27 Jan 2025 00:17:31 -0600 Subject: [PATCH 3/7] Update fraction according to feedback --- components/timezone/src/ixdtf.rs | 32 ++++---- utils/ixdtf/README.md | 6 +- utils/ixdtf/src/error.rs | 2 + utils/ixdtf/src/lib.rs | 6 +- utils/ixdtf/src/parsers/duration.rs | 18 +++-- utils/ixdtf/src/parsers/mod.rs | 8 +- utils/ixdtf/src/parsers/records.rs | 59 ++++++++++----- utils/ixdtf/src/parsers/tests.rs | 110 ++++++++++++++++++++++------ utils/ixdtf/src/parsers/time.rs | 38 ++-------- utils/ixdtf/src/parsers/timezone.rs | 4 +- 10 files changed, 176 insertions(+), 107 deletions(-) diff --git a/components/timezone/src/ixdtf.rs b/components/timezone/src/ixdtf.rs index deecc4e301e..65d92c23308 100644 --- a/components/timezone/src/ixdtf.rs +++ b/components/timezone/src/ixdtf.rs @@ -15,7 +15,7 @@ use icu_provider::prelude::*; use ixdtf::{ parsers::{ records::{ - DateRecord, Fraction, IxdtfParseRecord, TimeRecord, TimeZoneAnnotation, TimeZoneRecord, + DateRecord, IxdtfParseRecord, TimeRecord, TimeZoneAnnotation, TimeZoneRecord, UtcOffsetRecord, UtcOffsetRecordOrZ, }, IxdtfParser, @@ -42,6 +42,8 @@ pub enum ParseError { InconsistentTimeZoneOffsets, /// There was an invalid Offset. InvalidOffsetError, + /// There was an invalid subsecond fraction. + InvalidFraction, /// The set of time zone fields was not expected for the given type. /// For example, if a named time zone was present with offset-only parsing, /// or an offset was present with named-time-zone-only parsing. @@ -326,10 +328,9 @@ impl<'a> Intermediate<'a> { }; let time_zone_id = mapper.iana_bytes_to_bcp47(iana_identifier); let date = Date::::try_new_iso(self.date.year, self.date.month, self.date.day)?; - let nanosecond = match self.time.fraction { - Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), - _ => Err(IxdtfParseError::FractionPart), - }?; + let Some(nanosecond) = self.time.fraction.to_nanoseconds() else { + return Err(ParseError::InvalidFraction); + }; let time = Time::try_new( self.time.hour, self.time.minute, @@ -371,10 +372,9 @@ impl<'a> Intermediate<'a> { }, }; let date = Date::::try_new_iso(self.date.year, self.date.month, self.date.day)?; - let nanosecond = match self.time.fraction { - Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), - _ => Err(IxdtfParseError::FractionPart), - }?; + let Some(nanosecond) = self.time.fraction.to_nanoseconds() else { + return Err(ParseError::InvalidFraction); + }; let time = Time::try_new( self.time.hour, self.time.minute, @@ -397,10 +397,9 @@ impl<'a> Intermediate<'a> { }; let time_zone_id = mapper.iana_bytes_to_bcp47(iana_identifier); let date = Date::try_new_iso(self.date.year, self.date.month, self.date.day)?; - let nanosecond = match self.time.fraction { - Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), - _ => Err(IxdtfParseError::FractionPart), - }?; + let Some(nanosecond) = self.time.fraction.to_nanoseconds() else { + return Err(ParseError::InvalidFraction); + }; let time = Time::try_new( self.time.hour, self.time.minute, @@ -804,10 +803,9 @@ impl Time { fn try_from_ixdtf_record(ixdtf_record: &IxdtfParseRecord) -> Result { let time_record = ixdtf_record.time.ok_or(ParseError::MissingFields)?; - let nanosecond = match time_record.fraction { - Fraction::Nanoseconds(n) => u32::try_from(n).map_err(|_| IxdtfParseError::FractionPart), - _ => Err(IxdtfParseError::FractionPart), - }?; + let Some(nanosecond) = time_record.fraction.to_nanoseconds() else { + return Err(ParseError::InvalidFraction); + }; let time = Self::try_new( time_record.hour, time_record.minute, diff --git a/utils/ixdtf/README.md b/utils/ixdtf/README.md index ad387e80078..00985758ae2 100644 --- a/utils/ixdtf/README.md +++ b/utils/ixdtf/README.md @@ -42,7 +42,7 @@ assert_eq!(offset.sign, Sign::Negative); assert_eq!(offset.hour, 5); assert_eq!(offset.minute, 0); assert_eq!(offset.second, 0); -assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); +assert_eq!(offset.fraction, Fraction { digits: 0, value: 0}); assert!(!tz_annotation.critical); assert_eq!( tz_annotation.tz, @@ -96,7 +96,7 @@ assert_eq!(offset.sign, Sign::Negative); assert_eq!(offset.hour, 0); assert_eq!(offset.minute, 0); assert_eq!(offset.second, 0); -assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); +assert_eq!(offset.fraction, Fraction { digits: 0, value: 0}); assert!(!tz_annotation.critical); assert_eq!( tz_annotation.tz, @@ -152,7 +152,7 @@ assert_eq!(offset.sign, Sign::Negative); assert_eq!(offset.hour, 0); assert_eq!(offset.minute, 0); assert_eq!(offset.second, 0); -assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); +assert_eq!(offset.fraction, Fraction { digits: 0, value: 0}); assert!(tz_annotation.critical); assert_eq!( tz_annotation.tz, diff --git a/utils/ixdtf/src/error.rs b/utils/ixdtf/src/error.rs index 0efafdfbe74..196dfc3b942 100644 --- a/utils/ixdtf/src/error.rs +++ b/utils/ixdtf/src/error.rs @@ -46,6 +46,8 @@ pub enum ParseError { TimeSecond, #[displaydoc("Invalid character while parsing fraction part value.")] FractionPart, + #[displaydoc("Fraction part value exceeds a representable range.")] + InvalidFractionRange, #[displaydoc("Invalid character while parsing date separator.")] DateSeparator, #[displaydoc("Invalid character while parsing time separator.")] diff --git a/utils/ixdtf/src/lib.rs b/utils/ixdtf/src/lib.rs index 3e686684ad0..b77baffa5e8 100644 --- a/utils/ixdtf/src/lib.rs +++ b/utils/ixdtf/src/lib.rs @@ -42,7 +42,7 @@ //! assert_eq!(offset.hour, 5); //! assert_eq!(offset.minute, 0); //! assert_eq!(offset.second, 0); -//! assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); +//! assert_eq!(offset.fraction, Fraction { digits: 0, value: 0}); //! assert!(!tz_annotation.critical); //! assert_eq!( //! tz_annotation.tz, @@ -96,7 +96,7 @@ //! assert_eq!(offset.hour, 0); //! assert_eq!(offset.minute, 0); //! assert_eq!(offset.second, 0); -//! assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); +//! assert_eq!(offset.fraction, Fraction { digits: 0, value: 0}); //! assert!(!tz_annotation.critical); //! assert_eq!( //! tz_annotation.tz, @@ -152,7 +152,7 @@ //! assert_eq!(offset.hour, 0); //! assert_eq!(offset.minute, 0); //! assert_eq!(offset.second, 0); -//! assert_eq!(offset.fraction, Fraction::Nanoseconds(0)); +//! assert_eq!(offset.fraction, Fraction { digits: 0, value: 0}); //! assert!(tz_annotation.critical); //! assert_eq!( //! tz_annotation.tz, diff --git a/utils/ixdtf/src/parsers/duration.rs b/utils/ixdtf/src/parsers/duration.rs index cdadd4e55de..58a237a0ae2 100644 --- a/utils/ixdtf/src/parsers/duration.rs +++ b/utils/ixdtf/src/parsers/duration.rs @@ -197,6 +197,7 @@ pub(crate) fn parse_time_duration(cursor: &mut Cursor) -> ParserResult see test maximum_duration_fraction @@ -206,6 +207,7 @@ pub(crate) fn parse_time_duration(cursor: &mut Cursor) -> ParserResult Ok(Some(TimeDurationRecord::Seconds { @@ -218,11 +220,13 @@ pub(crate) fn parse_time_duration(cursor: &mut Cursor) -> ParserResult Fraction { - match fraction { - Fraction::Nanoseconds(d) => Fraction::Nanoseconds(unit * d), - Fraction::Picoseconds(d) => Fraction::Picoseconds(unit * d), - Fraction::Femtoseconds(d) => Fraction::Femtoseconds(unit * d), - Fraction::Truncated(d) => Fraction::Truncated(unit * d), - } +fn adjust_fraction_for_unit(fraction: Fraction, unit: u64) -> ParserResult { + let value = fraction + .value + .checked_mul(unit) + .ok_or(ParseError::InvalidFractionRange)?; + Ok(Fraction { + digits: fraction.digits, + value, + }) } diff --git a/utils/ixdtf/src/parsers/mod.rs b/utils/ixdtf/src/parsers/mod.rs index 4fa4c088427..2b7ea8e18b7 100644 --- a/utils/ixdtf/src/parsers/mod.rs +++ b/utils/ixdtf/src/parsers/mod.rs @@ -251,7 +251,7 @@ impl<'a> IxdtfParser<'a> { /// Some(TimeDurationRecord::Minutes{ hours, minutes, fraction }) => (hours, minutes, 0, fraction), /// // Seconds variant is defined as { hours: u32, minutes: u32, seconds: u32, fraction: Fraction } /// Some(TimeDurationRecord::Seconds{ hours, minutes, seconds, fraction }) => (hours, minutes, seconds, fraction), -/// None => (0,0,0, Fraction::Nanoseconds(0)), +/// None => (0,0,0, Fraction::default()), /// }; /// /// assert_eq!(result.sign, Sign::Positive); @@ -262,7 +262,7 @@ impl<'a> IxdtfParser<'a> { /// assert_eq!(hours, 2); /// assert_eq!(minutes, 10); /// assert_eq!(seconds, 30); -/// assert_eq!(fraction, Fraction::Nanoseconds(0)); +/// assert_eq!(fraction, Fraction { digits: 0, value: 0 }); /// ``` #[cfg(feature = "duration")] #[derive(Debug)] @@ -325,13 +325,13 @@ impl<'a> IsoDurationParser<'a> { /// Some(TimeDurationRecord::Minutes{ hours, minutes, fraction }) => (hours, minutes, 0, fraction), /// // Seconds variant is defined as { hours: u32, minutes: u32, seconds: u32, fraction: u32 } /// Some(TimeDurationRecord::Seconds{ hours, minutes, seconds, fraction }) => (hours, minutes, seconds, fraction), - /// None => (0,0,0,Fraction::Nanoseconds(0)), + /// None => (0,0,0,Fraction::default()), /// }; /// assert!(result.date.is_none()); /// assert_eq!(hours, 2); /// assert_eq!(minutes, 10); /// assert_eq!(seconds, 30); - /// assert_eq!(fraction, Fraction::Nanoseconds(0)); + /// assert_eq!(fraction, Fraction { digits: 0, value: 0 }); /// ``` pub fn parse(&mut self) -> ParserResult { duration::parse_duration(&mut self.cursor) diff --git a/utils/ixdtf/src/parsers/records.rs b/utils/ixdtf/src/parsers/records.rs index d59608b5c8a..48ed5621124 100644 --- a/utils/ixdtf/src/parsers/records.rs +++ b/utils/ixdtf/src/parsers/records.rs @@ -122,7 +122,10 @@ impl UtcOffsetRecord { hour: 0, minute: 0, second: 0, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 0, + value: 0, + }, } } } @@ -144,7 +147,7 @@ impl UtcOffsetRecordOrZ { hour: 0, minute: 0, second: 0, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction::default(), }, } } @@ -216,21 +219,41 @@ pub enum TimeDurationRecord { } /// A fraction value in nanoseconds or lower value. -#[derive(Debug, Clone, Copy, PartialEq)] -#[allow(clippy::exhaustive_enums)] // Parsed fraction must be one of the four provided options. -pub enum Fraction { - /// Parsed nanoseconds value (A fraction value from 1-9 digits length) - Nanoseconds(u64), - /// Parsed picoseconds value (A fraction value from 10-12 digits length) - Picoseconds(u64), - /// Parsed femtoseconds value (A fraction value from 12-15 digits length) - Femtoseconds(u64), - /// A parsed value truncated to nanoseconds (A fraction value of 15+ digits length) - Truncated(u64), // An unbound fraction value truncated to nanoseconds -} - -impl Default for Fraction { - fn default() -> Self { - Self::Nanoseconds(0) +/// +/// # Precision note +/// +/// `ixdtf` parses a fraction value to a precision of 18 digits of precision, but +/// preserves the fraction's digit length +#[derive(Debug, Default, Clone, Copy, PartialEq)] +#[allow(clippy::exhaustive_structs)] // A fraction is only a value and its digit length. +pub struct Fraction { + // The count of fraction digits, i.e. the fraction's digit length. + pub digits: u8, + // The parsed fraction value. + pub value: u64, +} + +impl Fraction { + /// Returns Some(`u32`) representing the `Fraction` as it's computed + /// nanosecond value or `None` if the digits exceeds 9 digits. + pub fn to_nanoseconds(&self) -> Option { + if self.digits <= 9 { + 10u32 + .checked_pow(9 - u32::from(self.digits)) + .map(|x| x * self.value as u32) + } else { + None + } + } + + /// Returns a `u64` representing the `Fraction` as it's computed + /// nanosecond value, truncating any value beyond 9 digits to + /// nanoseconds + pub fn to_truncated_nanoseconds(&self) -> u64 { + if self.digits <= 9 { + self.value + } else { + self.value / 10u64.pow(u32::from(self.digits - 9)) + } } } diff --git a/utils/ixdtf/src/parsers/tests.rs b/utils/ixdtf/src/parsers/tests.rs index 11d26a56a67..12fd81f5345 100644 --- a/utils/ixdtf/src/parsers/tests.rs +++ b/utils/ixdtf/src/parsers/tests.rs @@ -46,7 +46,10 @@ fn temporal_date_time_max() { hour: 12, minute: 28, second: 32, - fraction: Fraction::Nanoseconds(329402834), + fraction: Fraction { + digits: 9, + value: 329402834 + } }) ); } @@ -573,7 +576,10 @@ fn temporal_duration_parsing() { hours: 1, minutes: 1, seconds: 1, - fraction: Fraction::Nanoseconds(123456789) + fraction: Fraction { + digits: 9, + value: 123456789, + } }) }, "Failing to parse a valid Duration string: \"{}\" should pass.", @@ -593,7 +599,10 @@ fn temporal_duration_parsing() { }), time: Some(TimeDurationRecord::Hours { hours: 0, - fraction: Fraction::Nanoseconds(1_800_000_000_000), + fraction: Fraction { + digits: 1, + value: 18_000 + }, }) } ); @@ -650,7 +659,10 @@ fn duration_fraction_extended() { time: Some(TimeDurationRecord::Minutes { hours: 1, minutes: 1, - fraction: Fraction::Picoseconds(7_407_407_347_380) + fraction: Fraction { + digits: 12, + value: 7_407_407_347_380, + } }) }) ); @@ -799,7 +811,10 @@ fn test_correct_datetime() { hour: 4, minute: 0, second: 0, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 0, + value: 0 + }, }), offset: None, tz: None, @@ -821,7 +836,10 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 0, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 0, + value: 0 + }, }), offset: None, tz: None, @@ -843,7 +861,10 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 22, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 0, + value: 0 + }, }), offset: None, tz: None, @@ -865,7 +886,10 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 22, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 3, + value: 0 + }, }), offset: None, tz: None, @@ -887,7 +911,10 @@ fn test_correct_datetime() { hour: 4, minute: 34, second: 22, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 3, + value: 0 + }, }), offset: None, tz: None, @@ -992,7 +1019,10 @@ fn test_zulu_offset() { hour: 14, minute: 0, second: 0, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 0, + value: 0 + }, }), offset: Some(crate::parsers::records::UtcOffsetRecordOrZ::Z), tz: Some(TimeZoneAnnotation { @@ -1017,7 +1047,10 @@ fn test_zulu_offset() { hour: 14, minute: 0, second: 0, - fraction: Fraction::Nanoseconds(0), + fraction: Fraction { + digits: 0, + value: 0 + }, }), offset: Some(UtcOffsetRecordOrZ::Z), tz: None, @@ -1047,7 +1080,10 @@ fn subsecond_string_tests() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Nanoseconds(100_000_000), + fraction: Fraction { + digits: 1, + value: 1, + }, }), offset: None, tz: None, @@ -1069,7 +1105,10 @@ fn subsecond_string_tests() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Nanoseconds(123_456_780), + fraction: Fraction { + digits: 8, + value: 12_345_678 + }, }), offset: None, tz: None, @@ -1091,7 +1130,10 @@ fn subsecond_string_tests() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Nanoseconds(123_456_789), + fraction: Fraction { + digits: 9, + value: 123_456_789, + }, }), offset: None, tz: None, @@ -1113,7 +1155,10 @@ fn subseconds_parsing_extended_nanoseconds() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Nanoseconds(123_456_700), + fraction: Fraction { + digits: 7, + value: 1_234_567 + }, }), offset: None, tz: None, @@ -1132,7 +1177,10 @@ fn subseconds_parsing_extended_nanoseconds() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Nanoseconds(123_456_789), + fraction: Fraction { + digits: 9, + value: 123_456_789, + }, }), offset: None, tz: None, @@ -1154,7 +1202,10 @@ fn subseconds_parsing_extended_picoseconds() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Picoseconds(123_456_789_000), + fraction: Fraction { + digits: 10, + value: 1_234_567_890, + } }), offset: None, tz: None, @@ -1173,7 +1224,10 @@ fn subseconds_parsing_extended_picoseconds() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Picoseconds(123_456_789_876), + fraction: Fraction { + digits: 12, + value: 123_456_789_876, + } }), offset: None, tz: None, @@ -1195,7 +1249,10 @@ fn subseconds_parsing_extended_femtoseconds() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Femtoseconds(123_456_789_876_500), + fraction: Fraction { + digits: 13, + value: 1_234_567_898_765, + } }), offset: None, tz: None, @@ -1214,7 +1271,10 @@ fn subseconds_parsing_extended_femtoseconds() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Femtoseconds(123_456_789_876_543), + fraction: Fraction { + digits: 15, + value: 123_456_789_876_543 + } }), offset: None, tz: None, @@ -1236,7 +1296,10 @@ fn subseconds_parsing_extended_truncated() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Truncated(123_456_789), + fraction: Fraction { + digits: 16, + value: 1_234_567_898_765_432 + }, }), offset: None, tz: None, @@ -1255,7 +1318,10 @@ fn subseconds_parsing_extended_truncated() { hour: 15, minute: 23, second: 30, - fraction: Fraction::Truncated(123_456_789), + fraction: Fraction { + digits: 37, + value: 123_456_789_876_543_210, + }, }), offset: None, tz: None, diff --git a/utils/ixdtf/src/parsers/time.rs b/utils/ixdtf/src/parsers/time.rs index 3edb0502477..08e46a3c24a 100644 --- a/utils/ixdtf/src/parsers/time.rs +++ b/utils/ixdtf/src/parsers/time.rs @@ -151,43 +151,19 @@ pub(crate) fn parse_fraction(cursor: &mut Cursor) -> ParserResult Date: Mon, 27 Jan 2025 16:39:33 -0600 Subject: [PATCH 4/7] Change error to be more descriptive --- utils/ixdtf/src/error.rs | 4 ++-- utils/ixdtf/src/parsers/duration.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/ixdtf/src/error.rs b/utils/ixdtf/src/error.rs index 196dfc3b942..6e459eda3fb 100644 --- a/utils/ixdtf/src/error.rs +++ b/utils/ixdtf/src/error.rs @@ -46,8 +46,6 @@ pub enum ParseError { TimeSecond, #[displaydoc("Invalid character while parsing fraction part value.")] FractionPart, - #[displaydoc("Fraction part value exceeds a representable range.")] - InvalidFractionRange, #[displaydoc("Invalid character while parsing date separator.")] DateSeparator, #[displaydoc("Invalid character while parsing time separator.")] @@ -104,6 +102,8 @@ pub enum ParseError { DateDurationPartOrder, #[displaydoc("Invalid time duration part order.")] TimeDurationPartOrder, + #[displaydoc("Duration part fraction exceeded a representable range.")] + DurationFractionExceededRange, #[displaydoc("Invalid time duration designator.")] TimeDurationDesignator, } diff --git a/utils/ixdtf/src/parsers/duration.rs b/utils/ixdtf/src/parsers/duration.rs index 58a237a0ae2..031d0a2cfc9 100644 --- a/utils/ixdtf/src/parsers/duration.rs +++ b/utils/ixdtf/src/parsers/duration.rs @@ -224,7 +224,7 @@ fn adjust_fraction_for_unit(fraction: Fraction, unit: u64) -> ParserResult Date: Mon, 27 Jan 2025 16:43:10 -0600 Subject: [PATCH 5/7] Update error to ExcessivePrecision and add Time::try_from_time_record --- components/timezone/src/ixdtf.rs | 45 +++++++++----------------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/components/timezone/src/ixdtf.rs b/components/timezone/src/ixdtf.rs index 65d92c23308..8db57ff155d 100644 --- a/components/timezone/src/ixdtf.rs +++ b/components/timezone/src/ixdtf.rs @@ -42,8 +42,8 @@ pub enum ParseError { InconsistentTimeZoneOffsets, /// There was an invalid Offset. InvalidOffsetError, - /// There was an invalid subsecond fraction. - InvalidFraction, + /// Parsed fraction had excessive precision beyond nanosecond. + ExcessivePrecision, /// The set of time zone fields was not expected for the given type. /// For example, if a named time zone was present with offset-only parsing, /// or an offset was present with named-time-zone-only parsing. @@ -328,15 +328,7 @@ impl<'a> Intermediate<'a> { }; let time_zone_id = mapper.iana_bytes_to_bcp47(iana_identifier); let date = Date::::try_new_iso(self.date.year, self.date.month, self.date.day)?; - let Some(nanosecond) = self.time.fraction.to_nanoseconds() else { - return Err(ParseError::InvalidFraction); - }; - let time = Time::try_new( - self.time.hour, - self.time.minute, - self.time.second, - nanosecond, - )?; + let time = Time::try_from_time_record(&self.time)?; let offset = match time_zone_id.as_str() { "utc" | "gmt" => Some(UtcOffset::zero()), _ => None, @@ -372,15 +364,7 @@ impl<'a> Intermediate<'a> { }, }; let date = Date::::try_new_iso(self.date.year, self.date.month, self.date.day)?; - let Some(nanosecond) = self.time.fraction.to_nanoseconds() else { - return Err(ParseError::InvalidFraction); - }; - let time = Time::try_new( - self.time.hour, - self.time.minute, - self.time.second, - nanosecond, - )?; + let time = Time::try_from_time_record(&self.time)?; Ok(time_zone_id.with_offset(offset).at_time((date, time))) } @@ -397,15 +381,7 @@ impl<'a> Intermediate<'a> { }; let time_zone_id = mapper.iana_bytes_to_bcp47(iana_identifier); let date = Date::try_new_iso(self.date.year, self.date.month, self.date.day)?; - let Some(nanosecond) = self.time.fraction.to_nanoseconds() else { - return Err(ParseError::InvalidFraction); - }; - let time = Time::try_new( - self.time.hour, - self.time.minute, - self.time.second, - nanosecond, - )?; + let time = Time::try_from_time_record(&self.time)?; let offset = UtcOffset::try_from_utc_offset_record(offset)?; let zone_variant = match zone_offset_calculator .compute_offsets_from_time_zone(time_zone_id, (date, time)) @@ -803,16 +779,19 @@ impl Time { fn try_from_ixdtf_record(ixdtf_record: &IxdtfParseRecord) -> Result { let time_record = ixdtf_record.time.ok_or(ParseError::MissingFields)?; + Self::try_from_time_record(&time_record) + } + + fn try_from_time_record(time_record: &TimeRecord) -> Result { let Some(nanosecond) = time_record.fraction.to_nanoseconds() else { - return Err(ParseError::InvalidFraction); + return Err(ParseError::ExcessivePrecision); }; - let time = Self::try_new( + Ok(Self::try_new( time_record.hour, time_record.minute, time_record.second, nanosecond, - )?; - Ok(time) + )?) } } From 9a03d3b9b15358d0f049f3108f1d1ad78c267b6a Mon Sep 17 00:00:00 2001 From: Kevin Ness <46825870+nekevss@users.noreply.github.com> Date: Tue, 11 Feb 2025 11:33:34 -0600 Subject: [PATCH 6/7] Update components/timezone/src/ixdtf.rs Co-authored-by: Robert Bastian <4706271+robertbastian@users.noreply.github.com> --- components/timezone/src/ixdtf.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/timezone/src/ixdtf.rs b/components/timezone/src/ixdtf.rs index b1abd2dbf05..5857e2f3d53 100644 --- a/components/timezone/src/ixdtf.rs +++ b/components/timezone/src/ixdtf.rs @@ -42,7 +42,7 @@ pub enum ParseError { InconsistentTimeZoneOffsets, /// There was an invalid Offset. InvalidOffsetError, - /// Parsed fraction had excessive precision beyond nanosecond. + /// Parsed fractional digits had excessive precision beyond nanosecond. ExcessivePrecision, /// The set of time zone fields was not expected for the given type. /// For example, if a named time zone was present with offset-only parsing, From 468228addb6ebd0cfedaa9057e620c7776e815b4 Mon Sep 17 00:00:00 2001 From: Kevin Ness <46825870+nekevss@users.noreply.github.com> Date: Tue, 11 Feb 2025 11:44:30 -0600 Subject: [PATCH 7/7] Adjust error based on review feedback --- utils/ixdtf/src/error.rs | 2 +- utils/ixdtf/src/parsers/duration.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/ixdtf/src/error.rs b/utils/ixdtf/src/error.rs index 6e459eda3fb..6547925d1fb 100644 --- a/utils/ixdtf/src/error.rs +++ b/utils/ixdtf/src/error.rs @@ -103,7 +103,7 @@ pub enum ParseError { #[displaydoc("Invalid time duration part order.")] TimeDurationPartOrder, #[displaydoc("Duration part fraction exceeded a representable range.")] - DurationFractionExceededRange, + DurationFractionalDigitsExceededRange, #[displaydoc("Invalid time duration designator.")] TimeDurationDesignator, } diff --git a/utils/ixdtf/src/parsers/duration.rs b/utils/ixdtf/src/parsers/duration.rs index 031d0a2cfc9..ca1e261c782 100644 --- a/utils/ixdtf/src/parsers/duration.rs +++ b/utils/ixdtf/src/parsers/duration.rs @@ -224,7 +224,7 @@ fn adjust_fraction_for_unit(fraction: Fraction, unit: u64) -> ParserResult