From 63aa236141479678c0177b2e0470af810343154c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 17 May 2022 13:56:36 -0500 Subject: [PATCH] feat(parser): Convenient range value parsers There are several approaches with this - `value_parser(N..M)`: creates an i64 range - `value_parser(value_parser!(u16).range(10..))`: creates an u16 range that starts at 10 - `RangeI64ValueParser`: create whatever range you want I was hoping to generalize `RangeI64ValueParser` for any source type, not just i64, but ran into issues and decided to punt. I chose `i64` as the source type as it seemed the most general and didn't run into portability issues like `isize`. This is a step towards #3199. All that is left is for the derive to use this. --- src/builder/arg.rs | 9 +- src/builder/mod.rs | 1 + src/builder/value_parser.rs | 487 +++++++++++++++++++++++++++++++++++- 3 files changed, 485 insertions(+), 12 deletions(-) diff --git a/src/builder/arg.rs b/src/builder/arg.rs index 4de3c2fabc3..7909960233a 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -1010,6 +1010,7 @@ impl<'help> Arg<'help> { /// - [`value_parser!`][crate::value_parser!] for auto-selecting a value parser for a given type /// - [`BoolishValueParser`][crate::builder::BoolishValueParser], and [`FalseyValueParser`][crate::builder::FalseyValueParser] for alternative `bool` implementations /// - [`NonEmptyStringValueParser`][crate::builder::NonEmptyStringValueParser] for basic validation for strings + /// - [`RangedI64ValueParser`][crate::builder::RangedI64ValueParser] for numeric ranges /// - [`ArgEnumValueParser`][crate::builder::ArgEnumValueParser] and [`PossibleValuesParser`][crate::builder::PossibleValuesParser] for static enumerated values /// - or any other [`TypedValueParser`][crate::builder::TypedValueParser] implementation /// @@ -1031,13 +1032,13 @@ impl<'help> Arg<'help> { /// .arg( /// clap::Arg::new("port") /// .long("port") - /// .value_parser(clap::value_parser!(usize)) + /// .value_parser(clap::value_parser!(u16).range(3000..)) /// .takes_value(true) /// .required(true) /// ); /// /// let m = cmd.try_get_matches_from_mut( - /// ["cmd", "--hostname", "rust-lang.org", "--port", "80"] + /// ["cmd", "--hostname", "rust-lang.org", "--port", "3001"] /// ).unwrap(); /// /// let color: &String = m.get_one("color").unwrap().unwrap(); @@ -1046,8 +1047,8 @@ impl<'help> Arg<'help> { /// let hostname: &String = m.get_one("hostname").unwrap().unwrap(); /// assert_eq!(hostname, "rust-lang.org"); /// - /// let port: usize = *m.get_one("port").unwrap().unwrap(); - /// assert_eq!(port, 80); + /// let port: u16 = *m.get_one("port").unwrap().unwrap(); + /// assert_eq!(port, 3001); /// ``` pub fn value_parser(mut self, parser: impl Into) -> Self { self.value_parser = Some(parser.into()); diff --git a/src/builder/mod.rs b/src/builder/mod.rs index 60f6c6722b2..5cd8e154bcf 100644 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -41,6 +41,7 @@ pub use value_parser::NonEmptyStringValueParser; pub use value_parser::OsStringValueParser; pub use value_parser::PathBufValueParser; pub use value_parser::PossibleValuesParser; +pub use value_parser::RangedI64ValueParser; pub use value_parser::StringValueParser; pub use value_parser::TypedValueParser; pub use value_parser::ValueParser; diff --git a/src/builder/value_parser.rs b/src/builder/value_parser.rs index 8c360c4bdba..4aa8fc0b454 100644 --- a/src/builder/value_parser.rs +++ b/src/builder/value_parser.rs @@ -1,3 +1,5 @@ +use std::convert::TryInto; +use std::ops::RangeBounds; use std::sync::Arc; use crate::parser::AnyValue; @@ -34,13 +36,13 @@ use crate::parser::AnyValueId; /// .arg( /// clap::Arg::new("port") /// .long("port") -/// .value_parser(clap::value_parser!(usize)) +/// .value_parser(clap::value_parser!(u16).range(3000..)) /// .takes_value(true) /// .required(true) /// ); /// /// let m = cmd.try_get_matches_from_mut( -/// ["cmd", "--hostname", "rust-lang.org", "--port", "80"] +/// ["cmd", "--hostname", "rust-lang.org", "--port", "3001"] /// ).unwrap(); /// /// let color: &String = m.get_one("color").unwrap().unwrap(); @@ -49,8 +51,8 @@ use crate::parser::AnyValueId; /// let hostname: &String = m.get_one("hostname").unwrap().unwrap(); /// assert_eq!(hostname, "rust-lang.org"); /// -/// let port: usize = *m.get_one("port").unwrap().unwrap(); -/// assert_eq!(port, 80); +/// let port: u16 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 3001); /// ``` #[derive(Clone)] pub struct ValueParser(pub(crate) ValueParserInner); @@ -76,6 +78,7 @@ impl ValueParser { /// Pre-existing implementations include: /// - [`ArgEnumValueParser`] and [`PossibleValuesParser`] for static enumerated values /// - [`BoolishValueParser`] and [`FalseyValueParser`] for alternative `bool` implementations + /// - [`RangedI64ValueParser`] /// - [`NonEmptyStringValueParser`] /// /// # Example @@ -231,12 +234,218 @@ impl ValueParser { } } +/// Convert a [`TypedValueParser`] to [`ValueParser`] +/// +/// # Example +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("hostname") +/// .long("hostname") +/// .value_parser(clap::builder::NonEmptyStringValueParser) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut( +/// ["cmd", "--hostname", "rust-lang.org"] +/// ).unwrap(); +/// +/// let hostname: &String = m.get_one("hostname").unwrap().unwrap(); +/// assert_eq!(hostname, "rust-lang.org"); +/// ``` impl From

for ValueParser { fn from(p: P) -> Self { ValueParser(ValueParserInner::Other(Arc::new(p))) } } +/// Create an `i64` [`ValueParser`] from a `N..M` range +/// +/// See [`RangedI64ValueParser`] for more control over the output type. +/// +/// # Examples +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("port") +/// .long("port") +/// .value_parser(3000..4000) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut(["cmd", "--port", "3001"]).unwrap(); +/// let port: i64 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 3001); +/// ``` +impl From> for ValueParser { + fn from(value: std::ops::Range) -> Self { + let inner = RangedI64ValueParser::::new().range(value.start..value.end); + Self::from(inner) + } +} + +/// Create an `i64` [`ValueParser`] from a `N..=M` range +/// +/// See [`RangedI64ValueParser`] for more control over the output type. +/// +/// # Examples +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("port") +/// .long("port") +/// .value_parser(3000..=4000) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut(["cmd", "--port", "3001"]).unwrap(); +/// let port: i64 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 3001); +/// ``` +impl From> for ValueParser { + fn from(value: std::ops::RangeInclusive) -> Self { + let inner = RangedI64ValueParser::::new().range(value.start()..=value.end()); + Self::from(inner) + } +} + +/// Create an `i64` [`ValueParser`] from a `N..` range +/// +/// See [`RangedI64ValueParser`] for more control over the output type. +/// +/// # Examples +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("port") +/// .long("port") +/// .value_parser(3000..) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut(["cmd", "--port", "3001"]).unwrap(); +/// let port: i64 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 3001); +/// ``` +impl From> for ValueParser { + fn from(value: std::ops::RangeFrom) -> Self { + let inner = RangedI64ValueParser::::new().range(value.start..); + Self::from(inner) + } +} + +/// Create an `i64` [`ValueParser`] from a `..M` range +/// +/// See [`RangedI64ValueParser`] for more control over the output type. +/// +/// # Examples +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("port") +/// .long("port") +/// .value_parser(..3000) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut(["cmd", "--port", "80"]).unwrap(); +/// let port: i64 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 80); +/// ``` +impl From> for ValueParser { + fn from(value: std::ops::RangeTo) -> Self { + let inner = RangedI64ValueParser::::new().range(..value.end); + Self::from(inner) + } +} + +/// Create an `i64` [`ValueParser`] from a `..=M` range +/// +/// See [`RangedI64ValueParser`] for more control over the output type. +/// +/// # Examples +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("port") +/// .long("port") +/// .value_parser(..=3000) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut(["cmd", "--port", "80"]).unwrap(); +/// let port: i64 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 80); +/// ``` +impl From> for ValueParser { + fn from(value: std::ops::RangeToInclusive) -> Self { + let inner = RangedI64ValueParser::::new().range(..=value.end); + Self::from(inner) + } +} + +/// Create an `i64` [`ValueParser`] from a `..` range +/// +/// See [`RangedI64ValueParser`] for more control over the output type. +/// +/// # Examples +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("port") +/// .long("port") +/// .value_parser(..) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut(["cmd", "--port", "3001"]).unwrap(); +/// let port: i64 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 3001); +/// ``` +impl From for ValueParser { + fn from(value: std::ops::RangeFull) -> Self { + let inner = RangedI64ValueParser::::new().range(value); + Self::from(inner) + } +} + +/// Create a [`ValueParser`] with [`PossibleValuesParser`] +/// +/// See [`PossibleValuesParser`] for more flexibility in creating the +/// [`PossibleValue`][crate::PossibleValue]s. +/// +/// # Examples +/// +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("color") +/// .long("color") +/// .value_parser(["always", "auto", "never"]) +/// .default_value("auto") +/// ); +/// +/// let m = cmd.try_get_matches_from_mut( +/// ["cmd", "--color", "never"] +/// ).unwrap(); +/// +/// let color: &String = m.get_one("color").unwrap().unwrap(); +/// assert_eq!(color, "never"); +/// ``` impl From<[P; C]> for ValueParser where P: Into>, @@ -739,6 +948,201 @@ where } } +/// Parse number that fall within a range of values +/// +/// # Example +/// +/// Usage: +/// ```rust +/// let mut cmd = clap::Command::new("raw") +/// .arg( +/// clap::Arg::new("port") +/// .long("port") +/// .value_parser(clap::value_parser!(u16).range(3000..)) +/// .takes_value(true) +/// .required(true) +/// ); +/// +/// let m = cmd.try_get_matches_from_mut(["cmd", "--port", "3001"]).unwrap(); +/// let port: u16 = *m.get_one("port").unwrap().unwrap(); +/// assert_eq!(port, 3001); +/// ``` +/// +/// Semantics: +/// ```rust +/// # use std::ffi::OsStr; +/// # use clap::builder::TypedValueParser; +/// # let cmd = clap::Command::new("test"); +/// # let arg = None; +/// let value_parser = clap::builder::RangedI64ValueParser::::new().range(-1..200); +/// assert!(value_parser.parse_ref(&cmd, arg, OsStr::new("random")).is_err()); +/// assert!(value_parser.parse_ref(&cmd, arg, OsStr::new("")).is_err()); +/// assert!(value_parser.parse_ref(&cmd, arg, OsStr::new("-200")).is_err()); +/// assert!(value_parser.parse_ref(&cmd, arg, OsStr::new("300")).is_err()); +/// assert_eq!(value_parser.parse_ref(&cmd, arg, OsStr::new("-1")).unwrap(), -1); +/// assert_eq!(value_parser.parse_ref(&cmd, arg, OsStr::new("0")).unwrap(), 0); +/// assert_eq!(value_parser.parse_ref(&cmd, arg, OsStr::new("50")).unwrap(), 50); +/// ``` +#[derive(Copy, Clone, Debug)] +pub struct RangedI64ValueParser = i64> { + bounds: (std::ops::Bound, std::ops::Bound), + target: std::marker::PhantomData, +} + +impl> RangedI64ValueParser { + /// Select full range of `i64` + pub fn new() -> Self { + Self::from(..) + } + + /// Narrow the supported range + pub fn range>(mut self, range: B) -> Self { + // Consideration: when the user does `value_parser!(u8).range()` + // - Avoid programming mistakes by accidentally expanding the range + // - Make it convenient to limit the range like with `..10` + let start = match range.start_bound() { + l @ std::ops::Bound::Included(i) => { + debug_assert!( + self.bounds.contains(i), + "{} must be in {:?}", + i, + self.bounds + ); + l.cloned() + } + l @ std::ops::Bound::Excluded(i) => { + debug_assert!( + self.bounds.contains(&i.saturating_add(1)), + "{} must be in {:?}", + i, + self.bounds + ); + l.cloned() + } + std::ops::Bound::Unbounded => self.bounds.start_bound().cloned(), + }; + let end = match range.end_bound() { + l @ std::ops::Bound::Included(i) => { + debug_assert!( + self.bounds.contains(i), + "{} must be in {:?}", + i, + self.bounds + ); + l.cloned() + } + l @ std::ops::Bound::Excluded(i) => { + debug_assert!( + self.bounds.contains(&i.saturating_sub(1)), + "{} must be in {:?}", + i, + self.bounds + ); + l.cloned() + } + std::ops::Bound::Unbounded => self.bounds.end_bound().cloned(), + }; + self.bounds = (start, end); + self + } + + fn format_bounds(&self) -> String { + let mut result; + match self.bounds.0 { + std::ops::Bound::Included(i) => { + result = i.to_string(); + } + std::ops::Bound::Excluded(i) => { + result = i.saturating_add(1).to_string(); + } + std::ops::Bound::Unbounded => { + result = i64::MIN.to_string(); + } + } + result.push_str(".."); + match self.bounds.0 { + std::ops::Bound::Included(i) => { + result.push('='); + result.push_str(&i.to_string()); + } + std::ops::Bound::Excluded(i) => { + result.push_str(&i.to_string()); + } + std::ops::Bound::Unbounded => { + result.push_str(&i64::MAX.to_string()); + } + } + result + } +} + +impl> TypedValueParser for RangedI64ValueParser +where + >::Error: Send + Sync + 'static + std::error::Error + ToString, +{ + type Value = T; + + fn parse_ref( + &self, + cmd: &crate::Command, + arg: Option<&crate::Arg>, + raw_value: &std::ffi::OsStr, + ) -> Result { + let value = raw_value.to_str().ok_or_else(|| { + crate::Error::invalid_utf8( + cmd, + crate::output::Usage::new(cmd).create_usage_with_title(&[]), + ) + })?; + let value = value.parse::().map_err(|err| { + let arg = arg + .map(|a| a.to_string()) + .unwrap_or_else(|| "...".to_owned()); + crate::Error::value_validation( + arg, + raw_value.to_string_lossy().into_owned(), + err.into(), + ) + .with_cmd(cmd) + })?; + if !self.bounds.contains(&value) { + let arg = arg + .map(|a| a.to_string()) + .unwrap_or_else(|| "...".to_owned()); + return Err(crate::Error::value_validation( + arg, + raw_value.to_string_lossy().into_owned(), + format!("{} is not in {}", value, self.format_bounds()).into(), + ) + .with_cmd(cmd)); + } + + let value: Result = value.try_into(); + let value = value.map_err(|err| { + let arg = arg + .map(|a| a.to_string()) + .unwrap_or_else(|| "...".to_owned()); + crate::Error::value_validation( + arg, + raw_value.to_string_lossy().into_owned(), + err.into(), + ) + .with_cmd(cmd) + })?; + + Ok(value) + } +} + +impl, B: RangeBounds> From for RangedI64ValueParser { + fn from(range: B) -> Self { + Self { + bounds: (range.start_bound().cloned(), range.end_bound().cloned()), + target: Default::default(), + } + } +} + /// Implementation for [`ValueParser::bool`] /// /// Useful for composing new [`TypedValueParser`]s @@ -1035,23 +1439,81 @@ pub mod via_prelude { #[doc(hidden)] pub trait ValueParserViaBuiltIn: private::ValueParserViaBuiltInSealed { - fn value_parser(&self) -> ValueParser; + type Parser; + fn value_parser(&self) -> Self::Parser; } impl ValueParserViaBuiltIn for &&AutoValueParser { - fn value_parser(&self) -> ValueParser { + type Parser = ValueParser; + fn value_parser(&self) -> Self::Parser { ValueParser::string() } } impl ValueParserViaBuiltIn for &&AutoValueParser { - fn value_parser(&self) -> ValueParser { + type Parser = ValueParser; + fn value_parser(&self) -> Self::Parser { ValueParser::os_string() } } impl ValueParserViaBuiltIn for &&AutoValueParser { - fn value_parser(&self) -> ValueParser { + type Parser = ValueParser; + fn value_parser(&self) -> Self::Parser { ValueParser::path_buf() } } + impl ValueParserViaBuiltIn for &&AutoValueParser { + type Parser = RangedI64ValueParser; + fn value_parser(&self) -> Self::Parser { + let start: i64 = u8::MIN.into(); + let end: i64 = u8::MAX.into(); + RangedI64ValueParser::new().range(start..=end) + } + } + impl ValueParserViaBuiltIn for &&AutoValueParser { + type Parser = RangedI64ValueParser; + fn value_parser(&self) -> Self::Parser { + let start: i64 = i8::MIN.into(); + let end: i64 = i8::MAX.into(); + RangedI64ValueParser::new().range(start..=end) + } + } + impl ValueParserViaBuiltIn for &&AutoValueParser { + type Parser = RangedI64ValueParser; + fn value_parser(&self) -> Self::Parser { + let start: i64 = u16::MIN.into(); + let end: i64 = u16::MAX.into(); + RangedI64ValueParser::new().range(start..=end) + } + } + impl ValueParserViaBuiltIn for &&AutoValueParser { + type Parser = RangedI64ValueParser; + fn value_parser(&self) -> Self::Parser { + let start: i64 = i16::MIN.into(); + let end: i64 = i16::MAX.into(); + RangedI64ValueParser::new().range(start..=end) + } + } + impl ValueParserViaBuiltIn for &&AutoValueParser { + type Parser = RangedI64ValueParser; + fn value_parser(&self) -> Self::Parser { + let start: i64 = u32::MIN.into(); + let end: i64 = u32::MAX.into(); + RangedI64ValueParser::new().range(start..=end) + } + } + impl ValueParserViaBuiltIn for &&AutoValueParser { + type Parser = RangedI64ValueParser; + fn value_parser(&self) -> Self::Parser { + let start: i64 = i32::MIN.into(); + let end: i64 = i32::MAX.into(); + RangedI64ValueParser::new().range(start..=end) + } + } + impl ValueParserViaBuiltIn for &&AutoValueParser { + type Parser = RangedI64ValueParser; + fn value_parser(&self) -> Self::Parser { + RangedI64ValueParser::new() + } + } #[doc(hidden)] pub trait ValueParserViaArgEnum: private::ValueParserViaArgEnumSealed { @@ -1115,6 +1577,8 @@ pub mod via_prelude { /// assert_eq!(format!("{:?}", parser), "ValueParser::os_string"); /// let parser = clap::value_parser!(std::path::PathBuf); /// assert_eq!(format!("{:?}", parser), "ValueParser::path_buf"); +/// let parser = clap::value_parser!(u16).range(3000..); +/// assert_eq!(format!("{:?}", parser), "RangedI64ValueParser { bounds: (Included(3000), Included(65535)), target: PhantomData }"); /// /// // FromStr types /// let parser = clap::value_parser!(usize); @@ -1170,6 +1634,13 @@ mod private { impl ValueParserViaBuiltInSealed for &&AutoValueParser {} impl ValueParserViaBuiltInSealed for &&AutoValueParser {} impl ValueParserViaBuiltInSealed for &&AutoValueParser {} + impl ValueParserViaBuiltInSealed for &&AutoValueParser {} + impl ValueParserViaBuiltInSealed for &&AutoValueParser {} + impl ValueParserViaBuiltInSealed for &&AutoValueParser {} + impl ValueParserViaBuiltInSealed for &&AutoValueParser {} + impl ValueParserViaBuiltInSealed for &&AutoValueParser {} + impl ValueParserViaBuiltInSealed for &&AutoValueParser {} + impl ValueParserViaBuiltInSealed for &&AutoValueParser {} pub trait ValueParserViaArgEnumSealed {} impl ValueParserViaArgEnumSealed for &AutoValueParser {}