Skip to content

Commit

Permalink
Handle Z correctly in IXDTF (#5742)
Browse files Browse the repository at this point in the history
Fixes #5533, fixes #5653
  • Loading branch information
robertbastian authored Oct 29, 2024
1 parent a1a4e7d commit b982957
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 83 deletions.
2 changes: 1 addition & 1 deletion components/datetime/tests/patterns/tests/time_zones.json
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@
},
{
"locale": "ru",
"datetime": "2021-01-11T12:00:00.000Z[Antarctica/Troll]",
"datetime": "2021-01-11T12:00:00.000+00:00[Antarctica/Troll]",
"expectations": [
{
"patterns": [
Expand Down
104 changes: 76 additions & 28 deletions components/timezone/src/ixdtf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use ixdtf::{
parsers::{
records::{
DateRecord, IxdtfParseRecord, TimeRecord, TimeZoneAnnotation, TimeZoneRecord,
UTCOffsetRecord,
UtcOffsetRecord, UtcOffsetRecordOrZ,
},
IxdtfParser,
},
Expand Down Expand Up @@ -41,6 +41,23 @@ pub enum ParseError {
MismatchedTimeZoneFields,
/// An unknown calendar was provided.
UnknownCalendar,
/// A timezone calculation is required to interpret this string, which is not supported.
///
/// # Example
/// ```
/// use icu::timezone::{CustomZonedDateTime, ParseError};
///
/// // This timestamp is in UTC, and requires a time zone calculation in order to display a Zurich time.
/// assert_eq!(
/// CustomZonedDateTime::try_loose_iso_from_str("2024-08-12T12:32:00Z[Europe/Zurich]").unwrap_err(),
/// ParseError::RequiresCalculation,
/// );
///
/// // These timestamps are in local time
/// CustomZonedDateTime::try_loose_iso_from_str("2024-08-12T14:32:00+02:00[Europe/Zurich]").unwrap();
/// CustomZonedDateTime::try_loose_iso_from_str("2024-08-12T14:32:00[Europe/Zurich]").unwrap();
/// ```
RequiresCalculation,
}

impl From<IxdtfParseError> for ParseError {
Expand Down Expand Up @@ -68,7 +85,7 @@ impl From<InvalidOffsetError> for ParseError {
}

impl UtcOffset {
fn try_from_utc_offset_record(record: &UTCOffsetRecord) -> Result<Self, ParseError> {
fn try_from_utc_offset_record(record: UtcOffsetRecord) -> Result<Self, ParseError> {
let hour_seconds = i32::from(record.hour) * 3600;
let minute_seconds = i32::from(record.minute) * 60;
Self::try_from_seconds(
Expand All @@ -80,20 +97,21 @@ impl UtcOffset {
}

struct Intermediate<'a> {
offset: Option<&'a UTCOffsetRecord>,
offset: Option<UtcOffsetRecord>,
iana_identifier: Option<&'a [u8]>,
date: &'a DateRecord,
time: &'a TimeRecord,
date: DateRecord,
time: TimeRecord,
}

impl<'a> Intermediate<'a> {
fn try_from_ixdtf_record(ixdtf_record: &'a IxdtfParseRecord) -> Result<Self, ParseError> {
let (offset, iana_identifier) = match ixdtf_record {
// empty
// -0800
// Z
IxdtfParseRecord {
offset, tz: None, ..
} => (offset.as_ref(), None),
} => (offset.map(UtcOffsetRecordOrZ::resolve_rfc_9557), None),
// [-0800]
IxdtfParseRecord {
offset: None,
Expand All @@ -103,10 +121,10 @@ impl<'a> Intermediate<'a> {
..
}),
..
} => (Some(offset), None),
} => (Some(*offset), None),
// -0800[-0800]
IxdtfParseRecord {
offset: Some(offset),
offset: Some(UtcOffsetRecordOrZ::Offset(offset)),
tz:
Some(TimeZoneAnnotation {
tz: TimeZoneRecord::Offset(offset1),
Expand All @@ -117,34 +135,49 @@ impl<'a> Intermediate<'a> {
if offset != offset1 {
return Err(ParseError::InconsistentTimeZoneOffsets);
}
(Some(offset), None)
(Some(*offset), None)
}
// [America/Los_Angeles]
// -0800[America/Los_Angeles]
IxdtfParseRecord {
offset,
offset: Some(UtcOffsetRecordOrZ::Offset(offset)),
tz:
Some(TimeZoneAnnotation {
tz: TimeZoneRecord::Name(iana_identifier),
..
}),
..
} => (Some(*offset), Some(*iana_identifier)),
// Z[-0800]
// Z[America/Los_Angeles]
IxdtfParseRecord {
offset: Some(UtcOffsetRecordOrZ::Z),
tz: Some(_),
..
} => return Err(ParseError::RequiresCalculation),
// [America/Los_Angeles]
IxdtfParseRecord {
offset: None,
tz:
Some(TimeZoneAnnotation {
tz: TimeZoneRecord::Name(iana_identifier),
..
}),
..
} => (offset.as_ref(), Some(*iana_identifier)),
} => (None, Some(*iana_identifier)),
// non_exhaustive match: maybe something like [u-tz=uslax] in the future
IxdtfParseRecord {
offset,
tz: Some(TimeZoneAnnotation { tz, .. }),
..
} => {
debug_assert!(false, "unexpected TimeZoneRecord: {tz:?}");
(offset.as_ref(), None)
(None, None)
}
};
let IxdtfParseRecord {
date: Some(date),
time: Some(time),
..
} = ixdtf_record
} = *ixdtf_record
else {
// Date or time was missing
return Err(ParseError::MismatchedTimeZoneFields);
Expand Down Expand Up @@ -666,22 +699,37 @@ mod test {

#[test]
fn max_possible_ixdtf_utc_offset() {
let ixdtf_record =
IxdtfParser::from_utf8("2024-08-08T12:08:19+23:59:60.999999999".as_bytes())
.parse()
.unwrap();
let result = UtcOffset::try_from_utc_offset_record(&ixdtf_record.offset.unwrap());
assert_eq!(result, Err(ParseError::InvalidOffsetError));
assert_eq!(
CustomZonedDateTime::try_offset_only_iso_from_str(
"2024-08-08T12:08:19+23:59:60.999999999"
)
.unwrap_err(),
ParseError::InvalidOffsetError
);
}

#[test]
fn zone_calculations() {
CustomZonedDateTime::try_offset_only_iso_from_str("2024-08-08T12:08:19Z").unwrap();
assert_eq!(
CustomZonedDateTime::try_offset_only_iso_from_str("2024-08-08T12:08:19Z[+08:00]")
.unwrap_err(),
ParseError::RequiresCalculation
);
assert_eq!(
CustomZonedDateTime::try_offset_only_iso_from_str(
"2024-08-08T12:08:19Z[Europe/Zurich]"
)
.unwrap_err(),
ParseError::RequiresCalculation
);
}

#[test]
fn future_zone() {
let ixdtf_record = IxdtfParser::from_utf8("2024-08-08T12:08:19[Future/Zone]".as_bytes())
.parse()
.unwrap();
let intermediate = Intermediate::try_from_ixdtf_record(&ixdtf_record).unwrap();
let result = intermediate.loose().unwrap();
assert_eq!(result.time_zone_id, TimeZoneBcp47Id::unknown());
assert_eq!(result.offset, None);
let result =
CustomZonedDateTime::try_loose_from_str("2024-08-08T12:08:19[Future/Zone]").unwrap();
assert_eq!(result.zone.time_zone_id, TimeZoneBcp47Id::unknown());
assert_eq!(result.zone.offset, None);
}
}
8 changes: 4 additions & 4 deletions utils/ixdtf/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions utils/ixdtf/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
//!
//! let date = result.date.unwrap();
//! let time = result.time.unwrap();
//! let offset = result.offset.unwrap();
//! let offset = result.offset.unwrap().resolve_rfc_9557();
//! let tz_annotation = result.tz.unwrap();
//!
//! assert_eq!(date.year, 2024);
Expand Down Expand Up @@ -81,7 +81,7 @@
//!
//! let date = result.date.unwrap();
//! let time = result.time.unwrap();
//! let offset = result.offset.unwrap();
//! let offset = result.offset.unwrap().resolve_rfc_9557();
//! let tz_annotation = result.tz.unwrap();
//!
//! assert_eq!(date.year, 2024);
Expand Down Expand Up @@ -135,7 +135,7 @@
//! let result = IxdtfParser::from_str(zulu_offset).parse().unwrap();
//!
//! let tz_annotation = result.tz.unwrap();
//! let offset = result.offset.unwrap();
//! let offset = result.offset.unwrap().resolve_rfc_9557();
//!
//! // The offset is `Z`/`-00:00`, so the application can use the rules of
//! // "America/New_York" to calculate the time for IXDTF string.
Expand Down Expand Up @@ -284,7 +284,7 @@
//! let result = IxdtfParser::from_str(example_two).parse().unwrap();
//!
//! let tz_annotation = result.tz.unwrap();
//! let offset = result.offset.unwrap();
//! let offset = result.offset.unwrap().resolve_rfc_9557();
//!
//! // The time zone annotation and offset conflict with each other, and must therefore be
//! // resolved by the user.
Expand Down
4 changes: 2 additions & 2 deletions utils/ixdtf/src/parsers/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
ParseError, ParserResult,
};

use super::records::{Annotation, UTCOffsetRecord};
use super::records::{Annotation, UtcOffsetRecordOrZ};

#[derive(Debug, Default, Clone)]
/// A `DateTime` Parse Node that contains the date, time, and offset info.
Expand All @@ -28,7 +28,7 @@ pub(crate) struct DateTimeRecord {
/// Time
pub(crate) time: Option<TimeRecord>,
/// Tz Offset
pub(crate) time_zone: Option<UTCOffsetRecord>,
pub(crate) time_zone: Option<UtcOffsetRecordOrZ>,
}

/// This function handles parsing for [`AnnotatedDateTime`][datetime],
Expand Down
6 changes: 3 additions & 3 deletions utils/ixdtf/src/parsers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ macro_rules! assert_syntax {
///
/// ```rust
/// use ixdtf::parsers::{
/// records::{Sign, TimeZoneRecord, UTCOffsetRecord},
/// records::{Sign, TimeZoneRecord, UtcOffsetRecord},
/// IxdtfParser,
/// };
///
Expand All @@ -54,7 +54,7 @@ macro_rules! assert_syntax {
///
/// let date = result.date.unwrap();
/// let time = result.time.unwrap();
/// let offset = result.offset.unwrap();
/// let offset = result.offset.unwrap().resolve_rfc_9557();
/// let tz_annotation = result.tz.unwrap();
///
/// assert_eq!(date.year, 2024);
Expand Down Expand Up @@ -194,7 +194,7 @@ impl<'a> IxdtfParser<'a> {
/// let result = IxdtfParser::from_str(extended_time).parse_time().unwrap();
///
/// let time = result.time.unwrap();
/// let offset = result.offset.unwrap();
/// let offset = result.offset.unwrap().resolve_rfc_9557();
/// let tz_annotation = result.tz.unwrap();
///
/// assert_eq!(time.hour, 12);
Expand Down
29 changes: 26 additions & 3 deletions utils/ixdtf/src/parsers/records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub struct IxdtfParseRecord<'a> {
/// Parsed `TimeRecord`
pub time: Option<TimeRecord>,
/// Parsed UtcOffset
pub offset: Option<UTCOffsetRecord>,
pub offset: Option<UtcOffsetRecordOrZ>,
/// Parsed `TimeZone` annotation with critical flag and data (UTCOffset | IANA name)
pub tz: Option<TimeZoneAnnotation<'a>>,
/// The parsed calendar value.
Expand Down Expand Up @@ -75,7 +75,7 @@ pub enum TimeZoneRecord<'a> {
/// TimeZoneIANAName
Name(&'a [u8]),
/// TimeZoneOffset
Offset(UTCOffsetRecord),
Offset(UtcOffsetRecord),
}

/// The parsed sign value, representing whether its struct is positive or negative.
Expand All @@ -101,7 +101,7 @@ impl From<bool> for Sign {
/// A full precision `UtcOffsetRecord`
#[non_exhaustive]
#[derive(Debug, Clone, Copy, PartialEq)]
pub struct UTCOffsetRecord {
pub struct UtcOffsetRecord {
/// The `Sign` value of the `UtcOffsetRecord`.
pub sign: Sign,
/// The hour value of the `UtcOffsetRecord`.
Expand All @@ -114,6 +114,29 @@ pub struct UTCOffsetRecord {
pub nanosecond: u32,
}

#[non_exhaustive]
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum UtcOffsetRecordOrZ {
Offset(UtcOffsetRecord),
Z,
}

impl UtcOffsetRecordOrZ {
/// Resolves to a [`UtcOffsetRecord`] according to RFC9557: "Z" == "-00:00"
pub fn resolve_rfc_9557(self) -> UtcOffsetRecord {
match self {
UtcOffsetRecordOrZ::Offset(o) => o,
UtcOffsetRecordOrZ::Z => UtcOffsetRecord {
sign: Sign::Negative,
hour: 0,
minute: 0,
second: 0,
nanosecond: 0,
},
}
}
}

/// The resulting record of parsing a `Duration` string.
#[allow(clippy::exhaustive_structs)]
// A duration can only be a Sign, a DateDuration part, and a TimeDuration part that users need to match on.
Expand Down
Loading

0 comments on commit b982957

Please sign in to comment.