How do you implement a parser for a custom type that is fallible?

[theelderbeever]

I am trying to implement a customer non-unit enum type for parsing memory/storage requests. The idea being that it takes a string "5GB" and converts it to an enum StorageUnit::GB(5). The actual parsing is very simple however, I am struggling to make it play nicely with clap. The below example does indeed work since I impl From<&str> for StorageUnit however, my problem is that the parsing is inherently fallible which means now there are just unwraps and panics in what is supposed to be a non-fallible trait bound. this obviously makes the code not very portable. Is there any reason clap can't support TryFrom in addition to From and is there anyway to handle this particular parsing in a portable/friendly manner?

Clap Version: 4.4.18

#[derive(Debug, Args)]
#[command(args_conflicts_with_subcommands = false)]
pub struct TuneCommand {
    #[arg(
        help = "Amount of memory to provide postgres. ie: '32GB'. (default system memory)"
    )]
    memory_limit: Option<StorageUnit>,
}

#[derive(Debug, Clone)]
pub enum StorageUnit {
    B(u64),
    KiB(u64),
    MiB(u64),
    GiB(u64),
    TiB(u64),
}

impl From<&str> for StorageUnit {
    fn from(s: &str) -> Self {
        let value: u64 = s.trim_end_matches(char::is_alphabetic).parse().unwrap();
        let unit = s.trim_start_matches(char::is_numeric);
        match unit {
            "" => StorageUnit::B(value),
            "kB" => StorageUnit::KiB(value),
            "MB" => StorageUnit::MiB(value),
            "GB" => StorageUnit::GiB(value),
            "TB" => StorageUnit::TiB(value),
            _ => panic!("Invalid amount of storage"),
        }
    }
}

[epage]

How we pick the parser for a field is implemented by value_parser!. You can see its docs for how you can adapt your type to allow fallible parsing (likely by implementing FromStr). For more advanced cases, you can also specify the #[arg(value_parser)] attribute. See its docs for the different ways you can explicitly set the field's parser.

[theelderbeever]

I suppose I could have included those details as well as I have tried those.

The FromStr solution doesn't satisfy trait bounds for clap and consequently won't compile

#[derive(Debug, PartialEq, Eq)]
pub struct ParseStorageUnitError;

impl FromStr for StorageUnit {
    type Err = ParseStorageUnitError;
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let value: u64 = s
            .trim_end_matches(char::is_alphabetic)
            .parse()
            .map_err(|_| ParseStorageUnitError)?;
        let unit = s.trim_start_matches(char::is_numeric);
        match unit {
            "" => Ok(StorageUnit::B(value)),
            "kB" => Ok(StorageUnit::KiB(value)),
            "MB" => Ok(StorageUnit::MiB(value)),
            "GB" => Ok(StorageUnit::GiB(value)),
            "TB" => Ok(StorageUnit::TiB(value)),
            _ => Err(ParseStorageUnitError),
        }
    }
}

value_parser.rs(2646, 22): Actual error occurred here
tune.rs(30, 1): doesn't satisfy _: From<&OsStr>
tune.rs(30, 1): doesn't satisfy _: From<OsString>
tune.rs(30, 1): doesn't satisfy _: From<String>
tune.rs(30, 1): doesn't satisfy _: ValueParserFactory
tune.rs(30, 1): doesn't satisfy tune::StorageUnit: clap::ValueEnum
tune.rs(30, 1): doesn't satisfy tune::StorageUnit: std::convert::From<&'s str>
tune.rs(39, 1): doesn't satisfy _: Into<Box<dyn Error + Send + Sync>>
value_parser.rs(2462, 1): doesn't satisfy _: _ValueParserViaParse
mod.rs(445, 1): the traits clap::ValueEnum, clap::builder::ValueParserFactory, std::convert::Into, and std::convert::From must be implemented

Following your message I attempted to use FromStr and add value_parser = clap::value_parser!(String), however, this panics with

Mismatch between definition and access of memory_limit. Could not downcast to tusk::tune::StorageUnit, need to downcast to alloc::string::String

#[derive(Debug, Args)]
#[command(args_conflicts_with_subcommands = false)]
pub struct TuneCommand {
    #[arg(
        value_parser = clap::value_parser!(String),
        help = "Amount of memory to provide postgres. ie: '32GB'. (default system memory)"
    )]
    memory_limit: Option<StorageUnit>,
}

Maybe I am just misunderstanding how value_parser plays into this?

Additionally, impl ValueEnum doesn't work either due to the possible values

impl clap::ValueEnum for StorageUnit {
    fn value_variants<'a>() -> &'a [Self] {
        &[
            Self::B(1),
            Self::KiB(1),
            Self::MiB(1),
            Self::GiB(1),
            Self::TiB(1),
        ]
    }
    fn to_possible_value<'a>(&self) -> ::std::option::Option<clap::builder::PossibleValue> {
        match self {
            Self::B(_) => Some(clap::builder::PossibleValue::new(format!("B"))),
            Self::KiB(_) => Some(clap::builder::PossibleValue::new(format!("kB"))),
            Self::MiB(_) => Some(clap::builder::PossibleValue::new(format!("MB"))),
            Self::GiB(_) => Some(clap::builder::PossibleValue::new(format!("GB"))),
            Self::TiB(_) => Some(clap::builder::PossibleValue::new(format!("TB"))),
            _ => None,
        }
    }
    fn from_str(s: &str, _ignore_case: bool) -> Result<Self, String> {
        let value: u64 = s
            .trim_end_matches(char::is_alphabetic)
            .parse()
            .map_err(|_| "Could not parse amount into number")?;
        let unit = s.trim_start_matches(char::is_numeric);
        match unit {
            "" => Ok(StorageUnit::B(value)),
            "kB" => Ok(StorageUnit::KiB(value)),
            "MB" => Ok(StorageUnit::MiB(value)),
            "GB" => Ok(StorageUnit::GiB(value)),
            "TB" => Ok(StorageUnit::TiB(value)),
            _ => Err("Invalid units detected".to_string()),
        }
    }
}

[theelderbeever]

How do you tell it to use Fn(&str) -> Result<T, E> for the value_parser?

pub fn value_parser(self, parser: impl IntoResettable<ValueParser>) -> Arg
Specify the typed behavior of the argument.

This allows parsing and validating a value before storing it into ArgMatches as the given type.

Possible value parsers include:

value_parser!(T) for auto-selecting a value parser for a given type
Or range expressions like 0..=1 as a shorthand for RangedI64ValueParser
Fn(&str) -> Result<T, E>
[&str] and PossibleValuesParser for static enumerated values
BoolishValueParser, and FalseyValueParser for alternative bool implementations
NonEmptyStringValueParser for basic validation for strings
or any other TypedValueParser implementation
The default value is ValueParser::string.

[epage]

The FromStr solution doesn't satisfy trait bounds for clap and consequently won't compile

We have trait bounds on FromStr that requires that the Error type implements Display

Mismatch between definition and access of memory_limit. Could not downcast to tusk::tune::StorageUnit, need to downcast to alloc::string::String

As clap::value_parser!(String) means "parse into a string" which is invalid because your field isn't Option<String>

How do you tell it to use Fn(&str) -> Result<T, E> for the value_parser?

value_parser = my_func()

See also #5403 (comment) for an example (went to that just because I recently looked at it not because that is the most obvious and definitive place this is documented)

[theelderbeever]

Alright got it working. For some reason adding impl Display to my FromStr Error type didn't work but making the value parser function forcibly return a String error with Result<StorageUnit, String> did.

#[derive(Debug, Args)]
#[command(args_conflicts_with_subcommands = false)]
pub struct TuneCommand {
    #[arg(
        value_parser = parse_storage_unit,
        help = "Amount of memory to provide postgres. ie: '32GB'. (default system memory)"
    )]
    memory_limit: Option<StorageUnit>,
}
// Does work
fn parse_storage_unit(s: &str) -> Result<StorageUnit, String> {
    Ok(StorageUnit::from_str(s).map_err(|_| "Invalid storage unit")?)
}

// Doesn't work. Trait bound errors on `IntoResetable`
// fn parse_storage_unit(s: &str) -> Result<StorageUnit, ParseStorageUnitError> {
//     StorageUnit::from_str(s)
// }

#[derive(Debug, Clone)]
pub enum StorageUnit {
    B(u64),
    KiB(u64),
    MiB(u64),
    GiB(u64),
    TiB(u64),
}

#[derive(Debug, PartialEq, Eq)]
pub struct ParseStorageUnitError;

impl Display for ParseStorageUnitError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        f.write_str("Invalid storage unit")
    }
}

[epage]

Ah, I was mistaken. We are a bit more restrictive than Display

        <Parse as std::str::FromStr>::Err: Into<Box<dyn std::error::Error + Send + Sync + 'static>>,

[theelderbeever]

Yep all of those look like the traits that were being complained about. No worries it's simple enough to just make the parser function return a string.

Is there anywhere that these "special" examples are being tracked? Happy to submit a working example.

[epage]

We're not really actively tracking them but we can discuss them as needed.

We do have the this one which needs a builder variant and could have more cases added.