Fix pair parsing to work with fields with non-common specifiers (i.e. month and day of week)

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)})]

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<I, OP, E>,
     P4: Parser<I, OP, E>,
     P5: Parser<I, OP, E>,
-    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)?;

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<P, I, E>(
+    mut component_parser: P,
+) -> impl FnMut(I) -> IResult<I, CronSpecifier, E>
+where
+    P: Parser<I, CronSpecifier, E>,
+    I: InputIter + InputLength + Slice<RangeTo<usize>> + Slice<RangeFrom<usize>>,
+    <I as InputIter>::Item: AsChar,
+    E: nom::error::ParseError<I>,
+{
+    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.