From 2749125b1a453b21b386a493a6b733115827a7db Mon Sep 17 00:00:00 2001 From: kennytm Date: Wed, 8 Nov 2017 17:33:49 +0800 Subject: [PATCH 1/2] Implement custom string parser from either &str or &OsStr. Fix #2. Fix #3. --- structopt-derive/src/lib.rs | 203 ++++++++++++++++++++++++++++++--- tests/custom-string-parsers.rs | 125 ++++++++++++++++++++ 2 files changed, 311 insertions(+), 17 deletions(-) create mode 100644 tests/custom-string-parsers.rs diff --git a/structopt-derive/src/lib.rs b/structopt-derive/src/lib.rs index 655486aa..fba81852 100644 --- a/structopt-derive/src/lib.rs +++ b/structopt-derive/src/lib.rs @@ -64,7 +64,8 @@ //! The `FromStr` trait is used to convert the argument to the given //! type, and the `Arg::validator` method is set to a method using //! `FromStr::Err::description()` (`FromStr::Err` must implement -//! `std::Error::Error`). +//! `std::Error::Error`). If you would like to use a custom string parser other +//! than `FromStr`, see the [same titled section](#custom-string-parsers) below. //! //! Thus, the `speed` argument is generated as: //! @@ -227,6 +228,52 @@ //! Quux //! } //! ``` +//! +//! ## Custom string parsers +//! +//! If the field type does not have a `FromStr` implementation, or you would +//! like to provide a custom parsing scheme other than `FromStr`, you may +//! provide a custom string parser using `parse(...)` like this: +//! +//! ```ignore +//! use std::num::ParseIntError; +//! use std::path::PathBuf; +//! +//! fn parse_hex(src: &str) -> Result { +//! u32::from_str_radix(src, 16) +//! } +//! +//! #[derive(StructOpt)] +//! struct HexReader { +//! #[structopt(short = "n", parse(try_from_str = "parse_hex"))] +//! number: u32, +//! #[structopt(short = "o", parse(from_os_str))] +//! output: PathBuf, +//! } +//! ``` +//! +//! There are four kinds custom string parsers: +//! +//! | Kind | Signature | Default | +//! |-------------------|---------------------------------------|---------------------------------| +//! | `from_str` | `fn(&str) -> T` | `::std::convert::From::from` | +//! | `try_from_str` | `fn(&str) -> Result` | `::std::str::FromStr::from_str` | +//! | `from_os_str` | `fn(&OsStr) -> T` | `::std::convert::From::from` | +//! | `try_from_os_str` | `fn(&OsStr) -> Result` | (no default function) | +//! +//! When supplying a custom string parser, `bool` and `u64` will not be treated +//! specially: +//! +//! Type | Effect | Added method call to `clap::Arg` +//! ------------|-------------------|-------------------------------------- +//! `Option` | optional argument | `.takes_value(true).multiple(false)` +//! `Vec` | list of arguments | `.takes_value(true).multiple(true)` +//! `T` | required argument | `.takes_value(true).multiple(false).required(!has_default)` +//! +//! In the `try_from_*` variants, the function will run twice on valid input: +//! once to validate, and once to parse. Hence, make sure the function is +//! side-effect-free. + extern crate proc_macro; extern crate syn; @@ -286,6 +333,19 @@ fn sub_type(t: &syn::Ty) -> Option<&syn::Ty> { #[derive(Debug, Clone, Copy)] enum AttrSource { Struct, Field, } +#[derive(Debug)] +enum Parser { + /// Parse an option to using a `fn(&str) -> T` function. The function should never fail. + FromStr, + /// Parse an option to using a `fn(&str) -> Result` function. The error will be + /// converted to a string using `.to_string()`. + TryFromStr, + /// Parse an option to using a `fn(&OsStr) -> T` function. The function should never fail. + FromOsStr, + /// Parse an option to using a `fn(&OsStr) -> Result` function. + TryFromOsStr, +} + fn extract_attrs<'a>(attrs: &'a [Attribute], attr_source: AttrSource) -> Box + 'a> { let settings_attrs = attrs.iter() .filter_map(|attr| match attr.value { @@ -355,6 +415,67 @@ fn is_subcommand(field: &Field) -> bool { }) } +fn get_parser(field: &Field) -> Option<(Parser, quote::Tokens)> { + field.attrs.iter() + .flat_map(|attr| { + if let MetaItem::List(ref i, ref l) = attr.value { + if i == "structopt" { + return &**l; + } + } + &[] + }) + .filter_map(|attr| { + if let NestedMetaItem::MetaItem(MetaItem::List(ref i, ref l)) = *attr { + if i == "parse" { + return l.first(); + } + } + None + }) + .map(|attr| { + match *attr { + NestedMetaItem::MetaItem(MetaItem::NameValue(ref i, Lit::Str(ref v, _))) => { + let function = parse_path(v).expect("parser function path"); + let parser = if i == "from_str" { + Parser::FromStr + } else if i == "try_from_str" { + Parser::TryFromStr + } else if i == "from_os_str" { + Parser::FromOsStr + } else if i == "try_from_os_str" { + Parser::TryFromOsStr + } else { + panic!("unsupported parser {}", i); + }; + (parser, quote!(#function)) + } + NestedMetaItem::MetaItem(MetaItem::Word(ref i)) => { + if i == "from_str" { + (Parser::FromStr, quote!(::std::convert::From::from)) + } else if i == "try_from_str" { + (Parser::TryFromStr, quote!(::std::str::FromStr::from_str)) + } else if i == "from_os_str" { + (Parser::FromOsStr, quote!(::std::convert::From::from)) + } else if i == "try_from_os_str" { + panic!("cannot omit parser function name with `try_from_os_str`") + } else { + panic!("unsupported parser {}", i); + } + } + _ => panic!("unknown value parser specification"), + } + }) + .next() +} + +fn despecialize_bool_and_u64(cur_type: Ty) -> Ty { + match cur_type { + Ty::Bool | Ty::U64 => Ty::Other, + rest => rest, + } +} + /// Generate a block of code to add arguments/subcommands corresponding to /// the `fields` to an app. fn gen_augmentation(fields: &[Field], app_var: &Ident) -> quote::Tokens { @@ -382,22 +503,37 @@ fn gen_augmentation(fields: &[Field], app_var: &Ident) -> quote::Tokens { Ty::Vec | Ty::Option => sub_type(&field.ty).unwrap_or(&field.ty), _ => &field.ty, }; - let validator = quote! { - validator(|s| s.parse::<#convert_type>() - .map(|_| ()) - .map_err(|e| e.description().into())) + let (cur_type, validator) = match get_parser(field) { + None => (cur_type, quote! { + .validator(|s| { + s.parse::<#convert_type>() + .map(|_| ()) + .map_err(|e| e.description().into()) + }) + }), + Some((Parser::TryFromStr, f)) => (despecialize_bool_and_u64(cur_type), quote! { + .validator(|s| { + #f(&s) + .map(|_| ()) + .map_err(|e| e.to_string()) + }) + }), + Some((Parser::TryFromOsStr, f)) => (despecialize_bool_and_u64(cur_type), quote! { + .validator_os(|s| #f(&s).map(|_| ())) + }), + Some(_) => (despecialize_bool_and_u64(cur_type), quote! {}), }; let modifier = match cur_type { Ty::Bool => quote!( .takes_value(false).multiple(false) ), Ty::U64 => quote!( .takes_value(false).multiple(true) ), - Ty::Option => quote!( .takes_value(true).multiple(false).#validator ), - Ty::Vec => quote!( .takes_value(true).multiple(true).#validator ), + Ty::Option => quote!( .takes_value(true).multiple(false) #validator ), + Ty::Vec => quote!( .takes_value(true).multiple(true) #validator ), Ty::Other => { let required = extract_attrs(&field.attrs, AttrSource::Field) .find(|&(ref i, _)| i.as_ref() == "default_value" || i.as_ref() == "default_value_raw") .is_none(); - quote!( .takes_value(true).multiple(false).required(#required).#validator ) + quote!( .takes_value(true).multiple(false).required(#required) #validator ) }, }; let from_attr = extract_attrs(&field.attrs, AttrSource::Field) @@ -445,24 +581,57 @@ fn gen_constructor(fields: &[Field]) -> quote::Tokens { }; quote!( #field_name: #subcmd_type ::from_subcommand(matches.subcommand()) #unwrapper ) } else { - let convert = match ty(&field.ty) { + let cur_type = ty(&field.ty); + + let (cur_type, value_of, values_of, parse) = match get_parser(field) { + None => ( + cur_type, + quote!(value_of), + quote!(values_of), + quote!(|s| s.parse().unwrap()), + ), + Some((Parser::FromStr, f)) => ( + despecialize_bool_and_u64(cur_type), + quote!(value_of), + quote!(values_of), + f, + ), + Some((Parser::TryFromStr, f)) => ( + despecialize_bool_and_u64(cur_type), + quote!(value_of), + quote!(values_of), + quote!(|s| #f(s).unwrap()), + ), + Some((Parser::FromOsStr, f)) => ( + despecialize_bool_and_u64(cur_type), + quote!(value_of_os), + quote!(values_of_os), + f, + ), + Some((Parser::TryFromOsStr, f)) => ( + despecialize_bool_and_u64(cur_type), + quote!(value_of_os), + quote!(values_of_os), + quote!(|s| #f(s).unwrap()), + ), + }; + + let convert = match cur_type { Ty::Bool => quote!(is_present(stringify!(#name))), Ty::U64 => quote!(occurrences_of(stringify!(#name))), Ty::Option => quote! { - value_of(stringify!(#name)) + #value_of(stringify!(#name)) .as_ref() - .map(|s| s.parse().unwrap()) + .map(#parse) }, Ty::Vec => quote! { - values_of(stringify!(#name)) - .map(|v| v.map(|s| s.parse().unwrap()).collect()) + #values_of(stringify!(#name)) + .map(|v| v.map(#parse).collect()) .unwrap_or_else(Vec::new) }, Ty::Other => quote! { - value_of(stringify!(#name)) - .as_ref() - .unwrap() - .parse() + #value_of(stringify!(#name)) + .map(#parse) .unwrap() }, }; diff --git a/tests/custom-string-parsers.rs b/tests/custom-string-parsers.rs new file mode 100644 index 00000000..588d3de8 --- /dev/null +++ b/tests/custom-string-parsers.rs @@ -0,0 +1,125 @@ +// Copyright (c) 2017 structopt Developers +// +// This work is free. You can redistribute it and/or modify it under +// the terms of the Do What The Fuck You Want To Public License, +// Version 2, as published by Sam Hocevar. See the COPYING file for +// more details. + +extern crate structopt; +#[macro_use] +extern crate structopt_derive; + +use structopt::StructOpt; + +use std::path::PathBuf; +use std::num::ParseIntError; +use std::ffi::{OsStr, OsString}; + +#[derive(StructOpt, PartialEq, Debug)] +struct PathOpt { + #[structopt(short = "p", long = "path", parse(from_os_str))] + path: PathBuf, + + #[structopt(short = "d", default_value = "../", parse(from_os_str))] + default_path: PathBuf, + + #[structopt(short = "v", parse(from_os_str))] + vector_path: Vec, + + #[structopt(short = "o", parse(from_os_str))] + option_path_1: Option, + + #[structopt(short = "q", parse(from_os_str))] + option_path_2: Option, +} + +#[test] +fn test_path_opt_simple() { + assert_eq!( + PathOpt { + path: PathBuf::from("/usr/bin"), + default_path: PathBuf::from("../"), + vector_path: vec![ + PathBuf::from("/a/b/c"), + PathBuf::from("/d/e/f"), + PathBuf::from("/g/h/i"), + ], + option_path_1: None, + option_path_2: Some(PathBuf::from("j.zip")) + }, + PathOpt::from_clap(PathOpt::clap().get_matches_from(&[ + "test", + "-p", "/usr/bin", + "-v", "/a/b/c", + "-v", "/d/e/f", + "-v", "/g/h/i", + "-q", "j.zip", + ])) + ); +} + + + + +fn parse_hex(input: &str) -> Result { + u64::from_str_radix(input, 16) +} + +#[derive(StructOpt, PartialEq, Debug)] +struct HexOpt { + #[structopt(short = "n", parse(try_from_str = "parse_hex"))] + number: u64, +} + +#[test] +fn test_parse_hex() { + assert_eq!( + HexOpt { number: 5 }, + HexOpt::from_clap(HexOpt::clap().get_matches_from(&["test", "-n", "5"])) + ); + assert_eq!( + HexOpt { number: 0xabcdef }, + HexOpt::from_clap(HexOpt::clap().get_matches_from(&["test", "-n", "abcdef"])) + ); + + let err = HexOpt::clap().get_matches_from_safe(&["test", "-n", "gg"]).unwrap_err(); + assert!(err.message.contains("invalid digit found in string"), err); +} + + + + +fn custom_parser_1(_: &str) -> &'static str { + "A" +} +fn custom_parser_2(_: &str) -> Result<&'static str, u32> { + Ok("B") +} +fn custom_parser_3(_: &OsStr) -> &'static str { + "C" +} +fn custom_parser_4(_: &OsStr) -> Result<&'static str, OsString> { + Ok("D") +} + +#[derive(StructOpt, PartialEq, Debug)] +struct NoOpOpt { + #[structopt(short = "a", parse(from_str = "custom_parser_1"))] + a: &'static str, + #[structopt(short = "b", parse(try_from_str = "custom_parser_2"))] + b: &'static str, + #[structopt(short = "c", parse(from_os_str = "custom_parser_3"))] + c: &'static str, + #[structopt(short = "d", parse(try_from_os_str = "custom_parser_4"))] + d: &'static str, +} + +#[test] +fn test_every_custom_parser() { + assert_eq!( + NoOpOpt { a: "A", b: "B", c: "C", d: "D" }, + NoOpOpt::from_clap(NoOpOpt::clap().get_matches_from(&[ + "test", "-a=?", "-b=?", "-c=?", "-d=?", + ])) + ); +} \ No newline at end of file From 6962ec8678fb7604e1d051ee93b57cfd015cccd8 Mon Sep 17 00:00:00 2001 From: kennytm Date: Thu, 9 Nov 2017 03:03:43 +0800 Subject: [PATCH 2/2] Addressed review comments. --- structopt-derive/src/lib.rs | 69 ++++++++++++++++------------------ tests/custom-string-parsers.rs | 38 ++++++++++++++++++- 2 files changed, 69 insertions(+), 38 deletions(-) diff --git a/structopt-derive/src/lib.rs b/structopt-derive/src/lib.rs index fba81852..b6de4b39 100644 --- a/structopt-derive/src/lib.rs +++ b/structopt-derive/src/lib.rs @@ -63,9 +63,9 @@ //! //! The `FromStr` trait is used to convert the argument to the given //! type, and the `Arg::validator` method is set to a method using -//! `FromStr::Err::description()` (`FromStr::Err` must implement -//! `std::Error::Error`). If you would like to use a custom string parser other -//! than `FromStr`, see the [same titled section](#custom-string-parsers) below. +//! `to_string()` (`FromStr::Err` must implement `std::fmt::Display`). +//! If you would like to use a custom string parser other than `FromStr`, see +//! the [same titled section](#custom-string-parsers) below. //! //! Thus, the `speed` argument is generated as: //! @@ -415,6 +415,10 @@ fn is_subcommand(field: &Field) -> bool { }) } +fn get_default_parser() -> (Parser, quote::Tokens) { + (Parser::TryFromStr, quote!(::std::str::FromStr::from_str)) +} + fn get_parser(field: &Field) -> Option<(Parser, quote::Tokens)> { field.attrs.iter() .flat_map(|attr| { @@ -469,7 +473,7 @@ fn get_parser(field: &Field) -> Option<(Parser, quote::Tokens)> { .next() } -fn despecialize_bool_and_u64(cur_type: Ty) -> Ty { +fn convert_with_custom_parse(cur_type: Ty) -> Ty { match cur_type { Ty::Bool | Ty::U64 => Ty::Other, rest => rest, @@ -498,31 +502,30 @@ fn gen_augmentation(fields: &[Field], app_var: &Ident) -> quote::Tokens { .filter(|&field| !is_subcommand(field)) .map(|field| { let name = gen_name(field); - let cur_type = ty(&field.ty); + let mut cur_type = ty(&field.ty); let convert_type = match cur_type { Ty::Vec | Ty::Option => sub_type(&field.ty).unwrap_or(&field.ty), _ => &field.ty, }; - let (cur_type, validator) = match get_parser(field) { - None => (cur_type, quote! { - .validator(|s| { - s.parse::<#convert_type>() - .map(|_| ()) - .map_err(|e| e.description().into()) - }) - }), - Some((Parser::TryFromStr, f)) => (despecialize_bool_and_u64(cur_type), quote! { + + let parser = get_parser(field); + if parser.is_some() { + cur_type = convert_with_custom_parse(cur_type); + } + let validator = match parser.unwrap_or_else(get_default_parser) { + (Parser::TryFromStr, f) => quote! { .validator(|s| { #f(&s) - .map(|_| ()) + .map(|_: #convert_type| ()) .map_err(|e| e.to_string()) }) - }), - Some((Parser::TryFromOsStr, f)) => (despecialize_bool_and_u64(cur_type), quote! { - .validator_os(|s| #f(&s).map(|_| ())) - }), - Some(_) => (despecialize_bool_and_u64(cur_type), quote! {}), + }, + (Parser::TryFromOsStr, f) => quote! { + .validator_os(|s| #f(&s).map(|_: #convert_type| ())) + }, + _ => quote! {}, }; + let modifier = match cur_type { Ty::Bool => quote!( .takes_value(false).multiple(false) ), Ty::U64 => quote!( .takes_value(false).multiple(true) ), @@ -581,35 +584,29 @@ fn gen_constructor(fields: &[Field]) -> quote::Tokens { }; quote!( #field_name: #subcmd_type ::from_subcommand(matches.subcommand()) #unwrapper ) } else { - let cur_type = ty(&field.ty); + let mut cur_type = ty(&field.ty); + let parser = get_parser(field); + if parser.is_some() { + cur_type = convert_with_custom_parse(cur_type); + } - let (cur_type, value_of, values_of, parse) = match get_parser(field) { - None => ( - cur_type, - quote!(value_of), - quote!(values_of), - quote!(|s| s.parse().unwrap()), - ), - Some((Parser::FromStr, f)) => ( - despecialize_bool_and_u64(cur_type), + let (value_of, values_of, parse) = match parser.unwrap_or_else(get_default_parser) { + (Parser::FromStr, f) => ( quote!(value_of), quote!(values_of), f, ), - Some((Parser::TryFromStr, f)) => ( - despecialize_bool_and_u64(cur_type), + (Parser::TryFromStr, f) => ( quote!(value_of), quote!(values_of), quote!(|s| #f(s).unwrap()), ), - Some((Parser::FromOsStr, f)) => ( - despecialize_bool_and_u64(cur_type), + (Parser::FromOsStr, f) => ( quote!(value_of_os), quote!(values_of_os), f, ), - Some((Parser::TryFromOsStr, f)) => ( - despecialize_bool_and_u64(cur_type), + (Parser::TryFromOsStr, f) => ( quote!(value_of_os), quote!(values_of_os), quote!(|s| #f(s).unwrap()), diff --git a/tests/custom-string-parsers.rs b/tests/custom-string-parsers.rs index 588d3de8..9db105d7 100644 --- a/tests/custom-string-parsers.rs +++ b/tests/custom-string-parsers.rs @@ -45,7 +45,7 @@ fn test_path_opt_simple() { PathBuf::from("/g/h/i"), ], option_path_1: None, - option_path_2: Some(PathBuf::from("j.zip")) + option_path_2: Some(PathBuf::from("j.zip")), }, PathOpt::from_clap(PathOpt::clap().get_matches_from(&[ "test", @@ -122,4 +122,38 @@ fn test_every_custom_parser() { "test", "-a=?", "-b=?", "-c=?", "-d=?", ])) ); -} \ No newline at end of file +} + + +// Note: can't use `Vec` directly, as structopt would instead look for +// conversion function from `&str` to `u8`. +type Bytes = Vec; + +#[derive(StructOpt, PartialEq, Debug)] +struct DefaultedOpt { + #[structopt(short = "b", parse(from_str))] + bytes: Bytes, + + #[structopt(short = "i", parse(try_from_str))] + integer: u64, + + #[structopt(short = "p", parse(from_os_str))] + path: PathBuf, +} + +#[test] +fn test_parser_with_default_value() { + assert_eq!( + DefaultedOpt { + bytes: b"E\xc2\xb2=p\xc2\xb2c\xc2\xb2+m\xc2\xb2c\xe2\x81\xb4".to_vec(), + integer: 9000, + path: PathBuf::from("src/lib.rs"), + }, + DefaultedOpt::from_clap(DefaultedOpt::clap().get_matches_from(&[ + "test", + "-b", "E²=p²c²+m²c⁴", + "-i", "9000", + "-p", "src/lib.rs", + ])) + ); +}