Cleanup Now API in favor of system defined implementations (#182)

~~This adds a `SystemHooks` trait for providing system getters for
system time and system time zone. It also adjusts the "now" feature to a
"sys" feature flag for `DefaultSystemHooks`.~~

This PR provides more generalized methods that can be used in
combination with a system hooks.

Cargo.toml

@@ -67,9 +67,9 @@ combine = { workspace = true, optional = true }
 web-time = { workspace = true, optional =  true }
 
 [features]
-default = ["now"]
+default = ["sys"]
 log = ["dep:log"]
 compiled_data = ["tzdb"]
-now = ["std", "dep:web-time"]
+sys = ["std", "dep:web-time"]
 tzdb = ["dep:tzif", "std", "dep:jiff-tzdb", "dep:combine"]
 std = []

src/builtins/compiled/now.rs

@@ -2,39 +2,50 @@ use crate::builtins::{
     core::{Now, PlainDate, PlainDateTime, PlainTime},
     TZ_PROVIDER,
 };
-use crate::{TemporalError, TemporalResult, TimeZone};
+use crate::sys;
+use crate::{time::EpochNanoseconds, TemporalError, TemporalResult, TimeZone};
 
+#[cfg(feature = "sys")]
 impl Now {
     /// Returns the current system time as a [`PlainDateTime`] with an optional
     /// [`TimeZone`].
     ///
-    /// Enable with the `compiled_data` feature flag.
+    /// Enable with the `compiled_data` and `sys` feature flags.
     pub fn plain_datetime_iso(timezone: Option<TimeZone>) -> TemporalResult<PlainDateTime> {
         let provider = TZ_PROVIDER
             .lock()
             .map_err(|_| TemporalError::general("Unable to acquire lock"))?;
-        Now::plain_datetime_iso_with_provider(timezone, &*provider).map(Into::into)
+        let timezone = timezone.unwrap_or(TimeZone::IanaIdentifier(sys::get_system_timezone()?));
+        let system_nanos = sys::get_system_nanoseconds()?;
+        let epoch_nanos = EpochNanoseconds::try_from(system_nanos)?;
+        Now::plain_datetime_iso_with_provider(epoch_nanos, timezone, &*provider)
     }
 
     /// Returns the current system time as a [`PlainDate`] with an optional
     /// [`TimeZone`].
     ///
-    /// Enable with the `compiled_data` feature flag.
+    /// Enable with the `compiled_data` and `sys` feature flags.
     pub fn plain_date_iso(timezone: Option<TimeZone>) -> TemporalResult<PlainDate> {
         let provider = TZ_PROVIDER
             .lock()
             .map_err(|_| TemporalError::general("Unable to acquire lock"))?;
-        Now::plain_date_iso_with_provider(timezone, &*provider).map(Into::into)
+        let timezone = timezone.unwrap_or(TimeZone::IanaIdentifier(sys::get_system_timezone()?));
+        let system_nanos = sys::get_system_nanoseconds()?;
+        let epoch_nanos = EpochNanoseconds::try_from(system_nanos)?;
+        Now::plain_date_iso_with_provider(epoch_nanos, timezone, &*provider)
     }
 
     /// Returns the current system time as a [`PlainTime`] with an optional
     /// [`TimeZone`].
     ///
-    /// Enable with the `compiled_data` feature flag.
+    /// Enable with the `compiled_data` and `sys` feature flags.
     pub fn plain_time_iso(timezone: Option<TimeZone>) -> TemporalResult<PlainTime> {
         let provider = TZ_PROVIDER
             .lock()
             .map_err(|_| TemporalError::general("Unable to acquire lock"))?;
-        Now::plain_time_iso_with_provider(timezone, &*provider).map(Into::into)
+        let timezone = timezone.unwrap_or(TimeZone::IanaIdentifier(sys::get_system_timezone()?));
+        let system_nanos = sys::get_system_nanoseconds()?;
+        let epoch_nanos = EpochNanoseconds::try_from(system_nanos)?;
+        Now::plain_time_iso_with_provider(epoch_nanos, timezone, &*provider)
     }
 }

src/builtins/core/mod.rs

@@ -18,10 +18,8 @@ mod time;
 mod year_month;
 pub(crate) mod zoneddatetime;
 
-#[cfg(feature = "now")]
 mod now;
 
-#[cfg(feature = "now")]
 #[doc(inline)]
 pub use now::Now;
 

src/builtins/core/now.rs

@@ -1,11 +1,12 @@
 //! The Temporal Now component
 
+use crate::iso::IsoDateTime;
 use crate::provider::TimeZoneProvider;
-use crate::{iso::IsoDateTime, time::EpochNanoseconds, TemporalUnwrap};
-use crate::{sys, TemporalResult};
-use alloc::string::String;
+use crate::time::EpochNanoseconds;
+use crate::TemporalResult;
 
-use num_traits::FromPrimitive;
+#[cfg(feature = "sys")]
+use alloc::string::String;
 
 use super::{
     calendar::Calendar, timezone::TimeZone, Instant, PlainDate, PlainDateTime, PlainTime,
@@ -16,24 +17,57 @@ use super::{
 pub struct Now;
 
 impl Now {
-    /// Returns the current instant
-    pub fn instant() -> TemporalResult<Instant> {
-        system_instant()
-    }
-
-    /// Returns the current time zone.
-    pub fn time_zone_id() -> TemporalResult<String> {
-        sys::get_system_tz_identifier()
+    /// Returns the current system `DateTime` based off the provided system args
+    ///
+    /// ## Order of operations
+    ///
+    /// The order of operations for this method requires the `GetSystemTimeZone` call
+    /// to occur prior to calling system time and resolving the `EpochNanoseconds`
+    /// value.
+    ///
+    /// A correct implementation will follow the following steps:
+    ///
+    ///   1. Resolve user input `TimeZone` with the `SystemTimeZone`.
+    ///   2. Get the `SystemNanoseconds`
+    ///
+    /// For an example implementation see [`Self::zoneddatetime_iso`]
+    ///
+    pub(crate) fn system_datetime_with_provider(
+        epoch_nanoseconds: EpochNanoseconds,
+        timezone: TimeZone,
+        provider: &impl TimeZoneProvider,
+    ) -> TemporalResult<IsoDateTime> {
+        // 1. If temporalTimeZoneLike is undefined, then
+        // a. Let timeZone be SystemTimeZoneIdentifier().
+        // 2. Else,
+        // a. Let timeZone be ? ToTemporalTimeZoneIdentifier(temporalTimeZoneLike).
+        // 3. Let epochNs be SystemUTCEpochNanoseconds().
+        // 4. Return GetISODateTimeFor(timeZone, epochNs).
+        timezone.get_iso_datetime_for(&Instant::from(epoch_nanoseconds), provider)
     }
 
     /// Returns the current system time as a `ZonedDateTime` with an ISO8601 calendar.
     ///
     /// The time zone will be set to either the `TimeZone` if a value is provided, or
     /// according to the system timezone if no value is provided.
-    pub fn zoneddatetime_iso(timezone: Option<TimeZone>) -> TemporalResult<ZonedDateTime> {
-        let timezone =
-            timezone.unwrap_or(TimeZone::IanaIdentifier(sys::get_system_tz_identifier()?));
-        let instant = system_instant()?;
+    ///
+    /// ## Order of operations
+    ///
+    /// The order of operations for this method requires the `GetSystemTimeZone` call
+    /// to occur prior to calling system time and resolving the `EpochNanoseconds`
+    /// value.
+    ///
+    /// A correct implementation will follow the following steps:
+    ///
+    ///   1. Resolve user input `TimeZone` with the `SystemTimeZone`.
+    ///   2. Get the `SystemNanoseconds`
+    ///
+    /// For an example implementation see [`Self::zoneddatetime_iso`]
+    pub fn zoneddatetime_iso_with_system_values(
+        epoch_nanos: EpochNanoseconds,
+        timezone: TimeZone,
+    ) -> TemporalResult<ZonedDateTime> {
+        let instant = Instant::from(epoch_nanos);
         Ok(ZonedDateTime::new_unchecked(
             instant,
             Calendar::default(),
@@ -42,85 +76,124 @@ impl Now {
     }
 }
 
+#[cfg(feature = "sys")]
+impl Now {
+    /// Returns the current instant
+    ///
+    /// Enable with the `sys` feature flag.
+    pub fn instant() -> TemporalResult<Instant> {
+        let system_nanos = crate::sys::get_system_nanoseconds()?;
+        let epoch_nanos = EpochNanoseconds::try_from(system_nanos)?;
+        Ok(Instant::from(epoch_nanos))
+    }
+
+    /// Returns the current time zone.
+    ///
+    /// Enable with the `sys` feature flag.
+    pub fn time_zone_identifier() -> TemporalResult<String> {
+        crate::sys::get_system_timezone()
+    }
+
+    /// Returns the current system time as a [`PlainDateTime`] with an optional
+    /// [`TimeZone`].
+    ///
+    /// Enable with the `sys` feature flag.
+    pub fn zoneddatetime_iso(timezone: Option<TimeZone>) -> TemporalResult<ZonedDateTime> {
+        let timezone =
+            timezone.unwrap_or(TimeZone::IanaIdentifier(crate::sys::get_system_timezone()?));
+        let system_nanos = crate::sys::get_system_nanoseconds()?;
+        let epoch_nanos = EpochNanoseconds::try_from(system_nanos)?;
+        Now::zoneddatetime_iso_with_system_values(epoch_nanos, timezone)
+    }
+}
+
 impl Now {
     /// Returns the current system time as a `PlainDateTime` with an ISO8601 calendar.
     ///
-    /// The time zone used to calculate the `PlainDateTime` will be set to either the
-    /// `TimeZone` if a value is provided, or according to the system timezone if no
-    /// value is provided.
+    /// ## Order of operations
+    ///
+    /// The order of operations for this method requires the `GetSystemTimeZone` call
+    /// to occur prior to calling system time and resolving the `EpochNanoseconds`
+    /// value.
+    ///
+    /// A correct implementation will follow the following steps:
+    ///
+    ///   1. Resolve user input `TimeZone` with the `SystemTimeZone`.
+    ///   2. Get the `SystemNanoseconds`
+    ///
+    /// For an example implementation see [`Self::plain_datetime_iso`]
     pub fn plain_datetime_iso_with_provider(
-        timezone: Option<TimeZone>,
+        epoch_nanos: EpochNanoseconds,
+        timezone: TimeZone,
         provider: &impl TimeZoneProvider,
     ) -> TemporalResult<PlainDateTime> {
-        let iso = system_datetime(timezone, provider)?;
+        let iso = Self::system_datetime_with_provider(epoch_nanos, timezone, provider)?;
         Ok(PlainDateTime::new_unchecked(iso, Calendar::default()))
     }
 
     /// Returns the current system time as a `PlainDate` with an ISO8601 calendar.
     ///
-    /// The time zone used to calculate the `PlainDate` will be set to either the
-    /// `TimeZone` if a value is provided, or according to the system timezone if no
-    /// value is provided.
+    /// ## Order of operations
+    ///
+    /// The order of operations for this method requires the `GetSystemTimeZone` call
+    /// to occur prior to calling system time and resolving the `EpochNanoseconds`
+    /// value.
+    ///
+    /// A correct implementation will follow the following steps:
+    ///
+    ///   1. Resolve user input `TimeZone` with the `SystemTimeZone`.
+    ///   2. Get the `SystemNanoseconds`
+    ///
+    /// For an example implementation see [`Self::plain_date_iso`]
     pub fn plain_date_iso_with_provider(
-        timezone: Option<TimeZone>,
+        epoch_nanos: EpochNanoseconds,
+        timezone: TimeZone,
         provider: &impl TimeZoneProvider,
     ) -> TemporalResult<PlainDate> {
-        let iso = system_datetime(timezone, provider)?;
+        let iso = Self::system_datetime_with_provider(epoch_nanos, timezone, provider)?;
         Ok(PlainDate::new_unchecked(iso.date, Calendar::default()))
     }
 
     /// Returns the current system time as a `PlainTime` according to an ISO8601 calendar.
     ///
-    /// The time zone used to calculate the `PlainTime` will be set to either the
-    /// `TimeZone` if a value is provided, or according to the system timezone if no
-    /// value is provided.
+    /// ## Order of operations
+    ///
+    /// The order of operations for this method requires the `GetSystemTimeZone` call
+    /// to occur prior to calling system time and resolving the `EpochNanoseconds`
+    /// value.
+    ///
+    /// A correct implementation will follow the following steps:
+    ///
+    ///   1. Resolve user input `TimeZone` with the `SystemTimeZone`.
+    ///   2. Get the `SystemNanoseconds`
+    ///
+    /// For an example implementation see [`Self::plain_time_iso`]
     pub fn plain_time_iso_with_provider(
-        timezone: Option<TimeZone>,
+        epoch_nanos: EpochNanoseconds,
+        timezone: TimeZone,
         provider: &impl TimeZoneProvider,
     ) -> TemporalResult<PlainTime> {
-        let iso = system_datetime(timezone, provider)?;
+        let iso = Self::system_datetime_with_provider(epoch_nanos, timezone, provider)?;
         Ok(PlainTime::new_unchecked(iso.time))
     }
 }
 
-fn system_datetime(
-    tz: Option<TimeZone>,
-    provider: &impl TimeZoneProvider,
-) -> TemporalResult<IsoDateTime> {
-    // 1. If temporalTimeZoneLike is undefined, then
-    // a. Let timeZone be SystemTimeZoneIdentifier().
-    // 2. Else,
-    // a. Let timeZone be ? ToTemporalTimeZoneIdentifier(temporalTimeZoneLike).
-    let tz = tz.unwrap_or(TimeZone::IanaIdentifier(sys::get_system_tz_identifier()?));
-    // 3. Let epochNs be SystemUTCEpochNanoseconds().
-    // TODO: Handle u128 -> i128 better for system nanoseconds
-    let epoch_ns = EpochNanoseconds::try_from(sys::get_system_nanoseconds()?)?;
-    // 4. Return GetISODateTimeFor(timeZone, epochNs).
-    tz.get_iso_datetime_for(&Instant::from(epoch_ns), provider)
-}
-
-fn system_instant() -> TemporalResult<Instant> {
-    let nanos = sys::get_system_nanoseconds()?;
-    Instant::try_new(i128::from_u128(nanos).temporal_unwrap()?)
-}
-
-#[cfg(feature = "tzdb")]
+#[cfg(all(feature = "tzdb", feature = "sys"))]
 #[cfg(test)]
 mod tests {
+    use crate::builtins::core::Now;
     use std::thread;
     use std::time::Duration as StdDuration;
 
-    use crate::builtins::core::Now;
-    use crate::{options::DifferenceSettings, tzdb::FsTzdbProvider};
+    use crate::options::DifferenceSettings;
 
     #[test]
     fn now_datetime_test() {
-        let provider = &FsTzdbProvider::default();
         let sleep = 2;
 
-        let before = Now::plain_datetime_iso_with_provider(None, provider).unwrap();
+        let before = Now::plain_datetime_iso(None).unwrap();
         thread::sleep(StdDuration::from_secs(sleep));
-        let after = Now::plain_datetime_iso_with_provider(None, provider).unwrap();
+        let after = Now::plain_datetime_iso(None).unwrap();
 
         let diff = after.since(&before, DifferenceSettings::default()).unwrap();
 

src/lib.rs

@@ -53,12 +53,11 @@ pub mod parsers;
 pub mod primitive;
 pub mod provider;
 
-mod epoch_nanoseconds;
+#[cfg(feature = "sys")]
+pub(crate) mod sys;
 
 mod builtins;
-
-#[cfg(feature = "now")]
-mod sys;
+mod epoch_nanoseconds;
 
 #[cfg(feature = "tzdb")]
 pub mod tzdb;
@@ -97,13 +96,10 @@ pub mod time {
 }
 
 pub use crate::builtins::{
-    calendar::Calendar, core::timezone::TimeZone, DateDuration, Duration, Instant, PlainDate,
+    calendar::Calendar, core::timezone::TimeZone, DateDuration, Duration, Instant, Now, PlainDate,
     PlainDateTime, PlainMonthDay, PlainTime, PlainYearMonth, TimeDuration, ZonedDateTime,
 };
 
-#[cfg(feature = "now")]
-pub use crate::builtins::Now;
-
 /// A library specific trait for unwrapping assertions.
 pub(crate) trait TemporalUnwrap {
     type Output;

src/sys.rs

@@ -1,21 +1,23 @@
-use alloc::string::{String, ToString};
+use alloc::string::String;
 
-use crate::{TemporalError, TemporalResult};
+use crate::TemporalResult;
 
+use crate::TemporalError;
+use alloc::string::ToString;
 use web_time::{SystemTime, UNIX_EPOCH};
 
 // TODO: Need to implement SystemTime handling for non_std.
 
+#[inline]
+pub(crate) fn get_system_timezone() -> TemporalResult<String> {
+    iana_time_zone::get_timezone().map_err(|e| TemporalError::general(e.to_string()))
+}
+
 /// Returns the system time in nanoseconds.
-#[cfg(feature = "now")]
+#[cfg(feature = "sys")]
 pub(crate) fn get_system_nanoseconds() -> TemporalResult<u128> {
     SystemTime::now()
         .duration_since(UNIX_EPOCH)
         .map_err(|e| TemporalError::general(e.to_string()))
         .map(|d| d.as_nanos())
 }
-
-/// Returns the system tz identifier
-pub(crate) fn get_system_tz_identifier() -> TemporalResult<String> {
-    iana_time_zone::get_timezone().map_err(|e| TemporalError::general(e.to_string()))
-}