From e6e9c1818de850447627353e55763943c5db7308 Mon Sep 17 00:00:00 2001 From: Nick Krichevsky Date: Sun, 23 Jan 2022 17:23:40 -0500 Subject: [PATCH] Fix pair parsing to work with fields with non-common specifiers (i.e. month and day of week) --- src/lib.rs | 2 +- src/parse.rs | 48 ++++++++++++++++++--------- src/parse/compound.rs | 76 +++++++++++++++++++++++++------------------ 3 files changed, 78 insertions(+), 48 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 4ff51f4..4d239af 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,7 +48,7 @@ mod tests { #[test_case("* * * jan-jun *", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Range(1, 6), day_of_week: Any})] #[test_case("* * * 1,5 *", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Pair(Box::new(Specifically(1)), Box::new(Specifically(5))), day_of_week: Any})] #[test_case("* * * * 1,5", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Pair(Box::new(Specifically(1)), Box::new(Specifically(5)))})] - #[test_case("* * * * 2,2", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Pair(Box::new(Range(1, 3)), Box::new(Specifically(1)))})] + #[test_case("* * * * mon-wed,1", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Any, day_of_week: Pair(Box::new(Range(1, 3)), Box::new(Specifically(1)))})] // days of week #[test_case("* * * 1 mon", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Specifically(1), day_of_week: Specifically(1)})] #[test_case("* * * 2 MON", &CronEntry{minute: Any, hour: Any, day_of_month: Any, month: Specifically(2), day_of_week: Specifically(1)})] diff --git a/src/parse.rs b/src/parse.rs index 7c15df6..5dd0f9c 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -106,23 +106,15 @@ fn perform_entry_parse(entry: &str) -> IResult<&str, CronEntry> { let (remaining, specifiers) = separated_five_tuple( char(' '), // minute - parse_common_specifier, + possible_pair_specifier(parse_common_specifier), // hour - parse_common_specifier, + possible_pair_specifier(parse_common_specifier), // day of month - parse_common_specifier, + possible_pair_specifier(parse_common_specifier), // month - alt(( - compound::parse_month_range_specifier, - word::parse_month, - parse_common_specifier, - )), + possible_pair_specifier(parse_month_specifier), // day of week - alt(( - compound::parse_day_of_week_range_specifier, - word::parse_day_of_week, - parse_common_specifier, - )), + possible_pair_specifier(parse_day_of_week_specifier), )(entry)?; let parsed_entry = CronEntry { @@ -141,14 +133,39 @@ fn perform_entry_parse(entry: &str) -> IResult<&str, CronEntry> { Ok((remaining, parsed_entry)) } +fn parse_month_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { + alt(( + compound::parse_month_range_specifier, + word::parse_month, + parse_common_specifier, + ))(chunk) +} + +fn parse_day_of_week_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { + alt(( + compound::parse_day_of_week_range_specifier, + word::parse_day_of_week, + parse_common_specifier, + ))(chunk) +} + fn parse_common_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { alt(( - compound::parse_pair_specifier, compound::parse_numeric_range_specifier, simple::parse_simple_specifier, ))(chunk) } +fn possible_pair_specifier<'a, P, E>( + base_parser: P, +) -> impl FnMut(&'a str) -> IResult<&'a str, CronSpecifier, E> +where + P: Parser<&'a str, CronSpecifier, E> + Copy, + E: nom::error::ParseError<&'a str>, +{ + move |chunk: &str| alt((compound::make_pair_parser(base_parser), base_parser))(chunk) +} + /// Parse five elements separated by some kind of constant specifier. /// This is similar to [`nom::sequence::separated_pair`], but with five elements, specifically. /// For simplicity, these parsers must all return the same output type @@ -168,7 +185,6 @@ where P3: Parser, P4: Parser, P5: Parser, - OP: std::fmt::Debug, { const NUM_ELEMENTS: usize = 5; @@ -188,7 +204,7 @@ where for (i, parser) in parsers.into_iter().enumerate() { let (remaining_from_parser, parsed) = parser.parse(remaining)?; remaining = remaining_from_parser; - parser_outputs.push(dbg!(parsed)); + parser_outputs.push(parsed); if i != NUM_ELEMENTS - 1 { let (remaining_from_sep, _) = sep.parse(remaining)?; diff --git a/src/parse/compound.rs b/src/parse/compound.rs index 76b70cf..5eb6802 100644 --- a/src/parse/compound.rs +++ b/src/parse/compound.rs @@ -3,43 +3,57 @@ use crate::parse::{simple, word}; use crate::CronSpecifier; use nom::combinator::fail; +use nom::InputLength; use nom::{ branch::alt, character::complete::char, sequence::separated_pair, AsChar, IResult, InputIter, Parser, Slice, }; -use std::ops::RangeFrom; +use std::ops::{RangeFrom, RangeTo}; -pub(super) fn parse_pair_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { - let space_loc_candidate = chunk.find(' '); - let comma_loc_candidate = chunk.find(','); - if comma_loc_candidate.is_none() { - return fail(chunk); +/// Make a parser that will parse a pair specifier in a cron entry. Each part of the pair must be matched by +/// `component_parser` +pub(super) fn make_pair_parser( + mut component_parser: P, +) -> impl FnMut(I) -> IResult +where + P: Parser, + I: InputIter + InputLength + Slice> + Slice>, + ::Item: AsChar, + E: nom::error::ParseError, +{ + move |chunk: I| { + let space_loc_candidate = chunk.position(|c| c.as_char() == ' '); + let comma_loc_candidate = chunk.position(|c| c.as_char() == ','); + if comma_loc_candidate.is_none() { + return fail(chunk); + } + + let space_before_comma = if let Some(space_loc) = space_loc_candidate { + space_loc < comma_loc_candidate.unwrap() + } else { + false + }; + if space_before_comma { + return fail(chunk); + } + let comma_loc = comma_loc_candidate.unwrap(); + // If the comma is the last position, we can't split this into two segments + if comma_loc == chunk.input_len() - 1 { + return fail(chunk); + } + + // To prevent infinite recursion, we must split the comma ourselves, instead of using separated_pair. + // Otherwise, we will always detect that a comma is present and continue the recursion. + let (remaining, child_spec1) = component_parser.parse(chunk.slice(..comma_loc))?; + if !remaining.input_len() == 0 { + return fail(chunk); + } + + let (remaining, child_spec2) = component_parser.parse(chunk.slice(comma_loc + 1..))?; + let specifier = CronSpecifier::Pair(Box::new(child_spec1), Box::new(child_spec2)); + + Ok((remaining, specifier)) } - - let space_before_comma = if let Some(space_loc) = space_loc_candidate { - space_loc < comma_loc_candidate.unwrap() - } else { - false - }; - if space_before_comma { - return fail(chunk); - } - - let comma_loc = comma_loc_candidate.unwrap(); - // If the comma is the last position, we can't split this into two segments - if comma_loc == chunk.len() - 1 { - return fail(chunk); - } - - // To prevent infinite recursion, we must split the comma ourselves, instead of using separated_pair. - // Otherwise, we will always detect that a comma is present and continue the recursion. - let (remaining, child_spec1) = super::parse_common_specifier(&chunk[..comma_loc])?; - dbg!(&child_spec1); - let (remaining, child_spec2) = super::parse_common_specifier(&chunk[comma_loc + 1..])?; - dbg!(&child_spec2); - let specifier = CronSpecifier::Pair(Box::new(child_spec1), Box::new(child_spec2)); - - Ok((remaining, specifier)) } /// Parse a simple numeric range specifier, such as 5-10.