Skip to content

Commit

Permalink
Merge pull request #3799 from epage/value
Browse files Browse the repository at this point in the history
fix(derive): Clarify ArgEnum as ValueEnum
  • Loading branch information
epage committed Jun 8, 2022
2 parents 0377051 + 9e38353 commit 912a629
Show file tree
Hide file tree
Showing 31 changed files with 232 additions and 134 deletions.
4 changes: 2 additions & 2 deletions clap_complete/src/shells/shell.rs
Expand Up @@ -23,8 +23,8 @@ pub enum Shell {
}

impl Shell {
/// Deprecated, replaced with [`ArgEnumValueParser`][clap::builder::ArgEnumValueParser]
#[deprecated(since = "3.2.0", note = "Replaced with `ArgEnumValueParser`")]
/// Deprecated, replaced with [`EnumValueParser`][clap::builder::EnumValueParser]
#[deprecated(since = "3.2.0", note = "Replaced with `EnumValueParser`")]
pub fn possible_values() -> impl Iterator<Item = PossibleValue<'static>> {
Shell::value_variants()
.iter()
Expand Down
10 changes: 5 additions & 5 deletions clap_derive/src/attrs.rs
Expand Up @@ -486,7 +486,7 @@ impl Attrs {
self.push_method(ident, self.name.clone().translate(*self.env_casing));
}

ArgEnum(_) => self.is_enum = true,
ValueEnum(_) => self.is_enum = true,

FromGlobal(ident) => {
let ty = Sp::call_site(Ty::Other);
Expand Down Expand Up @@ -536,11 +536,11 @@ impl Attrs {
quote!(<#ty as ::std::default::Default>::default())
};

let val = if parsed.iter().any(|a| matches!(a, ArgEnum(_))) {
let val = if parsed.iter().any(|a| matches!(a, ValueEnum(_))) {
quote_spanned!(ident.span()=> {
{
let val: #ty = #val;
clap::ArgEnum::to_possible_value(&val).unwrap().get_name()
clap::ValueEnum::to_possible_value(&val).unwrap().get_name()
}
})
} else {
Expand Down Expand Up @@ -579,11 +579,11 @@ impl Attrs {
quote!(<#ty as ::std::default::Default>::default())
};

let val = if parsed.iter().any(|a| matches!(a, ArgEnum(_))) {
let val = if parsed.iter().any(|a| matches!(a, ValueEnum(_))) {
quote_spanned!(ident.span()=> {
{
let val: #ty = #val;
clap::ArgEnum::to_possible_value(&val).unwrap().get_name()
clap::ValueEnum::to_possible_value(&val).unwrap().get_name()
}
})
} else {
Expand Down
6 changes: 3 additions & 3 deletions clap_derive/src/derives/arg_enum.rs
Expand Up @@ -29,7 +29,7 @@ pub fn derive_arg_enum(input: &DeriveInput) -> TokenStream {

match input.data {
Data::Enum(ref e) => gen_for_enum(ident, &input.attrs, e),
_ => abort_call_site!("`#[derive(ArgEnum)]` only supports enums"),
_ => abort_call_site!("`#[derive(ValueEnum)]` only supports enums"),
}
}

Expand Down Expand Up @@ -60,7 +60,7 @@ pub fn gen_for_enum(name: &Ident, attrs: &[Attribute], e: &DataEnum) -> TokenStr
clippy::suspicious_else_formatting,
)]
#[deny(clippy::correctness)]
impl clap::ArgEnum for #name {
impl clap::ValueEnum for #name {
#value_variants
#to_possible_value
}
Expand All @@ -83,7 +83,7 @@ fn lits(
None
} else {
if !matches!(variant.fields, Fields::Unit) {
abort!(variant.span(), "`#[derive(ArgEnum)]` only supports non-unit variants, unless they are skipped");
abort!(variant.span(), "`#[derive(ValueEnum)]` only supports non-unit variants, unless they are skipped");
}
let fields = attrs.field_methods(false);
let name = attrs.cased_name();
Expand Down
4 changes: 2 additions & 2 deletions clap_derive/src/derives/args.rs
Expand Up @@ -426,7 +426,7 @@ pub fn gen_augment(

fn gen_arg_enum_possible_values(ty: &Type) -> TokenStream {
quote_spanned! { ty.span()=>
.possible_values(<#ty as clap::ArgEnum>::value_variants().iter().filter_map(clap::ArgEnum::to_possible_value))
.possible_values(<#ty as clap::ValueEnum>::value_variants().iter().filter_map(clap::ValueEnum::to_possible_value))
}
}

Expand Down Expand Up @@ -643,7 +643,7 @@ fn gen_parsers(
let ci = attrs.ignore_case();

parse = quote_spanned! { convert_type.span()=>
|s| <#convert_type as clap::ArgEnum>::from_str(s, #ci).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #id, err)))
|s| <#convert_type as clap::ValueEnum>::from_str(s, #ci).map_err(|err| clap::Error::raw(clap::ErrorKind::ValueValidation, format!("Invalid value for {}: {}", #id, err)))
}
}

Expand Down
2 changes: 1 addition & 1 deletion clap_derive/src/dummies.rs
Expand Up @@ -76,7 +76,7 @@ pub fn args(name: &Ident) {

pub fn arg_enum(name: &Ident) {
append_dummy(quote! {
impl clap::ArgEnum for #name {
impl clap::ValueEnum for #name {
fn value_variants<'a>() -> &'a [Self]{
unimplemented!()
}
Expand Down
10 changes: 9 additions & 1 deletion clap_derive/src/lib.rs
Expand Up @@ -28,7 +28,15 @@ mod dummies;
mod parse;
mod utils;

/// Generates the `ArgEnum` impl.
/// Generates the `ValueEnum` impl.
#[proc_macro_derive(ValueEnum, attributes(clap))]
#[proc_macro_error]
pub fn value_enum(input: TokenStream) -> TokenStream {
let input: DeriveInput = parse_macro_input!(input);
derives::derive_arg_enum(&input).into()
}

/// Generates the `ValueEnum` impl.
#[proc_macro_derive(ArgEnum, attributes(clap))]
#[proc_macro_error]
pub fn arg_enum(input: TokenStream) -> TokenStream {
Expand Down
5 changes: 3 additions & 2 deletions clap_derive/src/parse.rs
Expand Up @@ -30,7 +30,7 @@ pub enum ClapAttr {
Action(Ident),
Env(Ident),
Flatten(Ident),
ArgEnum(Ident),
ValueEnum(Ident),
FromGlobal(Ident),
Subcommand(Ident),
VerbatimDocComment(Ident),
Expand Down Expand Up @@ -188,7 +188,8 @@ impl Parse for ClapAttr {
"action" => Ok(Action(name)),
"env" => Ok(Env(name)),
"flatten" => Ok(Flatten(name)),
"arg_enum" => Ok(ArgEnum(name)),
"arg_enum" => Ok(ValueEnum(name)),
"value_enum" => Ok(ValueEnum(name)),
"from_global" => Ok(FromGlobal(name)),
"subcommand" => Ok(Subcommand(name)),
"external_subcommand" => Ok(ExternalSubcommand(name)),
Expand Down
14 changes: 7 additions & 7 deletions examples/derive_ref/README.md
Expand Up @@ -20,7 +20,7 @@ See [demo.rs](../demo.rs) and [demo.md](../demo.md) for a brief example.

Let's start by breaking down the anatomy of the derive attributes:
```rust
use clap::{Parser, Args, Subcommand, ArgEnum};
use clap::{Parser, Args, Subcommand, ValueEnum};

/// Doc comment
#[derive(Parser)]
Expand All @@ -30,7 +30,7 @@ struct Cli {
#[clap(ARG ATTRIBUTE)]
field: UserType,

#[clap(arg_enum, ARG ATTRIBUTE...)]
#[clap(value_enum, ARG ATTRIBUTE...)]
field: EnumValues,

#[clap(flatten)]
Expand Down Expand Up @@ -67,7 +67,7 @@ enum Command {
}

/// Doc comment
#[derive(ArgEnum)]
#[derive(ValueEnum)]
#[clap(ARG ENUM ATTRIBUTE)]
enum EnumValues {
/// Doc comment
Expand All @@ -84,7 +84,7 @@ fn main() {
- `Args` allows defining a set of re-usable arguments that get merged into their parent container.
- `Subcommand` defines available subcommands.
- Subcommand arguments can be defined in a struct-variant or automatically flattened with a tuple-variant.
- `ArgEnum` allows parsing a value directly into an `enum`, erroring on unsupported values.
- `ValueEnum` allows parsing a value directly into an `enum`, erroring on unsupported values.
- The derive doesn't work on enums that contain non-unit variants, unless they are skipped

See also the [tutorial](../tutorial_derive/README.md) and [examples](../README.md).
Expand Down Expand Up @@ -212,15 +212,15 @@ These correspond to a `clap::Arg`.
- Default: `try_from_str`
- Warning: for `Path` / `OsString`, be sure to use `try_from_os_str`
- See [Arg Types](#arg-types) for more details
- `arg_enum`: Parse the value using the `ArgEnum` trait
- `value_enum`: Parse the value using the `ValueEnum` trait
- `skip [= <expr>]`: Ignore this field, filling in with `<expr>`
- Without `<expr>`: fills the field with `Default::default()`
- `default_value = <str>`: `clap::Arg::default_value` and `clap::Arg::required(false)`
- `default_value_t [= <expr>]`: `clap::Arg::default_value` and `clap::Arg::required(false)`
- Requires `std::fmt::Display` or `#[clap(arg_enum)]`
- Requires `std::fmt::Display` or `#[clap(value_enum)]`
- Without `<expr>`, relies on `Default::default()`
- `default_value_os_t [= <expr>]`: `clap::Arg::default_value_os` and `clap::Arg::required(false)`
- Requires `std::convert::Into<OsString>` or `#[clap(arg_enum)]`
- Requires `std::convert::Into<OsString>` or `#[clap(value_enum)]`
- Without `<expr>`, relies on `Default::default()`

### Arg Enum Attributes
Expand Down
4 changes: 2 additions & 2 deletions examples/tutorial_builder/04_01_enum.rs
@@ -1,8 +1,8 @@
// Note: this requires the `cargo` feature

use clap::{arg, command, value_parser, ArgEnum};
use clap::{arg, command, value_parser, ValueEnum};

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ArgEnum)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
enum Mode {
Fast,
Slow,
Expand Down
2 changes: 1 addition & 1 deletion examples/tutorial_builder/README.md
Expand Up @@ -409,7 +409,7 @@ For more information try --help

```

When enabling the `derive` feature, you can use `ArgEnum` to take care of the boiler plate for you, giving the same results.
When enabling the `derive` feature, you can use `ValueEnum` to take care of the boiler plate for you, giving the same results.

[Example:](04_01_enum.rs)
```console
Expand Down
2 changes: 1 addition & 1 deletion examples/tutorial_derive/README.md
Expand Up @@ -378,7 +378,7 @@ name: "bob"
### Enumerated values

If you have arguments of specific values you want to test for, you can derive
`ArgEnum`.
`ValueEnum`.

This allows you specify the valid values for that argument. If the user does not use one of
those specific values, they will receive a graceful exit with error message informing them
Expand Down
2 changes: 1 addition & 1 deletion src/builder/arg.rs
Expand Up @@ -955,7 +955,7 @@ impl<'help> Arg<'help> {
/// - [`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] and [`RangedU64ValueParser`][crate::builder::RangedU64ValueParser] for numeric ranges
/// - [`ArgEnumValueParser`][crate::builder::ArgEnumValueParser] and [`PossibleValuesParser`][crate::builder::PossibleValuesParser] for static enumerated values
/// - [`EnumValueParser`][crate::builder::EnumValueParser] and [`PossibleValuesParser`][crate::builder::PossibleValuesParser] for static enumerated values
/// - or any other [`TypedValueParser`][crate::builder::TypedValueParser] implementation
///
/// ```rust
Expand Down
2 changes: 1 addition & 1 deletion src/builder/mod.rs
Expand Up @@ -34,10 +34,10 @@ pub use possible_value::PossibleValue;
pub use value_hint::ValueHint;
pub use value_parser::via_prelude;
pub use value_parser::AnyValueParser;
pub use value_parser::ArgEnumValueParser;
pub use value_parser::AutoValueParser;
pub use value_parser::BoolValueParser;
pub use value_parser::BoolishValueParser;
pub use value_parser::EnumValueParser;
pub use value_parser::FalseyValueParser;
pub use value_parser::NonEmptyStringValueParser;
pub use value_parser::OsStringValueParser;
Expand Down
42 changes: 21 additions & 21 deletions src/builder/value_parser.rs
Expand Up @@ -79,7 +79,7 @@ impl ValueParser {
/// To create a custom parser, see [`TypedValueParser`]
///
/// Pre-existing implementations include:
/// - [`ArgEnumValueParser`] and [`PossibleValuesParser`] for static enumerated values
/// - [`EnumValueParser`] and [`PossibleValuesParser`] for static enumerated values
/// - [`BoolishValueParser`] and [`FalseyValueParser`] for alternative `bool` implementations
/// - [`RangedI64ValueParser`] and [`RangedU64ValueParser`]
/// - [`NonEmptyStringValueParser`]
Expand Down Expand Up @@ -809,7 +809,7 @@ impl Default for PathBufValueParser {
}
}

/// Parse an [`ArgEnum`][crate::ArgEnum] value.
/// Parse an [`ValueEnum`][crate::ValueEnum] value.
///
/// See also:
/// - [`PossibleValuesParser`]
Expand All @@ -829,7 +829,7 @@ impl Default for PathBufValueParser {
/// Never,
/// }
///
/// impl clap::ArgEnum for ColorChoice {
/// impl clap::ValueEnum for ColorChoice {
/// fn value_variants<'a>() -> &'a [Self] {
/// &[Self::Always, Self::Auto, Self::Never]
/// }
Expand All @@ -847,7 +847,7 @@ impl Default for PathBufValueParser {
/// let mut cmd = clap::Command::new("raw")
/// .arg(
/// clap::Arg::new("color")
/// .value_parser(clap::builder::ArgEnumValueParser::<ColorChoice>::new())
/// .value_parser(clap::builder::EnumValueParser::<ColorChoice>::new())
/// .required(true)
/// );
///
Expand All @@ -857,7 +857,7 @@ impl Default for PathBufValueParser {
/// assert_eq!(port, ColorChoice::Always);
///
/// // Semantics
/// let value_parser = clap::builder::ArgEnumValueParser::<ColorChoice>::new();
/// let value_parser = clap::builder::EnumValueParser::<ColorChoice>::new();
/// // or
/// let value_parser = clap::value_parser!(ColorChoice);
/// assert!(value_parser.parse_ref(&cmd, arg, OsStr::new("random")).is_err());
Expand All @@ -867,19 +867,19 @@ impl Default for PathBufValueParser {
/// assert_eq!(value_parser.parse_ref(&cmd, arg, OsStr::new("never")).unwrap(), ColorChoice::Never);
/// ```
#[derive(Clone, Debug)]
pub struct ArgEnumValueParser<E: crate::ArgEnum + Clone + Send + Sync + 'static>(
pub struct EnumValueParser<E: crate::ValueEnum + Clone + Send + Sync + 'static>(
std::marker::PhantomData<E>,
);

impl<E: crate::ArgEnum + Clone + Send + Sync + 'static> ArgEnumValueParser<E> {
/// Parse an [`ArgEnum`][crate::ArgEnum]
impl<E: crate::ValueEnum + Clone + Send + Sync + 'static> EnumValueParser<E> {
/// Parse an [`ValueEnum`][crate::ValueEnum]
pub fn new() -> Self {
let phantom: std::marker::PhantomData<E> = Default::default();
Self(phantom)
}
}

impl<E: crate::ArgEnum + Clone + Send + Sync + 'static> TypedValueParser for ArgEnumValueParser<E> {
impl<E: crate::ValueEnum + Clone + Send + Sync + 'static> TypedValueParser for EnumValueParser<E> {
type Value = E;

fn parse_ref(
Expand Down Expand Up @@ -911,7 +911,7 @@ impl<E: crate::ArgEnum + Clone + Send + Sync + 'static> TypedValueParser for Arg
.iter()
.find(|v| {
v.to_possible_value()
.expect("ArgEnum::value_variants contains only values with a corresponding ArgEnum::to_possible_value")
.expect("ValueEnum::value_variants contains only values with a corresponding ValueEnum::to_possible_value")
.matches(value, ignore_case)
})
.ok_or_else(|| {
Expand All @@ -938,7 +938,7 @@ impl<E: crate::ArgEnum + Clone + Send + Sync + 'static> TypedValueParser for Arg
}
}

impl<E: crate::ArgEnum + Clone + Send + Sync + 'static> Default for ArgEnumValueParser<E> {
impl<E: crate::ValueEnum + Clone + Send + Sync + 'static> Default for EnumValueParser<E> {
fn default() -> Self {
Self::new()
}
Expand All @@ -947,7 +947,7 @@ impl<E: crate::ArgEnum + Clone + Send + Sync + 'static> Default for ArgEnumValue
/// Verify the value is from an enumerated set of [`PossibleValue`][crate::PossibleValue].
///
/// See also:
/// - [`ArgEnumValueParser`]
/// - [`EnumValueParser`]
///
/// # Example
///
Expand Down Expand Up @@ -1930,18 +1930,18 @@ pub mod via_prelude {
}

#[doc(hidden)]
pub trait ValueParserViaArgEnum: private::ValueParserViaArgEnumSealed {
pub trait ValueParserViaValueEnum: private::ValueParserViaValueEnumSealed {
type Output;

fn value_parser(&self) -> Self::Output;
}
impl<E: crate::ArgEnum + Clone + Send + Sync + 'static> ValueParserViaArgEnum
impl<E: crate::ValueEnum + Clone + Send + Sync + 'static> ValueParserViaValueEnum
for &AutoValueParser<E>
{
type Output = ArgEnumValueParser<E>;
type Output = EnumValueParser<E>;

fn value_parser(&self) -> Self::Output {
ArgEnumValueParser::<E>::new()
EnumValueParser::<E>::new()
}
}

Expand Down Expand Up @@ -2004,14 +2004,14 @@ pub mod via_prelude {
/// let parser = clap::value_parser!(usize);
/// assert_eq!(format!("{:?}", parser), "ValueParser::other(usize)");
///
/// // ArgEnum types
/// // ValueEnum types
/// #[derive(Copy, Clone, Debug, PartialEq, Eq)]
/// enum ColorChoice {
/// Always,
/// Auto,
/// Never,
/// }
/// impl clap::ArgEnum for ColorChoice {
/// impl clap::ValueEnum for ColorChoice {
/// // ...
/// # fn value_variants<'a>() -> &'a [Self] {
/// # &[Self::Always, Self::Auto, Self::Never]
Expand All @@ -2025,7 +2025,7 @@ pub mod via_prelude {
/// # }
/// }
/// let parser = clap::value_parser!(ColorChoice);
/// assert_eq!(format!("{:?}", parser), "ArgEnumValueParser(PhantomData)");
/// assert_eq!(format!("{:?}", parser), "EnumValueParser(PhantomData)");
/// ```
#[macro_export]
macro_rules! value_parser {
Expand Down Expand Up @@ -2053,8 +2053,8 @@ mod private {
pub trait ValueParserViaFactorySealed {}
impl<P: ValueParserFactory> ValueParserViaFactorySealed for &&AutoValueParser<P> {}

pub trait ValueParserViaArgEnumSealed {}
impl<E: crate::ArgEnum> ValueParserViaArgEnumSealed for &AutoValueParser<E> {}
pub trait ValueParserViaValueEnumSealed {}
impl<E: crate::ValueEnum> ValueParserViaValueEnumSealed for &AutoValueParser<E> {}

pub trait ValueParserViaFromStrSealed {}
impl<FromStr> ValueParserViaFromStrSealed for AutoValueParser<FromStr>
Expand Down

0 comments on commit 912a629

Please sign in to comment.