From 3c4d34e484b763e0483b76b85be72b785514c614 Mon Sep 17 00:00:00 2001 From: Nick Krichevsky Date: Mon, 24 Jan 2022 23:59:57 -0500 Subject: [PATCH] Clean up implementation of list or entry parsing, add error generics --- src/parse.rs | 65 ++++++++++--------------------------------- src/parse/compound.rs | 38 +++++++++++++++++++++---- src/parse/simple.rs | 21 +++++++++----- src/parse/word.rs | 43 +++++++++++++++++----------- 4 files changed, 88 insertions(+), 79 deletions(-) diff --git a/src/parse.rs b/src/parse.rs index f7be3cb..ac7e89e 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -106,15 +106,15 @@ fn perform_entry_parse(entry: &str) -> IResult<&str, CronEntry> { let (remaining, specifiers) = separated_five_tuple( char(' '), // minute - parse_common_specifier, + list_or_single_specifier(parse_common_specifier), // hour - parse_common_specifier, + list_or_single_specifier(parse_common_specifier), // day of month - parse_common_specifier, + list_or_single_specifier(parse_common_specifier), // month - parse_month_specifier, + list_or_single_specifier(parse_month_specifier), // day of week - parse_day_of_week_specifier, + list_or_single_specifier(parse_day_of_week_specifier), )(entry)?; let parsed_entry = CronEntry { @@ -134,76 +134,41 @@ fn perform_entry_parse(entry: &str) -> IResult<&str, CronEntry> { } fn parse_month_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { - // TODO: there's a lot of repetition right here I don't want to clean up this second, - // between this and the other callsites of parse_list_or_element - parse_list_or_element( - parse_month_list_element_specifier, - alt(( - parse_month_list_element_specifier, - simple::parse_star_specifier, - )), - )(chunk) -} - -fn parse_month_list_element_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { alt(( compound::parse_month_range_specifier, word::parse_month, - parse_list_element_common_specifier, + parse_common_specifier, ))(chunk) } fn parse_day_of_week_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { - parse_list_or_element( - parse_day_of_week_list_element_specifier, - alt(( - parse_day_of_week_list_element_specifier, - simple::parse_star_specifier, - )), - )(chunk) -} - -fn parse_day_of_week_list_element_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { alt(( compound::parse_day_of_week_range_specifier, word::parse_day_of_week, - parse_list_element_common_specifier, + parse_common_specifier, ))(chunk) } fn parse_common_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { - parse_list_or_element( - parse_list_element_common_specifier, - parse_standalone_common_specifier, - )(chunk) -} - -fn parse_standalone_common_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { - alt(( - parse_list_element_common_specifier, - simple::parse_star_specifier, - ))(chunk) -} - -fn parse_list_element_common_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { alt(( compound::parse_numeric_range_specifier, simple::parse_numeric_specifier, ))(chunk) } -fn parse_list_or_element<'a, PL, PB, E>( - list_element_parser: PL, - base_parser: PB, +/// Make a parser that will parse either a list of specifiers, or a single one. Each list element must match +/// the given parser. A single specifier must either match this parser, or be * +fn list_or_single_specifier<'a, P, E>( + list_element_parser: P, ) -> impl FnMut(&'a str) -> IResult<&'a str, CronSpecifier, E> where - PL: Parser<&'a str, CronSpecifier, E> + Copy, - PB: Parser<&'a str, CronSpecifier, E>, + P: Parser<&'a str, CronSpecifier, E> + Copy, E: nom::error::ParseError<&'a str>, { alt(( - compound::make_several_specifier_parser(list_element_parser), - base_parser, + compound::several_specifiers(list_element_parser), + list_element_parser, + simple::parse_star_specifier, )) } diff --git a/src/parse/compound.rs b/src/parse/compound.rs index b97e1b5..ed0229c 100644 --- a/src/parse/compound.rs +++ b/src/parse/compound.rs @@ -13,7 +13,7 @@ use std::ops::RangeFrom; /// Make a parser that will parse a list specifier in a cron entry. Each part of the list must be matched by /// `component_parser` -pub(super) fn make_several_specifier_parser( +pub(super) fn several_specifiers( component_parser: P, ) -> impl FnMut(I) -> IResult where @@ -42,13 +42,25 @@ where } /// Parse a simple numeric range specifier, such as 5-10. -pub(super) fn parse_numeric_range_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { +pub(super) fn parse_numeric_range_specifier<'a, E>( + chunk: &'a str, +) -> IResult<&'a str, CronSpecifier, E> +where + E: nom::error::ParseError<&'a str> + + nom::error::FromExternalError<&'a str, std::num::ParseIntError>, +{ build_range_parser(simple::parse_number, simple::parse_number)(chunk) } /// Parse a day of week range specifier, with the left/right of the ranges either being the numeric /// versions of a day of the week, their word forms, or a combination of both. -pub(super) fn parse_day_of_week_range_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { +pub(super) fn parse_day_of_week_range_specifier<'a, E>( + chunk: &'a str, +) -> IResult<&'a str, CronSpecifier, E> +where + E: nom::error::ParseError<&'a str> + + nom::error::FromExternalError<&'a str, std::num::ParseIntError>, +{ build_range_parser( parse_day_of_week_range_branch, parse_day_of_week_range_branch, @@ -57,18 +69,32 @@ pub(super) fn parse_day_of_week_range_specifier(chunk: &str) -> IResult<&str, Cr /// Parse a month range specifier, with the left/right of the ranges either being the numeric /// versions of a month, their word forms, or a combination of both. -pub(super) fn parse_month_range_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { +pub(super) fn parse_month_range_specifier<'a, E>( + chunk: &'a str, +) -> IResult<&'a str, CronSpecifier, E> +where + E: nom::error::ParseError<&'a str> + + nom::error::FromExternalError<&'a str, std::num::ParseIntError>, +{ build_range_parser(parse_month_range_branch, parse_month_range_branch)(chunk) } // unfortunately, these branch functions must be broken out into their own functions, // as in order for `build_range_parser` to work, need `Copy` types, which the impl that // alt returns is not. -fn parse_day_of_week_range_branch(chunk: &str) -> IResult<&str, u8> { +fn parse_day_of_week_range_branch<'a, E>(chunk: &'a str) -> IResult<&'a str, u8, E> +where + E: nom::error::ParseError<&'a str> + + nom::error::FromExternalError<&'a str, std::num::ParseIntError>, +{ alt((word::parse_day_of_week_raw, simple::parse_number))(chunk) } -fn parse_month_range_branch(chunk: &str) -> IResult<&str, u8> { +fn parse_month_range_branch<'a, E>(chunk: &'a str) -> IResult<&'a str, u8, E> +where + E: nom::error::ParseError<&'a str> + + nom::error::FromExternalError<&'a str, std::num::ParseIntError>, +{ alt((word::parse_month_raw, simple::parse_number))(chunk) } diff --git a/src/parse/simple.rs b/src/parse/simple.rs index c012a5b..b37e3d3 100644 --- a/src/parse/simple.rs +++ b/src/parse/simple.rs @@ -4,24 +4,31 @@ use nom::{ character::complete::{char, digit1}, combinator::map_res, + error::FromExternalError, IResult, }; use crate::CronSpecifier; -use std::str::FromStr; /// Parse a raw number, which can be later used to form a specifier. -pub(super) fn parse_number(chunk: &str) -> IResult<&str, u8> { +pub(super) fn parse_number<'a, E>(chunk: &'a str) -> IResult<&'a str, u8, E> +where + E: nom::error::ParseError<&'a str> + FromExternalError<&'a str, std::num::ParseIntError>, +{ map_res(digit1, str::parse)(chunk) } -pub(super) fn parse_numeric_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { - map_res::<_, _, _, _, ::Err, _, _>(parse_number, |num: u8| { - Ok(CronSpecifier::Specifically(num)) - })(chunk) +pub(super) fn parse_numeric_specifier<'a, E>(chunk: &'a str) -> IResult<&'a str, CronSpecifier, E> +where + E: nom::error::ParseError<&'a str> + FromExternalError<&'a str, std::num::ParseIntError>, +{ + map_res(parse_number, |num: u8| Ok(CronSpecifier::Specifically(num)))(chunk) } -pub(super) fn parse_star_specifier(chunk: &str) -> IResult<&str, CronSpecifier> { +pub(super) fn parse_star_specifier<'a, E>(chunk: &'a str) -> IResult<&'a str, CronSpecifier, E> +where + E: nom::error::ParseError<&'a str>, +{ let (remaining, _) = char('*')(chunk)?; Ok((remaining, CronSpecifier::Any)) diff --git a/src/parse/word.rs b/src/parse/word.rs index 30c17f4..3a6d855 100644 --- a/src/parse/word.rs +++ b/src/parse/word.rs @@ -12,16 +12,12 @@ use crate::CronSpecifier; /// `ParserIterator` exists as an implementation of nom's [`nom::branch::Alt`] trait so that we can use arbitrary /// iterators for [`nom::branch::alt`]. -struct ParserIterator<'a, Iter, I, O, E>(Iter) -where - I: 'a, - O: 'a, - E: 'a, - Iter: Iterator>; +struct ParserIterator(Iter); -impl<'a, Iter, I, O, E> Alt for ParserIterator<'a, Iter, I, O, E> +impl<'a, Iter, I, O, E> Alt for ParserIterator where I: Clone + 'a, + O: 'a, E: nom::error::ParseError + 'a, Iter: Iterator>, { @@ -57,7 +53,10 @@ where /// Parse a day of the week, which is a case-insensitive string of the first three letters /// of any given day of the week (e.g. tue). This "raw" form converts to a number directly, /// rather than wrapping it in a specifier. -pub(super) fn parse_day_of_week_raw(chunk: &str) -> IResult<&str, u8> { +pub(super) fn parse_day_of_week_raw<'a, E>(chunk: &'a str) -> IResult<&'a str, u8, E> +where + E: nom::error::ParseError<&'a str>, +{ parse_possible_word_set( chunk, &["sun", "mon", "tue", "wed", "thu", "fri", "sat", "sun"], @@ -66,7 +65,10 @@ pub(super) fn parse_day_of_week_raw(chunk: &str) -> IResult<&str, u8> { /// Parse a day of the week, which is a case-insensitive string of the first three letters /// of any given day of the week (e.g. tue). -pub(super) fn parse_day_of_week(chunk: &str) -> IResult<&str, CronSpecifier> { +pub(super) fn parse_day_of_week<'a, E>(chunk: &'a str) -> IResult<&'a str, CronSpecifier, E> +where + E: nom::error::ParseError<&'a str>, +{ let (remaining, raw_specifier) = parse_day_of_week_raw(chunk)?; Ok((remaining, CronSpecifier::Specifically(raw_specifier))) @@ -75,7 +77,10 @@ pub(super) fn parse_day_of_week(chunk: &str) -> IResult<&str, CronSpecifier> { /// Parse a month, which is a case-insensitive string of the first three letters /// of any given month (e.g. tue). This "raw" form converts to a number directly, /// rather than wrapping it in a specifier. -pub(super) fn parse_month_raw(chunk: &str) -> IResult<&str, u8> { +pub(super) fn parse_month_raw<'a, E>(chunk: &'a str) -> IResult<&'a str, u8, E> +where + E: nom::error::ParseError<&'a str>, +{ parse_possible_word_set( chunk, &[ @@ -93,7 +98,10 @@ pub(super) fn parse_month_raw(chunk: &str) -> IResult<&str, u8> { /// Parse a month, which is a case-insensitive string of the first three letters /// of any given month (e.g. tue). -pub(super) fn parse_month(chunk: &str) -> IResult<&str, CronSpecifier> { +pub(super) fn parse_month<'a, E>(chunk: &'a str) -> IResult<&'a str, CronSpecifier, E> +where + E: nom::error::ParseError<&'a str>, +{ let (remaining, raw_specifier) = parse_month_raw(chunk)?; Ok((remaining, CronSpecifier::Specifically(raw_specifier))) @@ -103,10 +111,13 @@ pub(super) fn parse_month(chunk: &str) -> IResult<&str, CronSpecifier> { /// the numeric index of the element that was matched. Because this returns a u8, it /// is a programmer error to pass any slice of length > 255 to this method, and as such, /// it will panic. -fn parse_possible_word_set<'a>( +fn parse_possible_word_set<'a, E>( chunk: &'a str, possibilities: &'a [&'a str], -) -> IResult<&'a str, u8> { +) -> IResult<&'a str, u8, E> +where + E: nom::error::ParseError<&'a str>, +{ // It is a propgrammer error to use a longer slice here; we must be able to enumerate these items into a u8 assert!(possibilities.len() <= 255); @@ -121,7 +132,7 @@ fn parse_possible_word_set<'a>( let parser_refs = parsers .iter_mut() - .map(|parser| parser as &mut dyn nom::Parser<&str, &str, nom::error::Error<&str>>); + .map(|parser| parser as &mut dyn nom::Parser<&str, &str, E>); let (remaining, candidate) = alt(ParserIterator(parser_refs))(chunk)?; @@ -147,7 +158,7 @@ mod tests { // to produce a ParserSlice #[test] fn test_alt_takes_first_matching_parser() { - let res = alt(ParserIterator::<_, _, _, nom::error::Error<_>>( + let res = alt(ParserIterator( [tag("abc"), tag("def"), tag("efg")] .iter_mut() .map(|x| x as &mut dyn nom::Parser<&str, &str, nom::error::Error<&str>>), @@ -159,7 +170,7 @@ mod tests { #[test] fn test_alt_will_fail_if_no_parsers_match() { - let res = alt(ParserIterator::<_, _, _, nom::error::Error<_>>( + let res = alt(ParserIterator( [tag("abc"), tag("def"), tag("efg")] .iter_mut() .map(|x| x as &mut dyn nom::Parser<&str, &str, nom::error::Error<&str>>),