Skip to content

fix: hour must be greater than 0 when meridiem is specified #169

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/items/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,17 +190,19 @@ pub fn parse(input: &mut &str) -> ModalResult<Vec<Item>> {
Ok(items)
}

#[allow(clippy::too_many_arguments)]
fn new_date(
year: i32,
month: u32,
day: u32,
hour: u32,
minute: u32,
second: u32,
nano: u32,
offset: FixedOffset,
) -> Option<DateTime<FixedOffset>> {
let newdate = NaiveDate::from_ymd_opt(year, month, day)
.and_then(|naive| naive.and_hms_opt(hour, minute, second))?;
.and_then(|naive| naive.and_hms_nano_opt(hour, minute, second, nano))?;

Some(DateTime::<FixedOffset>::from_local(newdate, offset))
}
Expand All @@ -220,7 +222,8 @@ fn with_timezone_restore(
.with_year(copy.year())?
.with_hour(copy.hour())?
.with_minute(copy.minute())?
.with_second(copy.second())?;
.with_second(copy.second())?
.with_nanosecond(copy.nanosecond())?;
Some(x)
}

Expand Down Expand Up @@ -274,6 +277,7 @@ fn at_date_inner(date: Vec<Item>, at: DateTime<FixedOffset>) -> Option<DateTime<
d.hour(),
d.minute(),
d.second(),
d.nanosecond(),
*d.offset(),
)?;
}
Expand All @@ -299,6 +303,7 @@ fn at_date_inner(date: Vec<Item>, at: DateTime<FixedOffset>) -> Option<DateTime<
hour,
minute,
second as u32,
(second.fract() * 10f64.powi(9)).round() as u32,
offset,
)?;
}
Expand All @@ -320,6 +325,7 @@ fn at_date_inner(date: Vec<Item>, at: DateTime<FixedOffset>) -> Option<DateTime<
hour,
minute,
second as u32,
(second.fract() * 10f64.powi(9)).round() as u32,
offset,
)?;
}
Expand Down Expand Up @@ -356,7 +362,7 @@ fn at_date_inner(date: Vec<Item>, at: DateTime<FixedOffset>) -> Option<DateTime<
let delta = (day.num_days_from_monday() as i32
- d.weekday().num_days_from_monday() as i32)
.rem_euclid(7)
+ x * 7;
+ x.checked_mul(7)?;

d = if delta < 0 {
d.checked_sub_days(chrono::Days::new((-delta) as u64))?
Expand Down Expand Up @@ -394,11 +400,11 @@ fn at_date_inner(date: Vec<Item>, at: DateTime<FixedOffset>) -> Option<DateTime<
relative::Relative::Days(x) => d += chrono::Duration::days(x.into()),
relative::Relative::Hours(x) => d += chrono::Duration::hours(x.into()),
relative::Relative::Minutes(x) => {
d += chrono::Duration::minutes(x.into());
d += chrono::Duration::try_minutes(x.into())?;
}
// Seconds are special because they can be given as a float
relative::Relative::Seconds(x) => {
d += chrono::Duration::seconds(x as i64);
d += chrono::Duration::try_seconds(x as i64)?;
}
}
}
Expand Down Expand Up @@ -495,6 +501,16 @@ mod tests {
test_eq_fmt("%Y-%m-%d %H:%M:%S %:z", "Jul 17 06:14:49 2024 GMT"),
);

assert_eq!(
"2024-07-17 06:14:49.567 +00:00",
test_eq_fmt("%Y-%m-%d %H:%M:%S%.f %:z", "Jul 17 06:14:49.567 2024 GMT"),
);

assert_eq!(
"2024-07-17 06:14:49.567 +00:00",
test_eq_fmt("%Y-%m-%d %H:%M:%S%.f %:z", "Jul 17 06:14:49,567 2024 GMT"),
);

assert_eq!(
"2024-07-17 06:14:49 -03:00",
test_eq_fmt("%Y-%m-%d %H:%M:%S %:z", "Jul 17 06:14:49 2024 BRT"),
Expand Down
8 changes: 6 additions & 2 deletions src/items/primitive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,13 @@ pub(super) fn float<'a, E>(input: &mut &'a str) -> winnow::Result<f64, E>
where
E: ParserError<&'a str>,
{
(opt(one_of(['+', '-'])), digit1, opt(preceded('.', digit1)))
(
opt(one_of(['+', '-'])),
digit1,
opt(preceded(one_of(['.', ',']), digit1)),
)
.void()
.take()
.verify_map(|s: &str| s.parse().ok())
.verify_map(|s: &str| s.replace(",", ".").parse().ok())
.parse_next(input)
}
12 changes: 7 additions & 5 deletions src/items/relative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,15 @@ pub enum Relative {
}

impl Relative {
// TODO: determine how to handle multiplication overflows,
// using saturating_mul for now.
fn mul(self, n: i32) -> Self {
match self {
Self::Years(x) => Self::Years(n * x),
Self::Months(x) => Self::Months(n * x),
Self::Days(x) => Self::Days(n * x),
Self::Hours(x) => Self::Hours(n * x),
Self::Minutes(x) => Self::Minutes(n * x),
Self::Years(x) => Self::Years(n.saturating_mul(x)),
Self::Months(x) => Self::Months(n.saturating_mul(x)),
Self::Days(x) => Self::Days(n.saturating_mul(x)),
Self::Hours(x) => Self::Hours(n.saturating_mul(x)),
Self::Minutes(x) => Self::Minutes(n.saturating_mul(x)),
Self::Seconds(x) => Self::Seconds(f64::from(n) * x),
}
}
Expand Down
62 changes: 41 additions & 21 deletions src/items/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use std::fmt::Display;

use chrono::FixedOffset;
use winnow::{
ascii::{digit1, float},
ascii::digit1,
combinator::{alt, opt, peek, preceded},
error::{ContextError, ErrMode, StrContext, StrContextValue},
seq,
Expand All @@ -53,7 +53,7 @@ use winnow::{
use crate::ParseDateTimeError;

use super::{
primitive::{dec_uint, s},
primitive::{dec_uint, float, s},
relative,
};

Expand Down Expand Up @@ -143,7 +143,7 @@ impl Display for Offset {
}

#[derive(Clone)]
enum Suffix {
enum Meridiem {
Am,
Pm,
}
Expand Down Expand Up @@ -178,30 +178,37 @@ pub fn iso(input: &mut &str) -> ModalResult<Time> {
///
/// The hours are restricted to 12 or lower in this format
fn am_pm_time(input: &mut &str) -> ModalResult<Time> {
seq!(
let (h, m, s, meridiem) = seq!(
hour12,
opt(preceded(colon, minute)),
opt(preceded(colon, second)),
alt((
s("am").value(Suffix::Am),
s("a.m.").value(Suffix::Am),
s("pm").value(Suffix::Pm),
s("p.m.").value(Suffix::Pm)
s("am").value(Meridiem::Am),
s("a.m.").value(Meridiem::Am),
s("pm").value(Meridiem::Pm),
s("p.m.").value(Meridiem::Pm)
)),
)
.map(|(h, m, s, suffix)| {
let mut h = h % 12;
if let Suffix::Pm = suffix {
h += 12;
}
Time {
hour: h,
minute: m.unwrap_or(0),
second: s.unwrap_or(0.0),
offset: None,
}
.parse_next(input)?;

if h == 0 {
let mut ctx_err = ContextError::new();
ctx_err.push(StrContext::Expected(StrContextValue::Description(
"hour must be greater than 0 when meridiem is specified",
)));
return Err(ErrMode::Cut(ctx_err));
}

let mut h = h % 12;
if let Meridiem::Pm = meridiem {
h += 12;
}
Ok(Time {
hour: h,
minute: m.unwrap_or(0),
second: s.unwrap_or(0.0),
offset: None,
})
.parse_next(input)
}

/// Parse a colon preceded by whitespace
Expand All @@ -226,7 +233,14 @@ fn minute(input: &mut &str) -> ModalResult<u32> {

/// Parse a number of seconds (preceded by whitespace)
fn second(input: &mut &str) -> ModalResult<f64> {
s(float).verify(|x| *x < 60.0).parse_next(input)
s(float)
.verify(|x| *x < 60.0)
.map(|x| {
// Truncates the fractional part of seconds to 9 digits.
let factor = 10f64.powi(9);
(x * factor).trunc() / factor
})
.parse_next(input)
}

pub(crate) fn timezone(input: &mut &str) -> ModalResult<Offset> {
Expand Down Expand Up @@ -626,6 +640,12 @@ mod tests {
}
}

#[test]
fn invalid() {
assert!(parse(&mut "00:00am").is_err());
assert!(parse(&mut "00:00:00am").is_err());
}

#[test]
fn hours_only() {
let reference = Time {
Expand Down
6 changes: 0 additions & 6 deletions tests/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@ pub fn check_time(input: &str, expected: &str, format: &str, base: Option<DateTi
#[case::full_time_with_spaces("12 : 34 : 56", "12:34:56.000000000")]
#[case::full_time_midnight("00:00:00", "00:00:00.000000000")]
#[case::full_time_almost_midnight("23:59:59", "23:59:59.000000000")]
/* TODO: https://github.com/uutils/parse_datetime/issues/165
#[case::full_time_decimal_seconds("12:34:56.666", "12:34:56.666000000")]
#[case::full_time_decimal_seconds("12:34:56.999999999", "12:34:56.999999999")]
#[case::full_time_decimal_seconds("12:34:56.9999999999", "12:34:56.999999999")]
#[case::full_time_decimal_seconds_after_comma("12:34:56,666", "12:34:56.666000000")]
*/
#[case::without_seconds("12:34", "12:34:00.000000000")]
fn test_time_24h_format(#[case] input: &str, #[case] expected: &str) {
check_time(input, expected, "%H:%M:%S%.9f", None);
Expand All @@ -54,10 +52,8 @@ fn test_time_24h_format(#[case] input: &str, #[case] expected: &str) {
#[case::full_time_capital("12:34:56pm", "12:34:56.000000000")]
#[case::full_time_midnight("00:00:00", "00:00:00.000000000")]
#[case::full_time_almost_midnight("23:59:59", "23:59:59.000000000")]
/* TODO: https://github.com/uutils/parse_datetime/issues/165
#[case::full_time_decimal_seconds("12:34:56.666pm", "12:34:56.666000000")]
#[case::full_time_decimal_seconds_after_comma("12:34:56,666pm", "12:34:56.666000000")]
*/
#[case::without_seconds("12:34pm", "12:34:00.000000000")]
fn test_time_12h_format(#[case] input: &str, #[case] expected: &str) {
check_time(input, expected, "%H:%M:%S%.9f", None);
Expand Down Expand Up @@ -111,10 +107,8 @@ fn test_time_correction_with_overflow(#[case] input: &str, #[case] expected: &st
#[case("23:59:60")]
#[case("13:00:00am")]
#[case("13:00:00pm")]
/* TODO: https://github.com/uutils/parse_datetime/issues/166
#[case("00:00:00am")]
#[case("00:00:00pm")]
*/
#[case("23:59:59 a.m")]
#[case("23:59:59 pm.")]
#[case("23:59:59+24:01")]
Expand Down