Skip to content

Commit

Permalink
Use NaiveDateTime for internal tz_info methods. (#1658)
Browse files Browse the repository at this point in the history
This replaces passing a timestamp and year to several internal functions with
passing a NaiveDateTime, making the function signature slightly clearer. The
values originated from in the a NaiveDateTime in first place, so it basically
just postpones the conversion.

Co-authored-by: Arjan <arjan@xldata.nl>
  • Loading branch information
AVee and Arjan authored Feb 18, 2025
1 parent 8317e7c commit 15e287b
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 25 deletions.
19 changes: 8 additions & 11 deletions src/offset/local/tz_info/rule.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::cmp::Ordering;

use super::parser::Cursor;
use super::timezone::{LocalTimeType, SECONDS_PER_WEEK};
use super::{
Error, CUMUL_DAY_IN_MONTHS_NORMAL_YEAR, DAYS_PER_WEEK, DAY_IN_MONTHS_NORMAL_YEAR,
SECONDS_PER_DAY,
};
use crate::{Datelike, NaiveDateTime};
use std::cmp::Ordering;

/// Transition rule
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -81,15 +81,14 @@ impl TransitionRule {
/// Find the local time type associated to the transition rule at the specified Unix time in seconds
pub(super) fn find_local_time_type_from_local(
&self,
local_time: i64,
year: i32,
local_time: NaiveDateTime,
) -> Result<crate::MappedLocalTime<LocalTimeType>, Error> {
match self {
TransitionRule::Fixed(local_time_type) => {
Ok(crate::MappedLocalTime::Single(*local_time_type))
}
TransitionRule::Alternate(alternate_time) => {
alternate_time.find_local_time_type_from_local(local_time, year)
alternate_time.find_local_time_type_from_local(local_time)
}
}
}
Expand Down Expand Up @@ -229,13 +228,11 @@ impl AlternateTime {

fn find_local_time_type_from_local(
&self,
local_time: i64,
current_year: i32,
local_time: NaiveDateTime,
) -> Result<crate::MappedLocalTime<LocalTimeType>, Error> {
// Check if the current year is valid for the following computations
if !(i32::MIN + 2..=i32::MAX - 2).contains(&current_year) {
return Err(Error::OutOfRange("out of range date time"));
}
// Year must be between i32::MIN + 2 and i32::MAX - 2, year in NaiveDate is always smaller.
let current_year = local_time.year();
let local_time = local_time.and_utc().timestamp();

let dst_start_transition_start =
self.dst_start.unix_time(current_year, 0) + i64::from(self.dst_start_time);
Expand Down
21 changes: 9 additions & 12 deletions src/offset/local/tz_info/timezone.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
//! Types related to a time zone.
use super::rule::{AlternateTime, TransitionRule};
use super::{parser, Error, DAYS_PER_WEEK, SECONDS_PER_DAY};
use crate::NaiveDateTime;
use std::fs::{self, File};
use std::io::{self, Read};
use std::path::{Path, PathBuf};
use std::{cmp::Ordering, fmt, str};

use super::rule::{AlternateTime, TransitionRule};
use super::{parser, Error, DAYS_PER_WEEK, SECONDS_PER_DAY};

#[cfg(target_env = "ohos")]
use crate::offset::local::tz_info::parser::Cursor;

Expand Down Expand Up @@ -133,13 +133,11 @@ impl TimeZone {
self.as_ref().find_local_time_type(unix_time)
}

// should we pass NaiveDateTime all the way through to this fn?
pub(crate) fn find_local_time_type_from_local(
&self,
local_time: i64,
year: i32,
local_time: NaiveDateTime,
) -> Result<crate::MappedLocalTime<LocalTimeType>, Error> {
self.as_ref().find_local_time_type_from_local(local_time, year)
self.as_ref().find_local_time_type_from_local(local_time)
}

/// Returns a reference to the time zone
Expand Down Expand Up @@ -225,8 +223,7 @@ impl<'a> TimeZoneRef<'a> {

pub(crate) fn find_local_time_type_from_local(
&self,
local_time: i64,
year: i32,
local_time: NaiveDateTime,
) -> Result<crate::MappedLocalTime<LocalTimeType>, Error> {
// #TODO: this is wrong as we need 'local_time_to_local_leap_time ?
// but ... does the local time even include leap seconds ??
Expand All @@ -235,10 +232,10 @@ impl<'a> TimeZoneRef<'a> {
// Err(Error::OutOfRange(error)) => return Err(Error::FindLocalTimeType(error)),
// Err(err) => return Err(err),
// };
let local_leap_time = local_time;
let local_leap_time = local_time.and_utc().timestamp();

// if we have at least one transition,
// we must check _all_ of them, incase of any Overlapping (MappedLocalTime::Ambiguous) or Skipping (MappedLocalTime::None) transitions
// we must check _all_ of them, in case of any Overlapping (MappedLocalTime::Ambiguous) or Skipping (MappedLocalTime::None) transitions
let offset_after_last = if !self.transitions.is_empty() {
let mut prev = self.local_time_types[0];

Expand Down Expand Up @@ -301,7 +298,7 @@ impl<'a> TimeZoneRef<'a> {
};

if let Some(extra_rule) = self.extra_rule {
match extra_rule.find_local_time_type_from_local(local_time, year) {
match extra_rule.find_local_time_type_from_local(local_time) {
Ok(local_time_type) => Ok(local_time_type),
Err(Error::OutOfRange(error)) => Err(Error::FindLocalTimeType(error)),
err => err,
Expand Down
4 changes: 2 additions & 2 deletions src/offset/local/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::{cell::RefCell, collections::hash_map, env, fs, hash::Hasher, time::Sys

use super::tz_info::TimeZone;
use super::{FixedOffset, NaiveDateTime};
use crate::{Datelike, MappedLocalTime};
use crate::MappedLocalTime;

pub(super) fn offset_from_utc_datetime(utc: &NaiveDateTime) -> MappedLocalTime<FixedOffset> {
offset(utc, false)
Expand Down Expand Up @@ -164,7 +164,7 @@ impl Cache {
// we pass through the year as the year of a local point in time must either be valid in that locale, or
// the entire time was skipped in which case we will return MappedLocalTime::None anyway.
self.zone
.find_local_time_type_from_local(d.and_utc().timestamp(), d.year())
.find_local_time_type_from_local(d)
.expect("unable to select local time type")
.and_then(|o| FixedOffset::east_opt(o.offset()))
}
Expand Down

0 comments on commit 15e287b

Please sign in to comment.