Skip to content

Commit

Permalink
fix(derive): Clarify ArgEnum as ValueEnum
Browse files Browse the repository at this point in the history
We aren't enumerating arguments but values for an argument, so the name
should reflect that.

This will be important as part of clap-rs#1807 when we have more specific
attribute names.
  • Loading branch information
epage committed Jun 8, 2022
1 parent 0377051 commit b08764c
Show file tree
Hide file tree
Showing 27 changed files with 148 additions and 136 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 b08764c

Please sign in to comment.