From 1003d3ecfdd8576037010c03aea4bfbcec77ffd9 Mon Sep 17 00:00:00 2001 From: raphaelroosz <121112446+raphaelroosz@users.noreply.github.com> Date: Tue, 20 Dec 2022 16:31:56 +0100 Subject: [PATCH 1/3] fix ordinal week calculation * math is 0 based while ordinal is 1 based => fix as 1 based logic * add extensive testing against the "date" command format * format: test sample instead of every day * 2007 starts with saturday * Last day of the year is thus the 52 on Monday weekly calendar, 53 on Sunday weekly calendar. * update %U expected value in test * Was the goal was to have a different value than with %W at next line ? another date to pick ? * update cfg("unix") into cfg(target_os = "linux") * format tests/dateutils.rs --- src/format/mod.rs | 4 +-- src/format/strftime.rs | 2 +- src/naive/date.rs | 2 +- tests/dateutils.rs | 55 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/format/mod.rs b/src/format/mod.rs index 5e706db998..5832ecdd00 100644 --- a/src/format/mod.rs +++ b/src/format/mod.rs @@ -534,10 +534,10 @@ fn format_inner<'a>( use self::Numeric::*; let week_from_sun = |d: &NaiveDate| { - (d.ordinal() as i32 - d.weekday().num_days_from_sunday() as i32 + 7) / 7 + (d.ordinal() as i32 - d.weekday().num_days_from_sunday() as i32 + 6) / 7 }; let week_from_mon = |d: &NaiveDate| { - (d.ordinal() as i32 - d.weekday().num_days_from_monday() as i32 + 7) / 7 + (d.ordinal() as i32 - d.weekday().num_days_from_monday() as i32 + 6) / 7 }; let (width, v) = match *spec { diff --git a/src/format/strftime.rs b/src/format/strftime.rs index 6338194366..b29a7c7373 100644 --- a/src/format/strftime.rs +++ b/src/format/strftime.rs @@ -584,7 +584,7 @@ fn test_strftime_docs() { assert_eq!(dt.format("%A").to_string(), "Sunday"); assert_eq!(dt.format("%w").to_string(), "0"); assert_eq!(dt.format("%u").to_string(), "7"); - assert_eq!(dt.format("%U").to_string(), "28"); + assert_eq!(dt.format("%U").to_string(), "27"); assert_eq!(dt.format("%W").to_string(), "27"); assert_eq!(dt.format("%G").to_string(), "2001"); assert_eq!(dt.format("%g").to_string(), "01"); diff --git a/src/naive/date.rs b/src/naive/date.rs index 3734460713..8c47c562c6 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -2857,7 +2857,7 @@ mod tests { // corner cases assert_eq!( NaiveDate::from_ymd_opt(2007, 12, 31).unwrap().format("%G,%g,%U,%W,%V").to_string(), - "2008,08,53,53,01" + "2008,08,52,53,01" ); assert_eq!( NaiveDate::from_ymd_opt(2010, 1, 3).unwrap().format("%G,%g,%U,%W,%V").to_string(), diff --git a/tests/dateutils.rs b/tests/dateutils.rs index 130649a71f..dec6bfe117 100644 --- a/tests/dateutils.rs +++ b/tests/dateutils.rs @@ -74,3 +74,58 @@ fn try_verify_against_date_command() { date += chrono::Duration::hours(1); } } + +#[cfg(target_os = "linux")] +fn verify_against_date_command_format_local(path: &'static str, dt: NaiveDateTime) { + let required_format = + "d%d D%D F%F H%H I%I j%j k%k l%l m%m M%M S%S T%T u%u U%U w%w W%W X%X y%y Y%Y z%:z"; + // a%a - depends from localization + // A%A - depends from localization + // b%b - depends from localization + // B%B - depends from localization + // h%h - depends from localization + // c%c - depends from localization + // p%p - depends from localization + // r%r - depends from localization + // x%x - fails, date is dd/mm/yyyy, chrono is dd/mm/yy, same as %D + // Z%Z - too many ways to represent it, will most likely fail + + let output = process::Command::new(path) + .arg("-d") + .arg(format!( + "{}-{:02}-{:02} {:02}:{:02}:{:02}", + dt.year(), + dt.month(), + dt.day(), + dt.hour(), + dt.minute(), + dt.second() + )) + .arg(format!("+{}", required_format)) + .output() + .unwrap(); + + let date_command_str = String::from_utf8(output.stdout).unwrap(); + let date = NaiveDate::from_ymd_opt(dt.year(), dt.month(), dt.day()).unwrap(); + let ldt = Local + .from_local_datetime(&date.and_hms_opt(dt.hour(), dt.minute(), dt.second()).unwrap()) + .unwrap(); + let formated_date = format!("{}\n", ldt.format(required_format)); + assert_eq!(date_command_str, formated_date); +} + +#[test] +#[cfg(target_os = "linux")] +fn try_verify_against_date_command_format() { + let date_path = "/usr/bin/date"; + + if !path::Path::new(date_path).exists() { + // date command not found, skipping + return; + } + let mut date = NaiveDate::from_ymd_opt(1970, 1, 1).unwrap().and_hms_opt(12, 11, 13).unwrap(); + while date.year() < 2008 { + verify_against_date_command_format_local(date_path, date); + date += chrono::Duration::days(55); + } +} From ab237b359615811acf36a4819100c1d320af7ef1 Mon Sep 17 00:00:00 2001 From: Eric Sheppard Date: Tue, 14 Feb 2023 11:41:14 +0000 Subject: [PATCH 2/3] apply same fix to parsing and add failing test cases as per issue #961 --- src/format/parsed.rs | 4 ++-- src/naive/date.rs | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/format/parsed.rs b/src/format/parsed.rs index 54679ccda1..6f668df4cf 100644 --- a/src/format/parsed.rs +++ b/src/format/parsed.rs @@ -380,8 +380,8 @@ impl Parsed { let verify_ordinal = |date: NaiveDate| { let ordinal = date.ordinal(); let weekday = date.weekday(); - let week_from_sun = (ordinal as i32 - weekday.num_days_from_sunday() as i32 + 7) / 7; - let week_from_mon = (ordinal as i32 - weekday.num_days_from_monday() as i32 + 7) / 7; + let week_from_sun = (ordinal as i32 - weekday.num_days_from_sunday() as i32 + 6) / 7; + let week_from_mon = (ordinal as i32 - weekday.num_days_from_monday() as i32 + 6) / 7; self.ordinal.unwrap_or(ordinal) == ordinal && self.week_from_sun.map_or(week_from_sun, |v| v as i32) == week_from_sun && self.week_from_mon.map_or(week_from_mon, |v| v as i32) == week_from_mon diff --git a/src/naive/date.rs b/src/naive/date.rs index 8c47c562c6..2cfd7cbeb0 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -2819,6 +2819,16 @@ mod tests { assert!(NaiveDate::parse_from_str("Sat, 09 Aug 2013", "%a, %d %b %Y").is_err()); assert!(NaiveDate::parse_from_str("2014-57", "%Y-%m-%d").is_err()); assert!(NaiveDate::parse_from_str("2014", "%Y").is_err()); // insufficient + + assert_eq!( + NaiveDate::parse_from_str("2020-01-0", "%Y-%W-%w").ok(), + NaiveDate::from_ymd_opt(2020, 1, 12), + ); + + assert_eq!( + NaiveDate::parse_from_str("2019-01-0", "%Y-%W-%w").ok(), + NaiveDate::from_ymd_opt(2019, 1, 13), + ); } #[test] From 151f96042a506545e921a3a27f6052b3e8720cb5 Mon Sep 17 00:00:00 2001 From: Eric Sheppard Date: Thu, 16 Feb 2023 12:33:39 +0000 Subject: [PATCH 3/3] factor calculations to weeks_from function and add tests tests for weeks_from and num_days_from fix array iter MSRV issue --- src/format/mod.rs | 8 +--- src/format/parsed.rs | 5 +-- src/naive/date.rs | 66 ++++++++++++++++++++++++++++++++ src/weekday.rs | 89 ++++++++++++++++++++++++++------------------ 4 files changed, 123 insertions(+), 45 deletions(-) diff --git a/src/format/mod.rs b/src/format/mod.rs index 5832ecdd00..8c9cb62ecd 100644 --- a/src/format/mod.rs +++ b/src/format/mod.rs @@ -533,12 +533,8 @@ fn format_inner<'a>( Item::Numeric(ref spec, ref pad) => { use self::Numeric::*; - let week_from_sun = |d: &NaiveDate| { - (d.ordinal() as i32 - d.weekday().num_days_from_sunday() as i32 + 6) / 7 - }; - let week_from_mon = |d: &NaiveDate| { - (d.ordinal() as i32 - d.weekday().num_days_from_monday() as i32 + 6) / 7 - }; + let week_from_sun = |d: &NaiveDate| d.weeks_from(Weekday::Sun); + let week_from_mon = |d: &NaiveDate| d.weeks_from(Weekday::Mon); let (width, v) = match *spec { Year => (4, date.map(|d| i64::from(d.year()))), diff --git a/src/format/parsed.rs b/src/format/parsed.rs index 6f668df4cf..0e2015d392 100644 --- a/src/format/parsed.rs +++ b/src/format/parsed.rs @@ -379,9 +379,8 @@ impl Parsed { // verify the ordinal and other (non-ISO) week dates. let verify_ordinal = |date: NaiveDate| { let ordinal = date.ordinal(); - let weekday = date.weekday(); - let week_from_sun = (ordinal as i32 - weekday.num_days_from_sunday() as i32 + 6) / 7; - let week_from_mon = (ordinal as i32 - weekday.num_days_from_monday() as i32 + 6) / 7; + let week_from_sun = date.weeks_from(Weekday::Sun); + let week_from_mon = date.weeks_from(Weekday::Mon); self.ordinal.unwrap_or(ordinal) == ordinal && self.week_from_sun.map_or(week_from_sun, |v| v as i32) == week_from_sun && self.week_from_mon.map_or(week_from_mon, |v| v as i32) == week_from_mon diff --git a/src/naive/date.rs b/src/naive/date.rs index 2cfd7cbeb0..1cf3a6abae 100644 --- a/src/naive/date.rs +++ b/src/naive/date.rs @@ -236,6 +236,9 @@ fn test_date_bounds() { } impl NaiveDate { + pub(crate) fn weeks_from(&self, day: Weekday) -> i32 { + (self.ordinal() as i32 - self.weekday().num_days_from(day) as i32 + 6) / 7 + } /// Makes a new `NaiveDate` from year and packed ordinal-flags, with a verification. fn from_of(year: i32, of: Of) -> Option { if (MIN_YEAR..=MAX_YEAR).contains(&year) && of.valid() { @@ -2916,4 +2919,67 @@ mod tests { assert!(days.contains(&date)); } } + + #[test] + fn test_weeks_from() { + // tests per: https://github.com/chronotope/chrono/issues/961 + // these internally use `weeks_from` via the parsing infrastructure + assert_eq!( + NaiveDate::parse_from_str("2020-01-0", "%Y-%W-%w").ok(), + NaiveDate::from_ymd_opt(2020, 1, 12), + ); + assert_eq!( + NaiveDate::parse_from_str("2019-01-0", "%Y-%W-%w").ok(), + NaiveDate::from_ymd_opt(2019, 1, 13), + ); + + // direct tests + for (y, starts_on) in &[ + (2019, Weekday::Tue), + (2020, Weekday::Wed), + (2021, Weekday::Fri), + (2022, Weekday::Sat), + (2023, Weekday::Sun), + (2024, Weekday::Mon), + (2025, Weekday::Wed), + (2026, Weekday::Thu), + ] { + for day in &[ + Weekday::Mon, + Weekday::Tue, + Weekday::Wed, + Weekday::Thu, + Weekday::Fri, + Weekday::Sat, + Weekday::Sun, + ] { + assert_eq!( + NaiveDate::from_ymd_opt(*y, 1, 1).map(|d| d.weeks_from(*day)), + Some(if day == starts_on { 1 } else { 0 }) + ); + + // last day must always be in week 52 or 53 + assert!([52, 53] + .contains(&NaiveDate::from_ymd_opt(*y, 12, 31).unwrap().weeks_from(*day)),); + } + } + + let base = NaiveDate::from_ymd_opt(2019, 1, 1).unwrap(); + + // 400 years covers all year types + for day in &[ + Weekday::Mon, + Weekday::Tue, + Weekday::Wed, + Weekday::Thu, + Weekday::Fri, + Weekday::Sat, + Weekday::Sun, + ] { + // must always be below 54 + for dplus in 1..(400 * 366) { + assert!((base + Days::new(dplus)).weeks_from(*day) < 54) + } + } + } } diff --git a/src/weekday.rs b/src/weekday.rs index bd30e1934d..c12dd01c64 100644 --- a/src/weekday.rs +++ b/src/weekday.rs @@ -73,15 +73,7 @@ impl Weekday { /// `w.number_from_monday()`: | 1 | 2 | 3 | 4 | 5 | 6 | 7 #[inline] pub fn number_from_monday(&self) -> u32 { - match *self { - Weekday::Mon => 1, - Weekday::Tue => 2, - Weekday::Wed => 3, - Weekday::Thu => 4, - Weekday::Fri => 5, - Weekday::Sat => 6, - Weekday::Sun => 7, - } + self.num_days_from(Weekday::Mon) + 1 } /// Returns a day-of-week number starting from Sunday = 1. @@ -91,15 +83,7 @@ impl Weekday { /// `w.number_from_sunday()`: | 2 | 3 | 4 | 5 | 6 | 7 | 1 #[inline] pub fn number_from_sunday(&self) -> u32 { - match *self { - Weekday::Mon => 2, - Weekday::Tue => 3, - Weekday::Wed => 4, - Weekday::Thu => 5, - Weekday::Fri => 6, - Weekday::Sat => 7, - Weekday::Sun => 1, - } + self.num_days_from(Weekday::Sun) + 1 } /// Returns a day-of-week number starting from Monday = 0. @@ -109,15 +93,7 @@ impl Weekday { /// `w.num_days_from_monday()`: | 0 | 1 | 2 | 3 | 4 | 5 | 6 #[inline] pub fn num_days_from_monday(&self) -> u32 { - match *self { - Weekday::Mon => 0, - Weekday::Tue => 1, - Weekday::Wed => 2, - Weekday::Thu => 3, - Weekday::Fri => 4, - Weekday::Sat => 5, - Weekday::Sun => 6, - } + self.num_days_from(Weekday::Mon) } /// Returns a day-of-week number starting from Sunday = 0. @@ -127,15 +103,17 @@ impl Weekday { /// `w.num_days_from_sunday()`: | 1 | 2 | 3 | 4 | 5 | 6 | 0 #[inline] pub fn num_days_from_sunday(&self) -> u32 { - match *self { - Weekday::Mon => 1, - Weekday::Tue => 2, - Weekday::Wed => 3, - Weekday::Thu => 4, - Weekday::Fri => 5, - Weekday::Sat => 6, - Weekday::Sun => 0, - } + self.num_days_from(Weekday::Sun) + } + + /// Returns a day-of-week number starting from the parameter `day` (D) = 0. + /// + /// `w`: | `D` | `D+1` | `D+2` | `D+3` | `D+4` | `D+5` | `D+6` + /// --------------------------- | ----- | ----- | ----- | ----- | ----- | ----- | ----- + /// `w.num_days_from(wd)`: | 0 | 1 | 2 | 3 | 4 | 5 | 6 + #[inline] + pub(crate) fn num_days_from(&self, day: Weekday) -> u32 { + (*self as u32 + 7 - day as u32) % 7 } } @@ -208,6 +186,45 @@ impl fmt::Debug for ParseWeekdayError { } } +#[cfg(test)] +mod tests { + use num_traits::FromPrimitive; + + use super::Weekday; + + #[test] + fn test_num_days_from() { + for i in 0..7 { + let base_day = Weekday::from_u64(i).unwrap(); + + assert_eq!(base_day.num_days_from_monday(), base_day.num_days_from(Weekday::Mon)); + assert_eq!(base_day.num_days_from_sunday(), base_day.num_days_from(Weekday::Sun)); + + assert_eq!(base_day.num_days_from(base_day), 0); + + assert_eq!(base_day.num_days_from(base_day.pred()), 1); + assert_eq!(base_day.num_days_from(base_day.pred().pred()), 2); + assert_eq!(base_day.num_days_from(base_day.pred().pred().pred()), 3); + assert_eq!(base_day.num_days_from(base_day.pred().pred().pred().pred()), 4); + assert_eq!(base_day.num_days_from(base_day.pred().pred().pred().pred().pred()), 5); + assert_eq!( + base_day.num_days_from(base_day.pred().pred().pred().pred().pred().pred()), + 6 + ); + + assert_eq!(base_day.num_days_from(base_day.succ()), 6); + assert_eq!(base_day.num_days_from(base_day.succ().succ()), 5); + assert_eq!(base_day.num_days_from(base_day.succ().succ().succ()), 4); + assert_eq!(base_day.num_days_from(base_day.succ().succ().succ().succ()), 3); + assert_eq!(base_day.num_days_from(base_day.succ().succ().succ().succ().succ()), 2); + assert_eq!( + base_day.num_days_from(base_day.succ().succ().succ().succ().succ().succ()), + 1 + ); + } + } +} + // the actual `FromStr` implementation is in the `format` module to leverage the existing code #[cfg(feature = "serde")]